[Github-comments] [geany/geany-plugins] Workbench (#598)

[Colomban Wendling]

b4n requested changes on this pull request.

A few problems, with proposed fixes in https://github.com/b4n/geany-plugins/tree/LarsGit223/workbench%2Bfollow-up

> @@ -0,0 +1,9 @@
+iconsdir = $(datadir)/icons/hicolor/16x16

wrong target directory, should be `24x24`

> @@ -0,0 +1,9 @@
+iconsdir = $(datadir)/icons/hicolor/16x16

should be `32x32`

> @@ -0,0 +1,9 @@
+iconsdir = $(datadir)/icons/hicolor/16x16

Should be `48x48`

> @@ -0,0 +1,9 @@
+iconsdir = $(datadir)/icons/hicolor/16x16

should be `scalable`

> +	/**** tree view ****/
+
+	sidebar.file_view = gtk_tree_view_new();
+	g_signal_connect(sidebar.file_view, "row-activated", (GCallback)sidebar_filew_view_on_row_activated, NULL);
+
+	sidebar.file_store = gtk_tree_store_new(FILEVIEW_N_COLUMNS, G_TYPE_ICON, G_TYPE_STRING, G_TYPE_UINT, G_TYPE_POINTER);
+	gtk_tree_view_set_model(GTK_TREE_VIEW(sidebar.file_view), GTK_TREE_MODEL(sidebar.file_store));
+
+	renderer = gtk_cell_renderer_pixbuf_new();
+	column = gtk_tree_view_column_new();
+	gtk_tree_view_column_pack_start(column, renderer, FALSE);
+	gtk_tree_view_column_add_attribute(column, renderer, "gicon", FILEVIEW_COLUMN_ICON);
+
+	renderer = gtk_cell_renderer_text_new();
+	gtk_tree_view_column_pack_start(column, renderer, TRUE);
+	gtk_tree_view_column_add_attribute(column, renderer, "markup", FILEVIEW_COLUMN_NAME);

you should use `"text"`" instead of `"markup"` as you pass raw filenames (and don't seem to need the markup features), otherwise it'll break with filenames containing `&`s, `<`s and the like.

> +}
+
+
+/* Get the list of files for root */
+static GSList *wb_project_dir_get_file_list(WB_PROJECT_DIR *root, const gchar *utf8_path, GSList *patterns,
+		GSList *ignored_dirs_patterns, GSList *ignored_file_patterns, GHashTable *visited_paths)
+{
+	GSList *list = NULL;
+	GDir *dir;
+	gchar *locale_path = utils_get_locale_from_utf8(utf8_path);
+	gchar *real_path = tm_get_real_path(locale_path);
+
+	dir = g_dir_open(locale_path, 0, NULL);
+	if (!dir || !real_path || g_hash_table_lookup(visited_paths, real_path))
+	{
+		g_dir_close(dir);

you should guard this call against `NULL` `dir`

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/598#pullrequestreview-57398419
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.geany.org/pipermail/github-comments/attachments/20170820/2b240f16/attachment.html>