From: Christian Weisgerber <naddy@mips.inka.de>
Subject: Re: tog: tree view move-to-parent bug
To: gameoftrees@openbsd.org
Date: Fri, 27 Nov 2020 00:11:11 +0100

Stefan Sperling:

> But if we start out by viewing a subtree and then move up, the scroll
> position of the parent isn't actually available since the parent tree
> was never nagivated by the user. In this case tree_view_walk_path()
> has to fill in some values. The current values are bogus and result in
> the behaviour you have found.
> 
> The only parent entry we know about in this case is the one which was
> traversed to reach the child. I believe the best we can do is to lock
> the parent's scroll position such that the traversed child entry appears
> at the top of the list if moving up to the parent's view. If we then
> navigate down again and return, the parent's scroll position will start
> to be retained and restored properly.

Your diff fixes the problem, but looking at the loop in
tree_view_walk_path(), I wonder if it isn't overly complicated:

> @@ -1907,7 +1910,8 @@ tree_view_walk_path(struct tog_tree_view_state *s,
>  		if (err)
>  			break;
>  
> -		err = tree_view_visit_subtree(tree, s);
> +		err = tree_view_visit_subtree(tree, s,
> +		    s->selected_entry, s->selected_entry, 0);
>  		if (err) {
>  			got_object_tree_close(tree);
>  			break;

You are effectively setting
  s->first_displayed_entry = s->selected_entry;
  s->selected = 0;
here.

But lo and behold, that's exactly what happens earlier for the
!S_ISDIR case.  And those are the only two non-error exits from the
loop.

So I think we can just always set
  s->first_displayed_entry = s->selected_entry;
and s->selected is initialized when the view is calloc()ed.

Am I missing something?

diff 16aeacf7088dcd3cd5e654af46a3015cecf41426 /home/naddy/got
blob - feaac17d0e83e59b9ef24e80a5f347f16db2f891
file + tog/tog.c
--- tog/tog.c
+++ tog/tog.c
@@ -1875,17 +1875,10 @@ tree_view_walk_path(struct tog_tree_view_state *s,
 			break;
 		}
 		free(te_name);
-		s->selected_entry = te;
-		s->selected = got_tree_entry_get_index(te);
-		if (s->tree != s->root)
-			s->selected++; /* skip '..' */
+		s->first_displayed_entry = s->selected_entry = te;
 
-		if (!S_ISDIR(got_tree_entry_get_mode(s->selected_entry))) {
-			/* Jump to this file's entry. */
-			s->first_displayed_entry = s->selected_entry;
-			s->selected = 0;
-			break;
-		}
+		if (!S_ISDIR(got_tree_entry_get_mode(s->selected_entry)))
+			break; /* jump to this file's entry */
 
 		slash = strchr(p, '/');
 		if (slash)
-- 
Christian "naddy" Weisgerber                          naddy@mips.inka.de