This is a fix of another diff view J keymap case that presents when
selecting the last visible commit in the horizontally split log view and
then toggling fullscreen. The view fails to load the next commit when
entering the J keymap due to too broad a predicate; the line selection
clamp should only be applied when scrolling from a focused, horizontally
split log view and not when scrolling from the diff view with J.

This can be seen by running the added regress without the fix applied,
although it might be best observed with a manual test by running tog in
an 80x24 xterm and entering:

	S	# toggle horizontal split
	4j	# jump to 5th commit
	return	# open diff view in hsplit
	F	# toggle fullscreen diff view
	J	# diff view won't load the next commit

There's also an off-by-one in the calculation to determine how many
commits we need to populate the log, which is revealed if that part of
this diff (i.e., first hunk) is reverted. By entering 20J instead of J
in the above manual test case to continue the scroll beyond the last
loaded (i.e., 24th) commit, the 25th commit will not be loaded--it gets
stuck on the 24th. There's a test added to cover this too.

ok?

-----------------------------------------------
commit 9edd7675a0eaa4433ac94be5e8f2c0c95abc1042
from: Mark Jamsek <mark@jamsek.dev>
date: Tue Jul  2 13:35:38 2024 UTC
 
 fix diff view J keymap to diff the next commit in the log
 
 When the last visible commit in the log view is selected while in a log/diff
 horizontal split, and fullscreen is then toggled in the diff view, the user
 cannot scroll to the next commit with the diff view J keymap due to a bad
 predicate guarding a clamp to prevent scrolling off-screen that should only
 be applied when in the horizontally split log view and not when scrolling
 from the diff view. The fix also reveals an off-by-one in the subsequent
 commit loading logic that also breaks J when attempting to scroll beyond the
 last loaded commit. New regress added to cover these cases.
 
 M  regress/tog/diff.sh  |  115+  6-
 M  tog/tog.c            |    3+  2-

2 files changed, 118 insertions(+), 8 deletions(-)

diff af206a198055a86eba0f4d54303cb456001ff133 9edd7675a0eaa4433ac94be5e8f2c0c95abc1042
commit - af206a198055a86eba0f4d54303cb456001ff133
commit + 9edd7675a0eaa4433ac94be5e8f2c0c95abc1042
blob - 479afa04f1c6c1a787ee26b12295c42f732bfa08
blob + 04f4fa4c3cfbec150ae760c462178f05c608149e
--- regress/tog/diff.sh
+++ regress/tog/diff.sh
@@ -131,9 +131,9 @@ EOF
 	test_done "$testroot" "$ret"
 }
 
-test_diff_J_keymap_on_last_loaded_commit()
+test_diff_J_keymap()
 {
-	test_init diff_J_keymap_on_last_loaded_commit 94 24
+	test_init diff_J_keymap 94 24
 
 	local i=0
 
@@ -142,19 +142,32 @@ test_diff_J_keymap_on_last_loaded_commit()
 	while [ "$i" -lt 32 ]; do
 		echo $i > alpha
 		git commit -aqm $i
+		# Get timestamp, and blob and commit IDs
+		# of the diff views to be screendumped.
 		if [ $i -eq 6 ]; then
 			local id6=$(git_show_head .)
 			local blobid6=$(get_blob_id . "" alpha)
 		elif [ $i -eq 7 ]; then
 			local id7=$(git_show_head .)
 			local blobid7=$(get_blob_id . "" alpha)
-			local author_time=$(git_show_author_time .)
+			local author_time7=$(git_show_author_time .)
+		elif [ $i -eq 25 ]; then
+			local id25=$(git_show_head .)
+			local blobid25=$(get_blob_id . "" alpha)
+		elif [ $i -eq 26 ]; then
+			local id26=$(git_show_head .)
+			local blobid26=$(get_blob_id . "" alpha)
+			local author_time26=$(git_show_author_time .)
 		fi
 		i=$(( i + 1 ))
 	done
 
-	local date=`date -u -r $author_time +"%a %b %e %X %Y UTC"`
+	local date7=`date -u -r $author_time7 +"%a %b %e %X %Y UTC"`
+	local date26=`date -u -r $author_time26 +"%a %b %e %X %Y UTC"`
 
+	# Test that J loads the diff view of the next commit when
+	# currently viewing the last commit loaded in the log view.
+
 	cat <<EOF >$TOG_TEST_SCRIPT
 KEY_ENTER	open diff view of selected commit
 S		toggle horizontal split
@@ -170,7 +183,7 @@ EOF
 [1/20] diff $id6 $id7
 commit $id7
 from: Flan Hacker <flan_hacker@openbsd.org>
-date: $date
+date: $date7
 
 7
 
@@ -202,6 +215,102 @@ EOF
 		return 1
 	fi
 
+	# Test that J loads the diff view of the next commit when currently
+	# viewing the last visible commit in the horizontally split log view.
+
+	cat <<EOF >$TOG_TEST_SCRIPT
+S		toggle horizontal split
+4j		move to last visible commit when in horizontal split
+KEY_ENTER	open diff view of selected commit
+F		toggle fullscreen
+J		move down to next commit in the log
+SCREENDUMP
+EOF
+
+	cat <<EOF >$testroot/view.expected
+[1/20] diff $id25 $id26
+commit $id26
+from: Flan Hacker <flan_hacker@openbsd.org>
+date: $date26
+
+26
+
+M  alpha  |  1+  1-
+
+1 file changed, 1 insertion(+), 1 deletion(-)
+
+commit - $id25
+commit + $id26
+blob - $blobid25
+blob + $blobid26
+--- alpha
++++ alpha
+@@ -1 +1 @@
+-25
++26
+
+
+
+(END)
+EOF
+
+	tog log
+	cmp -s $testroot/view.expected $testroot/view
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/view.expected $testroot/view
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	# Test J correctly requests the log to load more commits when
+	# scrolling beyond the last loaded commit from the diff view.
+
+	cat <<EOF >$TOG_TEST_SCRIPT
+S		toggle horizontal split
+4j		move to the 5th commit
+KEY_ENTER	open diff view of selected commit
+F		toggle fullscreen
+20J		scroll down and load diff of the 25th commit
+SCREENDUMP
+EOF
+
+	cat <<EOF >$testroot/view.expected
+[1/20] diff $id6 $id7
+commit $id7
+from: Flan Hacker <flan_hacker@openbsd.org>
+date: $date7
+
+7
+
+M  alpha  |  1+  1-
+
+1 file changed, 1 insertion(+), 1 deletion(-)
+
+commit - $id6
+commit + $id7
+blob - $blobid6
+blob + $blobid7
+--- alpha
++++ alpha
+@@ -1 +1 @@
+-6
++7
+
+
+
+(END)
+EOF
+
+	tog log
+	cmp -s $testroot/view.expected $testroot/view
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/view.expected $testroot/view
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
 	test_done "$testroot" "$ret"
 }
 
@@ -377,5 +486,5 @@ test_diff_commit_keywords()
 test_parseargs "$@"
 run_test test_diff_contiguous_commits
 run_test test_diff_arbitrary_commits
-run_test test_diff_J_keymap_on_last_loaded_commit
+run_test test_diff_J_keymap
 run_test test_diff_commit_keywords
blob - 499fad18a7c542dd22ce8d4b489207884559d884
blob + b8146cfeb30ec9156a8c21c672e2e62f2b8666cd
--- tog/tog.c
+++ tog/tog.c
@@ -3099,7 +3099,7 @@ log_scroll_down(struct tog_view *view, int maxscroll)
 	if (s->last_displayed_entry == NULL)
 		return NULL;
 
-	ncommits_needed = s->last_displayed_entry->idx + 1 + maxscroll;
+	ncommits_needed = s->last_displayed_entry->idx + 2 + maxscroll;
 	if (s->commits->ncommits < ncommits_needed &&
 	    !s->thread_args.log_complete) {
 		/*
@@ -4013,7 +4013,8 @@ log_move_cursor_down(struct tog_view *view, int page)
 	 * We might necessarily overshoot in horizontal
 	 * splits; if so, select the last displayed commit.
 	 */
-	if (s->first_displayed_entry && s->last_displayed_entry) {
+	if (view_is_hsplit_top(view) && s->first_displayed_entry &&
+	    s->last_displayed_entry) {
 		s->selected = MIN(s->selected,
 		    s->last_displayed_entry->idx -
 		    s->first_displayed_entry->idx);


-- 
Mark Jamsek <https://bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68