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

[Jacob Keller <jacob.keller@gmail.com>]

On Fri, Jun 1, 2018 at 10:46 AM, 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 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>
> ---
> My apologies to everyone who was affected by this regression.
>

Ahhh I suspect this is why my edited code in add -p was sometimes
failing to apply!

Thanks,
Jake

>  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..bb6f249f03 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 $_ 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
>