[PATCH 2/4] Extract and improve whitespace check from "git apply"

[Wincent Colaiuta <win@wincent.com>]

This refactoring extracts the whitespace checking formerly done in
builtin-apply.c into a function in ws.c so that the exact same
whitespace logic can be used by "git diff" as well, thus reducing
code duplication and the likelihood of errors.

At the same time we teach "git apply" to report all classes of
whitespace errors found in a given line rather than reporting only
the first found error.

Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
 builtin-apply.c |   60 +++++++++++-------------------------------------------
 cache.h         |    2 +
 ws.c            |   58 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 73 insertions(+), 47 deletions(-)

diff --git a/builtin-apply.c b/builtin-apply.c
index f2e9a33..cf349db 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -898,58 +898,24 @@ static int find_header(char *line, unsigned long size, int *hdrsize, struct patc
 	return -1;
 }
 
-static void check_whitespace(const char *line, int len, unsigned ws_rule)
+static void check_patch_whitespace(const char *line, int len,
+                                   unsigned ws_rule)
 {
-	const char *err = "Adds trailing whitespace";
-	int seen_space = 0;
-	int i;
-
-	/*
-	 * We know len is at least two, since we have a '+' and we
-	 * checked that the last character was a '\n' before calling
-	 * this function.  That is, an addition of an empty line would
-	 * check the '+' here.  Sneaky...
-	 */
-	if ((ws_rule & WS_TRAILING_SPACE) && isspace(line[len-2]))
-		goto error;
-
-	/*
-	 * Make sure that there is no space followed by a tab in
-	 * indentation.
-	 */
-	if (ws_rule & WS_SPACE_BEFORE_TAB) {
-		err = "Space in indent is followed by a tab";
-		for (i = 1; i < len; i++) {
-			if (line[i] == '\t') {
-				if (seen_space)
-					goto error;
-			}
-			else if (line[i] == ' ')
-				seen_space = 1;
-			else
-				break;
-		}
-	}
-
-	/*
-	 * Make sure that the indentation does not contain more than
-	 * 8 spaces.
-	 */
-	if ((ws_rule & WS_INDENT_WITH_NON_TAB) &&
-	    (8 < len) && !strncmp("+        ", line, 9)) {
-		err = "Indent more than 8 places with spaces";
-		goto error;
-	}
-	return;
+	char *err;
+	unsigned result = check_whitespace(line, len, ws_rule);
+	if (!result)
+		return;
 
- error:
 	whitespace_error++;
 	if (squelch_whitespace_errors &&
 	    squelch_whitespace_errors < whitespace_error)
 		;
-	else
+	else {
+		err = whitespace_error_string(result);
 		fprintf(stderr, "%s.\n%s:%d:%.*s\n",
-			err, patch_input_file, linenr, len-2, line+1);
+		     err, patch_input_file, linenr, len - 2, line + 1);
+		free(err);
+	}
 }
 
 /*
@@ -1001,7 +967,7 @@ static int parse_fragment(char *line, unsigned long size,
 		case '-':
 			if (apply_in_reverse &&
 			    ws_error_action != nowarn_ws_error)
-				check_whitespace(line, len, patch->ws_rule);
+				check_patch_whitespace(line, len, patch->ws_rule);
 			deleted++;
 			oldlines--;
 			trailing = 0;
@@ -1009,7 +975,7 @@ static int parse_fragment(char *line, unsigned long size,
 		case '+':
 			if (!apply_in_reverse &&
 			    ws_error_action != nowarn_ws_error)
-				check_whitespace(line, len, patch->ws_rule);
+				check_patch_whitespace(line, len, patch->ws_rule);
 			added++;
 			newlines--;
 			trailing = 0;
diff --git a/cache.h b/cache.h
index 1bcb3df..6c17f27 100644
--- a/cache.h
+++ b/cache.h
@@ -655,6 +655,8 @@ void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, i
 extern unsigned whitespace_rule_cfg;
 extern unsigned whitespace_rule(const char *);
 extern unsigned parse_whitespace_rule(const char *);
+extern unsigned check_whitespace(const char *line, int len, unsigned ws_rule);
+extern char *whitespace_error_string(unsigned ws);
 
 /* ls-files */
 int pathspec_match(const char **spec, char *matched, const char *filename, int skiplen);
diff --git a/ws.c b/ws.c
index 52c10ca..884d373 100644
--- a/ws.c
+++ b/ws.c
@@ -94,3 +94,61 @@ unsigned whitespace_rule(const char *pathname)
 		return whitespace_rule_cfg;
 	}
 }
+
+/* The returned string should be freed by the caller. */
+char *whitespace_error_string(unsigned ws)
+{
+	struct strbuf err;
+	strbuf_init(&err, 0);
+	if (ws & WS_TRAILING_SPACE)
+		strbuf_addstr(&err, "Adds trailing whitespace");
+	if (ws & WS_SPACE_BEFORE_TAB) {
+		if (err.len)
+			strbuf_addstr(&err, ", ");
+		strbuf_addstr(&err, "Space in indent is followed by a tab");
+	}
+	if (ws & WS_INDENT_WITH_NON_TAB) {
+		if (err.len)
+			strbuf_addstr(&err, ", ");
+		strbuf_addstr(&err, "Indent more than 8 places with spaces");
+	}
+	return strbuf_detach(&err, NULL);
+}
+
+unsigned check_whitespace(const char *line, int len, unsigned ws_rule)
+{
+	unsigned result = 0;
+	int seen_space = 0;
+	int i;
+
+	if (ws_rule & WS_TRAILING_SPACE) {
+		/* Lines start with "+" or "-" so length is at least 1 */
+		if (line[len - 1] == '\n') {
+			if (isspace(line[len - 2]))
+				result |= WS_TRAILING_SPACE;
+		}
+		else if (isspace(line[len - 1]))
+			result |= WS_TRAILING_SPACE;
+	}
+
+	if (ws_rule & WS_SPACE_BEFORE_TAB) {
+		for (i = 1; i < len; i++) {
+			if (line[i] == '\t') {
+				if (seen_space) {
+					result |= WS_SPACE_BEFORE_TAB;
+					break;
+				}
+			}
+			else if (line[i] == ' ')
+				seen_space = 1;
+			else
+				break;
+		}
+	}
+
+	if ((ws_rule & WS_INDENT_WITH_NON_TAB) &&
+	    (8 < len) && !strncmp("+        ", line, 9)) {
+		result |= WS_INDENT_WITH_NON_TAB;
+	}
+	return result;
+}
-- 
1.5.3.7.1159.g2f071-dirty