Re: [PATCH] New whitespace checking category 'trailing-blank-line'

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster <at> pobox.com> writes:

> 
> Bruno Haible <bruno <at> clisp.org> writes:
> 
...
> > Here is a proposed patch, to allow people to turn the check against trailing
> > blank lines independently from the whitespace-in-a-line checking. The default
> > behavior is not changed.
> 
> The "default" for people who do not have configuration may not change, but
> people who explicitly asked git to check the trailing blank lines by
> specifying "whitespace=trail" attribute, and have been relying on it, will
> now be unprotected. Such a regression/incompatibility should be noted here.
> 
> Better yet would be not to introduce such a regression, of course.
> 
> 
> 
... 
> Another idea would be to:
> 
>     - introduce two new settings:
> 
>         blank-at-eol		: SP/HT at EOL is an error
>         empty-lines-at-eof	: empty lines at the end of file is an error
> 
>     - make existing "trailing-space" a mere short-hand for "blank-at-eol"
>       and "empty-lines-at-eof".
> 
> 
> The "git diff" part looked reasonable from a quick glance, but I do not
> think I saw anything that affects "git apply --whitespace=fix" in the
> patch.
> 

Hi all!  How appropriate that this was posted so recently.  Seems to be a fairly
popular issue lately with the recent commits 735c674 and the correction 422a82f.

I was thinking of a fix for the disappearing EOL @ EOF and was hoping for some
input and direction, yet wasn't really sure if a new RFH thread would be better
than here or not.  Until otherwise noted how about just diving in...


My initial posting was to get input on which approach would be best received to
give a capability similar to what Junio's idea above states.  The problem is
which way to best go about it.  Obviously if someone is doing whitespace=fix
with trailing-space set then they indeed want it fixed, but having the blank
lines removed... ?

Currently the section in builtin-apply.c (v1.6.4) that is the culprit for
removing these lines is:

1999                 if (added_blank_line)
2000                         new_blank_lines_at_end++;
2001                 else
2002                         new_blank_lines_at_end = 0;

Having a --keep-new-blank-lines argument for apply, and am seems like a winner
that then doesn't effect regular operations.

1999                 if (added_blank_line && !keep_new_blank_lines_at_end)

I've tested having the added_blank_line reset to 0 during testing of whitespace
fixing from crlf to lf and it seems good to go for that purpose.

Is that something that I should try for?  Ie: having a new arg for am and apply,
or is a new core whitespace option the better route?

Sincerely,
Thell (almostautomated on freenode)



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]