commit aed1f7694fc4740d880a503eac8ebe1f6c4f3eca from: Mark Jamsek via: Thomas Adam date: Mon Jul 08 16:17:05 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 that's only applicable in the hsplit case 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. ok op@ (caveated that he's unsure exactly why the OB1 fix works) commit - 3f7ec816e916db9f4c429fe58385f2973e8469b0 commit + aed1f7694fc4740d880a503eac8ebe1f6c4f3eca 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 <$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 -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 <$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 <$testroot/view.expected +[1/20] diff $id25 $id26 +commit $id26 +from: Flan Hacker +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 <$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 <$testroot/view.expected +[1/20] diff $id6 $id7 +commit $id7 +from: Flan Hacker +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 - 094e8372a6a869db30b8b7943645cdb78c02103c blob + 181280f8ea352b18d6cf042612ac8ac22591aaae --- tog/tog.c +++ tog/tog.c @@ -3103,7 +3103,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) { /* @@ -4017,7 +4017,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);