Git development
 help / color / mirror / Atom feed
From: Thell Fowler <git@tbfowler.name>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 0/9] War on blank-at-eof
Date: Sat, 5 Sep 2009 16:28:22 -0500 (CDT)	[thread overview]
Message-ID: <alpine.WNT.2.00.0909051534380.7040@GWNotebook> (raw)
In-Reply-To: <1252061718-11579-1-git-send-email-gitster@pobox.com>

On Fri, 4 Sep 2009, Junio C Hamano wrote:

> Patch 5 corrects the definition of blank-at-eof.  If a patch adds an
> non-empty line that consists solely of whitespaces at the end of file, we
> should diagnose and strip it just line a new empty line.  After all, both
> are blank lines.
>

Thank you. Thank you, thank you. Thank you!  And did I mention thank you?

Tested this out after cherry-picking:
3b5ef0e xutils: Fix xdl_recmatch() on incomplete lines
78ed710 xutils: Fix hashing an incomplete line with whitespaces at the end

It worked as nicely!  I'm throwing away the --allow-whitelines-at-eof 
patch! :D  Converting a _real_ dirty whitespace branch into an 'almost' 
whitespace policy compliant branch with validation of the diffs was 
able to be done like so:
	git diff -b DIRTY CLEAN
	git diff DIRTY^ CLEAN > diff1
	git diff CLEAN^ DIRTY > diff2
	git diff -b diff1 diff2

I mention 'almost' above because unfortunately this type of conversion 
leaves extra line-spaces at the end of some files that you might not want 
to have in a whitespace policy.

While thinking about what appeared in:

http://article.gmane.org/gmane.comp.version-control.git/124138
Junio C Hamano <gitster <at> pobox.com> writes:
>Bruno Haible <bruno <at> clisp.org> writes:
>> In some GNU projects, there are file types for which trailing spaces in a line
>> ...
>> Currently the user has to turn off the 'trailing-space' whitespace attribute
>> in order for 'git diff --check' to not complain about such files. This has
>> the drawback that trailing spaces are not detected.
	
>Very good problem description.  Thanks.

I thought it might be interesting to throw this out there...  What do you 
think of an additional attribute value like
	core.whitespace blank-at-eof-min-<some 0 to N #>
	core.whitespace blank-at-eof-max-<some 0 to N #>
that could be read in when core.whitespace blank-at-eof is set.

If neither are present then use current. (No new eof blanks).
If min but not max is set then allow new blanks and ensure at least min.
If max but not min is set then only allow max blanks at eof.
If both then treat it as a boundary.

This could ensure a whitespace policy without the repository maintainer 
having to correct this type of minutia and without having to nit-pick 
contributors into submission.

Then perhaps diff could also recognize an in range blank-at-eof so a diff 
using one of the ignore whitespace options would ignore eof whitelines 
that are in range?


> The series applies to v1.6.0.6-87-g82d97da; merging the result to 'master'
> needs some conflict resolution.
>


-- 
Thell

  parent reply	other threads:[~2009-09-05 21:42 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-04 10:55 [PATCH 0/9] War on blank-at-eof Junio C Hamano
2009-09-04 10:55 ` [PATCH 1/9] apply --whitespace=fix: fix handling of blank lines at the eof Junio C Hamano
2009-09-04 10:55 ` [PATCH 2/9] apply --whitespace=fix: detect new blank lines at eof correctly Junio C Hamano
2009-09-04 12:02   ` Johannes Sixt
2009-09-04 16:26     ` Junio C Hamano
2009-09-04 10:55 ` [PATCH 3/9] apply.c: split check_whitespace() into two Junio C Hamano
2009-09-04 10:55 ` [PATCH 4/9] apply --whitespace=warn/error: diagnose blank at EOF Junio C Hamano
2009-09-04 10:55 ` [PATCH 5/9] apply --whitespace: warn blank but not necessarily empty lines " Junio C Hamano
2009-09-04 10:55 ` [PATCH 6/9] diff.c: the builtin_diff() deals with only two-file comparison Junio C Hamano
2009-09-04 10:55 ` [PATCH 7/9] diff --whitespace=warn/error: obey blank-at-eof Junio C Hamano
2009-09-04 10:55 ` [PATCH 8/9] diff --whitespace=warn/error: fix blank-at-eof check Junio C Hamano
2009-09-04 10:55 ` [PATCH 9/9] diff --color: color blank-at-eof Junio C Hamano
2009-09-05 21:28 ` Thell Fowler [this message]
2009-09-06  6:13   ` [PATCH 0/9] War on blank-at-eof Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.WNT.2.00.0909051534380.7040@GWNotebook \
    --to=git@tbfowler.name \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox