On Fri, Dec 04, 2020 at 01:19:33PM +0100, Christian Weisgerber wrote:
> Christian Weisgerber:
> 
> > By picking a branch in tog ref, and then opening a log view, you
> > can view the commits on that branch.  But if you open a diff view
> > from there, '<' and '>' for moving to a younger/older commit will
> > not stay on the branch.
> 
> Not sure what I saw there, but that's not quite what happens now.
> 
> Anyway, '<' and '>' in the diff view are broken and it has nothing
> to do with branches.  You can move by one entry in a direction, and
> you can move back and forth between the new and old entry, but any
> further movement blocks.
> 
> tog log -> diff                     ok
> tog ref -> log -> diff              broken
> tog ref -> tree -> log -> diff      ok
> 
> If you use a wide terminal, it's more obvious: '<' and '>' work
> when a log and diff view share a split screen.
> 
> input_diff_view() does
> 
>                 ls = &s->log_view->state.log;
>                 entry = TAILQ_PREV(ls->selected_entry,
>                     commit_queue_head, entry);
> 		err = input_log_view(NULL, NULL, s->log_view, KEY_UP);
> 
> and something analogous for the other direction.  The problem is
> that input_log_view() only sets ls->selected.  ls->selected_entry
> is not advanced.  That only happens in draw_commits().  If the log
> view isn't shown, the commit queue can't be traversed.

Thanks for digging into the issue.

The following works for me. Not sure if this is the best possible fix,
perhaps we'd want to decouple the diff/log views a bit more. But I am
happy enough with this approach for now:

diff 9970f7fca9084ec99cb608732155668240b318fe /home/stsp/src/got
blob - 50fefd43b456ae573fe7ef9c214a59c72cb42a9e
file + tog/tog.c
--- tog/tog.c
+++ tog/tog.c
@@ -1508,12 +1508,31 @@ queue_commits(struct got_commit_graph *graph, struct c
 	return err;
 }
 
+static void
+select_displayed_commit(struct tog_log_view_state *s, int selected_idx)
+{
+	struct commit_queue_entry *entry;
+	int ncommits = 0;
+
+	s->selected = selected_idx;
+
+	entry = s->first_displayed_entry;
+	while (entry) {
+		if (ncommits == s->selected) {
+			s->selected_entry = entry;
+			break;
+		}
+		entry = TAILQ_NEXT(entry, entry);
+		ncommits++;
+	}
+}
+
 static const struct got_error *
 draw_commits(struct tog_view *view)
 {
 	const struct got_error *err = NULL;
 	struct tog_log_view_state *s = &view->state.log;
-	struct commit_queue_entry *entry;
+	struct commit_queue_entry *entry = s->first_displayed_entry;
 	const int limit = view->nlines;
 	int width;
 	int ncommits, author_cols = 4;
@@ -1523,17 +1542,6 @@ draw_commits(struct tog_view *view)
 	struct tog_color *tc;
 	static const size_t date_display_cols = 12;
 
-	entry = s->first_displayed_entry;
-	ncommits = 0;
-	while (entry) {
-		if (ncommits == s->selected) {
-			s->selected_entry = entry;
-			break;
-		}
-		entry = TAILQ_NEXT(entry, entry);
-		ncommits++;
-	}
-
 	if (s->selected_entry &&
 	    !(view->searching && view->search_next_done == 0)) {
 		err = got_object_id_str(&id_str, s->selected_entry->id);
@@ -2347,7 +2355,7 @@ input_log_view(struct tog_view **new_view, struct tog_
 		if (s->first_displayed_entry == NULL)
 			break;
 		if (s->selected > 0)
-			s->selected--;
+			select_displayed_commit(s, s->selected - 1);
 		else
 			log_scroll_up(s, 1);
 		break;
@@ -2357,7 +2365,7 @@ input_log_view(struct tog_view **new_view, struct tog_
 			break;
 		if (TAILQ_FIRST(&s->commits.head) ==
 		    s->first_displayed_entry) {
-			s->selected = 0;
+			select_displayed_commit(s, 0);
 			break;
 		}
 		log_scroll_up(s, view->nlines - 1);
@@ -2370,7 +2378,7 @@ input_log_view(struct tog_view **new_view, struct tog_
 			break;
 		if (s->selected < MIN(view->nlines - 2,
 		    s->commits.ncommits - 1)) {
-			s->selected++;
+			select_displayed_commit(s, s->selected + 1);
 			break;
 		}
 		err = log_scroll_down(view, 1);
@@ -2388,17 +2396,17 @@ input_log_view(struct tog_view **new_view, struct tog_
 		    s->selected < MIN(view->nlines - 2,
 		    s->commits.ncommits - 1)) {
 			/* can't scroll further down */
-			s->selected = MIN(view->nlines - 2,
-			    s->commits.ncommits - 1);
+			select_displayed_commit(s,
+			    MIN(view->nlines - 2, s->commits.ncommits - 1));
 		}
 		err = NULL;
 		break;
 	}
 	case KEY_RESIZE:
 		if (s->selected > view->nlines - 2)
-			s->selected = view->nlines - 2;
+			select_displayed_commit(s, view->nlines - 2);
 		if (s->selected > s->commits.ncommits - 1)
-			s->selected = s->commits.ncommits - 1;
+			select_displayed_commit(s, s->commits.ncommits - 1);
 		if (s->commits.ncommits < view->nlines - 1 &&
 		    !s->thread_args.log_complete) {
 			s->thread_args.commits_needed += (view->nlines - 1) -
@@ -3588,7 +3596,6 @@ input_diff_view(struct tog_view **new_view, struct tog
 	const struct got_error *err = NULL;
 	struct tog_diff_view_state *s = &view->state.diff;
 	struct tog_log_view_state *ls;
-	struct commit_queue_entry *entry;
 	int i;
 
 	switch (ch) {
@@ -3661,16 +3668,12 @@ input_diff_view(struct tog_view **new_view, struct tog
 		if (s->log_view == NULL)
 			break;
 		ls = &s->log_view->state.log;
-		entry = TAILQ_PREV(ls->selected_entry,
-		    commit_queue_head, entry);
-		if (entry == NULL)
-			break;
 
 		err = input_log_view(NULL, NULL, s->log_view, KEY_UP);
 		if (err)
 			break;
 
-		err = set_selected_commit(s, entry);
+		err = set_selected_commit(s, ls->selected_entry);
 		if (err)
 			break;
 
@@ -3696,11 +3699,7 @@ input_diff_view(struct tog_view **new_view, struct tog
 		if (err)
 			break;
 
-		entry = TAILQ_NEXT(ls->selected_entry, entry);
-		if (entry == NULL)
-			break;
-
-		err = set_selected_commit(s, entry);
+		err = set_selected_commit(s, ls->selected_entry);
 		if (err)
 			break;