From c0bec06cfedb930ecb3cd8c8cdb0f3e14e5e9308 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 24 Sep 2025 13:57:15 -0700 Subject: [PATCH 01/12] diff --no-index: fix logic for paths ending in '/' If one of the two provided paths for git diff --no-index ends in a '/', a failure similar to the following occurs: $ git diff --no-index -- /tmp/ /tmp/ ':!' fatal: `pos + len' is too far after the end of the buffer This occurs because of an incorrect calculation of the skip lengths in diff_no_index(). The code wants to calculate the length of the string, but add one in case the string doesn't end with a slash. The method it uses is incorrect, as it always checks the trailing NUL character of the string. This will never be a '/', so we always add one. In the event that we *do* have a trailing slash, this will create an off-by-one length error later when using the skip value. The most straightforward fix would be to correct the skip1 and skip2 lengths by using ends_with(). However, Johannes made a good point that the existing logic is wasting a lot of computation. We generate the match string by copying the path in and then skipping almost all of it immediately with a potentially expensive memmove() from the strbuf_remove() call. We also re-initialize the match stringbuf each time we call read_directory_contents. The read_directory_contents really wants a path that is rooted at the start of the directory scan. We're currently building this by taking the full path and stripping out the start portion. Instead, replace this logic by building up the portion of the match as we go. Start by initializing two strbuf in diff_no_index containing the empty string. Pass these into queue_diff, which in turn passes the appropriate left or right side into read_directory_contents. As before, we build up the matches by appending elements to the match path and then clearing them using strbuf_setlen. In the recursive portion of the queue_diff algorithm, we build up new match paths the same way that we build up new buffer paths, by appending the elements and then clearing them with strbuf_setlen after each iteration. This is cheaper as it avoids repeated allocations, and is a bit simpler to track what is going on. Add a couple of test cases that pass in paths already ending in '/', to ensure the tests cover this regression. Reported-by: Johannes Schindelin Closes: https://lore.kernel.org/git/c75ec5f9-407a-6555-d4fb-bb629d54ec61@gmx.de/ Signed-off-by: Jacob Keller [jc: small leakfixes at the end of diff_no_index()] Signed-off-by: Junio C Hamano --- diff-no-index.c | 63 ++++++++++++++++++++++------------------ t/t4053-diff-no-index.sh | 16 ++++++++++ 2 files changed, 51 insertions(+), 28 deletions(-) diff --git a/diff-no-index.c b/diff-no-index.c index 88ae4cee56ba41..f320424f05fe0f 100644 --- a/diff-no-index.c +++ b/diff-no-index.c @@ -21,30 +21,21 @@ static int read_directory_contents(const char *path, struct string_list *list, const struct pathspec *pathspec, - int skip) + struct strbuf *match) { - struct strbuf match = STRBUF_INIT; - int len; + int len = match->len; DIR *dir; struct dirent *e; if (!(dir = opendir(path))) return error("Could not open directory %s", path); - if (pathspec) { - strbuf_addstr(&match, path); - strbuf_complete(&match, '/'); - strbuf_remove(&match, 0, skip); - - len = match.len; - } - while ((e = readdir_skip_dot_and_dotdot(dir))) { if (pathspec) { int is_dir = 0; - strbuf_setlen(&match, len); - strbuf_addstr(&match, e->d_name); + strbuf_setlen(match, len); + strbuf_addstr(match, e->d_name); if (NOT_CONSTANT(DTYPE(e)) != DT_UNKNOWN) { is_dir = (DTYPE(e) == DT_DIR); } else { @@ -57,7 +48,7 @@ static int read_directory_contents(const char *path, struct string_list *list, } if (!match_leading_pathspec(NULL, pathspec, - match.buf, match.len, + match->buf, match->len, 0, NULL, is_dir)) continue; } @@ -65,7 +56,7 @@ static int read_directory_contents(const char *path, struct string_list *list, string_list_insert(list, e->d_name); } - strbuf_release(&match); + strbuf_setlen(match, len); closedir(dir); return 0; } @@ -169,7 +160,8 @@ static struct diff_filespec *noindex_filespec(const struct git_hash_algo *algop, static int queue_diff(struct diff_options *o, const struct git_hash_algo *algop, const char *name1, const char *name2, int recursing, - const struct pathspec *ps, int skip1, int skip2) + const struct pathspec *ps, + struct strbuf *ps_match1, struct strbuf *ps_match2) { int mode1 = 0, mode2 = 0; enum special special1 = SPECIAL_NONE, special2 = SPECIAL_NONE; @@ -208,10 +200,12 @@ static int queue_diff(struct diff_options *o, const struct git_hash_algo *algop, struct string_list p2 = STRING_LIST_INIT_DUP; int i1, i2, ret = 0; size_t len1 = 0, len2 = 0; + size_t match1_len = ps_match1->len; + size_t match2_len = ps_match2->len; - if (name1 && read_directory_contents(name1, &p1, ps, skip1)) + if (name1 && read_directory_contents(name1, &p1, ps, ps_match1)) return -1; - if (name2 && read_directory_contents(name2, &p2, ps, skip2)) { + if (name2 && read_directory_contents(name2, &p2, ps, ps_match2)) { string_list_clear(&p1, 0); return -1; } @@ -235,6 +229,11 @@ static int queue_diff(struct diff_options *o, const struct git_hash_algo *algop, strbuf_setlen(&buffer1, len1); strbuf_setlen(&buffer2, len2); + if (ps) { + strbuf_setlen(ps_match1, match1_len); + strbuf_setlen(ps_match2, match2_len); + } + if (i1 == p1.nr) comp = 1; else if (i2 == p2.nr) @@ -245,18 +244,28 @@ static int queue_diff(struct diff_options *o, const struct git_hash_algo *algop, if (comp > 0) n1 = NULL; else { - strbuf_addstr(&buffer1, p1.items[i1++].string); + strbuf_addstr(&buffer1, p1.items[i1].string); + if (ps) { + strbuf_addstr(ps_match1, p1.items[i1].string); + strbuf_complete(ps_match1, '/'); + } n1 = buffer1.buf; + i1++; } if (comp < 0) n2 = NULL; else { - strbuf_addstr(&buffer2, p2.items[i2++].string); + strbuf_addstr(&buffer2, p2.items[i2].string); + if (ps) { + strbuf_addstr(ps_match2, p2.items[i2].string); + strbuf_complete(ps_match2, '/'); + } n2 = buffer2.buf; + i2++; } - ret = queue_diff(o, algop, n1, n2, 1, ps, skip1, skip2); + ret = queue_diff(o, algop, n1, n2, 1, ps, ps_match1, ps_match2); } string_list_clear(&p1, 0); string_list_clear(&p2, 0); @@ -346,7 +355,8 @@ int diff_no_index(struct rev_info *revs, const struct git_hash_algo *algop, int implicit_no_index, int argc, const char **argv) { struct pathspec pathspec, *ps = NULL; - int i, no_index, skip1 = 0, skip2 = 0; + struct strbuf ps_match1 = STRBUF_INIT, ps_match2 = STRBUF_INIT; + int i, no_index; int ret = 1; const char *paths[2]; char *to_free[ARRAY_SIZE(paths)] = { 0 }; @@ -387,11 +397,6 @@ int diff_no_index(struct rev_info *revs, const struct git_hash_algo *algop, NULL, &argv[2]); if (pathspec.nr) ps = &pathspec; - - skip1 = strlen(paths[0]); - skip1 += paths[0][skip1] == '/' ? 0 : 1; - skip2 = strlen(paths[1]); - skip2 += paths[1][skip2] == '/' ? 0 : 1; } else if (argc > 2) { warning(_("Limiting comparison with pathspecs is only " "supported if both paths are directories.")); @@ -415,7 +420,7 @@ int diff_no_index(struct rev_info *revs, const struct git_hash_algo *algop, revs->diffopt.flags.exit_with_status = 1; if (queue_diff(&revs->diffopt, algop, paths[0], paths[1], 0, ps, - skip1, skip2)) + &ps_match1, &ps_match2)) goto out; diff_set_mnemonic_prefix(&revs->diffopt, "1/", "2/"); diffcore_std(&revs->diffopt); @@ -431,6 +436,8 @@ int diff_no_index(struct rev_info *revs, const struct git_hash_algo *algop, for (i = 0; i < ARRAY_SIZE(to_free); i++) free(to_free[i]); strbuf_release(&replacement); + strbuf_release(&ps_match1); + strbuf_release(&ps_match2); if (ps) clear_pathspec(ps); return ret; diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh index 01db9243abfe4f..e0ea437685b0ab 100755 --- a/t/t4053-diff-no-index.sh +++ b/t/t4053-diff-no-index.sh @@ -322,6 +322,22 @@ test_expect_success 'diff --no-index with pathspec' ' test_cmp expect actual ' +test_expect_success 'diff --no-index first path ending in slash with pathspec' ' + test_expect_code 1 git diff --name-status --no-index a/ b 1 >actual && + cat >expect <<-EOF && + D a/1 + EOF + test_cmp expect actual +' + +test_expect_success 'diff --no-index second path ending in slash with pathspec' ' + test_expect_code 1 git diff --name-status --no-index a b/ 1 >actual && + cat >expect <<-EOF && + D a/1 + EOF + test_cmp expect actual +' + test_expect_success 'diff --no-index with pathspec no matches' ' test_expect_code 0 git diff --name-status --no-index a b missing ' From cf680cdb9543095bf75eefce7489c34282506353 Mon Sep 17 00:00:00 2001 From: Ezekiel Newren Date: Thu, 2 Oct 2025 23:27:26 +0000 Subject: [PATCH 02/12] make: delete XDIFF_LIB, add xdiff to LIB_OBJS In a future patch series the 'xdiff' Rust crate will be added. Delete the creation of the static library file for xdiff to avoid a name conflict. This also moves toward the goal of Rust only needing to link against libgit.a. Changes to Meson are not required as the xdiff library is already included in Meson's libgit.a. xdiff-objs was a historical make target to allow building just the objects in xdiff. Since it was defined in terms of XDIFF_OBJS (which no longer exists) this convenience make target no longer makes sense. Remove it. Signed-off-by: Ezekiel Newren Signed-off-by: Junio C Hamano --- Makefile | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/Makefile b/Makefile index 4c95affadb5e26..1a607eff939761 100644 --- a/Makefile +++ b/Makefile @@ -916,7 +916,6 @@ export PYTHON_PATH TEST_SHELL_PATH = $(SHELL_PATH) LIB_FILE = libgit.a -XDIFF_LIB = xdiff/lib.a REFTABLE_LIB = reftable/libreftable.a GENERATED_H += command-list.h @@ -1207,6 +1206,13 @@ LIB_OBJS += write-or-die.o LIB_OBJS += ws.o LIB_OBJS += wt-status.o LIB_OBJS += xdiff-interface.o +LIB_OBJS += xdiff/xdiffi.o +LIB_OBJS += xdiff/xemit.o +LIB_OBJS += xdiff/xhistogram.o +LIB_OBJS += xdiff/xmerge.o +LIB_OBJS += xdiff/xpatience.o +LIB_OBJS += xdiff/xprepare.o +LIB_OBJS += xdiff/xutils.o BUILTIN_OBJS += builtin/add.o BUILTIN_OBJS += builtin/am.o @@ -1388,8 +1394,8 @@ CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/test-lib.o -# xdiff and reftable libs may in turn depend on what is in libgit.a -GITLIBS = common-main.o $(LIB_FILE) $(XDIFF_LIB) $(REFTABLE_LIB) $(LIB_FILE) +# reftable lib may in turn depend on what is in libgit.a +GITLIBS = common-main.o $(LIB_FILE) $(REFTABLE_LIB) $(LIB_FILE) EXTLIBS = GIT_USER_AGENT = git/$(GIT_VERSION) @@ -2721,16 +2727,6 @@ reconfigure config.mak.autogen: config.status .PHONY: reconfigure # This is a convenience target. endif -XDIFF_OBJS += xdiff/xdiffi.o -XDIFF_OBJS += xdiff/xemit.o -XDIFF_OBJS += xdiff/xhistogram.o -XDIFF_OBJS += xdiff/xmerge.o -XDIFF_OBJS += xdiff/xpatience.o -XDIFF_OBJS += xdiff/xprepare.o -XDIFF_OBJS += xdiff/xutils.o -.PHONY: xdiff-objs -xdiff-objs: $(XDIFF_OBJS) - REFTABLE_OBJS += reftable/basics.o REFTABLE_OBJS += reftable/error.o REFTABLE_OBJS += reftable/block.o @@ -2765,7 +2761,6 @@ OBJECTS += $(GIT_OBJS) OBJECTS += $(SCALAR_OBJS) OBJECTS += $(PROGRAM_OBJS) OBJECTS += $(TEST_OBJS) -OBJECTS += $(XDIFF_OBJS) OBJECTS += $(FUZZ_OBJS) OBJECTS += $(REFTABLE_OBJS) $(REFTABLE_TEST_OBJS) OBJECTS += $(UNIT_TEST_OBJS) @@ -2919,9 +2914,6 @@ scalar$X: scalar.o GIT-LDFLAGS $(GITLIBS) $(LIB_FILE): $(LIB_OBJS) $(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^ -$(XDIFF_LIB): $(XDIFF_OBJS) - $(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^ - $(REFTABLE_LIB): $(REFTABLE_OBJS) $(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^ @@ -3763,7 +3755,7 @@ clean: profile-clean coverage-clean cocciclean $(RM) git.rc git.res $(RM) $(OBJECTS) $(RM) headless-git.o - $(RM) $(LIB_FILE) $(XDIFF_LIB) $(REFTABLE_LIB) + $(RM) $(LIB_FILE) $(REFTABLE_LIB) $(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) $(RM) $(TEST_PROGRAMS) $(RM) $(FUZZ_PROGRAMS) @@ -3957,7 +3949,6 @@ endif LIBGIT_PUB_OBJS += contrib/libgit-sys/public_symbol_export.o LIBGIT_PUB_OBJS += libgit.a LIBGIT_PUB_OBJS += reftable/libreftable.a -LIBGIT_PUB_OBJS += xdiff/lib.a LIBGIT_PARTIAL_EXPORT = contrib/libgit-sys/partial_symbol_export.o From f3b4c89d59f15f3b67f29bff6f1f53dbc11a5b58 Mon Sep 17 00:00:00 2001 From: Ezekiel Newren Date: Thu, 2 Oct 2025 23:27:27 +0000 Subject: [PATCH 03/12] make: delete REFTABLE_LIB, add reftable to LIB_OBJS Same idea as the previous commit except that I don't know when or if reftable will be turned into a Rust crate. Signed-off-by: Ezekiel Newren Signed-off-by: Junio C Hamano --- Makefile | 39 ++++++++++++++++----------------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/Makefile b/Makefile index 1a607eff939761..a6a14b038bb946 100644 --- a/Makefile +++ b/Makefile @@ -916,7 +916,6 @@ export PYTHON_PATH TEST_SHELL_PATH = $(SHELL_PATH) LIB_FILE = libgit.a -REFTABLE_LIB = reftable/libreftable.a GENERATED_H += command-list.h GENERATED_H += config-list.h @@ -1134,6 +1133,19 @@ LIB_OBJS += refs/iterator.o LIB_OBJS += refs/packed-backend.o LIB_OBJS += refs/ref-cache.o LIB_OBJS += refspec.o +LIB_OBJS += reftable/basics.o +LIB_OBJS += reftable/error.o +LIB_OBJS += reftable/block.o +LIB_OBJS += reftable/blocksource.o +LIB_OBJS += reftable/iter.o +LIB_OBJS += reftable/merged.o +LIB_OBJS += reftable/pq.o +LIB_OBJS += reftable/record.o +LIB_OBJS += reftable/stack.o +LIB_OBJS += reftable/system.o +LIB_OBJS += reftable/table.o +LIB_OBJS += reftable/tree.o +LIB_OBJS += reftable/writer.o LIB_OBJS += remote.o LIB_OBJS += replace-object.o LIB_OBJS += repo-settings.o @@ -1394,8 +1406,7 @@ CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/test-lib.o -# reftable lib may in turn depend on what is in libgit.a -GITLIBS = common-main.o $(LIB_FILE) $(REFTABLE_LIB) $(LIB_FILE) +GITLIBS = common-main.o $(LIB_FILE) EXTLIBS = GIT_USER_AGENT = git/$(GIT_VERSION) @@ -2727,20 +2738,6 @@ reconfigure config.mak.autogen: config.status .PHONY: reconfigure # This is a convenience target. endif -REFTABLE_OBJS += reftable/basics.o -REFTABLE_OBJS += reftable/error.o -REFTABLE_OBJS += reftable/block.o -REFTABLE_OBJS += reftable/blocksource.o -REFTABLE_OBJS += reftable/iter.o -REFTABLE_OBJS += reftable/merged.o -REFTABLE_OBJS += reftable/pq.o -REFTABLE_OBJS += reftable/record.o -REFTABLE_OBJS += reftable/stack.o -REFTABLE_OBJS += reftable/system.o -REFTABLE_OBJS += reftable/table.o -REFTABLE_OBJS += reftable/tree.o -REFTABLE_OBJS += reftable/writer.o - TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS)) .PHONY: test-objs @@ -2762,7 +2759,7 @@ OBJECTS += $(SCALAR_OBJS) OBJECTS += $(PROGRAM_OBJS) OBJECTS += $(TEST_OBJS) OBJECTS += $(FUZZ_OBJS) -OBJECTS += $(REFTABLE_OBJS) $(REFTABLE_TEST_OBJS) +OBJECTS += $(REFTABLE_TEST_OBJS) OBJECTS += $(UNIT_TEST_OBJS) OBJECTS += $(CLAR_TEST_OBJS) OBJECTS += $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(UNIT_TEST_PROGRAMS)) @@ -2914,9 +2911,6 @@ scalar$X: scalar.o GIT-LDFLAGS $(GITLIBS) $(LIB_FILE): $(LIB_OBJS) $(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^ -$(REFTABLE_LIB): $(REFTABLE_OBJS) - $(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^ - export DEFAULT_EDITOR DEFAULT_PAGER Documentation/GIT-EXCLUDED-PROGRAMS: FORCE @@ -3755,7 +3749,7 @@ clean: profile-clean coverage-clean cocciclean $(RM) git.rc git.res $(RM) $(OBJECTS) $(RM) headless-git.o - $(RM) $(LIB_FILE) $(REFTABLE_LIB) + $(RM) $(LIB_FILE) $(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) $(RM) $(TEST_PROGRAMS) $(RM) $(FUZZ_PROGRAMS) @@ -3948,7 +3942,6 @@ endif LIBGIT_PUB_OBJS += contrib/libgit-sys/public_symbol_export.o LIBGIT_PUB_OBJS += libgit.a -LIBGIT_PUB_OBJS += reftable/libreftable.a LIBGIT_PARTIAL_EXPORT = contrib/libgit-sys/partial_symbol_export.o From 2c3cc43f96f9568d5475e46bd1442c5551129ce8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Mon, 6 Oct 2025 19:19:23 +0200 Subject: [PATCH 04/12] add-patch: improve help for options j, J, k, and K MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The options j, J, k, and K don't affect the status of the current hunk. They just go to a different one. This is true whether the current hunk is undecided or not. Avoid misunderstanding by no longer mentioning the current hunk explicitly in their help texts. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- Documentation/git-add.adoc | 8 ++++---- add-patch.c | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Documentation/git-add.adoc b/Documentation/git-add.adoc index ad629c46c5f39a..3266ccf105f31e 100644 --- a/Documentation/git-add.adoc +++ b/Documentation/git-add.adoc @@ -342,10 +342,10 @@ patch:: d - do not stage this hunk or any of the later hunks in the file g - select a hunk to go to / - search for a hunk matching the given regex - j - leave this hunk undecided, see next undecided hunk - J - leave this hunk undecided, see next hunk - k - leave this hunk undecided, see previous undecided hunk - K - leave this hunk undecided, see previous hunk + j - go to the next undecided hunk + J - go to the next hunk + k - go to the previous undecided hunk + K - go to the previous hunk s - split the current hunk into smaller hunks e - manually edit the current hunk p - print the current hunk diff --git a/add-patch.c b/add-patch.c index b0389c5d5bad6d..912266a3f89a47 100644 --- a/add-patch.c +++ b/add-patch.c @@ -1397,10 +1397,10 @@ static size_t display_hunks(struct add_p_state *s, } static const char help_patch_remainder[] = -N_("j - leave this hunk undecided, see next undecided hunk\n" - "J - leave this hunk undecided, see next hunk\n" - "k - leave this hunk undecided, see previous undecided hunk\n" - "K - leave this hunk undecided, see previous hunk\n" +N_("j - go to the next undecided hunk\n" + "J - go to the next hunk\n" + "k - go to the previous undecided hunk\n" + "K - go to the previous hunk\n" "g - select a hunk to go to\n" "/ - search for a hunk matching the given regex\n" "s - split the current hunk into smaller hunks\n" From c309b65a7c8a0dc8a1566ac3587d37d935632e4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Mon, 6 Oct 2025 19:20:31 +0200 Subject: [PATCH 05/12] add-patch: document that option J rolls over MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The variable "permitted" is not reset after moving to a different hunk, so it only accumulates permission and doesn't necessarily reflect those of the current hunk. This may be a bug, but is actually useful with the option J, which can be used at the last hunk to roll over to the first hunk. Make this particular behavior official. Also adjust the error message, as it will only be shown if there's just a single hunk. Suggested-by: Junio C Hamano Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- Documentation/git-add.adoc | 2 +- add-patch.c | 6 +++--- t/t3701-add-interactive.sh | 18 ++++++++++++++---- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/Documentation/git-add.adoc b/Documentation/git-add.adoc index 3266ccf105f31e..5c05a3a7f9dffb 100644 --- a/Documentation/git-add.adoc +++ b/Documentation/git-add.adoc @@ -343,7 +343,7 @@ patch:: g - select a hunk to go to / - search for a hunk matching the given regex j - go to the next undecided hunk - J - go to the next hunk + J - go to the next hunk, roll over at the bottom k - go to the previous undecided hunk K - go to the previous hunk s - split the current hunk into smaller hunks diff --git a/add-patch.c b/add-patch.c index 912266a3f89a47..1f466ec9c08c86 100644 --- a/add-patch.c +++ b/add-patch.c @@ -1398,7 +1398,7 @@ static size_t display_hunks(struct add_p_state *s, static const char help_patch_remainder[] = N_("j - go to the next undecided hunk\n" - "J - go to the next hunk\n" + "J - go to the next hunk, roll over at the bottom\n" "k - go to the previous undecided hunk\n" "K - go to the previous hunk\n" "g - select a hunk to go to\n" @@ -1493,7 +1493,7 @@ static int patch_update_file(struct add_p_state *s, permitted |= ALLOW_GOTO_NEXT_UNDECIDED_HUNK; strbuf_addstr(&s->buf, ",j"); } - if (hunk_index + 1 < file_diff->hunk_nr) { + if (file_diff->hunk_nr > 1) { permitted |= ALLOW_GOTO_NEXT_HUNK; strbuf_addstr(&s->buf, ",J"); } @@ -1584,7 +1584,7 @@ static int patch_update_file(struct add_p_state *s, if (permitted & ALLOW_GOTO_NEXT_HUNK) hunk_index++; else - err(s, _("No next hunk")); + err(s, _("No other hunk")); } else if (s->answer.buf[0] == 'k') { if (permitted & ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK) hunk_index = undecided_previous; diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index d9fe289a7ad13a..d5d2e120ab34ed 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -334,7 +334,7 @@ test_expect_success 'different prompts for mode change/deleted' ' cat >expect <<-\EOF && (1/1) Stage deletion [y,n,q,a,d,p,?]? (1/2) Stage mode change [y,n,q,a,d,j,J,g,/,p,?]? - (2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,p,?]? + (2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,?]? EOF test_cmp expect actual.filtered ' @@ -521,7 +521,7 @@ test_expect_success 'split hunk setup' ' test_expect_success 'goto hunk 1 with "g 1"' ' test_when_finished "git reset" && tr _ " " >expect <<-EOF && - (2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,p,?]? + 1: -1,2 +1,3 +15 + (2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,?]? + 1: -1,2 +1,3 +15 _ 2: -2,4 +3,8 +21 go to which hunk? @@ -1,2 +1,3 @@ _10 @@ -550,7 +550,7 @@ test_expect_success 'goto hunk 1 with "g1"' ' test_expect_success 'navigate to hunk via regex /pattern' ' test_when_finished "git reset" && tr _ " " >expect <<-EOF && - (2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,p,?]? @@ -1,2 +1,3 @@ + (2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,?]? @@ -1,2 +1,3 @@ _10 +15 _20 @@ -805,7 +805,7 @@ test_expect_success 'colors can be overridden' ' (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]? @@ -3 +3,2 @@ more-context +another-one - (2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,p,?]? @@ -1,3 +1,3 @@ + (2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,?]? @@ -1,3 +1,3 @@ context -old +new @@ -1354,4 +1354,14 @@ do ' done +test_expect_success 'option J rolls over' ' + test_write_lines a b c d e f g h i >file && + git add file && + test_write_lines X b c d e f g h X >file && + test_write_lines J J q | git add -p >out && + test_write_lines 1 2 1 >expect && + sed -n -e "s-/.*--" -e "s/^(//p" actual && + test_cmp expect actual +' + test_done From 171c1688ccbe5e6d709444a65a5ca2e0a9175b16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Mon, 6 Oct 2025 19:21:19 +0200 Subject: [PATCH 06/12] add-patch: let options y, n, j, and e roll over to next undecided MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The options y, n, and e mark the current hunk as decided. If there's another undecided hunk towards the bottom of the hunk array they go there. If there isn't, but there is another undecided hunk towards the top then they go to the very first hunk, no matter if it has already been decided on. The option j does basically the same move. Technically it is not allowed if there's no undecided hunk towards the bottom, but the variable "permitted" is never reset, so this permission is retained from the very first hunk. That may a bug, but this behavior is at least consistent with y, n, and e and arguably more useful than refusing to move. Improve the roll-over behavior of these four options by moving to the first undecided hunk instead of hunk 1, consistent with what they do when not rolling over. Also adjust the error message for j, as it will only be shown if there's no other undecided hunk in either direction. Reported-by: Windl, Ulrich Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- Documentation/git-add.adoc | 2 +- add-patch.c | 13 ++++++++++--- t/t3701-add-interactive.sh | 22 ++++++++++++++++++++++ 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/Documentation/git-add.adoc b/Documentation/git-add.adoc index 5c05a3a7f9dffb..596cdeff93de8b 100644 --- a/Documentation/git-add.adoc +++ b/Documentation/git-add.adoc @@ -342,7 +342,7 @@ patch:: d - do not stage this hunk or any of the later hunks in the file g - select a hunk to go to / - search for a hunk matching the given regex - j - go to the next undecided hunk + j - go to the next undecided hunk, roll over at the bottom J - go to the next hunk, roll over at the bottom k - go to the previous undecided hunk K - go to the previous hunk diff --git a/add-patch.c b/add-patch.c index 1f466ec9c08c86..106bfcb275377b 100644 --- a/add-patch.c +++ b/add-patch.c @@ -1397,7 +1397,7 @@ static size_t display_hunks(struct add_p_state *s, } static const char help_patch_remainder[] = -N_("j - go to the next undecided hunk\n" +N_("j - go to the next undecided hunk, roll over at the bottom\n" "J - go to the next hunk, roll over at the bottom\n" "k - go to the previous undecided hunk\n" "K - go to the previous hunk\n" @@ -1408,6 +1408,11 @@ N_("j - go to the next undecided hunk\n" "p - print the current hunk, 'P' to use the pager\n" "? - print help\n"); +static size_t inc_mod(size_t a, size_t m) +{ + return a < m - 1 ? a + 1 : 0; +} + static int patch_update_file(struct add_p_state *s, struct file_diff *file_diff) { @@ -1451,7 +1456,9 @@ static int patch_update_file(struct add_p_state *s, break; } - for (i = hunk_index + 1; i < file_diff->hunk_nr; i++) + for (i = inc_mod(hunk_index, file_diff->hunk_nr); + i != hunk_index; + i = inc_mod(i, file_diff->hunk_nr)) if (file_diff->hunk[i].use == UNDECIDED_HUNK) { undecided_next = i; break; @@ -1594,7 +1601,7 @@ static int patch_update_file(struct add_p_state *s, if (permitted & ALLOW_GOTO_NEXT_UNDECIDED_HUNK) hunk_index = undecided_next; else - err(s, _("No next hunk")); + err(s, _("No other undecided hunk")); } else if (s->answer.buf[0] == 'g') { char *pend; unsigned long response; diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index d5d2e120ab34ed..8086d3da71405d 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -1364,4 +1364,26 @@ test_expect_success 'option J rolls over' ' test_cmp expect actual ' +test_expect_success 'options y, n, j, e roll over to next undecided (1)' ' + test_write_lines a b c d e f g h i j k l m n o p q >file && + git add file && + test_write_lines X b c d e f g h X j k l m n o p X >file && + test_set_editor : && + test_write_lines g3 y g3 n g3 j g3 e q | git add -p >out && + test_write_lines 1 3 1 3 1 3 1 3 1 >expect && + sed -n -e "s-/.*--" -e "s/^(//p" actual && + test_cmp expect actual +' + +test_expect_success 'options y, n, j, e roll over to next undecided (2)' ' + test_write_lines a b c d e f g h i j k l m n o p q >file && + git add file && + test_write_lines X b c d e f g h X j k l m n o p X >file && + test_set_editor : && + test_write_lines y g3 y g3 n g3 j g3 e q | git add -p >out && + test_write_lines 1 2 3 2 3 2 3 2 3 2 >expect && + sed -n -e "s-/.*--" -e "s/^(//p" actual && + test_cmp expect actual +' + test_done From 1967b60681256ed452ed70dedf381b5380697901 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Mon, 6 Oct 2025 19:22:38 +0200 Subject: [PATCH 07/12] add-patch: let options k and K roll over like j and J MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Options j and J roll over at the bottom and go to the first undecided hunk and hunk 1, respectively. Let options k and K do the same when they reach the top of the hunk array, so let them go to the last undecided hunk and the last hunk, respectively, for consistency. Also use the same direction-neutral error messages. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- Documentation/git-add.adoc | 4 ++-- add-patch.c | 22 ++++++++++++++------- t/t3701-add-interactive.sh | 40 +++++++++++++++++++------------------- 3 files changed, 37 insertions(+), 29 deletions(-) diff --git a/Documentation/git-add.adoc b/Documentation/git-add.adoc index 596cdeff93de8b..3116a2cac548d9 100644 --- a/Documentation/git-add.adoc +++ b/Documentation/git-add.adoc @@ -344,8 +344,8 @@ patch:: / - search for a hunk matching the given regex j - go to the next undecided hunk, roll over at the bottom J - go to the next hunk, roll over at the bottom - k - go to the previous undecided hunk - K - go to the previous hunk + k - go to the previous undecided hunk, roll over at the top + K - go to the previous hunk, roll over at the top s - split the current hunk into smaller hunks e - manually edit the current hunk p - print the current hunk diff --git a/add-patch.c b/add-patch.c index 106bfcb275377b..4f314c16ec824f 100644 --- a/add-patch.c +++ b/add-patch.c @@ -1399,8 +1399,8 @@ static size_t display_hunks(struct add_p_state *s, static const char help_patch_remainder[] = N_("j - go to the next undecided hunk, roll over at the bottom\n" "J - go to the next hunk, roll over at the bottom\n" - "k - go to the previous undecided hunk\n" - "K - go to the previous hunk\n" + "k - go to the previous undecided hunk, roll over at the top\n" + "K - go to the previous hunk, roll over at the top\n" "g - select a hunk to go to\n" "/ - search for a hunk matching the given regex\n" "s - split the current hunk into smaller hunks\n" @@ -1408,6 +1408,11 @@ N_("j - go to the next undecided hunk, roll over at the bottom\n" "p - print the current hunk, 'P' to use the pager\n" "? - print help\n"); +static size_t dec_mod(size_t a, size_t m) +{ + return a > 0 ? a - 1 : m - 1; +} + static size_t inc_mod(size_t a, size_t m) { return a < m - 1 ? a + 1 : 0; @@ -1450,7 +1455,9 @@ static int patch_update_file(struct add_p_state *s, undecided_next = -1; if (file_diff->hunk_nr) { - for (i = hunk_index - 1; i >= 0; i--) + for (i = dec_mod(hunk_index, file_diff->hunk_nr); + i != hunk_index; + i = dec_mod(i, file_diff->hunk_nr)) if (file_diff->hunk[i].use == UNDECIDED_HUNK) { undecided_previous = i; break; @@ -1492,7 +1499,7 @@ static int patch_update_file(struct add_p_state *s, permitted |= ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK; strbuf_addstr(&s->buf, ",k"); } - if (hunk_index) { + if (file_diff->hunk_nr > 1) { permitted |= ALLOW_GOTO_PREVIOUS_HUNK; strbuf_addstr(&s->buf, ",K"); } @@ -1584,9 +1591,10 @@ static int patch_update_file(struct add_p_state *s, } } else if (s->answer.buf[0] == 'K') { if (permitted & ALLOW_GOTO_PREVIOUS_HUNK) - hunk_index--; + hunk_index = dec_mod(hunk_index, + file_diff->hunk_nr); else - err(s, _("No previous hunk")); + err(s, _("No other hunk")); } else if (s->answer.buf[0] == 'J') { if (permitted & ALLOW_GOTO_NEXT_HUNK) hunk_index++; @@ -1596,7 +1604,7 @@ static int patch_update_file(struct add_p_state *s, if (permitted & ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK) hunk_index = undecided_previous; else - err(s, _("No previous hunk")); + err(s, _("No other undecided hunk")); } else if (s->answer.buf[0] == 'j') { if (permitted & ALLOW_GOTO_NEXT_UNDECIDED_HUNK) hunk_index = undecided_next; diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 8086d3da71405d..385e55c783a164 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -333,7 +333,7 @@ test_expect_success 'different prompts for mode change/deleted' ' sed -n "s/^\(([0-9/]*) Stage .*?\).*/\1/p" actual >actual.filtered && cat >expect <<-\EOF && (1/1) Stage deletion [y,n,q,a,d,p,?]? - (1/2) Stage mode change [y,n,q,a,d,j,J,g,/,p,?]? + (1/2) Stage mode change [y,n,q,a,d,k,K,j,J,g,/,p,?]? (2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,?]? EOF test_cmp expect actual.filtered @@ -527,7 +527,7 @@ test_expect_success 'goto hunk 1 with "g 1"' ' _10 +15 _20 - (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]?_ + (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,?]?_ EOF test_write_lines s y g 1 | git add -p >actual && tail -n 7 actual.trimmed && @@ -540,7 +540,7 @@ test_expect_success 'goto hunk 1 with "g1"' ' _10 +15 _20 - (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]?_ + (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,?]?_ EOF test_write_lines s y g1 | git add -p >actual && tail -n 4 actual.trimmed && @@ -554,7 +554,7 @@ test_expect_success 'navigate to hunk via regex /pattern' ' _10 +15 _20 - (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]?_ + (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,?]?_ EOF test_write_lines s y /1,2 | git add -p >actual && tail -n 5 actual.trimmed && @@ -567,7 +567,7 @@ test_expect_success 'navigate to hunk via regex / pattern' ' _10 +15 _20 - (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]?_ + (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,?]?_ EOF test_write_lines s y / 1,2 | git add -p >actual && tail -n 4 actual.trimmed && @@ -579,11 +579,11 @@ test_expect_success 'print again the hunk' ' tr _ " " >expect <<-EOF && +15 20 - (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]? @@ -1,2 +1,3 @@ + (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,?]? @@ -1,2 +1,3 @@ 10 +15 20 - (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]?_ + (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,?]?_ EOF test_write_lines s y g 1 p | git add -p >actual && tail -n 7 actual.trimmed && @@ -595,11 +595,11 @@ test_expect_success TTY 'print again the hunk (PAGER)' ' cat >expect <<-EOF && +15 20 - (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]? PAGER @@ -1,2 +1,3 @@ + (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,?]? PAGER @@ -1,2 +1,3 @@ PAGER 10 PAGER +15 PAGER 20 - (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]? + (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,?]? EOF test_write_lines s y g 1 P | ( @@ -802,7 +802,7 @@ test_expect_success 'colors can be overridden' ' -old +new more-context - (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]? @@ -3 +3,2 @@ + (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,?]? @@ -3 +3,2 @@ more-context +another-one (2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,?]? @@ -1,3 +1,3 @@ @@ -810,7 +810,7 @@ test_expect_success 'colors can be overridden' ' -old +new more-context - (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]? + (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,?]? EOF test_cmp expect actual ' @@ -1354,34 +1354,34 @@ do ' done -test_expect_success 'option J rolls over' ' +test_expect_success 'options J, K roll over' ' test_write_lines a b c d e f g h i >file && git add file && test_write_lines X b c d e f g h X >file && - test_write_lines J J q | git add -p >out && - test_write_lines 1 2 1 >expect && + test_write_lines J J K q | git add -p >out && + test_write_lines 1 2 1 2 >expect && sed -n -e "s-/.*--" -e "s/^(//p" actual && test_cmp expect actual ' -test_expect_success 'options y, n, j, e roll over to next undecided (1)' ' +test_expect_success 'options y, n, j, k, e roll over to next undecided (1)' ' test_write_lines a b c d e f g h i j k l m n o p q >file && git add file && test_write_lines X b c d e f g h X j k l m n o p X >file && test_set_editor : && - test_write_lines g3 y g3 n g3 j g3 e q | git add -p >out && - test_write_lines 1 3 1 3 1 3 1 3 1 >expect && + test_write_lines g3 y g3 n g3 j g3 e k q | git add -p >out && + test_write_lines 1 3 1 3 1 3 1 3 1 2 >expect && sed -n -e "s-/.*--" -e "s/^(//p" actual && test_cmp expect actual ' -test_expect_success 'options y, n, j, e roll over to next undecided (2)' ' +test_expect_success 'options y, n, j, k, e roll over to next undecided (2)' ' test_write_lines a b c d e f g h i j k l m n o p q >file && git add file && test_write_lines X b c d e f g h X j k l m n o p X >file && test_set_editor : && - test_write_lines y g3 y g3 n g3 j g3 e q | git add -p >out && - test_write_lines 1 2 3 2 3 2 3 2 3 2 >expect && + test_write_lines y g3 y g3 n g3 j g3 e g1 k q | git add -p >out && + test_write_lines 1 2 3 2 3 2 3 2 3 2 1 2 >expect && sed -n -e "s-/.*--" -e "s/^(//p" actual && test_cmp expect actual ' From e8c744dd9a0270d616ab10aaddfa07cfc071e382 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Mon, 6 Oct 2025 19:23:34 +0200 Subject: [PATCH 08/12] add-patch: let options a and d roll over like y and n MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Options a and d stage and unstage all undecided hunks towards the bottom of the array of hunks, respectively, and then roll over to the very first hunk. The first part is similar to y and n if the current hunk is the last one in the array, but they roll over to the next undecided hunk if there is any. That's more useful; do it for a and d as well. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- add-patch.c | 15 +++++++++++++++ t/t3701-add-interactive.sh | 12 ++++++------ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/add-patch.c b/add-patch.c index 4f314c16ec824f..6da13a78b5e186 100644 --- a/add-patch.c +++ b/add-patch.c @@ -1418,6 +1418,17 @@ static size_t inc_mod(size_t a, size_t m) return a < m - 1 ? a + 1 : 0; } +static bool get_first_undecided(const struct file_diff *file_diff, size_t *idx) +{ + for (size_t i = 0; i < file_diff->hunk_nr; i++) { + if (file_diff->hunk[i].use == UNDECIDED_HUNK) { + *idx = i; + return true; + } + } + return false; +} + static int patch_update_file(struct add_p_state *s, struct file_diff *file_diff) { @@ -1572,6 +1583,8 @@ static int patch_update_file(struct add_p_state *s, if (hunk->use == UNDECIDED_HUNK) hunk->use = USE_HUNK; } + if (!get_first_undecided(file_diff, &hunk_index)) + hunk_index = 0; } else if (hunk->use == UNDECIDED_HUNK) { hunk->use = USE_HUNK; } @@ -1582,6 +1595,8 @@ static int patch_update_file(struct add_p_state *s, if (hunk->use == UNDECIDED_HUNK) hunk->use = SKIP_HUNK; } + if (!get_first_undecided(file_diff, &hunk_index)) + hunk_index = 0; } else if (hunk->use == UNDECIDED_HUNK) { hunk->use = SKIP_HUNK; } diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 385e55c783a164..9d81b0542e0061 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -1364,24 +1364,24 @@ test_expect_success 'options J, K roll over' ' test_cmp expect actual ' -test_expect_success 'options y, n, j, k, e roll over to next undecided (1)' ' +test_expect_success 'options y, n, a, d, j, k, e roll over to next undecided (1)' ' test_write_lines a b c d e f g h i j k l m n o p q >file && git add file && test_write_lines X b c d e f g h X j k l m n o p X >file && test_set_editor : && - test_write_lines g3 y g3 n g3 j g3 e k q | git add -p >out && - test_write_lines 1 3 1 3 1 3 1 3 1 2 >expect && + test_write_lines g3 y g3 n g3 a g3 d g3 j g3 e k q | git add -p >out && + test_write_lines 1 3 1 3 1 3 1 3 1 3 1 3 1 2 >expect && sed -n -e "s-/.*--" -e "s/^(//p" actual && test_cmp expect actual ' -test_expect_success 'options y, n, j, k, e roll over to next undecided (2)' ' +test_expect_success 'options y, n, a, d, j, k, e roll over to next undecided (2)' ' test_write_lines a b c d e f g h i j k l m n o p q >file && git add file && test_write_lines X b c d e f g h X j k l m n o p X >file && test_set_editor : && - test_write_lines y g3 y g3 n g3 j g3 e g1 k q | git add -p >out && - test_write_lines 1 2 3 2 3 2 3 2 3 2 1 2 >expect && + test_write_lines y g3 y g3 n g3 a g3 d g3 j g3 e g1 k q | git add -p >out && + test_write_lines 1 2 3 2 3 2 3 2 3 2 3 2 3 2 1 2 >expect && sed -n -e "s-/.*--" -e "s/^(//p" actual && test_cmp expect actual ' From 208e23ea47ad71c20246ff234efa3abc8080513f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Mon, 6 Oct 2025 19:24:28 +0200 Subject: [PATCH 09/12] add-patch: reset "permitted" at loop start MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Don't accumulate allowed options from any visited hunks, start fresh at the top of the loop instead and only record the allowed options for the current hunk. Reported-by: Junio C Hamano Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- add-patch.c | 19 ++++++++++--------- t/t3701-add-interactive.sh | 14 ++++++++++++++ 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/add-patch.c b/add-patch.c index 6da13a78b5e186..45839ceac58bb4 100644 --- a/add-patch.c +++ b/add-patch.c @@ -1439,15 +1439,6 @@ static int patch_update_file(struct add_p_state *s, struct child_process cp = CHILD_PROCESS_INIT; int colored = !!s->colored.len, quit = 0, use_pager = 0; enum prompt_mode_type prompt_mode_type; - enum { - ALLOW_GOTO_PREVIOUS_HUNK = 1 << 0, - ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK = 1 << 1, - ALLOW_GOTO_NEXT_HUNK = 1 << 2, - ALLOW_GOTO_NEXT_UNDECIDED_HUNK = 1 << 3, - ALLOW_SEARCH_AND_GOTO = 1 << 4, - ALLOW_SPLIT = 1 << 5, - ALLOW_EDIT = 1 << 6 - } permitted = 0; /* Empty added files have no hunks */ if (!file_diff->hunk_nr && !file_diff->added) @@ -1457,6 +1448,16 @@ static int patch_update_file(struct add_p_state *s, render_diff_header(s, file_diff, colored, &s->buf); fputs(s->buf.buf, stdout); for (;;) { + enum { + ALLOW_GOTO_PREVIOUS_HUNK = 1 << 0, + ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK = 1 << 1, + ALLOW_GOTO_NEXT_HUNK = 1 << 2, + ALLOW_GOTO_NEXT_UNDECIDED_HUNK = 1 << 3, + ALLOW_SEARCH_AND_GOTO = 1 << 4, + ALLOW_SPLIT = 1 << 5, + ALLOW_EDIT = 1 << 6 + } permitted = 0; + if (hunk_index >= file_diff->hunk_nr) hunk_index = 0; hunk = file_diff->hunk_nr diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 9d81b0542e0061..403aaee356e6a2 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -1386,4 +1386,18 @@ test_expect_success 'options y, n, a, d, j, k, e roll over to next undecided (2) test_cmp expect actual ' +test_expect_success 'invalid option s is rejected' ' + test_write_lines a b c d e f g h i j k >file && + git add file && + test_write_lines X b X d e f g h i j X >file && + test_write_lines j s q | git add -p >out && + sed -ne "s/ @@.*//" -e "s/ \$//" -e "/^(/p" actual && + cat >expect <<-EOF && + (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,s,e,p,?]? + (2/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,?]? Sorry, cannot split this hunk + (2/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,?]? + EOF + test_cmp expect actual +' + test_done From 881445157279bc2319b7b3c1392d5083453f4662 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Tue, 7 Oct 2025 17:39:37 -0400 Subject: [PATCH 10/12] SubmittingPatches: extend release-notes experiment to topic names In d255105c99 (SubmittingPatches: release-notes entry experiment, 2024-03-25), we began an experiment to have contributors suggest a topic description to appear in our RelNotes and "What's cooking?" reports. Extend that experiment to also welcome suggested topic branch names in addition to descriptions. Suggested-by: Junio C Hamano Signed-off-by: Taylor Blau Signed-off-by: Junio C Hamano --- Documentation/SubmittingPatches | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index 86ca7f6a78a9b6..f48688e370068f 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches @@ -579,14 +579,19 @@ line via `git format-patch --notes`. [[the-topic-summary]] *This is EXPERIMENTAL*. -When sending a topic, you can propose a one-paragraph summary that -should appear in the "What's cooking" report when it is picked up to -explain the topic. If you choose to do so, please write a 2-5 line -paragraph that will fit well in our release notes (see many bulleted -entries in the Documentation/RelNotes/* files for examples), and make -it the first paragraph of the cover letter. For a single-patch -series, use the space between the three-dash line and the diffstat, as -described earlier. +When sending a topic, you can optionally propose a topic name and/or a +one-paragraph summary that should appear in the "What's cooking" +report when it is picked up to explain the topic. If you choose to do +so, please write a 2-5 line paragraph that will fit well in our +release notes (see many bulleted entries in the +Documentation/RelNotes/* files for examples), and make it the first +(or second, if including a suggested topic name) paragraph of the +cover letter. If suggesting a topic name, use the format +"XX/your-topic-name", where "XX" is a stand-in for the primary +author's initials, and "your-topic-name" is a brief, dash-delimited +description of what your topic does. For a single-patch series, use +the space between the three-dash line and the diffstat, as described +earlier. [[attachment]] Do not attach the patch as a MIME attachment, compressed or not. From 1a41698841065f7911f31f20cd1ba9ec7c297aae Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Tue, 7 Oct 2025 17:39:41 -0400 Subject: [PATCH 11/12] SubmittingPatches: guidance for multi-series efforts Occasionally there are efforts to contribute to the Git project that span more than one patch series in order to achieve a broader goal. By convention, the maintainer has typically suffixed the topic names with "-part-one", or "-part-1" and so on. Document that convention and suggest some guidance on how to structure proposed topic names for multi-series efforts. Suggested-by: Junio C Hamano Signed-off-by: Taylor Blau Signed-off-by: Junio C Hamano --- Documentation/SubmittingPatches | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index f48688e370068f..d620bd93bd92e4 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches @@ -593,6 +593,14 @@ description of what your topic does. For a single-patch series, use the space between the three-dash line and the diffstat, as described earlier. +[[multi-series-efforts]] +If your patch series is part of a larger effort spanning multiple +patch series, briefly describe the broader goal, and state where the +current series fits into that goal. If you are suggesting a topic +name as in <>, consider +"XX/the-broader-goal-part-one", "XX/the-broader-goal-part-two", and so +on. + [[attachment]] Do not attach the patch as a MIME attachment, compressed or not. Do not let your e-mail client send quoted-printable. Do not let From f229982df19c327876ce7ded40f6efefe20da5d4 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 17 Oct 2025 14:02:03 -0700 Subject: [PATCH 12/12] The twentieth batch Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.52.0.adoc | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/Documentation/RelNotes/2.52.0.adoc b/Documentation/RelNotes/2.52.0.adoc index 8c4ed4eef48321..ef5f91fcc034ad 100644 --- a/Documentation/RelNotes/2.52.0.adoc +++ b/Documentation/RelNotes/2.52.0.adoc @@ -127,6 +127,10 @@ Performance, Internal Implementation, Development Support etc. * Documentation for "git log --pretty" options has been updated to make it easier to translate. + * Instead of three library archives (one for git, one for reftable, + and one for xdiff), roll everything into a single libgit.a archive. + This would help later effort to FFI into Rust. + Fixes since v2.51 ----------------- @@ -329,6 +333,19 @@ including security updates, are included in this release. you would get from "git format-patch --notes=..." for a singleton patch. + * The code in "git add -p" and friends to iterate over hunks was + riddled with bugs, which has been corrected. + + * A few more things that patch authors can do to help maintainer to + keep track of their topics better. + (merge 1a41698841 tb/doc-submitting-patches later to maint). + + * An earlier addition to "git diff --no-index A B" to limit the + output with pathspec after the two directories misbehaved when + these directories were given with a trailing slash, which has been + corrected. + (merge c0bec06cfe jk/diff-no-index-with-pathspec-fix later to maint). + * Other code cleanup, docfix, build fix, etc. (merge 823d537fa7 kh/doc-git-log-markup-fix later to maint). (merge cf7efa4f33 rj/t6137-cygwin-fix later to maint).