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

[Antoine Pelisse <apelisse@gmail.com>]

On Sun, May 26, 2013 at 10:35 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> 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
>