Re: [PATCH] add -p: fix counting empty context lines in edited patches
- From
Jacob Keller <jacob.keller@gmail.com>
- Date
- Jun 1, 2018, 19:07 UTC
- Message-ID
- <CA+P7+xokjMcbbrH2iCt5SWMxgsstA+VXHOQgx4debd7Oou-RRA@mail.gmail.com>
- In-Reply-To
- <20180601174644.13055-1-phillip.wood@talktalk.net>
On Fri, Jun 1, 2018 at 10:46 AM, Phillip Wood <phillip.wood@talktalk.net> wrote:
Show 27 quoted lines
> 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
Show 74 quoted lines
> 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
>