Re* [RFC 1/1] Add new indent-with-tab whitespace check

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

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

>> diff --git a/ws.c b/ws.c
>> index c089338..e44a711 100644
>> --- a/ws.c
>> +++ b/ws.c
>> @@ -18,6 +18,7 @@ static struct whitespace_rule {
>>  	{ "cr-at-eol", WS_CR_AT_EOL, 1 },
>>  	{ "blank-at-eol", WS_BLANK_AT_EOL, 0 },
>>  	{ "blank-at-eof", WS_BLANK_AT_EOF, 0 },
>> +	{ "indent-with-tab", WS_INDENT_WITH_TAB, 0 },
>
> User's existing attribute setting that uses "indent" as an abbreviation
> for "indent-with-non-tab" will probably be broken by this naming.

There is another issue with this change.  Because "whitespace" without any
string in .gitattributes are defined to cause all the whitespace breakages
known to git to be caught, and tab-in-indent is inherently incompatible
with indent-with-non-tab, this cannot be supported without changing the
definition of "default set of whitespace breakage classes".

The intention of allowing .gitattributes to say "*.txt whitespace" is to
let the users and projects say:

    I trust the competence and good judgement made by git developers
    regarding whitespace issues.  They may devise a new algorithm to catch
    common whitespace errors that the current tool may not catch, and when
    that happens, I'd like my project to take advantage of the new code
    and catch the newly defined classes of errors.
    
and that is why we include all whitespace-rule except for the ones that
loosens error conditions to the set of breakages we catch for such a
specification.

So on top of the replacement I sent earlier, I think we would need a patch
like the attached.  The topic still needs:

>  - detecting incompatible settings;
>  - tests;
>  - docs;
>  - matching change to apply --whitespace=fix;

but otherwise with this patch I think it is usable by me ;-)

Thanks.

-- >8 --
whitespace: we cannot "catch all errors known to git" anymore

Traditionally, "*.txt whitespace" in .gitattributes file has been an
instruction to catch _all_ classes of whitespace errors known to git.

This however has to change, as the recently introduced "tab-in-indent"
is inherently incompatible with "indent-with-non-tab".  As we do not want
to break configuration of existing users, add a mechanism to allow marking
selected rules to be excluded from "all rules known to git".

Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
 ws.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/ws.c b/ws.c
index d0b6c54..c7b4f91 100644
--- a/ws.c
+++ b/ws.c
@@ -10,7 +10,8 @@
 static struct whitespace_rule {
 	const char *rule_name;
 	unsigned rule_bits;
-	unsigned loosens_error;
+	unsigned loosens_error:1,
+		not_in_all:1;
 } whitespace_rule_names[] = {
 	{ "trailing-space", WS_TRAILING_SPACE, 0 },
 	{ "space-before-tab", WS_SPACE_BEFORE_TAB, 0 },
@@ -18,7 +19,7 @@ static struct whitespace_rule {
 	{ "cr-at-eol", WS_CR_AT_EOL, 1 },
 	{ "blank-at-eol", WS_BLANK_AT_EOL, 0 },
 	{ "blank-at-eof", WS_BLANK_AT_EOF, 0 },
-	{ "tab-in-indent", WS_TAB_IN_INDENT, 0 },
+	{ "tab-in-indent", WS_TAB_IN_INDENT, 0, 1 },
 };
 
 unsigned parse_whitespace_rule(const char *string)
@@ -83,7 +84,8 @@ unsigned whitespace_rule(const char *pathname)
 			unsigned all_rule = 0;
 			int i;
 			for (i = 0; i < ARRAY_SIZE(whitespace_rule_names); i++)
-				if (!whitespace_rule_names[i].loosens_error)
+				if (!whitespace_rule_names[i].loosens_error &&
+				    !whitespace_rule_names[i].not_in_all)
 					all_rule |= whitespace_rule_names[i].rule_bits;
 			return all_rule;
 		} else if (ATTR_FALSE(value)) {
--
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]