GNOME Bugzilla – Bug 750818
Do not add random directories to the list of recent projects
Last modified: 2016-02-22 00:39:00 UTC
Created attachment 305109 [details] screenshot Whenever I open any individual file with Builder (e.g. by clicking on a source code file in nautilus), the directory it is in gets treated as a "project" and appears on a big list of Recent Projects that appear when starting Builder. (There is also a list of Other Projects, but nobody will ever see it since it's down too far.) These are actually "projects" per se, they are just random directories. I have three "projects" under .cache, for example, each named "bug" (and I don't remember how I managed that :). My favorite is the one named "wtf" which I did not create intentionally but which describes this list pretty well. I think projects should only be added to this list if I explicitly tell Builder that a directory represents a project by using the New Project command.
We probably can do this by checking of the project is a IdeDirectoryBuildSystem and an additional check to see if the project was created via the recent-projects/new-project-dialog. Although, what if you open a file that is part of an autotools project. Today, that would get added to the recent projects (since it would discover an autotools project by walking the filesystem tree). Not clear to me what we should do here yet.
> Although, what if you open a file that is part of an autotools project. > Today, that would get added to the recent projects (since it would discover > an autotools project by walking the filesystem tree). Not clear to me what > we should do here yet. But it would discover the top-level directory of the project that contains configure.ac, right? (Rather than random subdirectories.) That seems fine, since you can delete entries from the list now. (Although I think it would also be fine to just not add the project unless it's added with New Project.)
perhaps not adding the entire folder unless its added from the new project (what Michael was saying?). this would allow builder to edit simple python scripts as well as transition to big projects (typically all in one folder) while a python file might just be in Documents
It would be nice if we could at least, in the short term: - ignore ~/.cache/* - ignore ~/Downloads/* Seems like a good newcomer bug. libide/ide-context.c in ide_context_init_add_recent() is where to look.
Created attachment 321683 [details] [review] This patch avoids bookmarking undesired directories. With this patch, gnome builder only lists those recent projects that are folders within the current user home directory. Dot preceded folders will not be listed.
Review of attachment 321683 [details] [review]: Definitely on the right track here, but lets use GFile instead of string URIs for comparisons. ::: libide/ide-context.c @@ +1272,3 @@ +static +gboolean directory_is_ignored (gchar *uri) I think we should use the GFile* here instead of the URI. All sorts of weird stuff can land in a URI that makes correct string checks non-trivial. @@ +1274,3 @@ +gboolean directory_is_ignored (gchar *uri) +{ + const gchar *user_dir = g_environ_getenv (g_get_environ (), "HOME"); Use g_get_home_dir() for the home directory. @@ +1275,3 @@ +{ + const gchar *user_dir = g_environ_getenv (g_get_environ (), "HOME"); + gboolean ignore = (!g_str_has_prefix (uri, g_strconcat ("file://", user_dir, "/", NULL)) || Lets create GFiles for the things we don't care about, and check to see if the project file is a descendant of that directory. static gboolean is_descendant (GFile *parent, GFile *descendant) { g_autofree gchar *relative_path = g_file_get_relative_path (parent, descendant); return (relative_path != NULL); } @@ +1277,3 @@ + gboolean ignore = (!g_str_has_prefix (uri, g_strconcat ("file://", user_dir, "/", NULL)) || + g_str_has_prefix (uri, g_strconcat ("file://", user_dir, "/.", NULL)) || + g_str_has_prefix (uri, g_strconcat ("file://", user_dir, "/Downloads", NULL))); The downloads directory can be fetched using g_get_user_special_dir (G_USER_DIRECTORY_DOWNLOAD).
Review of attachment 321683 [details] [review]: ::: libide/ide-context.c @@ +1275,3 @@ +{ + const gchar *user_dir = g_environ_getenv (g_get_environ (), "HOME"); + gboolean ignore = (!g_str_has_prefix (uri, g_strconcat ("file://", user_dir, "/", NULL)) || Also, g_strconcat is transfer full, you need to free the result with g_free.
To be clear, that means you can't ever use the result of g_strconcat as a temporary.
Created attachment 321718 [details] [review] Avoid bookmarking undesired directories. I switched to using GFile everywhere except where I check if path contains hidden folders that are directly under the home directory. I have also added another condition so that the home folder is not added as a recent project if a file directly under the home folder is opened.
Review of attachment 321718 [details] [review]: ::: libide/ide-context.c @@ +52,3 @@ +#include <glib.h> +#include <glib/gprintf.h> Looks like you were using this for debugging; don't forget to remove it.
Created attachment 321755 [details] [review] Removed includes used for debugging
Review of attachment 321755 [details] [review]: A few comments attached below. I've pushed a modified version of the patch with the fixes below. ::: libide/ide-context.c @@ +1271,3 @@ +directory_is_ignored (GFile *file) +{ + g_autofree const gchar *home_dir = g_get_home_dir (); don't free const strings @@ +1272,3 @@ +{ + g_autofree const gchar *home_dir = g_get_home_dir (); + g_autofree const gchar *downloads_dir = g_get_user_special_dir (G_USER_DIRECTORY_DOWNLOAD); same @@ +1286,3 @@ + if (g_str_has_prefix (relative_path, ".")) + return TRUE; + if (type != G_FILE_TYPE_DIRECTORY && g_file_get_parent() leaks parent @@ -1270,0 +1270,22 @@ +static gboolean +directory_is_ignored (GFile *file) +{ ... 19 more ... instead of using else here, we generally just do a return FALSE outside the conditional @@ +1315,2 @@ task = g_task_new (self, cancellable, callback, user_data); + Trailing whitespace.
Created attachment 321795 [details] [review] updated patch with review fixes Attaching updated patch for posterity
Thank you for reviewing my first patch! :)