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

["Phillip Wood via GitGitGadget" <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