git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Christian Holtje <docwhat@gmail.com>
Cc: Alex Riesen <raa.lkml@gmail.com>, git@vger.kernel.org
Subject: Re: [PATCH v2] pre-commit hook should ignore carriage returns at EOL
Date: Thu, 26 Jun 2008 16:01:40 -0700	[thread overview]
Message-ID: <7vprq3ol63.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <7vk5gbq10p.fsf@gitster.siamese.dyndns.org> (Junio C. Hamano's message of "Thu, 26 Jun 2008 15:33:58 -0700")

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

> Christian Holtje <docwhat@gmail.com> writes:
>
>>> I suggested using "diff --check" (and possibly teaching "diff --check"
>>> other things the scripted example checks, such as conflict markers),
>>> which would know to honor the line endings specified per path via
>>> gitattributes(5), instead of building on top of the big Perl script,
>>> and I
>>> had an impression that you agreed to the approach.
>>
>> I'm completely confused how gitattributes and core.autocrlf interact,
>> etc.
>
> Here is a series I just cooked up so that we can remove the whole Perl
> script and replace it by adding --check to "diff-index" used there. 
>
> The first three are code clean-ups and the last two implements necessary
> new features to "diff --check".  The whole series somewhat depend on the
> fix to 'maint' not to lose the exit status I sent earlier.
>
> [PATCH 1/5] diff --check: explain why we do not care whether old side is binary
> [PATCH 2/5] check_and_emit_line(): rename and refactor
> [PATCH 3/5] checkdiff: pass diff_options to the callback
> [PATCH 4/5] Teach "diff --check" about a new blank lines at end
> [PATCH 5/5] diff --check: detect leftover conflict markers

With these enhancements in place, I think the pre-commit hook to find
problematic change would become essentially a one-liner, something like:

	git diff-index --check -M --cached

and the checking will obey what you configured with core.whitespace, which
globally defines what kind of whitespace breakages are "problematic",
and/or whitespace attribute which determines the same per path.

If you have for example Python source files that you would want all the
default whitespace checks (that is, trailing whitespaces are not allowed,
initial indentation part should not have SP followed by HT), you would
have

	*.py whitespace=trail,space-before-tab

in your .gitattributes, and the above command would catch such a
breakage.  If you further want to catch indentation with more than 8
SPs that can be replaced with HTs in your C sources, you would say:

	*.[ch] whitespace=indent-with-no-tab,trail,space-before-tab

You could choose to have CRLF line endings in the repository [*1*], and
for such projects, diff output would have tons of lines that end with
CRs.  To consider these CRs part of the line terminator, add cr-at-eol
to the value of whitespace attribute, like so:

	*.py whitespace=trail,space,cr-at-eol
	*.[ch] whitespace=indent,trail,space,cr-at-eol

[Footnote]

*1* I do not do Windows, but my understanding is that this practice is not
recommended because it would hurt cross-platform use of the project.  You
would instead keep your repository copy with LF line endings, and make
your checkouts have CRLF line endings by core.autocrlf configuration.

  parent reply	other threads:[~2008-06-26 23:03 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-24 19:21 [PATCH v2] pre-commit hook should ignore carriage returns at EOL Christian Holtje
2008-06-25 18:14 ` Alex Riesen
2008-06-25 18:47   ` Christian Holtje
2008-06-25 19:14     ` Junio C Hamano
2008-06-26  2:41       ` Christian Holtje
2008-06-26 22:33         ` Junio C Hamano
2008-06-26 22:34           ` [PATCH 1/5] diff --check: explain why we do not care whether old side is binary Junio C Hamano
2008-06-26 22:35           ` [PATCH 2/5] check_and_emit_line(): rename and refactor Junio C Hamano
2008-06-26 22:36           ` [PATCH 3/5] checkdiff: pass diff_options to the callback Junio C Hamano
2008-06-26 22:36           ` [PATCH 4/5] Teach "diff --check" about a new blank lines at end Junio C Hamano
2008-06-26 22:37           ` [PATCH 5/5] diff --check: detect leftover conflict markers Junio C Hamano
2008-06-26 23:01           ` Junio C Hamano [this message]
2008-06-26 23:06             ` [PATCH v2] pre-commit hook should ignore carriage returns at EOL Junio C Hamano
2008-06-27  4:24               ` Jeff King

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=7vprq3ol63.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=docwhat@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=raa.lkml@gmail.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;
as well as URLs for NNTP newsgroup(s).