Skip to content

Commit c1795bb

Browse files
wincentgitster
wincent
authored and
gitster
committed
Unify whitespace checking
This commit unifies three separate places where whitespace checking was performed: - the whitespace checking previously done in builtin-apply.c is extracted into a function in ws.c - the equivalent logic in "git diff" is removed - the emit_line_with_ws() function is also removed because that also rechecks the whitespace, and its functionality is rolled into ws.c 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. At the same time we teach it to report all classes of whitespace errors found for a given line rather than reporting only the first found error. Signed-off-by: Wincent Colaiuta <win@wincent.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent da31b35 commit c1795bb

File tree

5 files changed

+139
-164
lines changed

5 files changed

+139
-164
lines changed

‎builtin-apply.c‎

Lines changed: 10 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -900,56 +900,22 @@ static int find_header(char *line, unsigned long size, int *hdrsize, struct patc
900900

901901
static void check_whitespace(const char *line, int len, unsigned ws_rule)
902902
{
903-
const char *err = "Adds trailing whitespace";
904-
int seen_space = 0;
905-
int i;
906-
907-
/*
908-
* We know len is at least two, since we have a '+' and we
909-
* checked that the last character was a '\n' before calling
910-
* this function. That is, an addition of an empty line would
911-
* check the '+' here. Sneaky...
912-
*/
913-
if ((ws_rule & WS_TRAILING_SPACE) && isspace(line[len-2]))
914-
goto error;
915-
916-
/*
917-
* Make sure that there is no space followed by a tab in
918-
* indentation.
919-
*/
920-
if (ws_rule & WS_SPACE_BEFORE_TAB) {
921-
err = "Space in indent is followed by a tab";
922-
for (i = 1; i < len; i++) {
923-
if (line[i] == '\t') {
924-
if (seen_space)
925-
goto error;
926-
}
927-
else if (line[i] == ' ')
928-
seen_space = 1;
929-
else
930-
break;
931-
}
932-
}
933-
934-
/*
935-
* Make sure that the indentation does not contain more than
936-
* 8 spaces.
937-
*/
938-
if ((ws_rule & WS_INDENT_WITH_NON_TAB) &&
939-
(8 < len) && !strncmp("+ ", line, 9)) {
940-
err = "Indent more than 8 places with spaces";
941-
goto error;
942-
}
943-
return;
903+
char *err;
904+
unsigned result = check_and_emit_line(line + 1, len - 1, ws_rule,
905+
NULL, NULL, NULL, NULL);
906+
if (!result)
907+
return;
944908

945-
error:
946909
whitespace_error++;
947910
if (squelch_whitespace_errors &&
948911
squelch_whitespace_errors < whitespace_error)
949912
;
950-
else
913+
else {
914+
err = whitespace_error_string(result);
951915
fprintf(stderr, "%s.\n%s:%d:%.*s\n",
952-
err, patch_input_file, linenr, len-2, line+1);
916+
err, patch_input_file, linenr, len - 2, line + 1);
917+
free(err);
918+
}
953919
}
954920

955921
/*

‎cache.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -655,6 +655,10 @@ void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, i
655655
extern unsigned whitespace_rule_cfg;
656656
extern unsigned whitespace_rule(const char *);
657657
extern unsigned parse_whitespace_rule(const char *);
658+
extern unsigned check_and_emit_line(const char *line, int len, unsigned ws_rule,
659+
FILE *stream, const char *set,
660+
const char *reset, const char *ws);
661+
extern char *whitespace_error_string(unsigned ws);
658662

659663
/* ls-files */
660664
int pathspec_match(const char **spec, char *matched, const char *filename, int skiplen);

‎diff.c‎

Lines changed: 19 additions & 119 deletions
Original file line numberDiff line numberDiff line change
@@ -486,88 +486,9 @@ const char *diff_get_color(int diff_use_color, enum color_diff ix)
486486

