Re: [PATCH] diff: add --ignore-blank-lines option

From: Antoine Pelisse <apelisse@gmail.com>
Date: Jun 9, 2013, 20:32 UTC
Message-ID: <CALWbr2xijB+UD9gwc+HmMdHM9OT+2Lzr9w3h22=CegKHK-Ocng@mail.gmail.com>
In-Reply-To: <7vsj0roxnr.fsf@alter.siamese.dyndns.org>

On Sun, Jun 9, 2013 at 10:07 PM, Junio C Hamano <gitster@pobox.com> wrote:

> by
> the way, do we also handle deletions and do your new tests check
> them?

As stated in the commit message, yes we should, but we don't have tests for that. I will need to add some as I think I found a bug when removing blank lines.

Show 24 quoted lines

>> +     git diff --inter-hunk-context=100 --ignore-blank-lines >out.tmp &&
>> +     cat <<-\EOF >expected &&
>> +     diff --git a/x b/x
>> +     --- a/x
>> +     +++ b/x
>> +     @@ -1,7 +1,10 @@
>> +     +change
>> +      1
>> +      2
>> +     +
>> +      3
>> +      4
>> +     +
>> +      5
>> +      6
>> +      7
>> +     EOF
>> +     compare_diff_patch expected out.tmp
>> +'
>
> And from that point of view, this expected output may be excessively
> noisy.
>
> So I dunno.

It might be kind of noisy, but I think trying to improve the solution might lead to over-engineering. How would we compute the "minimal distance between interesting and blank" so that the blank becomes interesting ? Using the context size for that is quite convenient, while creating another variable would probably become overkill..

The original goal is to remove hunks created solely for addition/suppression, and I think it's what it should do for the moment. But of course, I have no strong opinion about that.

And by the way, I have also another bug, so you can expect another re-roll (sorry about that, it's more complex than I initially thought).

Thanks a lot, Antoine