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

2018-07-11 Thread Jeff Felchner
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  wrote:
> 
> From: Phillip Wood 
> 
> 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 
> Reported-by: Oliver Joseph Ash 
> Reported-by: Jeff Felchner 
> Signed-off-by: Phillip Wood 
> ---
> 
> 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
> 



Re: 2.17.0 Regression When Adding Patches Without Whitespace In Initial Column

2018-05-26 Thread Jeff Felchner
Todd, it looks like that may very well be the same issue.  And it looks like 
it's planning on being fixed in the next release?  Would that be 2.17.1 since 
it's a regression?

> On 2018 May 26, at 16:07, Todd Zullinger  wrote:
> 
> Hi Jeff,
> 
> Jeff Felchner wrote:
>> Ever since 2.17.0, when saving a patch (using add --patch
>> but probably other ways as well), if the whitespace is
>> removed from the initial column, the patch doesn't apply.
> 
> This sounds a bit like the issue discussed in this thread a
> few weeks ago:
> 
> https://public-inbox.org/git/e8aedc6b-5b3e-cfb2-be9d-971bfd9ad...@talktalk.net/
> 
> But I didn't download or watch the video, so I'm not positive.
> 
>> Full walkthrough (including comparison with 2.16.3) in the
>> video attached to this link:
>> 
>> https://www.dropbox.com/s/s1ophi4mwmf9ogv/git-add-patch-whitespace-bug.mp4?dl=1
> 
> -- 
> Todd
> ~~
> Everybody knows how to raise children, except the people who have
> them.
>-- P.J. O'Rourke
> 



2.17.0 Regression When Adding Patches Without Whitespace In Initial Column

2018-05-26 Thread Jeff Felchner
Ever since 2.17.0, when saving a patch (using add --patch but probably other 
ways as well), if the whitespace is removed from the initial column, the patch 
doesn't apply.

Full walkthrough (including comparison with 2.16.3) in the video attached to 
this link:

https://www.dropbox.com/s/s1ophi4mwmf9ogv/git-add-patch-whitespace-bug.mp4?dl=1