After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 750818 - Do not add random directories to the list of recent projects
Do not add random directories to the list of recent projects
Status: RESOLVED FIXED
Product: gnome-builder
Classification: Other
Component: general
3.19.x
Other Linux
: Normal minor
: ---
Assigned To: GNOME Builder Maintainers
GNOME Builder Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-06-12 01:45 UTC by Michael Catanzaro
Modified: 2016-02-22 00:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
screenshot (64.47 KB, image/png)
2015-06-12 01:45 UTC, Michael Catanzaro
  Details
This patch avoids bookmarking undesired directories. (1.91 KB, patch)
2016-02-19 18:45 UTC, Akshaya Kakkilaya
none Details | Review
Avoid bookmarking undesired directories. (2.52 KB, patch)
2016-02-20 11:53 UTC, Akshaya Kakkilaya
none Details | Review
Removed includes used for debugging (2.36 KB, patch)
2016-02-20 23:12 UTC, Akshaya Kakkilaya
committed Details | Review
updated patch with review fixes (8.88 KB, patch)
2016-02-21 22:52 UTC, Christian Hergert
committed Details | Review

Description Michael Catanzaro 2015-06-12 01:45:23 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.
Comment 1 Christian Hergert 2015-06-12 03:20:31 UTC
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.
Comment 2 Michael Catanzaro 2015-06-12 03:55:35 UTC
> 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.)
Comment 3 justyn temme 2015-07-14 04:43:04 UTC
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
Comment 4 Christian Hergert 2016-02-12 19:16:12 UTC
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.
Comment 5 Akshaya Kakkilaya 2016-02-19 18:45:24 UTC
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.
Comment 6 Christian Hergert 2016-02-19 19:26:30 UTC
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).
Comment 7 Michael Catanzaro 2016-02-19 19:41:26 UTC
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.
Comment 8 Michael Catanzaro 2016-02-19 19:42:05 UTC
To be clear, that means you can't ever use the result of g_strconcat as a temporary.
Comment 9 Akshaya Kakkilaya 2016-02-20 11:53:40 UTC
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.
Comment 10 Michael Catanzaro 2016-02-20 16:43:59 UTC
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.
Comment 11 Akshaya Kakkilaya 2016-02-20 23:12:03 UTC
Created attachment 321755 [details] [review]
Removed includes used for debugging
Comment 12 Christian Hergert 2016-02-21 22:51:31 UTC
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.
Comment 13 Christian Hergert 2016-02-21 22:52:17 UTC
Created attachment 321795 [details] [review]
updated patch with review fixes

Attaching updated patch for posterity
Comment 14 Akshaya Kakkilaya 2016-02-22 00:39:00 UTC
Thank you for reviewing my first patch! :)