Re: [PATCH 3/5] Unify whitespace checking

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

 



Wincent Colaiuta <win@xxxxxxxxxxx> writes:

> 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.

> 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?

> +/* 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.
-
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]

  Powered by Linux