{"thread":{"id":"15019","subject":"[PATCH] add boolean diff.suppress-blank-empty config option","startedAt":"2008-08-15T12:47:07Z","lastAt":"2008-08-19T13:29:48Z","messageCount":5,"participants":["Jim Meyering","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"87302","messageId":"87k5eiphro.fsf@rho.meyering.net","threadId":"15019","inReplyTo":null,"subject":"[PATCH] add boolean diff.suppress-blank-empty config option","fromName":"Jim Meyering","fromEmail":"jim@meyering.net","sentAt":"2008-08-15T12:47:07Z","receivedAt":"2008-08-15T12:47:07Z","isPatch":true,"sender":{"key":"jim@meyering.net","avatar":"https://avatars.githubusercontent.com/u/710630?v=4"},"body":"\nGNU diff's --suppress-blank-empty option makes it so that diff does not\nadd a space or tab before each empty output line of context.  With this\noption, empty context lines are empty also in \"git diff\" output.\nBefore (and without the option), they'd have a single trailing space.\n\n* diff.c (diff_suppress_blank_empty): New global.\n(git_diff_basic_config): Set it.\n(builtin_diff): Use it to set the bit in ecbdata, aka ecb.priv.\n* xdiff-interface.c (xdiff_outf): Honor the new option.\n* xdiff-interface.h (struct xdiff_emit_state) [suppress_blank_empty]:\nNew member.\n* t/t4029-diff-trailing-space.sh: New file.\n* Documentation/config.txt: Document it.\n---\nI'm posting this solely in case someone else finds it useful.\nOf course, if you're interested in making it part of git, I'll\nbe happy to make any required adjustments.\nI've been using a variant of this for over a year with no ill effects.\n[FWIW, this is relative to today's \"maint\"]\n\n Documentation/config.txt       |    4 ++++\n diff.c                         |    8 ++++++++\n t/t4029-diff-trailing-space.sh |   39 +++++++++++++++++++++++++++++++++++++++\n xdiff-interface.c              |    7 +++++++\n xdiff-interface.h              |    1 +\n 5 files changed, 59 insertions(+), 0 deletions(-)\n create mode 100755 t/t4029-diff-trailing-space.sh\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex b8ec01c..ccc2432 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -567,6 +567,10 @@ diff.autorefreshindex::\n \taffects only 'git-diff' Porcelain, and not lower level\n \t'diff' commands, such as 'git-diff-files'.\n\n+diff.suppress-blank-empty::\n+\tA boolean to inhibit the standard behavior of printing a\n+\tspace or tab before each empty output line. Defaults to false.\n+\n diff.external::\n \tIf this config variable is set, diff generation is not\n \tperformed using the internal diff machinery, but using the\ndiff --git a/diff.c b/diff.c\nindex bf5d5f1..f0fe746 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -20,6 +20,7 @@\n\n static int diff_detect_rename_default;\n static int diff_rename_limit_default = 200;\n+static int diff_suppress_blank_empty;\n int diff_use_color_default = -1;\n static const char *external_diff_cmd_cfg;\n int diff_auto_refresh_index = 1;\n@@ -176,6 +177,12 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n\n+\t/* like GNU diff's --suppress-blank-empty option  */\n+\tif (!strcmp(var, \"diff.suppress-blank-empty\")) {\n+\t\tdiff_suppress_blank_empty = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \tif (!prefixcmp(var, \"diff.\")) {\n \t\tconst char *ep = strrchr(var, '.');\n \t\tif (ep != var + 4) {\n@@ -1523,6 +1530,7 @@ static void builtin_diff(const char *name_a,\n \t\t\txecfg.ctxlen = strtoul(diffopts + 2, NULL, 10);\n \t\tecb.outf = xdiff_outf;\n \t\tecb.priv = &ecbdata;\n+\t\tecbdata.xm.suppress_blank_empty = diff_suppress_blank_empty;\n \t\tecbdata.xm.consume = fn_out_consume;\n \t\tif (DIFF_OPT_TST(o, COLOR_DIFF_WORDS)) {\n \t\t\tecbdata.diff_words =\ndiff --git a/t/t4029-diff-trailing-space.sh b/t/t4029-diff-trailing-space.sh\nnew file mode 100755\nindex 0000000..f3351dc\n--- /dev/null\n+++ b/t/t4029-diff-trailing-space.sh\n@@ -0,0 +1,39 @@\n+#!/bin/bash\n+#\n+# Copyright (c) Jim Meyering\n+#\n+test_description='diff does not add trailing spaces'\n+\n+. ./test-lib.sh\n+\n+cat <<\\EOF > exp ||\n+diff --git a/f b/f\n+index 5f6a263..8cb8bae 100644\n+--- a/f\n++++ b/f\n+@@ -1,2 +1,2 @@\n+\n+-x\n++y\n+EOF\n+exit 1\n+\n+test_expect_success \\\n+    \"$test_description\" \\\n+    'printf \"\\nx\\n\" > f &&\n+     git add f &&\n+     git commit -q -m. f &&\n+     printf \"\\ny\\n\" > f &&\n+     git config --bool diff.suppress-blank-empty true &&\n+     git diff f > actual &&\n+     test_cmp exp actual &&\n+     perl -i.bak -p -e \"s/^\\$/ /\" exp &&\n+     git config --bool diff.suppress-blank-empty false &&\n+     git diff f > actual &&\n+     test_cmp exp actual &&\n+     git config --bool --unset diff.suppress-blank-empty &&\n+     git diff f > actual &&\n+     test_cmp exp actual\n+     '\n+\n+test_done\ndiff --git a/xdiff-interface.c b/xdiff-interface.c\nindex 61dc5c5..5544e5a 100644\n--- a/xdiff-interface.c\n+++ b/xdiff-interface.c\n@@ -66,6 +66,13 @@ int xdiff_outf(void *priv_, mmbuffer_t *mb, int nbuf)\n \tstruct xdiff_emit_state *priv = priv_;\n \tint i;\n\n+\tif (priv->suppress_blank_empty\n+\t    && mb[0].size == 1\n+\t    && mb[0].ptr[0] == ' '\n+\t    && mb[1].size == 1\n+\t    && mb[1].ptr[0] == '\\n')\n+\t  mb[0].size = 0;\n+\n \tfor (i = 0; i < nbuf; i++) {\n \t\tif (mb[i].ptr[mb[i].size-1] != '\\n') {\n \t\t\t/* Incomplete line */\ndiff --git a/xdiff-interface.h b/xdiff-interface.h\nindex f7f791d..dc305ba 100644\n--- a/xdiff-interface.h\n+++ b/xdiff-interface.h\n@@ -11,6 +11,7 @@ struct xdiff_emit_state {\n \txdiff_emit_consume_fn consume;\n \tchar *remainder;\n \tunsigned long remainder_size;\n+\tunsigned int suppress_blank_empty:1;\n };\n\n int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t const *xecfg, xdemitcb_t *ecb);\n-- \n1.6.0.rc3.12.g5b095\n"},{"id":"87337","messageId":"7v4p5mb0a1.fsf@gitster.siamese.dyndns.org","threadId":"15019","inReplyTo":"87k5eiphro.fsf@rho.meyering.net","subject":"Re: [PATCH] add boolean diff.suppress-blank-empty config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-15T18:28:38Z","receivedAt":"2008-08-15T18:28:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jim Meyering <jim@meyering.net> writes:\n\n> GNU diff's --suppress-blank-empty option makes it so that diff does not\n> add a space or tab before each empty output line of context.  With this\n> option, empty context lines are empty also in \"git diff\" output.\n\nAFAIK, ERN 103/120 inspired by your comment merely says an empty context\nis either an empty line or a line with a single space on it and does not\ndeprecate the latter, more traditional, form (I do not know the current\nstatus of the update to the POSIX --- what's the recommended way to find\nit out by outsiders?)\n\nWe've hacked the patch application side to accomodate this special case to\ngrok output from recent GNU diff already.  Can't we just stop at that,\nwithout having to do the same for generation side?  What's the downside of\nnot doing so?\n"},{"id":"87374","messageId":"loom.20080816T105522-117@post.gmane.org","threadId":"15019","inReplyTo":"7v4p5mb0a1.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] add boolean diff.suppress-blank-empty config option","fromName":"Jim Meyering","fromEmail":"meyering@gmail.com","sentAt":"2008-08-16T11:07:40Z","receivedAt":"2008-08-16T11:07:40Z","isPatch":true,"sender":{"key":"meyering@gmail.com","avatar":null},"body":"Junio C Hamano <gitster <at> pobox.com> writes:\n> Jim Meyering <jim <at> meyering.net> writes:\n> > GNU diff's --suppress-blank-empty option makes it so that diff does not\n> > add a space or tab before each empty output line of context.  With this\n> > option, empty context lines are empty also in \"git diff\" output.\n>\n> AFAIK, ERN 103/120 inspired by your comment merely says an empty context\n> is either an empty line or a line with a single space on it and does not\n> deprecate the latter, more traditional, form (I do not know the current\n\nThis is not a POSIX compliance issue.\nEither format may be emitted.\n\n> status of the update to the POSIX --- what's the recommended way to find\n> it out by outsiders?)\n\nHere's one copy of what's being added to the latest version of POSIX:\nhttp://www.opengroup.org/austin/aardvark/latest/xcubug2.txt\nAnyone can register (see Q2: http://www.opengroup.org/austin/faq.html)\nand then get the lastest draft, but it should be final (and hence public)\npretty soon.\n\n> We've hacked the patch application side to accomodate this special case to\n> grok output from recent GNU diff already.  Can't we just stop at that,\n> without having to do the same for generation side?  What's the downside of\n> not doing so?\n\nNo pressure to add it to git.  I deliberately did not propose that,\nbased on the reception an earlier proposal received in this forum.\nI tried to make that clear in the header comments.\n\nHowever, here's why I use this patch:\n  - there are many contexts where I try to avoid trailing blanks, and\n    eliminating spurious ones makes it easier to spot possibly-problematic ones.\n  - I version-control some diff output, and having diff-generating programs\n    handle the elimination of unnecessary trailing blanks is slightly safer\n    than doing it from each context.\n  - some tools systematically remove or highlight trailing blanks.  If I can\n    make my diff output less susceptible with such an option, I'll use it.\n\nBottom line, it doesn't really matter.\nI posted the patch in case someone else would find it useful.\n"},{"id":"87568","messageId":"7vmyjam5xh.fsf@gitster.siamese.dyndns.org","threadId":"15019","inReplyTo":"87k5eiphro.fsf@rho.meyering.net","subject":"Re: [PATCH] add boolean diff.suppress-blank-empty config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-18T20:20:10Z","receivedAt":"2008-08-18T20:20:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jim Meyering <jim@meyering.net> writes:\n\n> GNU diff's --suppress-blank-empty option makes it so that diff does not\n> add a space or tab before each empty output line of context.  With this\n> option, empty context lines are empty also in \"git diff\" output.\n> Before (and without the option), they'd have a single trailing space.\n\n> diff --git a/xdiff-interface.c b/xdiff-interface.c\n> index 61dc5c5..5544e5a 100644\n> --- a/xdiff-interface.c\n> +++ b/xdiff-interface.c\n> @@ -66,6 +66,13 @@ int xdiff_outf(void *priv_, mmbuffer_t *mb, int nbuf)\n>  \tstruct xdiff_emit_state *priv = priv_;\n>  \tint i;\n>\n> +\tif (priv->suppress_blank_empty\n> +\t    && mb[0].size == 1\n> +\t    && mb[0].ptr[0] == ' '\n> +\t    && mb[1].size == 1\n> +\t    && mb[1].ptr[0] == '\\n')\n> +\t  mb[0].size = 0;\n> +\n>  \tfor (i = 0; i < nbuf; i++) {\n>  \t\tif (mb[i].ptr[mb[i].size-1] != '\\n') {\n>  \t\t\t/* Incomplete line */\n\nI do not have a fundamental objection to the optional behaviour, but from\ntechnical point of view, I had to wonder if hooking to xdiff_outf() has\nfunny interactions with codepaths that use patch-id (namely, git-rebase,\ngit-format-patch, and git-cherry).  Luckily, patch-ids are computed over\nnon whitespaces, so it turns out to be Ok, but there may be other\nunintended side effects that I haven't thought about.\n\nI would have preferred the option to hook into a bit higher layer (namely,\nthe part that actually writes textual diff to the output stream).\n"},{"id":"87689","messageId":"87d4k5yvxv.fsf@rho.meyering.net","threadId":"15019","inReplyTo":"7vmyjam5xh.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] add boolean diff.suppress-blank-empty config option","fromName":"Jim Meyering","fromEmail":"meyering@redhat.com","sentAt":"2008-08-19T13:29:48Z","receivedAt":"2008-08-19T13:29:48Z","isPatch":true,"sender":{"key":"jim@meyering.net","avatar":"https://avatars.githubusercontent.com/u/710630?v=4"},"body":"\nGNU diff's --suppress-blank-empty option makes it so that diff no\nlonger outputs trailing white space unless the input data has it.\nWith this option, empty context lines are now empty also in diff -u output.\nBefore, they would have a single trailing space.\n\n* diff.c (diff_suppress_blank_empty): New global.\n(git_diff_basic_config): Set it.\n(fn_out_consume): Honor it.\n* t/t4029-diff-trailing-space.sh: New file.\n* Documentation/config.txt: Document it.\n---\nJunio C Hamano <gitster@pobox.com> wrote:\n...\n> I would have preferred the option to hook into a bit higher layer (namely,\n> the part that actually writes textual diff to the output stream).\n\nGood point.  I've done that.\nHere's the result:\n\n Documentation/config.txt       |    4 ++++\n diff.c                         |   13 +++++++++++++\n t/t4029-diff-trailing-space.sh |   39 +++++++++++++++++++++++++++++++++++++++\n 3 files changed, 56 insertions(+), 0 deletions(-)\n create mode 100755 t/t4029-diff-trailing-space.sh\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 676c39b..9020675 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -567,6 +567,10 @@ diff.autorefreshindex::\n \taffects only 'git-diff' Porcelain, and not lower level\n \t'diff' commands, such as 'git-diff-files'.\n\n+diff.suppress-blank-empty::\n+\tA boolean to inhibit the standard behavior of printing a space\n+\tbefore each empty output line. Defaults to false.\n+\n diff.external::\n \tIf this config variable is set, diff generation is not\n \tperformed using the internal diff machinery, but using the\ndiff --git a/diff.c b/diff.c\nindex bf5d5f1..fe43407 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -20,6 +20,7 @@\n\n static int diff_detect_rename_default;\n static int diff_rename_limit_default = 200;\n+static int diff_suppress_blank_empty;\n int diff_use_color_default = -1;\n static const char *external_diff_cmd_cfg;\n int diff_auto_refresh_index = 1;\n@@ -176,6 +177,12 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n\n+\t/* like GNU diff's --suppress-blank-empty option  */\n+\tif (!strcmp(var, \"diff.suppress-blank-empty\")) {\n+\t\tdiff_suppress_blank_empty = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \tif (!prefixcmp(var, \"diff.\")) {\n \t\tconst char *ep = strrchr(var, '.');\n \t\tif (ep != var + 4) {\n@@ -580,6 +587,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n \t\tecbdata->label_path[0] = ecbdata->label_path[1] = NULL;\n \t}\n\n+\tif (diff_suppress_blank_empty\n+\t    && len == 2 && line[0] == ' ' && line[1] == '\\n') {\n+\t\tline[0] = '\\n';\n+\t\tlen = 1;\n+\t}\n+\n \t/* This is not really necessary for now because\n \t * this codepath only deals with two-way diffs.\n \t */\ndiff --git a/t/t4029-diff-trailing-space.sh b/t/t4029-diff-trailing-space.sh\nnew file mode 100755\nindex 0000000..4ca65e0\n--- /dev/null\n+++ b/t/t4029-diff-trailing-space.sh\n@@ -0,0 +1,39 @@\n+#!/bin/bash\n+#\n+# Copyright (c) Jim Meyering\n+#\n+test_description='diff honors config option, diff.suppress-blank-empty'\n+\n+. ./test-lib.sh\n+\n+cat <<\\EOF > exp ||\n+diff --git a/f b/f\n+index 5f6a263..8cb8bae 100644\n+--- a/f\n++++ b/f\n+@@ -1,2 +1,2 @@\n+\n+-x\n++y\n+EOF\n+exit 1\n+\n+test_expect_success \\\n+    \"$test_description\" \\\n+    'printf \"\\nx\\n\" > f &&\n+     git add f &&\n+     git commit -q -m. f &&\n+     printf \"\\ny\\n\" > f &&\n+     git config --bool diff.suppress-blank-empty true &&\n+     git diff f > actual &&\n+     test_cmp exp actual &&\n+     perl -i.bak -p -e \"s/^\\$/ /\" exp &&\n+     git config --bool diff.suppress-blank-empty false &&\n+     git diff f > actual &&\n+     test_cmp exp actual &&\n+     git config --bool --unset diff.suppress-blank-empty &&\n+     git diff f > actual &&\n+     test_cmp exp actual\n+     '\n+\n+test_done\n--\n1.6.0.4.g750768\n"}]}