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

Chris Webb [off-list ref] writes:

User's existing attribute setting that uses "indent" as an abbreviation
for "indent-with-non-tab" will probably be broken by this naming.

This sounds more like "tab-in-indent".  Naming the option that way would
give it an added benefit of allowing it to be abbreviated to "tab" by the
parser.

I think WS_TAB_IN_INDENT are incompatible with either WS_SPACE_BEFORE_TAB
or WS_INDENT_WITH_TAB.  Don't we want to notice such a combination as an
error?

-		if ((ws_rule & WS_SPACE_BEFORE_TAB) && written < i) {
+		if (ws_rule & WS_INDENT_WITH_TAB) {
+			result |= WS_INDENT_WITH_TAB;
+			if (stream) {
+				fwrite(line + written, i - written, 1, stream);
+				fputs(ws, stream);
+				fwrite(line + i, 1, 1, stream);
+				fputs(reset, stream);
+			}
+		} else if ((ws_rule & WS_SPACE_BEFORE_TAB) && written < i) {

Adding this new code as a new second branch for if-elif-else chain would
match the order of options better.

 			result |= WS_SPACE_BEFORE_TAB;
 			if (stream) {
 				fputs(ws, stream);
 				fwrite(line + written, i - written, 1, stream);
 				fputs(reset, stream);
+				fwrite(line + i, 1, 1, stream);
 			}
-		} else if (stream)
-			fwrite(line + written, i - written, 1, stream);
-		if (stream)
-			fwrite(line + i, 1, 1, stream);
+		} else if (stream) {
+			fwrite(line + written, i - written + 1, 1, stream);
+		}

I think it is a good change, and a necessary one for adding this new
feature.

The original code assumed that this loop always detected an after seeing a
"good" letter (hence the writing of the current letter was unconditional),
but the patch makes it more clear what is being checked and what is being
highlighted by the logic between SPACE_BEFORE_TAB and TAB_IN_INDENT
codepaths.

I'll queue this with s/indent-with-tab/tab-in-indent/ tweak to 'pu' but
the topic would need:

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

before moving to 'next', I think.  Here is what I tentatively will queue,
with an updated log message.

Thanks.

-- >8 --
From: Chris Webb <redacted>
Date: Sat, 27 Mar 2010 14:08:01 +0000
Subject: [PATCH] Add tab-in-indent whitespace error class

Some projects and languages use coding style where no tab character
is used to indent the lines.

This only adds support for "apply --whitespace=warn" and "diff --check";
follow-up patches need to add "apply --whitespace=fix", tests and docs.

Signed-off-by: Chris Webb <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
 cache.h |    1 +
 ws.c    |   24 +++++++++++++++++++-----
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/cache.h b/cache.h
index 6dcb100..030af32 100644
--- a/cache.h
+++ b/cache.h

@@ -1040,6 +1040,7 @@ void shift_tree_by(const unsigned char *, const unsigned char *, unsigned char *
 #define WS_INDENT_WITH_NON_TAB	04
 #define WS_CR_AT_EOL           010
 #define WS_BLANK_AT_EOF        020
+#define WS_TAB_IN_INDENT       040
 #define WS_TRAILING_SPACE      (WS_BLANK_AT_EOL|WS_BLANK_AT_EOF)
 #define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB)
 extern unsigned whitespace_rule_cfg;

diff --git a/ws.c b/ws.c
index c089338..d0b6c54 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 },
+	{ "tab-in-indent", WS_TAB_IN_INDENT, 0 },
 };
 
 unsigned parse_whitespace_rule(const char *string)

@@ -125,6 +126,11 @@ char *whitespace_error_string(unsigned ws)
 			strbuf_addstr(&err, ", ");
 		strbuf_addstr(&err, "indent with spaces");
 	}
+	if (ws & WS_TAB_IN_INDENT) {
+		if (err.len)
+			strbuf_addstr(&err, ", ");
+		strbuf_addstr(&err, "tab in indent");
+	}
 	return strbuf_detach(&err, NULL);
 }
 

@@ -163,7 +169,7 @@ static unsigned ws_check_emit_1(const char *line, int len, unsigned ws_rule,
 		}
 	}
 
-	/* Check for space before tab in initial indent. */
+	/* Check indentation */
 	for (i = 0; i < len; i++) {
 		if (line[i] == ' ')
 			continue;

@@ -175,11 +181,19 @@ static unsigned ws_check_emit_1(const char *line, int len, unsigned ws_rule,
 				fputs(ws, stream);
 				fwrite(line + written, i - written, 1, stream);
 				fputs(reset, stream);
+				fwrite(line + i, 1, 1, stream);
 			}
-		} else if (stream)
-			fwrite(line + written, i - written, 1, stream);
-		if (stream)
-			fwrite(line + i, 1, 1, stream);
+		} else if (ws_rule & WS_TAB_IN_INDENT) {
+			result |= WS_TAB_IN_INDENT;
+			if (stream) {
+				fwrite(line + written, i - written, 1, stream);
+				fputs(ws, stream);
+				fwrite(line + i, 1, 1, stream);
+				fputs(reset, stream);
+			}
+		} else if (stream) {
+			fwrite(line + written, i - written + 1, 1, stream);
+		}
 		written = i + 1;
 	}
 

-- 
1.7.0.3.513.gc8ed0