# [PATCH 2/2] git-diff: Respect core.whitespace.{space-indent,space-before-tab,trailing}. 4 messages from 2007-11-02 to 2007-11-03. Participants: David Symonds, Junio C Hamano. Thread: https://gitlist.dev/t/10591 ## David Symonds, 2007-11-02 15:08 Subject: [PATCH 1/2] Implement parsing for new core.whitespace.* options. Message-ID: <11940160932021-git-send-email-dsymonds@gmail.com> URL: https://gitlist.dev/e/11940160932021-git-send-email-dsymonds%40gmail.com ``` Each of the new core.whitespace.* options (enumerated below) can be set to one of: * okay (default): Whitespace of this type is okay * warn: Whitespace of this type should be warned about * error: Whitespace of this type should raise an error * autofix: Whitespace of this type should be automatically fixed The initial options are: * trailing: Whitespace at the end of a line * space-before-tab: SP HT sequence in the initial whitespace of a line * space-indent: At least 8 spaces in a row at the start of a line Example usage: [core "whitespace"] trailing = autofix space-before-tab = error space-indent = warn Signed-off-by: David Symonds --- cache.h | 16 ++++++++++++++++ config.c | 28 ++++++++++++++++++++++++++++ environment.c | 3 +++ 3 files changed, 47 insertions(+), 0 deletions(-) diff --git a/cache.h b/cache.h index bfffa05..51e3982 100644 --- a/cache.h +++ b/cache.h @@ -602,4 +602,20 @@ 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 + */ +enum whitespace_mode { + WS_OKAY = 0, + WS_WARN, + WS_ERROR, + WS_AUTOFIX +}; +extern enum whitespace_mode ws_mode_trailing; +extern enum whitespace_mode ws_mode_space_before_tab; +extern enum whitespace_mode ws_mode_space_indent; +extern enum whitespace_mode git_config_whitespace_mode(const char *, const char *); + + #endif /* CACHE_H */ diff --git a/config.c b/config.c index dc3148d..8e6f252 100644 --- a/config.c +++ b/config.c @@ -297,6 +297,19 @@ int git_config_bool(const char *name, const char *value) return git_config_int(name, value) != 0; } +enum whitespace_mode git_config_whitespace_mode(const char *name, const char *value) +{ + if (!strcasecmp(value, "okay")) + return WS_OKAY; + if (!strcasecmp(value, "warn")) + return WS_WARN; + if (!strcasecmp(value, "error")) + return WS_ERROR; + if (!strcasecmp(value, "autofix")) + return WS_AUTOFIX; + die("bad config value for '%s' in %s", name, config_file_name); +} + int git_default_config(const char *var, const char *value) { /* This needs a better name */ @@ -431,6 +444,21 @@ int git_default_config(const char *var, const char *value) return 0; } + if (!strcmp(var, "core.whitespace.trailing")) { + ws_mode_trailing = git_config_whitespace_mode(var, value); + return 0; + } + + if (!strcmp(var, "core.whitespace.space-before-tab")) { + ws_mode_space_before_tab = git_config_whitespace_mode(var, value); + return 0; + } + + if (!strcmp(var, "core.whitespace.space-indent")) { + ws_mode_space_indent = git_config_whitespace_mode(var, value); + return 0; + } + /* Add other config variables here and to Documentation/config.txt. */ return 0; } diff --git a/environment.c b/environment.c index b5a6c69..71502fc 100644 --- a/environment.c +++ b/environment.c @@ -35,6 +35,9 @@ int pager_in_use; int pager_use_color = 1; char *editor_program; int auto_crlf = 0; /* 1: both ways, -1: only when adding git objects */ +enum whitespace_mode ws_mode_trailing = WS_OKAY; +enum whitespace_mode ws_mode_space_before_tab = WS_OKAY; +enum whitespace_mode ws_mode_space_indent = WS_OKAY; /* This is set by setup_git_dir_gently() and/or git_default_config() */ char *git_work_tree_cfg; -- 1.5.3.1 ``` ## David Symonds, 2007-11-02 15:08 Subject: [PATCH 2/2] git-diff: Respect core.whitespace.{space-indent,space-before-tab,trailing}. Message-ID: <11940160942802-git-send-email-dsymonds@gmail.com> URL: https://gitlist.dev/e/11940160942802-git-send-email-dsymonds%40gmail.com In-Reply-To: <11940160932021-git-send-email-dsymonds@gmail.com> ``` Signed-off-by: David Symonds --- diff.c | 24 ++++++++++++++++++------ 1 files changed, 18 insertions(+), 6 deletions(-) diff --git a/diff.c b/diff.c index a6aaaf7..aa86fa1 100644 --- a/diff.c +++ b/diff.c @@ -503,12 +503,15 @@ static void emit_line_with_ws(int nparents, 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. + * whitespaces? In indent, SP should never precede a TAB. In + * addition, under "indent with non tab" rule, there should not + * be 8 or more 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_mode_space_before_tab != WS_OKAY) && + (0 <= last_space_in_indent)) need_highlight_leading_space = 1; } else if (line[i] == ' ') @@ -516,6 +519,13 @@ static void emit_line_with_ws(int nparents, else break; } + if ((ws_mode_space_indent != WS_OKAY) && + (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 +550,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_mode_trailing != WS_OKAY) { + while (i < tail) { + if (!isspace(line[tail])) + break; + tail--; + } } if ((i < tail && line[tail + 1] != '\n')) { /* This has whitespace between tail+1..len */ -- 1.5.3.1 ``` ## Junio C Hamano, 2007-11-02 20:02 Subject: Re: [PATCH 1/2] Implement parsing for new core.whitespace.* options. Message-ID: <7v3avo2z6x.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7v3avo2z6x.fsf%40gitster.siamese.dyndns.org In-Reply-To: <11940160932021-git-send-email-dsymonds@gmail.com> ``` David Symonds writes: > Each of the new core.whitespace.* options (enumerated below) can be set to one > of: > * okay (default): Whitespace of this type is okay > * warn: Whitespace of this type should be warned about > * error: Whitespace of this type should raise an error > * autofix: Whitespace of this type should be automatically fixed Many problems at the conceptual level (I haven't look at the patch yet). We call these options (nowarn,warn,error,strip) in apply.whitespace. "strip" is a bit of misnomer, as we only handled the trailing whitespace initially. We should add "fix" as a synonym to "strip". The intention is to define what is an anomaly with core.whitespace and then define what to do with it with apply.whitespace. And that is a good distinction. You may usually use "fix", but occasionally you would want to override it one-shot (when you do want to have byte-to-byte identical application of the patch), and the command line option "--whitespace=" lets you do so. At least you need to extend --whitespace command line option handling to allow these overridden. Adding the "error" and "fix" to "diff" is a mistake --- there is no error condition nor fixing there. That shows how the approach of your patch is inappropriate by trying to mix what core.whitespace (give the definition of what is an error) and apply.whitespace (specify what to do with an error) are designed to do. Defaulting to "nowarn" is wrong. Trailing whitespace errors and space before tab errors should be turned on by default as before. ``` ## David Symonds, 2007-11-03 02:35 Subject: Re: [PATCH 1/2] Implement parsing for new core.whitespace.* options. Message-ID: URL: https://gitlist.dev/e/ee77f5c20711021935u16fe0eb2n202dfca3fc220e2d%40mail.gmail.com In-Reply-To: <7v3avo2z6x.fsf@gitster.siamese.dyndns.org> ``` On 11/3/07, Junio C Hamano wrote: > David Symonds writes: > > > Each of the new core.whitespace.* options (enumerated below) can be set to one > > of: > > * okay (default): Whitespace of this type is okay > > * warn: Whitespace of this type should be warned about > > * error: Whitespace of this type should raise an error > > * autofix: Whitespace of this type should be automatically fixed > > Many problems at the conceptual level (I haven't look at the > patch yet). Sure, I thought there might be. I'm still finding my way around the git code. > We call these options (nowarn,warn,error,strip) in > apply.whitespace. "strip" is a bit of misnomer, as we only > handled the trailing whitespace initially. We should add "fix" > as a synonym to "strip". I can whip that up in a separate patch if you'd like. However, it still doesn't allow for different handling of the different whitespace errors, does it? > The intention is to define what is an anomaly with > core.whitespace and then define what to do with it with > apply.whitespace. That seems a little counterintuitive to be splitting like that. For overriding, a simple environment variable like GIT_EXTRA_CONFIG (or whatever) could pass in arbitrary one-shot configuration parameters, which seems like a better (more general) solution. > Adding the "error" and "fix" to "diff" is a mistake --- there is > no error condition nor fixing there. That shows how the > approach of your patch is inappropriate by trying to mix what > core.whitespace (give the definition of what is an error) and > apply.whitespace (specify what to do with an error) are designed > to do. Yes, I agree that there's no place for "error" or "fix" in git-diff; that was the reason for me resending the series, because I adjusted the diff warnings to happen whenever the relevant setting was anything but "okay", with the idea being that any incorrect whitespace should be flagged in git-diff, and there's a natural split between "okay" and "warn"/"error"/"fix". > Defaulting to "nowarn" is wrong. Trailing whitespace errors and > space before tab errors should be turned on by default as > before. Yes, you're correct. That's easy to fix. Dave. ```