Implement parsing for new core.whitespace.* options.
PATCH 1/2 Implement parsing for new core.whitespace.* options.
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
--- cache.h | 16 ++++++++++++++++ config.c | 28 ++++++++++++++++++++++++++++ environment.c | 3 +++ 3 files changed, 47 insertions(+), 0 deletions(-)
cache.h +16 -0
@@ -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 */
config.c +28 -0
@@ -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 ; }
environment.c +3 -0
@@ -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;
Signature · 1 line
1.5.3.1
PATCH 2/2 git-diff: Respect core.whitespace.{space-indent,space-before-tab,trailing}.
--- diff.c | 24 ++++++++++++++++++------ 1 files changed, 18 insertions(+), 6 deletions(-)
diff.c +18 -6
@@ -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 */
Signature · 1 line
1.5.3.1
Quoted 6 lines · David Symonds
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.
Quoted 11 lines · Junio C Hamano
Quoted 6 lines · David Symonds
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 fixedMany 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.
Quoted 4 lines
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?
Quoted 3 lines
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.
Quoted 6 lines
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".
Quoted 3 lines
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.