Re: [PATCH 3/5] Unify whitespace checking
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 14, 2007, 00:03 UTC
- Message-ID
- <7vd4taywfw.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <1197552751-53480-4-git-send-email-win@wincent.com>
Wincent Colaiuta <win@wincent.com> writes:
Show 6 quoted lines
> The new function is called check_and_emit_line() and it does two things: > checks a line for whitespace errors and optionally emits it. The checking > is based on lines of content rather than patch lines (in other words, the > caller must strip the leading "+" or "-"); this was suggested by Junio on > the mailing list to allow for a future extension to "git show" to display > whitespace errors in blobs.
I do not think "git show" is a realistic "future extension", by the way. But at least from the interface-cleanliness point of view, I think not passing the leading "+" to the function is a right thing to do.
Show 12 quoted lines
> diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh > index dc538b3..a0a47dd 100755 > --- a/t/t4015-diff-whitespace.sh > +++ b/t/t4015-diff-whitespace.sh > @@ -121,7 +121,7 @@ test_expect_success 'check mixed spaces and tabs in indent' ' > > # This is indented with SP HT SP. > echo " foo();" > x && > - git diff --check | grep "space before tab" > + git diff --check | grep "Space in indent is followed by a tab" > > '
Hmph. I think with the multiple detection this rewording would make the error message very long to read. Was the rewording really necessary?
Show 5 quoted lines
> +/* If stream is non-NULL, emits the line after checking. */
> +unsigned check_and_emit_line(const char *line, int len, unsigned ws_rule,
> + FILE *stream, const char *set,
> + const char *reset, const char *ws)
> +{Honestly, I regretted suggesting this, fearing that it might make the checking too costly, but the code is clean, readable, and does not look costly at all. Nice job.