git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 0/2] builtin add -p: fix hunk splitting

From
PGPhillip Wood via GitGitGadget <gitgitgadget@gmail.com>
Date
Jan 11, 2022, 11:12 UTC
Message-ID
<pull.1100.v2.git.1641899530.gitgitgadget@gmail.com>
In-Reply-To
<pull.1100.git.1640010777.gitgitgadget@gmail.com>

Thanks to Junio and Ævar for their comments on V1. I've updated the commit message and added a helper function as suggested.

V1 Cover Letter: Fix a small regression in the hunk splitting of the builtin version compared to the perl version. Thanks to Szeder for the easy to follow bug report.

Phillip Wood (2):
  t3701: clean up hunk splitting tests
  builtin add -p: fix hunk splitting
 add-patch.c                | 20 ++++++++++------
 t/t3701-add-interactive.sh | 48 ++++++++++++++++++++++++++++++++++----
 2 files changed, 56 insertions(+), 12 deletions(-)
base-commit: cd3e606211bb1cf8bc57f7d76bab98cc17a150bc
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1100%2Fphillipwood%2Fwip%2Fadd-p-fix-hunk-splitting-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1100/phillipwood/wip/add-p-fix-hunk-splitting-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1100
Range-diff vs v1:
 1:  cc8639fc29d = 1:  cc8639fc29d t3701: clean up hunk splitting tests
 2:  5d5639c2b04 ! 2:  b698989e265 builtin add -p: fix hunk splitting
     @@ Metadata
       ## Commit message ##
          builtin add -p: fix hunk splitting
      
     +    The C reimplementation of "add -p" fails to split the last hunk in a
     +    file if hunk ends with an addition or deletion without any post context
     +    line unless it is the last file to be processed.
     +
          To determine whether a hunk can be split a counter is incremented each
          time a context line follows an insertion or deletion. If at the end of
          the hunk the value of this counter is greater than one then the hunk
     @@ Commit message
          Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
      
       ## add-patch.c ##
     +@@ add-patch.c: static int is_octal(const char *p, size_t len)
     + 	return 1;
     + }
     + 
     ++static void complete_file(char marker, struct hunk *hunk)
     ++{
     ++	if (marker == '-' || marker == '+')
     ++		/*
     ++		 * Last hunk ended in non-context line (i.e. it
     ++		 * appended lines to the file, so there are no
     ++		 * trailing context lines).
     ++		 */
     ++		hunk->splittable_into++;
     ++}
     ++
     + static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
     + {
     + 	struct strvec args = STRVEC_INIT;
      @@ add-patch.c: static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
       			eol = pend;
       
       		if (starts_with(p, "diff ")) {
     -+			if (marker == '-' || marker == '+')
     -+				/*
     -+				 * Last hunk ended in non-context line (i.e. it
     -+				 * appended lines to the file, so there are no
     -+				 * trailing context lines).
     -+				 */
     -+				hunk->splittable_into++;
     ++			complete_file(marker, hunk);
       			ALLOC_GROW_BY(s->file_diff, s->file_diff_nr, 1,
       				   file_diff_alloc);
       			file_diff = s->file_diff + s->file_diff_nr - 1;
     +@@ add-patch.c: static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
     + 				file_diff->hunk->colored_end = hunk->colored_end;
     + 		}
     + 	}
     +-
     +-	if (marker == '-' || marker == '+')
     +-		/*
     +-		 * Last hunk ended in non-context line (i.e. it appended lines
     +-		 * to the file, so there are no trailing context lines).
     +-		 */
     +-		hunk->splittable_into++;
     ++	complete_file(marker, hunk);
     + 
     + 	/* non-colored shorter than colored? */
     + 	if (colored_p != colored_pend) {
      
       ## t/t3701-add-interactive.sh ##
      @@ t/t3701-add-interactive.sh: test_expect_success 'correct message when there is nothing to do' '
-- 
gitgitgadget
Previous: Junio C HamanoNext: Phillip Wood via GitGitGadget
Message 9 of 21 in “builtin add -p: fix hunk splitting”
  1. 0/2 builtin add -p: fix hunk splittingPhillip Wood via GitGitGadget, Dec 20, 2021
  2. 1/2 t3701: clean up hunk splitting testsPhillip Wood via GitGitGadget, Dec 20, 2021
  3. Junio C HamanoDec 20, 2021
  4. 2/2 builtin add -p: fix hunk splittingPhillip Wood via GitGitGadget, Dec 20, 2021
  5. Ævar Arnfjörð BjarmasonDec 20, 2021
  6. Phillip WoodJan 11, 2022
  7. Ævar Arnfjörð BjarmasonJan 11, 2022
  8. Junio C HamanoDec 20, 2021
  9. 0/2 builtin add -p: fix hunk splittingPhillip Wood via GitGitGadget, Jan 11, 2022
  10. 1/2 t3701: clean up hunk splitting testsPhillip Wood via GitGitGadget, Jan 11, 2022
  11. 2/2 builtin add -p: fix hunk splittingPhillip Wood via GitGitGadget, Jan 11, 2022
  12. Ævar Arnfjörð BjarmasonJan 11, 2022
  13. Phillip WoodJan 11, 2022
  14. Junio C HamanoJan 12, 2022
  15. Phillip WoodJan 19, 2022
  16. Junio C HamanoJan 20, 2022
  17. Ævar Arnfjörð BjarmasonJan 20, 2022
  18. Junio C HamanoJan 20, 2022
  19. Johannes SchindelinJan 22, 2022
  20. Phillip WoodJan 24, 2022
  21. Junio C HamanoJan 12, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.