Re: [PATCH v2] add -p: fix counting empty context lines in edited patches

[Jeff Felchner <jfelchner1@gmail.com>]

Hey all, I assumed this was going to be in 2.18, but I'm still having the same issue.  What's the plan for release of this?

> On 2018 Jun 11, at 4:46, Phillip Wood <phillip.wood@talktalk.net> wrote:
> 
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
> 
> recount_edited_hunk() introduced in commit 2b8ea7f3c7 ("add -p:
> calculate offset delta for edited patches", 2018-03-05) required all
> context lines to start with a space, empty lines are not counted. This
> was intended to avoid any recounting problems if the user had
> introduced empty lines at the end when editing the patch. However this
> introduced a regression into 'git add -p' as it seems it is common for
> editors to strip the trailing whitespace from empty context lines when
> patches are edited thereby introducing empty lines that should be
> counted. 'git apply' knows how to deal with such empty lines and POSIX
> states that whether or not there is an space on an empty context line
> is implementation defined [1].
> 
> Fix the regression by counting lines that consist solely of a newline
> as well as lines starting with a space as context lines and add a test
> to prevent future regressions.
> 
> [1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/diff.html
> 
> Reported-by: Mahmoud Al-Qudsi <mqudsi@neosmart.net>
> Reported-by: Oliver Joseph Ash <oliverjash@gmail.com>
> Reported-by: Jeff Felchner <jfelchner1@gmail.com>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
> 
> Thanks for the feedback, the only changes since v1 are to fix the
> commit message to match what was in pu and to change '$_' to '$mode'
> in the comparison as I think that is clearer. In the end I decided to
> leave the tests as they are.
> 
> git-add--interactive.perl  |  2 +-
> t/t3701-add-interactive.sh | 43 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index ab022ec073..8361ef45e7 100755
> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -1047,7 +1047,7 @@ sub recount_edited_hunk {
> 			$o_cnt++;
> 		} elsif ($mode eq '+') {
> 			$n_cnt++;
> -		} elsif ($mode eq ' ') {
> +		} elsif ($mode eq ' ' or $mode eq "\n") {
> 			$o_cnt++;
> 			$n_cnt++;
> 		}
> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
> index e5c66f7500..f1bb879ea4 100755
> --- a/t/t3701-add-interactive.sh
> +++ b/t/t3701-add-interactive.sh
> @@ -175,6 +175,49 @@ test_expect_success 'real edit works' '
> 	diff_cmp expected output
> '
> 
> +test_expect_success 'setup file' '
> +	test_write_lines a "" b "" c >file &&
> +	git add file &&
> +	test_write_lines a "" d "" c >file
> +'
> +
> +test_expect_success 'setup patch' '
> +	SP=" " &&
> +	NULL="" &&
> +	cat >patch <<-EOF
> +	@@ -1,4 +1,4 @@
> +	 a
> +	$NULL
> +	-b
> +	+f
> +	$SP
> +	c
> +	EOF
> +'
> +
> +test_expect_success 'setup expected' '
> +	cat >expected <<-EOF
> +	diff --git a/file b/file
> +	index b5dd6c9..f910ae9 100644
> +	--- a/file
> +	+++ b/file
> +	@@ -1,5 +1,5 @@
> +	 a
> +	$SP
> +	-f
> +	+d
> +	$SP
> +	 c
> +	EOF
> +'
> +
> +test_expect_success 'edit can strip spaces from empty context lines' '
> +	test_write_lines e n q | git add -p 2>error &&
> +	test_must_be_empty error &&
> +	git diff >output &&
> +	diff_cmp expected output
> +'
> +
> test_expect_success 'skip files similarly as commit -a' '
> 	git reset &&
> 	echo file >.gitignore &&
> -- 
> 2.17.0
>