487487
static void emit_line(const char *set, const char *reset, const char *line, int len)
488488
{
489-
if (len > 0 && line[len-1] == '\n')
490-
len--;
491489
fputs(set, stdout);
492490
fwrite(line, len, 1, stdout);
493-
puts(reset);
494-
}
495-
496-
static void emit_line_with_ws(int nparents,
497-
const char *set, const char *reset, const char *ws,
498-
const char *line, int len, unsigned ws_rule)
499-
{
500-
int col0 = nparents;
501-
int last_tab_in_indent = -1;
502-
int last_space_in_indent = -1;
503-
int i;
504-
int tail = len;
505-
int need_highlight_leading_space = 0;
506-
/*
507-
* The line is a newly added line. Does it have funny leading
508-
* whitespaces? In indent, SP should never precede a TAB. In
509-
* addition, under "indent with non tab" rule, there should not
510-
* be more than 8 consecutive spaces.
511-
*/
512-
for (i = col0; i < len; i++) {
513-
if (line[i] == '\t') {
514-
last_tab_in_indent = i;
515-
if ((ws_rule & WS_SPACE_BEFORE_TAB) &&
516-
0 <= last_space_in_indent)
517-
need_highlight_leading_space = 1;
518-
}
519-
else if (line[i] == ' ')
520-
last_space_in_indent = i;
521-
else
522-
break;
523-
}
524-
if ((ws_rule & WS_INDENT_WITH_NON_TAB) &&
525-
0 <= last_space_in_indent &&
526-
last_tab_in_indent < 0 &&
527-
8 <= (i - col0)) {
528-
last_tab_in_indent = i;
529-
need_highlight_leading_space = 1;
530-
}
531-
fputs(set, stdout);
532-
fwrite(line, col0, 1, stdout);
533491
fputs(reset, stdout);
534-
if (((i == len) || line[i] == '\n') && i != col0) {
535-
/* The whole line was indent */
536-
emit_line(ws, reset, line + col0, len - col0);
537-
return;
538-
}
539-
i = col0;
540-
if (need_highlight_leading_space) {
541-
while (i < last_tab_in_indent) {
542-
if (line[i] == ' ') {
543-
fputs(ws, stdout);
544-
putchar(' ');
545-
fputs(reset, stdout);
546-
}
547-
else
548-
putchar(line[i]);
549-
i++;
550-
}
551-
}
552-
tail = len - 1;
553-
if (line[tail] == '\n' && i < tail)
554-
tail--;
555-
if (ws_rule & WS_TRAILING_SPACE) {
556-
while (i < tail) {
557-
if (!isspace(line[tail]))
558-
break;
559-
tail--;
560-
}
561-
}
562-
if ((i < tail && line[tail + 1] != '\n')) {
563-
/* This has whitespace between tail+1..len */
564-
fputs(set, stdout);
565-
fwrite(line + i, tail - i + 1, 1, stdout);
566-
fputs(reset, stdout);
567-
emit_line(ws, reset, line + tail + 1, len - tail - 1);
568-
}
569-
else
570-
emit_line(set, reset, line + i, len - i);
571492
}
572493

573494
static void emit_add_line(const char *reset, struct emit_callback *ecbdata, const char *line, int len)
@@ -577,9 +498,13 @@ static void emit_add_line(const char *reset, struct emit_callback *ecbdata, cons
577498

578499
if (!*ws)
579500
emit_line(set, reset, line, len);
580-
else
581-
emit_line_with_ws(ecbdata->nparents, set, reset, ws,
582-
line, len, ecbdata->ws_rule);
501+
else {
502+
/* Emit just the prefix, then the rest. */
503+
emit_line(set, reset, line, ecbdata->nparents);
504+
(void)check_and_emit_line(line + ecbdata->nparents,
505+
len - ecbdata->nparents, ecbdata->ws_rule,
506+
stdout, set, reset, ws);
507+
}
583508
}
584509

585510
static void fn_out_consume(void *priv, char *line, unsigned long len)
@@ -1040,45 +965,20 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len)
1040965
const char *ws = diff_get_color(data->color_diff, DIFF_WHITESPACE);
1041966
const char *reset = diff_get_color(data->color_diff, DIFF_RESET);
1042967
const char *set = diff_get_color(data->color_diff, DIFF_FILE_NEW);
968+
char *err;
1043969

1044970
if (line[0] == '+') {
1045-
int i, spaces = 0, space_before_tab = 0, white_space_at_end = 0;
1046-
1047-
/* check space before tab */
1048-
for (i = 1; i < len; i++) {
1049-
if (line[i] == ' ')
1050-
spaces++;
1051-
else if (line[i] == '\t') {
1052-
if (spaces) {
1053-
space_before_tab = 1;
1054-
break;
1055-
}
1056-
}
1057-
else
1058-
break;
1059-
}
1060-
1061-
/* check whitespace at line end */
1062-
if (line[len - 1] == '\n')
1063-
len--;
1064-
if (isspace(line[len - 1]))
1065-
white_space_at_end = 1;
1066-
1067-
if (space_before_tab || white_space_at_end) {
1068-
data->status = 1;
1069-
printf("%s:%d: %s", data->filename, data->lineno, ws);
1070-
if (space_before_tab) {
1071-
printf("space before tab");
1072-
if (white_space_at_end)
1073-
putchar(',');
1074-
}
1075-
if (white_space_at_end)
1076-
printf("whitespace at end");
1077-
printf(":%s ", reset);
1078-
emit_line_with_ws(1, set, reset, ws, line, len,
1079-
data->ws_rule);
1080-
}
1081-
971+
data->status = check_and_emit_line(line + 1, len - 1,
972+
data->ws_rule, NULL, NULL, NULL, NULL);
973+
if (!data->status)
974+
return;
975+
err = whitespace_error_string(data->status);
976+
printf("%s:%d: %s%s:%s ", data->filename, data->lineno,
977+
ws, err, reset);
978+
free(err);
979+
emit_line(set, reset, line, 1);
980+
(void)check_and_emit_line(line + 1, len - 1, data->ws_rule,
981+
stdout, set, reset, ws);
1082982
data->lineno++;
1083983
} else if (line[0] == ' ')
1084984
data->lineno++;

