{"thread":{"id":"10591","subject":"[PATCH 2/2] git-diff: Respect core.whitespace.{space-indent,space-before-tab,trailing}.","startedAt":"2007-11-02T15:08:12Z","lastAt":"2007-11-03T02:35:35Z","messageCount":4,"participants":["David Symonds","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"57990","messageId":"11940160932021-git-send-email-dsymonds@gmail.com","threadId":"10591","inReplyTo":null,"subject":"[PATCH 1/2] Implement parsing for new core.whitespace.* options.","fromName":"David Symonds","fromEmail":"dsymonds@gmail.com","sentAt":"2007-11-02T15:08:12Z","receivedAt":"2007-11-02T15:08:12Z","isPatch":true,"sender":{"key":"dsymonds@gmail.com","avatar":"https://gravatar.com/avatar/b22f5051cbfc11836e36cf7a690e6cde4e225d835e13295ff98d15c7a9ee3c0f?d=mp&s=160"},"body":"Each of the new core.whitespace.* options (enumerated below) can be set to one\nof:\n\t* okay (default): Whitespace of this type is okay\n\t* warn: Whitespace of this type should be warned about\n\t* error: Whitespace of this type should raise an error\n\t* autofix: Whitespace of this type should be automatically fixed\n\nThe initial options are:\n\t* trailing: Whitespace at the end of a line\n\t* space-before-tab: SP HT sequence in the initial whitespace of a line\n\t* space-indent: At least 8 spaces in a row at the start of a line\n\nExample usage:\n\t[core \"whitespace\"]\n\t\ttrailing = autofix\n\t\tspace-before-tab = error\n\t\tspace-indent = warn\n\nSigned-off-by: David Symonds <dsymonds@gmail.com>\n---\n cache.h       |   16 ++++++++++++++++\n config.c      |   28 ++++++++++++++++++++++++++++\n environment.c |    3 +++\n 3 files changed, 47 insertions(+), 0 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex bfffa05..51e3982 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -602,4 +602,20 @@ extern int diff_auto_refresh_index;\n /* match-trees.c */\n void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, int);\n \n+/*\n+ * whitespace rules.\n+ * used by both diff and apply\n+ */\n+enum whitespace_mode {\n+\tWS_OKAY = 0,\n+\tWS_WARN,\n+\tWS_ERROR,\n+\tWS_AUTOFIX\n+};\n+extern enum whitespace_mode ws_mode_trailing;\n+extern enum whitespace_mode ws_mode_space_before_tab;\n+extern enum whitespace_mode ws_mode_space_indent;\n+extern enum whitespace_mode git_config_whitespace_mode(const char *, const char *);\n+\n+\n #endif /* CACHE_H */\ndiff --git a/config.c b/config.c\nindex dc3148d..8e6f252 100644\n--- a/config.c\n+++ b/config.c\n@@ -297,6 +297,19 @@ int git_config_bool(const char *name, const char *value)\n \treturn git_config_int(name, value) != 0;\n }\n \n+enum whitespace_mode git_config_whitespace_mode(const char *name, const char *value)\n+{\n+\tif (!strcasecmp(value, \"okay\"))\n+\t\treturn WS_OKAY;\n+\tif (!strcasecmp(value, \"warn\"))\n+\t\treturn WS_WARN;\n+\tif (!strcasecmp(value, \"error\"))\n+\t\treturn WS_ERROR;\n+\tif (!strcasecmp(value, \"autofix\"))\n+\t\treturn WS_AUTOFIX;\n+\tdie(\"bad config value for '%s' in %s\", name, config_file_name);\n+}\n+\n int git_default_config(const char *var, const char *value)\n {\n \t/* This needs a better name */\n@@ -431,6 +444,21 @@ int git_default_config(const char *var, const char *value)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"core.whitespace.trailing\")) {\n+\t\tws_mode_trailing = git_config_whitespace_mode(var, value);\n+\t\treturn 0;\n+\t}\n+\n+\tif (!strcmp(var, \"core.whitespace.space-before-tab\")) {\n+\t\tws_mode_space_before_tab = git_config_whitespace_mode(var, value);\n+\t\treturn 0;\n+\t}\n+\n+\tif (!strcmp(var, \"core.whitespace.space-indent\")) {\n+\t\tws_mode_space_indent = git_config_whitespace_mode(var, value);\n+\t\treturn 0;\n+\t}\n+\n \t/* Add other config variables here and to Documentation/config.txt. */\n \treturn 0;\n }\ndiff --git a/environment.c b/environment.c\nindex b5a6c69..71502fc 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -35,6 +35,9 @@ int pager_in_use;\n int pager_use_color = 1;\n char *editor_program;\n int auto_crlf = 0;\t/* 1: both ways, -1: only when adding git objects */\n+enum whitespace_mode ws_mode_trailing = WS_OKAY;\n+enum whitespace_mode ws_mode_space_before_tab = WS_OKAY;\n+enum whitespace_mode ws_mode_space_indent = WS_OKAY;\n \n /* This is set by setup_git_dir_gently() and/or git_default_config() */\n char *git_work_tree_cfg;\n-- \n1.5.3.1\n"},{"id":"57989","messageId":"11940160942802-git-send-email-dsymonds@gmail.com","threadId":"10591","inReplyTo":"11940160932021-git-send-email-dsymonds@gmail.com","subject":"[PATCH 2/2] git-diff: Respect core.whitespace.{space-indent,space-before-tab,trailing}.","fromName":"David Symonds","fromEmail":"dsymonds@gmail.com","sentAt":"2007-11-02T15:08:13Z","receivedAt":"2007-11-02T15:08:13Z","isPatch":true,"sender":{"key":"dsymonds@gmail.com","avatar":"https://gravatar.com/avatar/b22f5051cbfc11836e36cf7a690e6cde4e225d835e13295ff98d15c7a9ee3c0f?d=mp&s=160"},"body":"Signed-off-by: David Symonds <dsymonds@gmail.com>\n---\n diff.c |   24 ++++++++++++++++++------\n 1 files changed, 18 insertions(+), 6 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex a6aaaf7..aa86fa1 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -503,12 +503,15 @@ static void emit_line_with_ws(int nparents,\n \tint tail = len;\n \tint need_highlight_leading_space = 0;\n \t/* The line is a newly added line.  Does it have funny leading\n-\t * whitespaces?  In indent, SP should never precede a TAB.\n+\t * whitespaces?  In indent, SP should never precede a TAB. In\n+\t * addition, under \"indent with non tab\" rule, there should not\n+\t * be 8 or more consecutive spaces.\n \t */\n \tfor (i = col0; i < len; i++) {\n \t\tif (line[i] == '\\t') {\n \t\t\tlast_tab_in_indent = i;\n-\t\t\tif (0 <= last_space_in_indent)\n+\t\t\tif ((ws_mode_space_before_tab != WS_OKAY) &&\n+\t\t\t    (0 <= last_space_in_indent))\n \t\t\t\tneed_highlight_leading_space = 1;\n \t\t}\n \t\telse if (line[i] == ' ')\n@@ -516,6 +519,13 @@ static void emit_line_with_ws(int nparents,\n \t\telse\n \t\t\tbreak;\n \t}\n+\tif ((ws_mode_space_indent != WS_OKAY) &&\n+\t    (0 <= last_space_in_indent) &&\n+\t    (last_tab_in_indent < 0) &&\n+\t    (8 <= (i - col0))) {\n+\t\tlast_tab_in_indent = i;\n+\t\tneed_highlight_leading_space = 1;\n+\t}\n \tfputs(set, stdout);\n \tfwrite(line, col0, 1, stdout);\n \tfputs(reset, stdout);\n@@ -540,10 +550,12 @@ static void emit_line_with_ws(int nparents,\n \ttail = len - 1;\n \tif (line[tail] == '\\n' && i < tail)\n \t\ttail--;\n-\twhile (i < tail) {\n-\t\tif (!isspace(line[tail]))\n-\t\t\tbreak;\n-\t\ttail--;\n+\tif (ws_mode_trailing != WS_OKAY) {\n+\t\twhile (i < tail) {\n+\t\t\tif (!isspace(line[tail]))\n+\t\t\t\tbreak;\n+\t\t\ttail--;\n+\t\t}\n \t}\n \tif ((i < tail && line[tail + 1] != '\\n')) {\n \t\t/* This has whitespace between tail+1..len */\n-- \n1.5.3.1\n"},{"id":"58026","messageId":"7v3avo2z6x.fsf@gitster.siamese.dyndns.org","threadId":"10591","inReplyTo":"11940160932021-git-send-email-dsymonds@gmail.com","subject":"Re: [PATCH 1/2] Implement parsing for new core.whitespace.* options.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-02T20:02:30Z","receivedAt":"2007-11-02T20:02:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Symonds <dsymonds@gmail.com> writes:\n\n> Each of the new core.whitespace.* options (enumerated below) can be set to one\n> of:\n> \t* okay (default): Whitespace of this type is okay\n> \t* warn: Whitespace of this type should be warned about\n> \t* error: Whitespace of this type should raise an error\n> \t* autofix: Whitespace of this type should be automatically fixed\n\nMany problems at the conceptual level (I haven't look at the\npatch yet).\n\nWe call these options (nowarn,warn,error,strip) in\napply.whitespace.  \"strip\" is a bit of misnomer, as we only\nhandled the trailing whitespace initially.  We should add \"fix\"\nas a synonym to \"strip\".\n\nThe intention is to define what is an anomaly with\ncore.whitespace and then define what to do with it with\napply.whitespace.\n\nAnd that is a good distinction.  You may usually use \"fix\", but\noccasionally you would want to override it one-shot (when you do\nwant to have byte-to-byte identical application of the patch),\nand the command line option \"--whitespace=\" lets you do so.\nAt least you need to extend --whitespace command line option\nhandling to allow these overridden.\n\nAdding the \"error\" and \"fix\" to \"diff\" is a mistake --- there is\nno error condition nor fixing there.  That shows how the\napproach of your patch is inappropriate by trying to mix what\ncore.whitespace (give the definition of what is an error) and\napply.whitespace (specify what to do with an error) are designed\nto do.\n\nDefaulting to \"nowarn\" is wrong.  Trailing whitespace errors and\nspace before tab errors should be turned on by default as\nbefore.\n"},{"id":"58063","messageId":"ee77f5c20711021935u16fe0eb2n202dfca3fc220e2d@mail.gmail.com","threadId":"10591","inReplyTo":"7v3avo2z6x.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/2] Implement parsing for new core.whitespace.* options.","fromName":"David Symonds","fromEmail":"dsymonds@gmail.com","sentAt":"2007-11-03T02:35:35Z","receivedAt":"2007-11-03T02:35:35Z","isPatch":true,"sender":{"key":"dsymonds@gmail.com","avatar":"https://gravatar.com/avatar/b22f5051cbfc11836e36cf7a690e6cde4e225d835e13295ff98d15c7a9ee3c0f?d=mp&s=160"},"body":"On 11/3/07, Junio C Hamano <gitster@pobox.com> wrote:\n> David Symonds <dsymonds@gmail.com> writes:\n>\n> > Each of the new core.whitespace.* options (enumerated below) can be set to one\n> > of:\n> >       * okay (default): Whitespace of this type is okay\n> >       * warn: Whitespace of this type should be warned about\n> >       * error: Whitespace of this type should raise an error\n> >       * autofix: Whitespace of this type should be automatically fixed\n>\n> Many problems at the conceptual level (I haven't look at the\n> patch yet).\n\nSure, I thought there might be. I'm still finding my way around the git code.\n\n> We call these options (nowarn,warn,error,strip) in\n> apply.whitespace.  \"strip\" is a bit of misnomer, as we only\n> handled the trailing whitespace initially.  We should add \"fix\"\n> as a synonym to \"strip\".\n\nI can whip that up in a separate patch if you'd like. However, it\nstill doesn't allow for different handling of the different whitespace\nerrors, does it?\n\n> The intention is to define what is an anomaly with\n> core.whitespace and then define what to do with it with\n> apply.whitespace.\n\nThat seems a little counterintuitive to be splitting like that. For\noverriding, a simple environment variable like GIT_EXTRA_CONFIG (or\nwhatever) could pass in arbitrary one-shot configuration parameters,\nwhich seems like a better (more general) solution.\n\n> Adding the \"error\" and \"fix\" to \"diff\" is a mistake --- there is\n> no error condition nor fixing there.  That shows how the\n> approach of your patch is inappropriate by trying to mix what\n> core.whitespace (give the definition of what is an error) and\n> apply.whitespace (specify what to do with an error) are designed\n> to do.\n\nYes, I agree that there's no place for \"error\" or \"fix\" in git-diff;\nthat was the reason for me resending the series, because I adjusted\nthe diff warnings to happen whenever the relevant setting was anything\nbut \"okay\", with the idea being that any incorrect whitespace should\nbe flagged in git-diff, and there's a natural split between \"okay\" and\n\"warn\"/\"error\"/\"fix\".\n\n> Defaulting to \"nowarn\" is wrong.  Trailing whitespace errors and\n> space before tab errors should be turned on by default as\n> before.\n\nYes, you're correct. That's easy to fix.\n\n\nDave.\n"}]}