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

From: Antoine Pelisse <apelisse@gmail.com>
Date: May 27, 2013, 07:14 UTC
Message-ID: <CALWbr2xzSC8ZGggCUYbS7ZWEOHnZnxFJWOfHCOBkVZFQS9KcEw@mail.gmail.com>
In-Reply-To: <51A2722A.8040200@kdbg.org>

On Sun, May 26, 2013 at 10:35 PM, Johannes Sixt <j6t@kdbg.org> wrote:

Show 12 quoted lines

> Am 26.05.2013 19:58, schrieb Antoine Pelisse:
>> The goal of the patch is to introduce the GNU diff
>> -B/--ignore-blank-lines as closely as possible. The short option is not
>> available because it's already used for "break-rewrites".
>>
>> When this option is used, git-diff will not create hunks that simply
>> adds or removes empty lines, but will still show empty lines
>> addition/suppression if they are close enough to "valuable" changes.
>
> So when an addition or removal of a blank line appears in a hunk that
> also has non-blank-line changes, the addition or removal is not treated
> specially?

Exactly.

> How is a blank line defined? What happens if a line that has only
> whitespace is added or removed?

xdl_blankline() is the best description of what I considered a blank line. If no --ignore-space-* option is given, it's a line that starts and ends with '\n'. If any --ignore-space-* option is given, it's a line that has any number of isspace(3)-defined characters, followed by '\n'.

> I'm thinking of diffs of files with CRLF

Good you did, because I didn't ;-)

> line breaks, where the CR would count as whitespace in the line, I think.

With the current implementation, an empty line with CRLF will not show as a blank line if no space option is given. As CR is a space according to isspace(3), the line will be removed with any space option.

>> +--ignore-blank-lines::
>> +     Ignore changes whose lines are all blank.
>
> I think this is too terse and does not convey what the option really does.

That's the description from GNU diff man page. But indeed it could be more precise.

>> +test_expect_success 'ignore-blank-lines: only new lines' '
>> +     seq 5 >x &&
>
> Please use test_seq instead of seq in all new tests.

Will fix.

> -- Hannes
>