diff options
| -rw-r--r-- | Documentation/config.txt | 18 | ||||
| -rw-r--r-- | Documentation/git-apply.txt | 35 | ||||
| -rw-r--r-- | Documentation/gitattributes.txt | 31 | ||||
| -rw-r--r-- | Makefile | 2 | ||||
| -rw-r--r-- | builtin-apply.c | 238 | ||||
| -rw-r--r-- | cache.h | 12 | ||||
| -rw-r--r-- | config.c | 5 | ||||
| -rw-r--r-- | diff.c | 40 | ||||
| -rw-r--r-- | environment.c | 1 | ||||
| -rwxr-xr-x | t/t4019-diff-wserror.sh | 123 | ||||
| -rwxr-xr-x | t/t4124-apply-ws-rule.sh | 151 | ||||
| -rw-r--r-- | ws.c | 96 |
12 files changed, 641 insertions, 111 deletions
diff --git a/Documentation/config.txt b/Documentation/config.txt index 79d51f26cc..fabe7f859f 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -295,6 +295,20 @@ core.pager:: The command that git will use to paginate output. Can be overridden with the `GIT_PAGER` environment variable. +core.whitespace:: + A comma separated list of common whitespace problems to + notice. `git diff` will use `color.diff.whitespace` to + highlight them, and `git apply --whitespace=error` will + consider them as errors: ++ +* `trailing-space` treats trailing whitespaces at the end of the line + as an error (enabled by default). +* `space-before-tab` treats a space character that appears immediately + before a tab character in the initial indent part of the line as an + error (enabled by default). +* `indent-with-non-tab` treats a line that is indented with 8 or more + space characters that can be replaced with tab characters. + alias.*:: Command aliases for the gitlink:git[1] command wrapper - e.g. after defining "alias.last = cat-file commit HEAD", the invocation @@ -387,8 +401,8 @@ color.diff.<slot>:: which part of the patch to use the specified color, and is one of `plain` (context text), `meta` (metainformation), `frag` (hunk header), `old` (removed lines), `new` (added lines), - `commit` (commit headers), or `whitespace` (highlighting dubious - whitespace). The values of these variables may be specified as + `commit` (commit headers), or `whitespace` (highlighting + whitespace errors). The values of these variables may be specified as in color.branch.<slot>. color.interactive:: diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt index c1c54bfe0b..bae3e7b909 100644 --- a/Documentation/git-apply.txt +++ b/Documentation/git-apply.txt @@ -13,7 +13,7 @@ SYNOPSIS [--apply] [--no-add] [--build-fake-ancestor <file>] [-R | --reverse] [--allow-binary-replacement | --binary] [--reject] [-z] [-pNUM] [-CNUM] [--inaccurate-eof] [--cached] - [--whitespace=<nowarn|warn|error|error-all|strip>] + [--whitespace=<nowarn|warn|fix|error|error-all>] [--exclude=PATH] [--verbose] [<patch>...] DESCRIPTION @@ -135,25 +135,32 @@ discouraged. be useful when importing patchsets, where you want to exclude certain files or directories. ---whitespace=<option>:: - When applying a patch, detect a new or modified line - that ends with trailing whitespaces (this includes a - line that solely consists of whitespaces). By default, - the command outputs warning messages and applies the - patch. - When gitlink:git-apply[1] is used for statistics and not applying a - patch, it defaults to `nowarn`. - You can use different `<option>` to control this - behavior: +--whitespace=<action>:: + When applying a patch, detect a new or modified line that has + whitespace errors. What are considered whitespace errors is + controlled by `core.whitespace` configuration. By default, + trailing whitespaces (including lines that solely consist of + whitespaces) and a space character that is immediately followed + by a tab character inside the initial indent of the line are + considered whitespace errors. ++ +By default, the command outputs warning messages but applies the patch. +When gitlink:git-apply[1] is used for statistics and not applying a +patch, it defaults to `nowarn`. ++ +You can use different `<action>` to control this +behavior: + * `nowarn` turns off the trailing whitespace warning. * `warn` outputs warnings for a few such errors, but applies the - patch (default). + patch as-is (default). +* `fix` outputs warnings for a few such errors, and applies the + patch after fixing them (`strip` is a synonym --- the tool + used to consider only trailing whitespaces as errors, and the + fix involved 'stripping' them, but modern gits do more). * `error` outputs warnings for a few such errors, and refuses to apply the patch. * `error-all` is similar to `error` but shows all errors. -* `strip` outputs warnings for a few such errors, strips out the - trailing whitespaces and applies the patch. --inaccurate-eof:: Under certain circumstances, some versions of diff do not correctly diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt index 19bd25f299..71c7ad76d5 100644 --- a/Documentation/gitattributes.txt +++ b/Documentation/gitattributes.txt @@ -361,6 +361,37 @@ When left unspecified, the driver itself is used for both internal merge and the final merge. +Checking whitespace errors +~~~~~~~~~~~~~~~~~~~~~~~~~~ + +`whitespace` +^^^^^^^^^^^^ + +The `core.whitespace` configuration variable allows you to define what +`diff` and `apply` should consider whitespace errors for all paths in +the project (See gitlink:git-config[1]). This attribute gives you finer +control per path. + +Set:: + + Notice all types of potential whitespace errors known to git. + +Unset:: + + Do not notice anything as error. + +Unspecified:: + + Use the value of `core.whitespace` configuration variable to + decide what to notice as error. + +String:: + + Specify a comma separate list of common whitespace problems to + notice in the same format as `core.whitespace` configuration + variable. + + EXAMPLE ------- @@ -313,7 +313,7 @@ LIB_OBJS = \ alloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \ color.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \ convert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o \ - transport.o bundle.o walker.o parse-options.o + transport.o bundle.o walker.o parse-options.o ws.o BUILTIN_OBJS = \ builtin-add.o \ diff --git a/builtin-apply.c b/builtin-apply.c index 91f8752ff7..f2e9a332ca 100644 --- a/builtin-apply.c +++ b/builtin-apply.c @@ -45,14 +45,14 @@ static const char *fake_ancestor; static int line_termination = '\n'; static unsigned long p_context = ULONG_MAX; static const char apply_usage[] = -"git-apply [--stat] [--numstat] [--summary] [--check] [--index] [--cached] [--apply] [--no-add] [--index-info] [--allow-binary-replacement] [--reverse] [--reject] [--verbose] [-z] [-pNUM] [-CNUM] [--whitespace=<nowarn|warn|error|error-all|strip>] <patch>..."; - -static enum whitespace_eol { - nowarn_whitespace, - warn_on_whitespace, - error_on_whitespace, - strip_whitespace, -} new_whitespace = warn_on_whitespace; +"git-apply [--stat] [--numstat] [--summary] [--check] [--index] [--cached] [--apply] [--no-add] [--index-info] [--allow-binary-replacement] [--reverse] [--reject] [--verbose] [-z] [-pNUM] [-CNUM] [--whitespace=<nowarn|warn|fix|error|error-all>] <patch>..."; + +static enum ws_error_action { + nowarn_ws_error, + warn_on_ws_error, + die_on_ws_error, + correct_ws_error, +} ws_error_action = warn_on_ws_error; static int whitespace_error; static int squelch_whitespace_errors = 5; static int applied_after_fixing_ws; @@ -61,28 +61,28 @@ static const char *patch_input_file; static void parse_whitespace_option(const char *option) { if (!option) { - new_whitespace = warn_on_whitespace; + ws_error_action = warn_on_ws_error; return; } if (!strcmp(option, "warn")) { - new_whitespace = warn_on_whitespace; + ws_error_action = warn_on_ws_error; return; } if (!strcmp(option, "nowarn")) { - new_whitespace = nowarn_whitespace; + ws_error_action = nowarn_ws_error; return; } if (!strcmp(option, "error")) { - new_whitespace = error_on_whitespace; + ws_error_action = die_on_ws_error; return; } if (!strcmp(option, "error-all")) { - new_whitespace = error_on_whitespace; + ws_error_action = die_on_ws_error; squelch_whitespace_errors = 0; return; } - if (!strcmp(option, "strip")) { - new_whitespace = strip_whitespace; + if (!strcmp(option, "strip") || !strcmp(option, "fix")) { + ws_error_action = correct_ws_error; return; } die("unrecognized whitespace option '%s'", option); @@ -90,11 +90,8 @@ static void parse_whitespace_option(const char *option) static void set_default_whitespace_mode(const char *whitespace_option) { - if (!whitespace_option && !apply_default_whitespace) { - new_whitespace = (apply - ? warn_on_whitespace - : nowarn_whitespace); - } + if (!whitespace_option && !apply_default_whitespace) + ws_error_action = (apply ? warn_on_ws_error : nowarn_ws_error); } /* @@ -137,11 +134,17 @@ struct fragment { #define BINARY_DELTA_DEFLATED 1 #define BINARY_LITERAL_DEFLATED 2 +/* + * This represents a "patch" to a file, both metainfo changes + * such as creation/deletion, filemode and content changes represented + * as a series of fragments. + */ struct patch { char *new_name, *old_name, *def_name; unsigned int old_mode, new_mode; int is_new, is_delete; /* -1 = unknown, 0 = false, 1 = true */ int rejected; + unsigned ws_rule; unsigned long deflate_origlen; int lines_added, lines_deleted; int score; @@ -158,7 +161,8 @@ struct patch { struct patch *next; }; -static void say_patch_name(FILE *output, const char *pre, struct patch *patch, const char *post) +static void say_patch_name(FILE *output, const char *pre, + struct patch *patch, const char *post) { fputs(pre, output); if (patch->old_name && patch->new_name && @@ -229,7 +233,8 @@ static char *find_name(const char *line, char *def, int p_value, int terminate) if (*line == '"') { struct strbuf name; - /* Proposed "new-style" GNU patch/diff format; see + /* + * Proposed "new-style" GNU patch/diff format; see * http://marc.theaimsgroup.com/?l=git&m=112927316408690&w=2 */ strbuf_init(&name, 0); @@ -499,7 +504,8 @@ static int gitdiff_dissimilarity(const char *line, struct patch *patch) static int gitdiff_index(const char *line, struct patch *patch) { - /* index line is N hexadecimal, "..", N hexadecimal, + /* + * index line is N hexadecimal, "..", N hexadecimal, * and optional space with octal mode. */ const char *ptr, *eol; @@ -550,7 +556,8 @@ static const char *stop_at_slash(const char *line, int llen) return NULL; } -/* This is to extract the same name that appears on "diff --git" +/* + * This is to extract the same name that appears on "diff --git" * line. We do not find and return anything if it is a rename * patch, and it is OK because we will find the name elsewhere. * We need to reliably find name only when it is mode-change only, @@ -584,7 +591,8 @@ static char *git_header_name(char *line, int llen) goto free_and_fail1; strbuf_remove(&first, 0, cp + 1 - first.buf); - /* second points at one past closing dq of name. + /* + * second points at one past closing dq of name. * find the second name. */ while ((second < line + llen) && isspace(*second)) @@ -627,7 +635,8 @@ static char *git_header_name(char *line, int llen) return NULL; name++; - /* since the first name is unquoted, a dq if exists must be + /* + * since the first name is unquoted, a dq if exists must be * the beginning of the second name. */ for (second = name; second < line + llen; second++) { @@ -758,7 +767,7 @@ static int parse_num(const char *line, unsigned long *p) } static int parse_range(const char *line, int len, int offset, const char *expect, - unsigned long *p1, unsigned long *p2) + unsigned long *p1, unsigned long *p2) { int digits, ex; @@ -867,14 +876,14 @@ static int find_header(char *line, unsigned long size, int *hdrsize, struct patc return offset; } - /** --- followed by +++ ? */ + /* --- followed by +++ ? */ if (memcmp("--- ", line, 4) || memcmp("+++ ", line + len, 4)) continue; /* * We only accept unified patches, so we want it to * at least have "@@ -a,b +c,d @@\n", which is 14 chars - * minimum + * minimum ("@@ -0,0 +1 @@\n" is the shortest). */ nextlen = linelen(line + len, size - len); if (size < nextlen + 14 || memcmp("@@ -", line + len + nextlen, 4)) @@ -889,7 +898,7 @@ static int find_header(char *line, unsigned long size, int *hdrsize, struct patc return -1; } -static void check_whitespace(const char *line, int len) +static void check_whitespace(const char *line, int len, unsigned ws_rule) { const char *err = "Adds trailing whitespace"; int seen_space = 0; @@ -901,23 +910,35 @@ static void check_whitespace(const char *line, int len) * this function. That is, an addition of an empty line would * check the '+' here. Sneaky... */ - if (isspace(line[len-2])) + 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. */ - 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; + 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; @@ -931,14 +952,14 @@ static void check_whitespace(const char *line, int len) err, patch_input_file, linenr, len-2, line+1); } - /* * Parse a unified diff. Note that this really needs to parse each * fragment separately, since the only way to know the difference * between a "---" that is part of a patch, and a "---" that starts * the next patch is to look at the line counts.. */ -static int parse_fragment(char *line, unsigned long size, struct patch *patch, struct fragment *fragment) +static int parse_fragment(char *line, unsigned long size, + struct patch *patch, struct fragment *fragment) { int added, deleted; int len = linelen(line, size), offset; @@ -979,22 +1000,23 @@ static int parse_fragment(char *line, unsigned long size, struct patch *patch, s break; case '-': if (apply_in_reverse && - new_whitespace != nowarn_whitespace) - check_whitespace(line, len); + ws_error_action != nowarn_ws_error) + check_whitespace(line, len, patch->ws_rule); deleted++; oldlines--; trailing = 0; break; case '+': if (!apply_in_reverse && - new_whitespace != nowarn_whitespace) - check_whitespace(line, len); + ws_error_action != nowarn_ws_error) + check_whitespace(line, len, patch->ws_rule); added++; newlines--; trailing = 0; break; - /* We allow "\ No newline at end of file". Depending + /* + * We allow "\ No newline at end of file". Depending * on locale settings when the patch was produced we * don't know what this line looks like. The only * thing we do know is that it begins with "\ ". @@ -1012,7 +1034,8 @@ static int parse_fragment(char *line, unsigned long size, struct patch *patch, s fragment->leading = leading; fragment->trailing = trailing; - /* If a fragment ends with an incomplete line, we failed to include + /* + * If a fragment ends with an incomplete line, we failed to include * it in the above loop because we hit oldlines == newlines == 0 * before seeing it. */ @@ -1140,7 +1163,8 @@ static struct fragment *parse_binary_hunk(char **buf_p, int *status_p, int *used_p) { - /* Expect a line that begins with binary patch method ("literal" + /* + * Expect a line that begins with binary patch method ("literal" * or "delta"), followed by the length of data before deflating. * a sequence of 'length-byte' followed by base-85 encoded data * should follow, terminated by a newline. @@ -1189,7 +1213,8 @@ static struct fragment *parse_binary_hunk(char **buf_p, size--; break; } - /* Minimum line is "A00000\n" which is 7-byte long, + /* + * Minimum line is "A00000\n" which is 7-byte long, * and the line length must be multiple of 5 plus 2. */ if ((llen < 7) || (llen-2) % 5) @@ -1240,7 +1265,8 @@ static struct fragment *parse_binary_hunk(char **buf_p, static int parse_binary(char *buffer, unsigned long size, struct patch *patch) { - /* We have read "GIT binary patch\n"; what follows is a line + /* + * We have read "GIT binary patch\n"; what follows is a line * that says the patch method (currently, either "literal" or * "delta") and the length of data before deflating; a * sequence of 'length-byte' followed by base-85 encoded data @@ -1270,7 +1296,8 @@ static int parse_binary(char *buffer, unsigned long size, struct patch *patch) if (reverse) used += used_1; else if (status) { - /* not having reverse hunk is not an error, but having + /* + * Not having reverse hunk is not an error, but having * a corrupt reverse hunk is. */ free((void*) forward->patch); @@ -1291,7 +1318,12 @@ static int parse_chunk(char *buffer, unsigned long size, struct patch *patch) if (offset < 0) return offset; - patchsize = parse_single_patch(buffer + offset + hdrsize, size - offset - hdrsize, patch); + patch->ws_rule = whitespace_rule(patch->new_name + ? patch->new_name + : patch->old_name); + + patchsize = parse_single_patch(buffer + offset + hdrsize, + size - offset - hdrsize, patch); if (!patchsize) { static const char *binhdr[] = { @@ -1367,8 +1399,10 @@ static void reverse_patches(struct patch *p) } } -static const char pluses[] = "++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++"; -static const char minuses[]= "----------------------------------------------------------------------"; +static const char pluses[] = +"++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++"; +static const char minuses[]= +"----------------------------------------------------------------------"; static void show_stats(struct patch *patch) { @@ -1437,7 +1471,9 @@ static int read_old_data(struct stat *st, const char *path, struct strbuf *buf) } } -static int find_offset(const char *buf, unsigned long size, const char *fragment, unsigned long fragsize, int line, int *lines) +static int find_offset(const char *buf, unsigned long size, + const char *fragment, unsigned long fragsize, + int line, int *lines) { int i; unsigned long start, backwards, forwards; @@ -1536,9 +1572,11 @@ static void remove_last_line(const char **rbuf, int *rsize) *rsize = offset + 1; } -static int apply_line(char *output, const char *patch, int plen) +static int apply_line(char *output, const char *patch, int plen, + unsigned ws_rule) { - /* plen is number of bytes to be copied from patch, + /* + * plen is number of bytes to be copied from patch, * starting at patch+1 (patch[0] is '+'). Typically * patch[plen] is '\n', unless this is the incomplete * last line. @@ -1551,13 +1589,17 @@ static int apply_line(char *output, const char *patch, int plen) int need_fix_leading_space = 0; char *buf; - if ((new_whitespace != strip_whitespace) || !whitespace_error || + if ((ws_error_action != correct_ws_error) || !whitespace_error || *patch != '+') { memcpy(output, patch + 1, plen); return plen; } - if (1 < plen && isspace(patch[plen-1])) { + /* + * Strip trailing whitespace + */ + if ((ws_rule & WS_TRAILING_SPACE) && + (1 < plen && isspace(patch[plen-1]))) { if (patch[plen] == '\n') add_nl_to_tail = 1; plen--; @@ -1566,15 +1608,23 @@ static int apply_line(char *output, const char *patch, int plen) fixed = 1; } + /* + * Check leading whitespaces (indent) + */ for (i = 1; i < plen; i++) { char ch = patch[i]; if (ch == '\t') { last_tab_in_indent = i; - if (0 <= last_space_in_indent) + if ((ws_rule & WS_SPACE_BEFORE_TAB) && + 0 <= last_space_in_indent) + need_fix_leading_space = 1; + } else if (ch == ' ') { + last_space_in_indent = i; + if ((ws_rule & WS_INDENT_WITH_NON_TAB) && + last_tab_in_indent < 0 && + 8 <= i) need_fix_leading_space = 1; } - else if (ch == ' ') - last_space_in_indent = i; else break; } @@ -1582,10 +1632,21 @@ static int apply_line(char *output, const char *patch, int plen) buf = output; if (need_fix_leading_space) { int consecutive_spaces = 0; - /* between patch[1..last_tab_in_indent] strip the - * funny spaces, updating them to tab as needed. + int last = last_tab_in_indent + 1; + + if (ws_rule & WS_INDENT_WITH_NON_TAB) { + /* have "last" point at one past the indent */ + if (last_tab_in_indent < last_space_in_indent) + last = last_space_in_indent + 1; + else + last = last_tab_in_indent + 1; + } + + /* + * between patch[1..last], strip the funny spaces, + * updating them to tab as needed. */ - for (i = 1; i < last_tab_in_indent; i++, plen--) { + for (i = 1; i < last; i++, plen--) { char ch = patch[i]; if (ch != ' ') { consecutive_spaces = 0; @@ -1598,8 +1659,10 @@ static int apply_line(char *output, const char *patch, int plen) } } } + while (0 < consecutive_spaces--) + *output++ = ' '; fixed = 1; - i = last_tab_in_indent; + i = last; } else i = 1; @@ -1612,7 +1675,8 @@ static int apply_line(char *output, const char *patch, int plen) return output + plen - buf; } -static int apply_one_fragment(struct strbuf *buf, struct fragment *frag, int inaccurate_eof) +static int apply_one_fragment(struct strbuf *buf, struct fragment *frag, + int inaccurate_eof, unsigned ws_rule) { int match_beginning, match_end; const char *patch = frag->patch; @@ -1671,7 +1735,7 @@ static int apply_one_fragment(struct strbuf *buf, struct fragment *frag, int ina case '+': if (first != '+' || !no_add) { int added = apply_line(new + newsize, patch, - plen); + plen, ws_rule); newsize += added; if (first == '+' && added == 1 && new[newsize-1] == '\n') @@ -1694,8 +1758,9 @@ static int apply_one_fragment(struct strbuf *buf, struct fragment *frag, int ina size -= len; } - if (inaccurate_eof && oldsize > 0 && old[oldsize - 1] == '\n' && - newsize > 0 && new[newsize - 1] == '\n') { + if (inaccurate_eof && + oldsize > 0 && old[oldsize - 1] == '\n' && + newsize > 0 && new[newsize - 1] == '\n') { oldsize--; newsize--; } @@ -1732,7 +1797,7 @@ static int apply_one_fragment(struct strbuf *buf, struct fragment *frag, int ina if (match_beginning && offset) offset = -1; if (offset >= 0) { - if (new_whitespace == strip_whitespace && + if (ws_error_action == correct_ws_error && (buf->len - oldsize - offset == 0)) /* end of file? */ newsize -= new_blank_lines_at_end; @@ -1757,9 +1822,10 @@ static int apply_one_fragment(struct strbuf *buf, struct fragment *frag, int ina match_beginning = match_end = 0; continue; } - /* Reduce the number of context lines - * Reduce both leading and trailing if they are equal - * otherwise just reduce the larger context. + /* + * Reduce the number of context lines; reduce both + * leading and trailing if they are equal otherwise + * just reduce the larger context. */ if (leading >= trailing) { remove_first_line(&oldlines, &oldsize); @@ -1819,7 +1885,8 @@ static int apply_binary(struct strbuf *buf, struct patch *patch) const char *name = patch->old_name ? patch->old_name : patch->new_name; unsigned char sha1[20]; - /* For safety, we require patch index line to contain + /* + * For safety, we require patch index line to contain * full 40-byte textual SHA1 for old and new, at least for now. */ if (strlen(patch->old_sha1_prefix) != 40 || @@ -1830,7 +1897,8 @@ static int apply_binary(struct strbuf *buf, struct patch *patch) "without full index line", name); if (patch->old_name) { - /* See if the old one matches what the patch + /* + * See if the old one matches what the patch * applies to. */ hash_sha1_file(buf->buf, buf->len, blob_type, sha1); @@ -1867,7 +1935,8 @@ static int apply_binary(struct strbuf *buf, struct patch *patch) /* XXX read_sha1_file NUL-terminates */ strbuf_attach(buf, result, size, size + 1); } else { - /* We have verified buf matches the preimage; + /* + * We have verified buf matches the preimage; * apply the patch data to it, which is stored * in the patch->fragments->{patch,size}. */ @@ -1889,12 +1958,14 @@ static int apply_fragments(struct strbuf *buf, struct patch *patch) { struct fragment *frag = patch->fragments; const char *name = patch->old_name ? patch->old_name : patch->new_name; + unsigned ws_rule = patch->ws_rule; + unsigned inaccurate_eof = patch->inaccurate_eof; if (patch->is_binary) return apply_binary(buf, patch); while (frag) { - if (apply_one_fragment(buf, frag, patch->inaccurate_eof)) { + if (apply_one_fragment(buf, frag, inaccurate_eof, ws_rule)) { error("patch failed: %s:%ld", name, frag->oldpos); if (!apply_with_reject) return -1; @@ -2066,7 +2137,8 @@ static int check_patch(struct patch *patch, struct patch *prev_patch) if (new_name && prev_patch && 0 < prev_patch->is_delete && !strcmp(prev_patch->old_name, new_name)) - /* A type-change diff is always split into a patch to + /* + * A type-change diff is always split into a patch to * delete old, immediately followed by a patch to * create new (see diff.c::run_diff()); in such a case * it is Ok that the entry to be deleted by the @@ -2670,7 +2742,7 @@ static int apply_patch(int fd, const char *filename, int inaccurate_eof) offset += nr; } - if (whitespace_error && (new_whitespace == error_on_whitespace)) + if (whitespace_error && (ws_error_action == die_on_ws_error)) apply = 0; update_index = check_index && apply; @@ -2865,7 +2937,7 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix) squelched, squelched == 1 ? "" : "s"); } - if (new_whitespace == error_on_whitespace) + if (ws_error_action == die_on_ws_error) die("%d line%s add%s whitespace errors.", whitespace_error, whitespace_error == 1 ? "" : "s", @@ -644,6 +644,18 @@ extern int diff_auto_refresh_index; /* match-trees.c */ void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, int); +/* + * whitespace rules. + * used by both diff and apply + */ +#define WS_TRAILING_SPACE 01 +#define WS_SPACE_BEFORE_TAB 02 +#define WS_INDENT_WITH_NON_TAB 04 +#define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB) +extern unsigned whitespace_rule_cfg; +extern unsigned whitespace_rule(const char *); +extern unsigned parse_whitespace_rule(const char *); + /* ls-files */ int pathspec_match(const char **spec, char *matched, const char *filename, int skiplen); int report_path_error(const char *ps_matched, const char **pathspec, int prefix_offset); @@ -439,6 +439,11 @@ int git_default_config(const char *var, const char *value) return 0; } + if (!strcmp(var, "core.whitespace")) { + whitespace_rule_cfg = parse_whitespace_rule(value); + return 0; + } + /* Add other config variables here and to Documentation/config.txt. */ return 0; } @@ -454,6 +454,7 @@ static void diff_words_show(struct diff_words_data *diff_words) struct emit_callback { struct xdiff_emit_state xm; int nparents, color_diff; + unsigned ws_rule; const char **label_path; struct diff_words_data *diff_words; int *found_changesp; @@ -493,8 +494,8 @@ static void emit_line(const char *set, const char *reset, const char *line, int } static void emit_line_with_ws(int nparents, - const char *set, const char *reset, const char *ws, - const char *line, int len) + const char *set, const char *reset, const char *ws, + const char *line, int len, unsigned ws_rule) { int col0 = nparents; int last_tab_in_indent = -1; @@ -502,13 +503,17 @@ static void emit_line_with_ws(int nparents, int i; int tail = len; int need_highlight_leading_space = 0; - /* The line is a newly added line. Does it have funny leading - * whitespaces? In indent, SP should never precede a TAB. + /* + * The line is a newly added line. Does it have funny leading + * whitespaces? In indent, SP should never precede a TAB. In + * addition, under "indent with non tab" rule, there should not + * be more than 8 consecutive spaces. */ for (i = col0; i < len; i++) { if (line[i] == '\t') { last_tab_in_indent = i; - if (0 <= last_space_in_indent) + if ((ws_rule & WS_SPACE_BEFORE_TAB) && + 0 <= last_space_in_indent) need_highlight_leading_space = 1; } else if (line[i] == ' ') @@ -516,6 +521,13 @@ static void emit_line_with_ws(int nparents, else break; } + if ((ws_rule & WS_INDENT_WITH_NON_TAB) && + 0 <= last_space_in_indent && + last_tab_in_indent < 0 && + 8 <= (i - col0)) { + last_tab_in_indent = i; + need_highlight_leading_space = 1; + } fputs(set, stdout); fwrite(line, col0, 1, stdout); fputs(reset, stdout); @@ -540,10 +552,12 @@ static void emit_line_with_ws(int nparents, tail = len - 1; if (line[tail] == '\n' && i < tail) tail--; - while (i < tail) { - if (!isspace(line[tail])) - break; - tail--; + if (ws_rule & WS_TRAILING_SPACE) { + while (i < tail) { + if (!isspace(line[tail])) + break; + tail--; + } } if ((i < tail && line[tail + 1] != '\n')) { /* This has whitespace between tail+1..len */ @@ -565,7 +579,7 @@ static void emit_add_line(const char *reset, struct emit_callback *ecbdata, cons emit_line(set, reset, line, len); else emit_line_with_ws(ecbdata->nparents, set, reset, ws, - line, len); + line, len, ecbdata->ws_rule); } static void fn_out_consume(void *priv, char *line, unsigned long len) @@ -981,6 +995,7 @@ struct checkdiff_t { struct xdiff_emit_state xm; const char *filename; int lineno, color_diff; + unsigned ws_rule; }; static void checkdiff_consume(void *priv, char *line, unsigned long len) @@ -1016,7 +1031,8 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len) if (white_space_at_end) printf("white space at end"); printf(":%s ", reset); - emit_line_with_ws(1, set, reset, ws, line, len); + emit_line_with_ws(1, set, reset, ws, line, len, + data->ws_rule); } data->lineno++; @@ -1317,6 +1333,7 @@ static void builtin_diff(const char *name_a, ecbdata.label_path = lbl; ecbdata.color_diff = DIFF_OPT_TST(o, COLOR_DIFF); ecbdata.found_changesp = &o->found_changes; + ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a); xpp.flags = XDF_NEED_MINIMAL | o->xdl_opts; xecfg.ctxlen = o->context; xecfg.flags = XDL_EMIT_FUNCNAMES; @@ -1410,6 +1427,7 @@ static void builtin_checkdiff(const char *name_a, const char *name_b, data.filename = name_b ? name_b : name_a; data.lineno = 0; data.color_diff = DIFF_OPT_TST(o, COLOR_DIFF); + data.ws_rule = whitespace_rule(data.filename); if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0) die("unable to read files to diff"); diff --git a/environment.c b/environment.c index 1dab72ec15..f3e3d4138d 100644 --- a/environment.c +++ b/environment.c @@ -36,6 +36,7 @@ int pager_use_color = 1; char *editor_program; char *excludes_file; int auto_crlf = 0; /* 1: both ways, -1: only when adding git objects */ +unsigned whitespace_rule_cfg = WS_DEFAULT_RULE; /* This is set by setup_git_dir_gently() and/or git_default_config() */ char *git_work_tree_cfg; diff --git a/t/t4019-diff-wserror.sh b/t/t4019-diff-wserror.sh new file mode 100755 index 0000000000..67e080bdbe --- /dev/null +++ b/t/t4019-diff-wserror.sh @@ -0,0 +1,123 @@ +#!/bin/sh + +test_description='diff whitespace error detection' + +. ./test-lib.sh + +test_expect_success setup ' + + git config diff.color.whitespace "blue reverse" && + >F && + git add F && + echo " Eight SP indent" >>F && + echo " HT and SP indent" >>F && + echo "With trailing SP " >>F && + echo "No problem" >>F + +' + +blue_grep='7;34m' ;# ESC [ 7 ; 3 4 m + +test_expect_success default ' + + git diff --color >output + grep "$blue_grep" output >error + grep -v "$blue_grep" output >normal + + grep Eight normal >/dev/null && + grep HT error >/dev/null && + grep With error >/dev/null && + grep No normal >/dev/null + +' + +test_expect_success 'without -trail' ' + + git config core.whitespace -trail + git diff --color >output + grep "$blue_grep" output >error + grep -v "$blue_grep" output >normal + + grep Eight normal >/dev/null && + grep HT error >/dev/null && + grep With normal >/dev/null && + grep No normal >/dev/null + +' + +test_expect_success 'without -trail (attribute)' ' + + git config --unset core.whitespace + echo "F whitespace=-trail" >.gitattributes + git diff --color >output + grep "$blue_grep" output >error + grep -v "$blue_grep" output >normal + + grep Eight normal >/dev/null && + grep HT error >/dev/null && + grep With normal >/dev/null && + grep No normal >/dev/null + +' + +test_expect_success 'without -space' ' + + rm -f .gitattributes + git config core.whitespace -space + git diff --color >output + grep "$blue_grep" output >error + grep -v "$blue_grep" output >normal + + grep Eight normal >/dev/null && + grep HT normal >/dev/null && + grep With error >/dev/null && + grep No normal >/dev/null + +' + +test_expect_success 'without -space (attribute)' ' + + git config --unset core.whitespace + echo "F whitespace=-space" >.gitattributes + git diff --color >output + grep "$blue_grep" output >error + grep -v "$blue_grep" output >normal + + grep Eight normal >/dev/null && + grep HT normal >/dev/null && + grep With error >/dev/null && + grep No normal >/dev/null + +' + +test_expect_success 'with indent-non-tab only' ' + + rm -f .gitattributes + git config core.whitespace indent,-trailing,-space + git diff --color >output + grep "$blue_grep" output >error + grep -v "$blue_grep" output >normal + + grep Eight error >/dev/null && + grep HT normal >/dev/null && + grep With normal >/dev/null && + grep No normal >/dev/null + +' + +test_expect_success 'with indent-non-tab only (attribute)' ' + + git config --unset core.whitespace + echo "F whitespace=indent,-trailing,-space" >.gitattributes + git diff --color >output + grep "$blue_grep" output >error + grep -v "$blue_grep" output >normal + + grep Eight error >/dev/null && + grep HT normal >/dev/null && + grep With normal >/dev/null && + grep No normal >/dev/null + +' + +test_done diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh new file mode 100755 index 0000000000..85f3da2b98 --- /dev/null +++ b/t/t4124-apply-ws-rule.sh @@ -0,0 +1,151 @@ +#!/bin/sh + +test_description='core.whitespace rules and git-apply' + +. ./test-lib.sh + +prepare_test_file () { + + # A line that has character X is touched iff RULE is in effect: + # X RULE + # ! trailing-space + # @ space-before-tab + # # indent-with-non-tab + sed -e "s/_/ /g" -e "s/>/ /" <<-\EOF + An_SP in an ordinary line>and a HT. + >A HT. + _>A SP and a HT (@). + _>_A SP, a HT and a SP (@). + _______Seven SP. + ________Eight SP (#). + _______>Seven SP and a HT (@). + ________>Eight SP and a HT (@#). + _______>_Seven SP, a HT and a SP (@). + ________>_Eight SP, a HT and a SP (@#). + _______________Fifteen SP (#). + _______________>Fifteen SP and a HT (@#). + ________________Sixteen SP (#). + ________________>Sixteen SP and a HT (@#). + _____a__Five SP, a non WS, two SP. + A line with a (!) trailing SP_ + A line with a (!) trailing HT> + EOF +} + +apply_patch () { + >target && + sed -e "s|\([ab]\)/file|\1/target|" <patch | + git apply "$@" +} + +test_fix () { + + # fix should not barf + apply_patch --whitespace=fix || return 1 + + # find touched lines + diff file target | sed -n -e "s/^> //p" >fixed + + # the changed lines are all expeced to change + fixed_cnt=$(wc -l <fixed) + case "$1" in + '') expect_cnt=$fixed_cnt ;; + ?*) expect_cnt=$(grep "[$1]" <fixed | wc -l) ;; + esac + test $fixed_cnt -eq $expect_cnt || return 1 + + # and we are not missing anything + case "$1" in + '') expect_cnt=0 ;; + ?*) expect_cnt=$(grep "[$1]" <file | wc -l) ;; + esac + test $fixed_cnt -eq $expect_cnt || return 1 + + # Get the patch actually applied + git diff-files -p target >fixed-patch + test -s fixed-patch && return 0 + + # Make sure it is complaint-free + >target + git apply --whitespace=error-all <fixed-patch + +} + +test_expect_success setup ' + + >file && + git add file && + prepare_test_file >file && + git diff-files -p >patch && + >target && + git add target + +' + +test_expect_success 'whitespace=nowarn, default rule' ' + + apply_patch --whitespace=nowarn && + diff file target + +' + +test_expect_success 'whitespace=warn, default rule' ' + + apply_patch --whitespace=warn && + diff file target + +' + +test_expect_success 'whitespace=error-all, default rule' ' + + apply_patch --whitespace=error-all && return 1 + test -s target && return 1 + : happy + +' + +test_expect_success 'whitespace=error-all, no rule' ' + + git config core.whitespace -trailing,-space-before,-indent && + apply_patch --whitespace=error-all && + diff file target + +' + +test_expect_success 'whitespace=error-all, no rule (attribute)' ' + + git config --unset core.whitespace && + echo "target -whitespace" >.gitattributes && + apply_patch --whitespace=error-all && + diff file target + +' + +for t in - '' +do + case "$t" in '') tt='!' ;; *) tt= ;; esac + for s in - '' + do + case "$s" in '') ts='@' ;; *) ts= ;; esac + for i in - '' + do + case "$i" in '') ti='#' ;; *) ti= ;; esac + rule=${t}trailing,${s}space,${i}indent + + rm -f .gitattributes + test_expect_success "rule=$rule" ' + git config core.whitespace "$rule" && + test_fix "$tt$ts$ti" + ' + + test_expect_success "rule=$rule (attributes)" ' + git config --unset core.whitespace && + echo "target whitespace=$rule" >.gitattributes && + test_fix "$tt$ts$ti" + ' + + done + done +done + +test_done @@ -0,0 +1,96 @@ +/* + * Whitespace rules + * + * Copyright (c) 2007 Junio C Hamano + */ + +#include "cache.h" +#include "attr.h" + +static struct whitespace_rule { + const char *rule_name; + unsigned rule_bits; +} whitespace_rule_names[] = { + { "trailing-space", WS_TRAILING_SPACE }, + { "space-before-tab", WS_SPACE_BEFORE_TAB }, + { "indent-with-non-tab", WS_INDENT_WITH_NON_TAB }, +}; + +unsigned parse_whitespace_rule(const char *string) +{ + unsigned rule = WS_DEFAULT_RULE; + + while (string) { + int i; + size_t len; + const char *ep; + int negated = 0; + + string = string + strspn(string, ", \t\n\r"); + ep = strchr(string, ','); + if (!ep) + len = strlen(string); + else + len = ep - string; + + if (*string == '-') { + negated = 1; + string++; + len--; + } + if (!len) + break; + for (i = 0; i < ARRAY_SIZE(whitespace_rule_names); i++) { + if (strncmp(whitespace_rule_names[i].rule_name, + string, len)) + continue; + if (negated) + rule &= ~whitespace_rule_names[i].rule_bits; + else + rule |= whitespace_rule_names[i].rule_bits; + break; + } + string = ep; + } + return rule; +} + +static void setup_whitespace_attr_check(struct git_attr_check *check) +{ + static struct git_attr *attr_whitespace; + + if (!attr_whitespace) + attr_whitespace = git_attr("whitespace", 10); + check[0].attr = attr_whitespace; +} + +unsigned whitespace_rule(const char *pathname) +{ + struct git_attr_check attr_whitespace_rule; + + setup_whitespace_attr_check(&attr_whitespace_rule); + if (!git_checkattr(pathname, 1, &attr_whitespace_rule)) { + const char *value; + + value = attr_whitespace_rule.value; + if (ATTR_TRUE(value)) { + /* true (whitespace) */ + unsigned all_rule = 0; + int i; + for (i = 0; i < ARRAY_SIZE(whitespace_rule_names); i++) + all_rule |= whitespace_rule_names[i].rule_bits; + return all_rule; + } else if (ATTR_FALSE(value)) { + /* false (-whitespace) */ + return 0; + } else if (ATTR_UNSET(value)) { + /* reset to default (!whitespace) */ + return whitespace_rule_cfg; + } else { + /* string */ + return parse_whitespace_rule(value); + } + } else { + return whitespace_rule_cfg; + } +} |