‎t/t4015-diff-whitespace.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ test_expect_success 'check mixed spaces and tabs in indent' '
121121
122122
# This is indented with SP HT SP.
123123
echo " foo();" > x &&
124-
git diff --check | grep "space before tab"
124+
git diff --check | grep "Space in indent is followed by a tab"
125125
126126
'
127127

‎ws.c‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,3 +94,108 @@ unsigned whitespace_rule(const char *pathname)
9494
return whitespace_rule_cfg;
9595
}
9696
}
97+
98+
/* The returned string should be freed by the caller. */
99+
char *whitespace_error_string(unsigned ws)
100+
{
101+
struct strbuf err;
102+
strbuf_init(&err, 0);
103+
if (ws & WS_TRAILING_SPACE)
104+
strbuf_addstr(&err, "Adds trailing whitespace");
105+
if (ws & WS_SPACE_BEFORE_TAB) {
106+
if (err.len)
107+
strbuf_addstr(&err, ", ");
108+
strbuf_addstr(&err, "Space in indent is followed by a tab");
109+
}
110+
if (ws & WS_INDENT_WITH_NON_TAB) {
111+
if (err.len)
112+
strbuf_addstr(&err, ", ");
113+
strbuf_addstr(&err, "Indent more than 8 places with spaces");
114+
}
115+
return strbuf_detach(&err, NULL);
116+
}
117+
118+
/* If stream is non-NULL, emits the line after checking. */
119+
unsigned check_and_emit_line(const char *line, int len, unsigned ws_rule,
120+
FILE *stream, const char *set,
121+
const char *reset, const char *ws)
122+
{
123+
unsigned result = 0;
124+
int leading_space = -1;
125+
int trailing_whitespace = -1;
126+
int trailing_newline = 0;
127+
int i;
128+
129+
/* Logic is simpler if we temporarily ignore the trailing newline. */
130+
if (len > 0 && line[len - 1] == '\n') {
131+
trailing_newline = 1;
132+
len--;
133+
}
134+
135+
/* Check for trailing whitespace. */
136+
if (ws_rule & WS_TRAILING_SPACE) {
137+
for (i = len - 1; i >= 0; i--) {
138+
if (isspace(line[i])) {
139+
trailing_whitespace = i;
140+
result |= WS_TRAILING_SPACE;
141+
}
142+
else
143+
break;
144+
}
145+
}
146+
147+
/* Check for space before tab in initial indent. */
148+
for (i = 0; i < len; i++) {
149+
if (line[i] == '\t') {
150+
if ((ws_rule & WS_SPACE_BEFORE_TAB) &&
151+
(leading_space != -1))
152+
result |= WS_SPACE_BEFORE_TAB;
153+
break;
154+
}
155+
else if (line[i] == ' ')
156+
leading_space = i;
157+
else
158+
break;
159+
}
160+
161+
/* Check for indent using non-tab. */
162+
if ((ws_rule & WS_INDENT_WITH_NON_TAB) && leading_space >= 8)
163+
result |= WS_INDENT_WITH_NON_TAB;
164+
165+
if (stream) {
166+
/* Highlight errors in leading whitespace. */
167+
if ((result & WS_SPACE_BEFORE_TAB) ||
168+
(result & WS_INDENT_WITH_NON_TAB)) {
169+
fputs(ws, stream);
170+
fwrite(line, leading_space + 1, 1, stream);
171+
fputs(reset, stream);
172+
leading_space++;
173+
}
174+
else
175+
leading_space = 0;
176+
177+
/* Now the rest of the line starts at leading_space.
178+
* The non-highlighted part ends at trailing_whitespace. */
179+
if (trailing_whitespace == -1)
180+
trailing_whitespace = len;
181+
182+
/* Emit non-highlighted (middle) segment. */
183+
if (trailing_whitespace - leading_space > 0) {
184+
fputs(set, stream);
185+
fwrite(line + leading_space,
186+
trailing_whitespace - leading_space, 1, stream);
187+
fputs(reset, stream);
188+
}
189+
190+
/* Highlight errors in trailing whitespace. */
191+
if (trailing_whitespace != len) {
192+
fputs(ws, stream);
193+
fwrite(line + trailing_whitespace,
194+
len - trailing_whitespace, 1, stream);
195+
fputs(reset, stream);
196+
}
197+
if (trailing_newline)
198+
fputc('\n', stream);
199+
}
200+
return result;
201+
}

0 commit comments

Comments
 (0)