{"thread":{"id":"9880","subject":"whitespace-stripping","startedAt":"2007-09-16T22:48:59Z","lastAt":"2007-10-03T01:00:27Z","messageCount":16,"participants":["J. Bruce Fields","Martin Langhoff","Junio C Hamano","Krzysztof Halasa","David Kastrup"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"53230","messageId":"11899829424040-git-send-email-bfields@citi.umich.edu","threadId":"9880","inReplyTo":null,"subject":"whitespace-stripping","fromName":"J. Bruce Fields","fromEmail":"bfields@citi.umich.edu","sentAt":"2007-09-16T22:48:59Z","receivedAt":"2007-09-16T22:48:59Z","isPatch":false,"sender":{"key":"bfields@citi.umich.edu","avatar":null},"body":"The following patches fix one (probably rare) bug in\n\n git apply --whitespace=strip\n\nand then teach it to also complain about initial consecutive spaces that\ncould be tabs.\n\nThe latter is the standard for the kernel, but may not be appropriate\nfor other projects. I'd like to make the whitespace code handle the\nkernel style completely first, then consider configuration to handle\nother styles if people complain. But maybe the change of behavior would\nbe an unpleasant surprise for someone with apply.whitespace=strip and a\nproject that always uses spaces for indents.\n\n--b.\n"},{"id":"53231","messageId":"11899829424173-git-send-email-bfields@citi.umich.edu","threadId":"9880","inReplyTo":"11899829424040-git-send-email-bfields@citi.umich.edu","subject":"[PATCH 1/3] git-apply: fix whitespace stripping","fromName":"J. Bruce Fields","fromEmail":"bfields@citi.umich.edu","sentAt":"2007-09-16T22:49:00Z","receivedAt":"2007-09-16T22:49:00Z","isPatch":true,"sender":{"key":"bfields@citi.umich.edu","avatar":null},"body":"The algorithm isn't right here: it accumulates any set of 8 spaces into\ntabs even if they're separated by tabs, so\n\n\t\n\nis converted to\n\n\t\n\nwhen it should be just\n\n\t\n\nSo teach git-apply that a tab hides any group of less than 8 previous\nspaces in a row.\n\nSigned-off-by: J. Bruce Fields \n---\n builtin-apply.c | 13 ++++++++++---\n 1 files changed, 10 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 976ec77..70359c1 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -1642,15 +1642,22 @@ static int apply_line(char *output, const char *patch, int plen)\n \n \tbuf = output;\n \tif (need_fix_leading_space) {\n+\t\tint consecutive_spaces = 0;\n \t\t/* between patch[1..last_tab_in_indent] strip the\n \t\t * funny spaces, updating them to tab as needed.\n \t\t */\n \t\tfor (i = 1; i < last_tab_in_indent; i++, plen--) {\n \t\t\tchar ch = patch[i];\n-\t\t\tif (ch != ' ')\n+\t\t\tif (ch != ' ') {\n+\t\t\t\tconsecutive_spaces = 0;\n \t\t\t\t*output++ = ch;\n-\t\t\telse if ((i % 8) == 0)\n-\t\t\t\t*output++ = '\\t';\n+\t\t\t} else {\n+\t\t\t\tconsecutive_spaces++;\n+\t\t\t\tif (consecutive_spaces == 8) {\n+\t\t\t\t\t*output++ = '\\t';\n+\t\t\t\t\tconsecutive_spaces = 0;\n+\t\t\t\t}\n+\t\t\t}\n \t\t}\n \t\tfixed = 1;\n \t\ti = last_tab_in_indent;\n-- \n1.5.3.1.42.gfe5df\n"},{"id":"53232","messageId":"1189982942187-git-send-email-bfields@citi.umich.edu","threadId":"9880","inReplyTo":"11899829424173-git-send-email-bfields@citi.umich.edu","subject":"[PATCH 2/3] git-apply: complain about >=8 consecutive spaces in initial indent","fromName":"J. Bruce Fields","fromEmail":"bfields@citi.umich.edu","sentAt":"2007-09-16T22:49:01Z","receivedAt":"2007-09-16T22:49:01Z","isPatch":true,"sender":{"key":"bfields@citi.umich.edu","avatar":null},"body":"Complain if we find 8 spaces or more in a row as part of the initial\nwhitespace on a line, and (with --whitespace=stripspace) replace such by\na tab.\n\nWell, linux's checkpatch.pl complains about this sort of thing.\n\nSigned-off-by: J. Bruce Fields \n---\n builtin-apply.c | 34 +++++++++++++++++++++++++++-------\n 1 files changed, 27 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 70359c1..fb63089 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -918,6 +918,7 @@ static void check_whitespace(const char *line, int len)\n {\n \tconst char *err = \"Adds trailing whitespace\";\n \tint seen_space = 0;\n+\tint consecutive_spaces = 0;\n \tint i;\n \n \t/*\n@@ -944,6 +945,18 @@ static void check_whitespace(const char *line, int len)\n \t\telse\n \t\t\tbreak;\n \t}\n+\n+\terr = \"Initial indent contains eight or more spaces in a row\";\n+\tfor (i = 1; i < len; i++) {\n+\t\tif (line[i] == ' ')\n+\t\t\tconsecutive_spaces++;\n+\t\telse if (line[i] == '\\t')\n+\t\t\tconsecutive_spaces = 0;\n+\t\telse\n+\t\t\tbreak;\n+\t\tif (consecutive_spaces == 8)\n+\t\t\tgoto error;\n+\t}\n \treturn;\n \n error:\n@@ -1607,9 +1620,10 @@ static int apply_line(char *output, const char *patch, int plen)\n \tint i;\n \tint add_nl_to_tail = 0;\n \tint fixed = 0;\n-\tint last_tab_in_indent = -1;\n+\tint after_indent = -1;\n \tint last_space_in_indent = -1;\n \tint need_fix_leading_space = 0;\n+\tint consecutive_spaces = 0;\n \tchar *buf;\n \n \tif ((new_whitespace != strip_whitespace) || !whitespace_error ||\n@@ -1630,23 +1644,27 @@ static int apply_line(char *output, const char *patch, int plen)\n \tfor (i = 1; i < plen; i++) {\n \t\tchar ch = patch[i];\n \t\tif (ch == '\\t') {\n-\t\t\tlast_tab_in_indent = i;\n+\t\t\tconsecutive_spaces = 0;\n \t\t\tif (0 <= last_space_in_indent)\n \t\t\t\tneed_fix_leading_space = 1;\n \t\t}\n-\t\telse if (ch == ' ')\n+\t\telse if (ch == ' ') {\n+\t\t\tconsecutive_spaces++;\n \t\t\tlast_space_in_indent = i;\n-\t\telse\n+\t\t} else\n \t\t\tbreak;\n+\t\tif (consecutive_spaces == 8)\n+\t\t\tneed_fix_leading_space = 1;\n \t}\n+\tafter_indent=i;\n \n \tbuf = output;\n \tif (need_fix_leading_space) {\n-\t\tint consecutive_spaces = 0;\n+\t\tconsecutive_spaces = 0;\n \t\t/* between patch[1..last_tab_in_indent] strip the\n \t\t * funny spaces, updating them to tab as needed.\n \t\t */\n-\t\tfor (i = 1; i < last_tab_in_indent; i++, plen--) {\n+\t\tfor (i = 1; i < after_indent; i++, plen--) {\n \t\t\tchar ch = patch[i];\n \t\t\tif (ch != ' ') {\n \t\t\t\tconsecutive_spaces = 0;\n@@ -1660,7 +1678,9 @@ static int apply_line(char *output, const char *patch, int plen)\n \t\t\t}\n \t\t}\n \t\tfixed = 1;\n-\t\ti = last_tab_in_indent;\n+\t\ti = after_indent;\n+\t\ti -= consecutive_spaces;\n+\t\tplen += consecutive_spaces;\n \t}\n \telse\n \t\ti = 1;\n-- \n1.5.3.1.42.gfe5df\n"},{"id":"53233","messageId":"11899829421064-git-send-email-bfields@citi.umich.edu","threadId":"9880","inReplyTo":"1189982942187-git-send-email-bfields@citi.umich.edu","subject":"[PATCH 3/3] git-apply: add tests for stripping of leading and trailing whitespace","fromName":"J. Bruce Fields","fromEmail":"bfields@citi.umich.edu","sentAt":"2007-09-16T22:49:02Z","receivedAt":"2007-09-16T22:49:02Z","isPatch":true,"sender":{"key":"bfields@citi.umich.edu","avatar":null},"body":"Add tests to make sure we strip leading and trailing whitespace correctly.\n\nOf the four tests, the first two should always have passed, the third\nrequires the \"fix whitespace stripping\" patch, and the fourth requires\nthe \"complain about >= 8 consecutive spaces in initial indent\" patch.\n\nNote that this patch itself adds leading and trailing whitespace.\n\nSigned-off-by: J. Bruce Fields \n---\n t/t4124-apply-whitespace-strip.sh | 43 +++++++++++++++++++++++++++++++++++++\n t/t4124/1-after | 3 ++\n t/t4124/1-before | 3 ++\n t/t4124/2-after | 3 ++\n t/t4124/2-before | 3 ++\n t/t4124/3-after | 1 +\n t/t4124/3-before | 1 +\n t/t4124/4-after | 5 ++++\n t/t4124/4-before | 5 ++++\n 9 files changed, 67 insertions(+), 0 deletions(-)\n create mode 100644 t/t4124-apply-whitespace-strip.sh\n create mode 100644 t/t4124/1-after\n create mode 100644 t/t4124/1-before\n create mode 100644 t/t4124/2-after\n create mode 100644 t/t4124/2-before\n create mode 100644 t/t4124/3-after\n create mode 100644 t/t4124/3-before\n create mode 100644 t/t4124/4-after\n create mode 100644 t/t4124/4-before\n\ndiff --git a/t/t4124-apply-whitespace-strip.sh b/t/t4124-apply-whitespace-strip.sh\nnew file mode 100644\nindex 0000000..3b5f58b\n--- /dev/null\n+++ b/t/t4124-apply-whitespace-strip.sh\n@@ -0,0 +1,43 @@\n+#!/bin/sh\n+\n+test_description='handle space and tab combinations with --whitespace=strip'\n+\n+. ./test-lib.sh\n+\n+# The directory t4124/ contains pairs of files \"n-before\" and \"n-after\",\n+# identicaly except leading and trailing whitespace are stripped from\n+# the latter.\n+#\n+# Check that we strip whitespace correctly by checking that the diff\n+# between the two files, applied to the first (with --whitespace=strip)\n+# produces the second.\n+\n+mkpatch () {\n+\tcp \"$1\" foo\n+\tgit diff /dev/null foo >patch\n+\trm foo\n+}\n+\n+checkstrip () {\n+\tmkpatch \"../t4124/$1-before\"\n+\tgit apply --whitespace=strip patch\n+\tgit diff foo \"../t4124/$1-after\"\n+}\n+\n+test_expect_success \\\n+\t'trailing tabs and spaces' \\\n+\t'checkstrip 1'\n+\n+test_expect_success \\\n+\t'spaces before tabs' \\\n+\t'checkstrip 2' \n+\n+test_expect_success \\\n+\t'8 or more non-consecutive initial spaces' \\\n+\t'checkstrip 3'\n+\n+test_expect_success \\\n+\t'8 or more consecutive initial spaces' \\\n+\t'checkstrip 4'\n+\n+test_done\ndiff --git a/t/t4124/1-after b/t/t4124/1-after\nnew file mode 100644\nindex 0000000..cf5dfce\n--- /dev/null\n+++ b/t/t4124/1-after\n@@ -0,0 +1,3 @@\n+trailing space\n+trailing tab\n+trailing spaces and tabs\ndiff --git a/t/t4124/1-before b/t/t4124/1-before\nnew file mode 100644\nindex 0000000..1f2505b\n--- /dev/null\n+++ b/t/t4124/1-before\n@@ -0,0 +1,3 @@\n+trailing space \n+trailing tab \n+trailing spaces and tabs \t \t \t\ndiff --git a/t/t4124/2-after b/t/t4124/2-after\nnew file mode 100644\nindex 0000000..f198144\n--- /dev/null\n+++ b/t/t4124/2-after\n@@ -0,0 +1,3 @@\n+\tspace tab\n+\tspace space tab\n+\t\ttab space tab\ndiff --git a/t/t4124/2-before b/t/t4124/2-before\nnew file mode 100644\nindex 0000000..8fc35bb\n--- /dev/null\n+++ b/t/t4124/2-before\n@@ -0,0 +1,3 @@\n+ \tspace tab\n+ \tspace space tab\n+\t \ttab space tab\ndiff --git a/t/t4124/3-after b/t/t4124/3-after\nnew file mode 100644\nindex 0000000..4db0e80\n--- /dev/null\n+++ b/t/t4124/3-after\n@@ -0,0 +1 @@\n+\t\t4 spaces, tab, 4 spaces, tab\ndiff --git a/t/t4124/3-before b/t/t4124/3-before\nnew file mode 100644\nindex 0000000..f0e2b9c\n--- /dev/null\n+++ b/t/t4124/3-before\n@@ -0,0 +1 @@\n+ \t \t4 spaces, tab, 4 spaces, tab\ndiff --git a/t/t4124/4-after b/t/t4124/4-after\nnew file mode 100644\nindex 0000000..a9b8cf6\n--- /dev/null\n+++ b/t/t4124/4-after\n@@ -0,0 +1,5 @@\n+ 7 spaces\n+\t8 spaces\n+\t 9 spaces\n+\t\ttab 8 spaces\n+\t\t tab 9 spaces\ndiff --git a/t/t4124/4-before b/t/t4124/4-before\nnew file mode 100644\nindex 0000000..a35b624\n--- /dev/null\n+++ b/t/t4124/4-before\n@@ -0,0 +1,5 @@\n+ 7 spaces\n+ 8 spaces\n+ 9 spaces\n+\t tab 8 spaces\n+\t tab 9 spaces\n-- \n1.5.3.1.42.gfe5df\n"},{"id":"53237","messageId":"46a038f90709161624j6eb55de6m61aab9e585e22a05@mail.gmail.com","threadId":"9880","inReplyTo":"1189982942187-git-send-email-bfields@citi.umich.edu","subject":"Re: [PATCH 2/3] git-apply: complain about >=8 consecutive spaces in initial indent","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2007-09-16T23:24:12Z","receivedAt":"2007-09-16T23:24:12Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On 9/17/07, J. Bruce Fields wrote:\n> Complain if we find 8 spaces or more in a row as part of the initial\n> whitespace on a line, and (with --whitespace=stripspace) replace such by\n> a tab.\n\nI do quite a bit of hacking on \"spaces-for-indentation\" projects and\nstill use stripspace to cleanup my patches. So no, thanks.\n\nPerhaps split it off to a separate option? I'm not opposed to the\nfunctionality per-se, but don't put together with\ntrailing-space-trimming. It's a different beast. Everyone agrees\ntrimming trailing spaces as much as everyone disagrees on\ntabs-vs-spaces.\n\ncheers,\n\n\n\nm\n"},{"id":"53238","messageId":"7vy7f63zr4.fsf@gitster.siamese.dyndns.org","threadId":"9880","inReplyTo":"1189982942187-git-send-email-bfields@citi.umich.edu","subject":"Re: [PATCH 2/3] git-apply: complain about >=8 consecutive spaces in initial indent","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-17T00:24:31Z","receivedAt":"2007-09-17T00:24:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"J. Bruce Fields\" writes:\n\n> Complain if we find 8 spaces or more in a row as part of the initial\n> whitespace on a line, and (with --whitespace=stripspace) replace such by\n> a tab.\n>\n> Well, linux's checkpatch.pl complains about this sort of thing.\n\nSome people program in Python, so I am afraid that this needs to\nbe a separate option.\n\nMaybe it is time to redo the --whitespace options as bitmasks so\nthat we can say --whitespace-fix=tab,tail,lines to pick and\nchoose which kinds of breakage to fix?\n"},{"id":"53245","messageId":"20070917024453.GA24675@fieldses.org","threadId":"9880","inReplyTo":"7vy7f63zr4.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/3] git-apply: complain about >=8 consecutive spaces in initial indent","fromName":"J. Bruce Fields","fromEmail":"bfields@fieldses.org","sentAt":"2007-09-17T02:44:53Z","receivedAt":"2007-09-17T02:44:53Z","isPatch":true,"sender":{"key":"bfields@citi.umich.edu","avatar":null},"body":"On Sun, Sep 16, 2007 at 05:24:31PM -0700, Junio C Hamano wrote:\n> \"J. Bruce Fields\" writes:\n> \n> > Complain if we find 8 spaces or more in a row as part of the initial\n> > whitespace on a line, and (with --whitespace=stripspace) replace such by\n> > a tab.\n> >\n> > Well, linux's checkpatch.pl complains about this sort of thing.\n> \n> Some people program in Python, so I am afraid that this needs to\n> be a separate option.\n\nOK.\n\n> Maybe it is time to redo the --whitespace options as bitmasks so\n> that we can say --whitespace-fix=tab,tail,lines to pick and\n> choose which kinds of breakage to fix?\n\nOK. Or maybe keep the current commandline options and have a\nwhitespace-style config option someplace?\n\nI'm afraid I won't get to either anytime soon, though, so that project's\nup for grabs....\n\n--b.\n"},{"id":"53246","messageId":"20070917024528.GB24675@fieldses.org","threadId":"9880","inReplyTo":"46a038f90709161624j6eb55de6m61aab9e585e22a05@mail.gmail.com","subject":"Re: [PATCH 2/3] git-apply: complain about >=8 consecutive spaces in initial indent","fromName":"J. Bruce Fields","fromEmail":"bfields@fieldses.org","sentAt":"2007-09-17T02:45:28Z","receivedAt":"2007-09-17T02:45:28Z","isPatch":true,"sender":{"key":"bfields@citi.umich.edu","avatar":null},"body":"On Mon, Sep 17, 2007 at 11:24:12AM +1200, Martin Langhoff wrote:\n> On 9/17/07, J. Bruce Fields wrote:\n> > Complain if we find 8 spaces or more in a row as part of the initial\n> > whitespace on a line, and (with --whitespace=stripspace) replace such by\n> > a tab.\n> \n> I do quite a bit of hacking on \"spaces-for-indentation\" projects and\n> still use stripspace to cleanup my patches. So no, thanks.\n\nOK, fair enough.\n\n--b.\n"},{"id":"53318","messageId":"m3myvlv0m0.fsf@maximus.localdomain","threadId":"9880","inReplyTo":"11899829421064-git-send-email-bfields@citi.umich.edu","subject":"Re: [PATCH 3/3] git-apply: add tests for stripping of leading and trailing whitespace","fromName":"Krzysztof Halasa","fromEmail":"khc@pm.waw.pl","sentAt":"2007-09-17T14:16:07Z","receivedAt":"2007-09-17T14:16:07Z","isPatch":true,"sender":{"key":"khc@pm.waw.pl","avatar":null},"body":"\"J. Bruce Fields\" writes:\n\n> +test_expect_success \\\n> +\t'8 or more consecutive initial spaces' \\\n> +\t'checkstrip 4'\n\nIt may be valid, some projects use tabs for indentation and spaces\nfor alignment, e.g.:\n\n\tif (cond && (cond1 ||\n\t cond2))\n\t\t...\n\nThe second line is actually:\n\t cond2))\nSSSSSSSSSSSSScond2))\nwhere 'S' means space.\n\nThis is the only way to write code which display correctly with\ndifferent tab sizes.\n\nWith tab = 4 spaces it would be expanded to:\n if (cond && (cond1 ||\n cond2))\n ...\nI.e., it would be still fine.\n\n\nMost of the formating tools probably can't do it automatically.\n-- \nKrzysztof Halasa\n"},{"id":"53319","messageId":"20070917150213.GB4957@fieldses.org","threadId":"9880","inReplyTo":"m3myvlv0m0.fsf@maximus.localdomain","subject":"Re: [PATCH 3/3] git-apply: add tests for stripping of leading and trailing whitespace","fromName":"J. Bruce Fields","fromEmail":"bfields@fieldses.org","sentAt":"2007-09-17T15:02:13Z","receivedAt":"2007-09-17T15:02:13Z","isPatch":true,"sender":{"key":"bfields@citi.umich.edu","avatar":null},"body":"On Mon, Sep 17, 2007 at 04:16:07PM +0200, Krzysztof Halasa wrote:\n> \"J. Bruce Fields\" writes:\n> \n> > +test_expect_success \\\n> > +\t'8 or more consecutive initial spaces' \\\n> > +\t'checkstrip 4'\n> \n> It may be valid, some projects use tabs for indentation and spaces\n> for alignment, e.g.:\n\nYeah, I know. I was hoping that the stripspace behavior was already\nspecific enough to the linux-kernel style that we could just assume that \nit was only used by developers on projects with the same style. I agree\nthat I was wrong--apologies.\n\n--b.\n"},{"id":"53370","messageId":"m31wcwsvpt.fsf@maximus.localdomain","threadId":"9880","inReplyTo":"20070917150213.GB4957@fieldses.org","subject":"Re: [PATCH 3/3] git-apply: add tests for stripping of leading and trailing whitespace","fromName":"Krzysztof Halasa","fromEmail":"khc@pm.waw.pl","sentAt":"2007-09-17T23:44:46Z","receivedAt":"2007-09-17T23:44:46Z","isPatch":true,"sender":{"key":"khc@pm.waw.pl","avatar":null},"body":"\"J. Bruce Fields\" writes:\n\n>> It may be valid, some projects use tabs for indentation and spaces\n>> for alignment, e.g.:\n>\n> Yeah, I know. I was hoping that the stripspace behavior was already\n> specific enough to the linux-kernel style that we could just assume that \n> it was only used by developers on projects with the same style.\n\nActually I would consider linux kernel an example of such project\nwith spaces for alignment. Except that current tools are not to\nthe task. Someday...\n\nObviously other cases are valid, especially 'spaces before tabs'\nwhich IMHO includes '8 or more non-consecutive initial spaces'.\nAnd all trailing whitespace of course.\n-- \nKrzysztof Halasa\n"},{"id":"53385","messageId":"20070918005349.GB2443@fieldses.org","threadId":"9880","inReplyTo":"m31wcwsvpt.fsf@maximus.localdomain","subject":"Re: [PATCH 3/3] git-apply: add tests for stripping of leading and trailing whitespace","fromName":"J. Bruce Fields","fromEmail":"bfields@fieldses.org","sentAt":"2007-09-18T00:53:49Z","receivedAt":"2007-09-18T00:53:49Z","isPatch":true,"sender":{"key":"bfields@citi.umich.edu","avatar":null},"body":"On Tue, Sep 18, 2007 at 01:44:46AM +0200, Krzysztof Halasa wrote:\n> \"J. Bruce Fields\" writes:\n> \n> >> It may be valid, some projects use tabs for indentation and spaces\n> >> for alignment, e.g.:\n> >\n> > Yeah, I know. I was hoping that the stripspace behavior was already\n> > specific enough to the linux-kernel style that we could just assume that \n> > it was only used by developers on projects with the same style.\n> \n> Actually I would consider linux kernel an example of such project\n> with spaces for alignment. Except that current tools are not to\n> the task. Someday...\n\nI guess I haven't followed previous arguments on the subject, but based\non checkpatch.pl I was assuming the tabs-whenever-possible policy had\nwon out.\n\nAnyway, I can only care about whitespace for so long....\n\n--b.\n"},{"id":"53419","messageId":"86r6kw4aki.fsf@lola.quinscape.zz","threadId":"9880","inReplyTo":"11899829424173-git-send-email-bfields@citi.umich.edu","subject":"Re: [PATCH 1/3] git-apply: fix whitespace stripping","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-09-18T08:55:25Z","receivedAt":"2007-09-18T08:55:25Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"\"J. Bruce Fields\" writes:\n\n> The algorithm isn't right here: it accumulates any set of 8 spaces into\n> tabs even if they're separated by tabs, so\n>\n> \t\n>\n> is converted to\n>\n> \t\n>\n> when it should be just\n>\n> \t\n>\n> So teach git-apply that a tab hides any group of less than 8 previous\n> spaces in a row.\n>\n> Signed-off-by: J. Bruce Fields \n> ---\n> builtin-apply.c | 13 ++++++++++---\n> 1 files changed, 10 insertions(+), 3 deletions(-)\n>\n> diff --git a/builtin-apply.c b/builtin-apply.c\n> index 976ec77..70359c1 100644\n> --- a/builtin-apply.c\n> +++ b/builtin-apply.c\n> @@ -1642,15 +1642,22 @@ static int apply_line(char *output, const char *patch, int plen)\n> \n> \tbuf = output;\n> \tif (need_fix_leading_space) {\n> +\t\tint consecutive_spaces = 0;\n> \t\t/* between patch[1..last_tab_in_indent] strip the\n> \t\t * funny spaces, updating them to tab as needed.\n> \t\t */\n> \t\tfor (i = 1; i < last_tab_in_indent; i++, plen--) {\n> \t\t\tchar ch = patch[i];\n> -\t\t\tif (ch != ' ')\n> +\t\t\tif (ch != ' ') {\n> +\t\t\t\tconsecutive_spaces = 0;\n> \t\t\t\t*output++ = ch;\n> -\t\t\telse if ((i % 8) == 0)\n> -\t\t\t\t*output++ = '\\t';\n> +\t\t\t} else {\n> +\t\t\t\tconsecutive_spaces++;\n> +\t\t\t\tif (consecutive_spaces == 8) {\n> +\t\t\t\t\t*output++ = '\\t';\n> +\t\t\t\t\tconsecutive_spaces = 0;\n> +\t\t\t\t}\n> +\t\t\t}\n> \t\t}\n> \t\tfixed = 1;\n> \t\ti = last_tab_in_indent;\n> -- \n> 1.5.3.1.42.gfe5df\n\nAs far as I can see, this does not really work since it does not\nmaintain an idea of a current column.\n\nIf you have\n\nabcd\n\nthen indeed the resulting conversion needs to be \nwhereas with\n\nabc\n\nthe resulting conversion needs to be just \n\n\n-- \nDavid Kastrup\n"},{"id":"53471","messageId":"20070918131237.GA12120@fieldses.org","threadId":"9880","inReplyTo":"86r6kw4aki.fsf@lola.quinscape.zz","subject":"Re: [PATCH 1/3] git-apply: fix whitespace stripping","fromName":"J. Bruce Fields","fromEmail":"bfields@fieldses.org","sentAt":"2007-09-18T13:12:37Z","receivedAt":"2007-09-18T13:12:37Z","isPatch":true,"sender":{"key":"bfields@citi.umich.edu","avatar":null},"body":"On Tue, Sep 18, 2007 at 10:55:25AM +0200, David Kastrup wrote:\n> As far as I can see, this does not really work since it does not\n> maintain an idea of a current column.\n> \n> If you have\n> \n> abcd\n> \n> then indeed the resulting conversion needs to be \n> whereas with\n> \n> abc\n> \n> the resulting conversion needs to be just \n\nNote that this code *only* handles whitespace in the initial indent;\nprocessing stops as soon as it hits anything other than a tab or an\nindent.\n\nGiven that, I believe the proposed patch is correct. Am I missing\nsomething else?\n\n--b.\n"},{"id":"53484","messageId":"867imo3vmh.fsf@lola.quinscape.zz","threadId":"9880","inReplyTo":"20070918131237.GA12120@fieldses.org","subject":"Re: [PATCH 1/3] git-apply: fix whitespace stripping","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-09-18T14:18:14Z","receivedAt":"2007-09-18T14:18:14Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"\"J. Bruce Fields\" writes:\n\n> On Tue, Sep 18, 2007 at 10:55:25AM +0200, David Kastrup wrote:\n>> As far as I can see, this does not really work since it does not\n>> maintain an idea of a current column.\n>> \n>> If you have\n>> \n>> abcd\n>> \n>> then indeed the resulting conversion needs to be \n>> whereas with\n>> \n>> abc\n>> \n>> the resulting conversion needs to be just \n>\n> Note that this code *only* handles whitespace in the initial indent;\n> processing stops as soon as it hits anything other than a tab or an\n> indent.\n>\n> Given that, I believe the proposed patch is correct. Am I missing\n> something else?\n\nDon't think so. Sorry for the noise.\n\n-- \nDavid Kastrup\n"},{"id":"54635","messageId":"7vfy0thv10.fsf_-_@gitster.siamese.dyndns.org","threadId":"9880","inReplyTo":"1189982942187-git-send-email-bfields@citi.umich.edu","subject":"[PATCH] git-diff: complain about >=8 consecutive spaces in initial indent","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-10-03T01:00:27Z","receivedAt":"2007-10-03T01:00:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This teaches coloring code in \"diff\" to detect indent of 8 or\nmore places using SP, which can and should (in some projects\nincluding the kernel and git itself) use HT instead.\n\n---\n\n * This is primarily meant as a \"reminder\" patch, and not for\n inclusion. We earlier saw a patch to \"git-apply\" to rewrite\n them to HT but rejected it, because some projects use \"no HT,\n all SP\" policy (e.g. Python).\n\n We probably should resurrect the earlier \"git-apply\" patch,\n and teach it and this patch to selectively enable/disable\n detection of different kinds of whitespace breakages.\n\n diff.c | 11 +++++++++--\n 1 files changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 0ee9ea1..647377b 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -531,8 +531,10 @@ static void emit_line_with_ws(int nparents,\n \tint i;\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/*\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, and\n+\t * there shouldn't be more than 8 consecutive spaces.\n \t */\n \tfor (i = col0; i < len; i++) {\n \t\tif (line[i] == '\\t') {\n@@ -545,6 +547,11 @@ static void emit_line_with_ws(int nparents,\n \t\telse\n \t\t\tbreak;\n \t}\n+\tif (0 <= last_space_in_indent && 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"}]}