Created attachment 340481 [details] [review]
project-tree: Expand tree properly after a refresh

After refreshing the project tree (which checks for new files in the
filesystem), the tree will have newly created IdeTreeNode instances, so
the code we were using to find the node that was selected before the
rebuild wasn't working, meaning that the tree was always collapsed after
a rebuild. This commit changes gb_project_tree_actions_refresh to find
the node representing the file that was previously selected even if the
GbProjectFile instance is different (using g_file_equal). That way we
can expand the correct node to make the tree look like the user would
expect.

Review of attachment 340481 [details] [review]:

I think we can just rely on the reveal helper now since we only have files in the project tree. Someday, when we support more node types we can try harder to do this in a more abstract fashion.

::: plugins/project-tree/gb-project-tree-actions.c
@@ +64,3 @@
 
+static gboolean
+compare_to_file (gconstpointer a,

Drop this function.

@@ +93,3 @@
   IdeTreeNode *selected;
   GObject *item = NULL;
+  gboolean expanded;

Drop this. Add:

g_autoptr(GFile) expand_to = NULL;

@@ +100,3 @@
     {
       item = ide_tree_node_get_item (selected);
+      if (item != NULL && GB_IS_PROJECT_FILE (item))

No need to do the item != NULL and the GB_IS_PROJECT_FILE(). The type check macros are NULL safe. So just:

  if (GB_IS_PROJECT_FILE (item))
    expand_to = g_object_ref (gb_project_file_get_file (GB_PROJECT_FILE (item)));

@@ -83,3 +109,3 @@
   ide_tree_rebuild (IDE_TREE (self));
 
-  if (item != NULL)
+  /* Re-select the file that was selected before the rebuild if it's still there */

Probably can remove this whole branch now. Replace with:

  if (expand_to != NULL)
    gb_project_tree_reveal (self, expand_to, FALSE);

That does make the code much simpler, but it doesn't work if the selected node was the top level folder or if the selected node's file was deleted. And if the selected node was an expanded folder it becomes a collapsed one.

So it's a question of if we care about those edge cases enough to make the code more complex.

Created attachment 340484 [details] [review]
project-tree: Expand tree properly after a refresh

After refreshing the project tree (which checks for new files in the
filesystem), the tree will have newly created IdeTreeNode instances, so
the code we were using to find the node that was selected before the
rebuild wasn't working, meaning that the tree was always collapsed after
a rebuild. This commit changes gb_project_tree_actions_refresh to find
the node representing the file that was previously selected even if the
GbProjectFile instance is different (using g_file_equal). That way we
can expand the correct node to make the tree look like the user would
expect.

(In reply to Matthew Leeds from comment #3)
> That does make the code much simpler, but it doesn't work if the selected
> node was the top level folder or if the selected node's file was deleted.
> And if the selected node was an expanded folder it becomes a collapsed one.

We can add a new parameter to the reveal function that indicates if the match should be expanded (and stash the expanded state before rebuilding).

Also, we should make sure that reveal ensures the tree is expanded as far down as it can go in case the file was deleted.

Created attachment 340579 [details] [review]
project-tree: Expand tree properly after a refresh

After refreshing the project tree (which checks for new files in the
filesystem), the tree will have newly created IdeTreeNode instances, so
the code we were using to find the node that was selected before the
rebuild wasn't working, meaning that the tree was always collapsed after
a rebuild. This commit changes gb_project_tree_actions_refresh to use
gb_project_tree_reveal, and changes that function to allow expansions of
matching folders, and to behave reasonably for deleted files and for the
the top level folder. It also changes the places where that reveal
function is called to make them work.

Review of attachment 340579 [details] [review]:

LGTM, thanks!

::: plugins/project-tree/gb-project-tree.c
@@ -141,3 +139,3 @@
-  if (NULL != (node = ide_tree_get_selected (IDE_TREE (self))) &&
-      NULL != (item = ide_tree_node_get_item (node)) &&
-      GB_IS_PROJECT_FILE (item))
+  group = gtk_widget_get_action_group (GTK_WIDGET (self), "project-tree");
+  if (group != NULL)
+    g_action_group_activate_action (group, "refresh", NULL);

This is something I don't like about how the actions are implemented today. I should have been more diligent in making them API, and then only wrapping API with the actions :(

Pushed as commit 5b92c1efb529c0fa294c71fcce57f6521cfc5b4d with a few changes and a follow-up commit.