{"thread":{"id":"3464","subject":"the war on trailing whitespace","startedAt":"2006-02-26T01:40:47Z","lastAt":"2006-02-28T09:46:45Z","messageCount":40,"participants":["Andrew Morton","Junio C Hamano","Linus Torvalds","Sam Ravnborg","Dave Jones","MIke Galbraith","Johannes Schindelin","Adrien Beau","Andreas Ericsson","Uwe Zeisberger","Peter Hagervall","Randal L. Schwartz","Josef Weidendorfer","Peter Williams","A Large Angry SCM"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"16735","messageId":"20060225174047.0e9a6d29.akpm@osdl.org","threadId":"3464","inReplyTo":null,"subject":"the war on trailing whitespace","fromName":"Andrew Morton","fromEmail":"akpm@osdl.org","sentAt":"2006-02-26T01:40:47Z","receivedAt":"2006-02-26T01:40:47Z","isPatch":false,"sender":{"key":"akpm@osdl.org","avatar":null},"body":"\nIt's invariably pointless to add lines which have trailing whitespace. \nNobody cares much, but my scripts spam me when it happens, so I've become\nobsessive. Looking at Dave Miller's current net devel tree:\n\nbix:/usr/src/25> grep '^+.*[ ]$' patches/git-net.patch | wc -l\n 170\n\nNote that this is purely _added_ trailing whitespace. I'm not proposing\nthat the revision control system should care about pre-existing trailing\nwhitespace.\n\nI got the quilt guys to generate warnings when patches add trailing\nwhitespace, and to provide the tools to strip it. And I believe Larry made\nsimilar changes to bk.\n\nI realise that we cannot do this when doing git fetches, but when importing\npatches and mboxes, git ought to whine loudly about input which matches the\nabove regexp, and it should offer an option to tidy it up. Perhaps by\ndefault.\n\nOf course, this means that the person who sent the patch will find that his\nas-yet-unsent patches don't apply to the upstream tree any more. Well,\nthat's tough luck - perhaps it'll motivate him to stop adding trailing\nwhitespace. The patches will still apply with `patch -l'.\n\nThanks for listening ;)\n"},{"id":"16738","messageId":"7v1wxq7psj.fsf@assigned-by-dhcp.cox.net","threadId":"3464","inReplyTo":"20060225174047.0e9a6d29.akpm@osdl.org","subject":"Re: the war on trailing whitespace","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-02-26T03:38:20Z","receivedAt":"2006-02-26T03:38:20Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andrew Morton writes:\n\n> It's invariably pointless to add lines which have trailing whitespace. \n> Nobody cares much, but my scripts spam me when it happens, so I've become\n> obsessive....\n\nI do not call me obsessive, but I do enable pre-commit and\npre-applypatch hooks I ship with git myself.\n\n> I realise that we cannot do this when doing git fetches, but when importing\n> patches and mboxes, git ought to whine loudly about input which matches the\n> above regexp, and it should offer an option to tidy it up. Perhaps by\n> default.\n\nI stole the policy the sample hook scripts use from you; it is\nnot enabled by default, and as the tool manufacturer I am a bit\nreluctant to do so.\n\nHowever, as a kernel project maintainer high in the foodchain,\nI'd imagine your plea to your fellow maintainers who apply\npatches using git tools would be heard well.\n"},{"id":"16740","messageId":"20060225210712.29b30f59.akpm@osdl.org","threadId":"3464","inReplyTo":"7v1wxq7psj.fsf@assigned-by-dhcp.cox.net","subject":"Re: the war on trailing whitespace","fromName":"Andrew Morton","fromEmail":"akpm@osdl.org","sentAt":"2006-02-26T05:07:12Z","receivedAt":"2006-02-26T05:07:12Z","isPatch":false,"sender":{"key":"akpm@osdl.org","avatar":null},"body":"Junio C Hamano wrote:\n>\n> Andrew Morton writes:\n> \n> > It's invariably pointless to add lines which have trailing whitespace. \n> > Nobody cares much, but my scripts spam me when it happens, so I've become\n> > obsessive....\n> \n> I do not call me obsessive, but I do enable pre-commit and\n> pre-applypatch hooks I ship with git myself.\n\nIt's apparent that few others do this.\n\n> > I realise that we cannot do this when doing git fetches, but when importing\n> > patches and mboxes, git ought to whine loudly about input which matches the\n> > above regexp, and it should offer an option to tidy it up. Perhaps by\n> > default.\n> \n> I stole the policy the sample hook scripts use from you; it is\n> not enabled by default, and as the tool manufacturer I am a bit\n> reluctant to do so.\n> \n\nIt's not strong enough. I mean, if you ask a developer \"do you wish to add\nnew trialing whitespace to the kernel\" then obviously their answer would be\n\"no\". So how do we help them in this?\n\nI'd suggest a) git will simply refuse to apply such a patch unless given a\nspecial `forcing' flag, b) even when thus forced, it will still warn and c)\nwith a different flag, it will strip-then-apply, without generating a\nwarning.\n"},{"id":"16753","messageId":"Pine.LNX.4.64.0602260925170.22647@g5.osdl.org","threadId":"3464","inReplyTo":"20060225210712.29b30f59.akpm@osdl.org","subject":"Re: the war on trailing whitespace","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-02-26T17:29:00Z","receivedAt":"2006-02-26T17:29:00Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 25 Feb 2006, Andrew Morton wrote:\n> \n> I'd suggest a) git will simply refuse to apply such a patch unless given a\n> special `forcing' flag, b) even when thus forced, it will still warn and c)\n> with a different flag, it will strip-then-apply, without generating a\n> warning.\n\nThis doesn't do the \"strip-then-apply\" thing, but it allows you to make \ngit-apply generate a warning or error on extraneous whitespace.\n\nUse --whitespace=warn to warn, and (surprise, surprise) --whitespace=error \nto make it a fatal error to have whitespace at the end.\n\nTotally untested, of course. But it compiles, so it must be fine.\n\nHOWEVER! Note that this literally will check every single patch-line with \n\"+\" at the beginning. Which means that if you fix a simple typo, and the \nline had a space at the end before, and you didn't remove it, that's still \nconsidered a \"new line with whitespace at the end\", even though obviously \nthe line wasn't really new.\n\nI assume this is what you wanted, and there isn't really any sane \nalternatives (you could make the warning activate only for _pure_ \nadditions with no deletions at all in that hunk, but that sounds a bit \ninsane).\n\n\t\tLinus\n\n---\ndiff --git a/apply.c b/apply.c\nindex 244718c..e7b3dca 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -34,6 +34,12 @@ static int line_termination = '\\n';\n static const char apply_usage[] =\n \"git-apply [--stat] [--numstat] [--summary] [--check] [--index] [--apply] [--no-add] [--index-info] [--allow-binary-replacement] [-z] [-pNUM] ...\";\n \n+static enum whitespace_eol {\n+\tnowarn,\n+\twarn_on_whitespace,\n+\terror_on_whitespace\n+} new_whitespace = nowarn;\n+\n /*\n * For \"diff-stat\" like behaviour, we keep track of the biggest change\n * we've seen, and the longest filename. That allows us to do simple\n@@ -815,6 +821,22 @@ static int parse_fragment(char *line, un\n \t\t\toldlines--;\n \t\t\tbreak;\n \t\tcase '+':\n+\t\t\t/*\n+\t\t\t * We know len is at least two, since we have a '+' and\n+\t\t\t * we checked that the last character was a '\\n' above\n+\t\t\t */\n+\t\t\tif (isspace(line[len-2])) {\n+\t\t\t\tswitch (new_whitespace) {\n+\t\t\t\tcase nowarn:\n+\t\t\t\t\tbreak;\n+\t\t\t\tcase warn_on_whitespace:\n+\t\t\t\t\tnew_whitespace = nowarn;\t/* Just once */\n+\t\t\t\t\terror(\"Added whitespace at end of line at line %d\", linenr);\n+\t\t\t\t\tbreak;\n+\t\t\t\tcase error_on_whitespace:\n+\t\t\t\t\tdie(\"Added whitespace at end of line at line %d\", linenr);\n+\t\t\t\t}\n+\t\t\t}\n \t\t\tadded++;\n \t\t\tnewlines--;\n \t\t\tbreak;\n@@ -1839,6 +1861,17 @@ int main(int argc, char **argv)\n \t\t\tline_termination = 0;\n \t\t\tcontinue;\n \t\t}\n+\t\tif (!strncmp(arg, \"--whitespace=\", 13)) {\n+\t\t\tif (strcmp(arg+13, \"warn\")) {\n+\t\t\t\tnew_whitespace = warn_on_whitespace;\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t\tif (strcmp(arg+13, \"error\")) {\n+\t\t\t\tnew_whitespace = error_on_whitespace;\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t\tdie(\"unrecognixed whitespace option '%s'\", arg+13);\n+\t\t}\n \n \t\tif (check_index && prefix_length < 0) {\n \t\t\tprefix = setup_git_directory();\n"},{"id":"16756","messageId":"20060226103604.2d97696c.akpm@osdl.org","threadId":"3464","inReplyTo":"Pine.LNX.4.64.0602260925170.22647@g5.osdl.org","subject":"Re: the war on trailing whitespace","fromName":"Andrew Morton","fromEmail":"akpm@osdl.org","sentAt":"2006-02-26T18:36:04Z","receivedAt":"2006-02-26T18:36:04Z","isPatch":false,"sender":{"key":"akpm@osdl.org","avatar":null},"body":"Linus Torvalds wrote:\n>\n> \n> \n> On Sat, 25 Feb 2006, Andrew Morton wrote:\n> > \n> > I'd suggest a) git will simply refuse to apply such a patch unless given a\n> > special `forcing' flag, b) even when thus forced, it will still warn and c)\n> > with a different flag, it will strip-then-apply, without generating a\n> > warning.\n> \n> This doesn't do the \"strip-then-apply\" thing, but it allows you to make \n> git-apply generate a warning or error on extraneous whitespace.\n> \n> Use --whitespace=warn to warn, and (surprise, surprise) --whitespace=error \n> to make it a fatal error to have whitespace at the end.\n\nThanks. But it defaults to nowarn. Nobody will turn it on and nothing\nimproves.\n\n> Totally untested, of course. But it compiles, so it must be fine.\n\nWho cares, as long as the patch doesn't add trailing whitespace? ) ;)\n\n> HOWEVER! Note that this literally will check every single patch-line with \n> \"+\" at the beginning. Which means that if you fix a simple typo, and the \n> line had a space at the end before, and you didn't remove it, that's still \n> considered a \"new line with whitespace at the end\", even though obviously \n> the line wasn't really new.\n> \n> I assume this is what you wanted, and there isn't really any sane \n> alternatives (you could make the warning activate only for _pure_ \n> additions with no deletions at all in that hunk, but that sounds a bit \n> insane).\n\nYup. So by the time we've patched every line in the kernel, it's\ntrailing-whitespace-free.\n"},{"id":"16758","messageId":"20060226194529.GA21009@mars.ravnborg.org","threadId":"3464","inReplyTo":"Pine.LNX.4.64.0602260925170.22647@g5.osdl.org","subject":"Re: the war on trailing whitespace","fromName":"Sam Ravnborg","fromEmail":"sam@ravnborg.org","sentAt":"2006-02-26T19:45:29Z","receivedAt":"2006-02-26T19:45:29Z","isPatch":false,"sender":{"key":"sam@ravnborg.org","avatar":"https://gravatar.com/avatar/168a912606ed0742d840bb365e3cc21db390c36531a58341dc7a069cc1f15f62?d=mp&s=160"},"body":"On Sun, Feb 26, 2006 at 09:29:00AM -0800, Linus Torvalds wrote:\n> \n> \n> On Sat, 25 Feb 2006, Andrew Morton wrote:\n> > \n> > I'd suggest a) git will simply refuse to apply such a patch unless given a\n> > special `forcing' flag, b) even when thus forced, it will still warn and c)\n> > with a different flag, it will strip-then-apply, without generating a\n> > warning.\n> \n> This doesn't do the \"strip-then-apply\" thing, but it allows you to make \n> git-apply generate a warning or error on extraneous whitespace.\n\nCan this somehow be done in a way so everyone that clones your tree\nwill inherit the warn/error on whitespace setting?\nIn this way we make sure it gets enabled automagically in many trees\nand I do not have to remember yet another options.\n\nAlternatively something that is enabled for a tree so I only have to do\nsomething once - a trigger maybe?\n\n\tSam\n"},{"id":"16760","messageId":"Pine.LNX.4.64.0602261213340.22647@g5.osdl.org","threadId":"3464","inReplyTo":"20060226103604.2d97696c.akpm@osdl.org","subject":"Re: the war on trailing whitespace","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-02-26T20:16:25Z","receivedAt":"2006-02-26T20:16:25Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 26 Feb 2006, Andrew Morton wrote:\n> \n> Thanks. But it defaults to nowarn. Nobody will turn it on and nothing\n> improves.\n\nFew enough people run \"git-apply\" on its own. Most people (certainly me) \nend up using it through some email-applicator script or other. So the plan \nwas that the --whitespace=warn/error flag would go there, and that \ngit-apply by default would work more like \"patch\".\n\nBut hey, I have no strong preferences, and it's easy enough to make the \ndefault be warn (and add a \"--whitespace=ok\" flag to turn it off).\n\nPersonally, I don't mind whitespace that much. In particular, I _suspect_ \nI often have empty lines like\n\n\tint i;\n\t\n\ti = 10;\n\nwhere the \"empty\" line actually has the same indentation as the lines \naround it. Is that wrong? Perhaps.\n\n\t\tLinus\n"},{"id":"16762","messageId":"20060226202617.GH7851@redhat.com","threadId":"3464","inReplyTo":"Pine.LNX.4.64.0602261213340.22647@g5.osdl.org","subject":"Re: the war on trailing whitespace","fromName":"Dave Jones","fromEmail":"davej@redhat.com","sentAt":"2006-02-26T20:26:17Z","receivedAt":"2006-02-26T20:26:17Z","isPatch":false,"sender":{"key":"davej@redhat.com","avatar":null},"body":"On Sun, Feb 26, 2006 at 12:16:25PM -0800, Linus Torvalds wrote:\n\n > Few enough people run \"git-apply\" on its own. Most people (certainly me)\n > end up using it through some email-applicator script or other. So the plan\n > was that the --whitespace=warn/error flag would go there, and that\n > git-apply by default would work more like \"patch\".\n >\n > But hey, I have no strong preferences, and it's easy enough to make the\n > default be warn (and add a \"--whitespace=ok\" flag to turn it off).\n >\n > Personally, I don't mind whitespace that much. In particular, I _suspect_\n > I often have empty lines like\n >\n >\tint i;\n >\n >\ti = 10;\n >\n > where the \"empty\" line actually has the same indentation as the lines\n > around it. Is that wrong? Perhaps.\n\nI think I have the same anal-retentive problem Andrew has, because I have ..\n\nhighlight RedundantSpaces term=standout ctermbg=red guibg=red\nmatch RedundantSpaces /\\s\\+$\\| \\+\\ze\\t/\n\nin my .vimrc, which highlights this (and other trailing whitespace) as\na big red blob. I do this in part for the same reason Andrew does,\nso that when someone sends me a diff with a zillion spaces at the EOL,\nit screams at me, I spot them, and chop them out.\n\n\t\tDave\n"},{"id":"16763","messageId":"7vhd6l6ezq.fsf@assigned-by-dhcp.cox.net","threadId":"3464","inReplyTo":"20060226103604.2d97696c.akpm@osdl.org","subject":"Re: the war on trailing whitespace","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-02-26T20:29:13Z","receivedAt":"2006-02-26T20:29:13Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andrew Morton writes:\n\n> Thanks. But it defaults to nowarn. Nobody will turn it on and nothing\n> improves.\n\nThis WS is clearly a policy, and while I personally agree that\nit is a _good_ policy, I am a bit hesitant to hardcode this\nstricter policy as the default to lower level tools.\n\nI have a feeling that Linus is saying that pre-applypatch hook\nis good enough, and you have to educate people who feed things\nto you ;-)\n"},{"id":"16765","messageId":"20060226203102.GI7851@redhat.com","threadId":"3464","inReplyTo":"20060226202617.GH7851@redhat.com","subject":"Re: the war on trailing whitespace","fromName":"Dave Jones","fromEmail":"davej@redhat.com","sentAt":"2006-02-26T20:31:02Z","receivedAt":"2006-02-26T20:31:02Z","isPatch":false,"sender":{"key":"davej@redhat.com","avatar":null},"body":"On Sun, Feb 26, 2006 at 03:26:17PM -0500, Dave Jones wrote:\n\n > in my .vimrc, which highlights this (and other trailing whitespace) as\n > a big red blob. I do this in part for the same reason Andrew does,\n > so that when someone sends me a diff with a zillion spaces at the EOL,\n > it screams at me, I spot them, and chop them out.\n\n(seconds later, I find my .vimrc on master.k.o hasn't had this turned\n on so I find a billion instances of this mess in agp/cpufreq -- Bah).\n\n\t\tDave\n"},{"id":"16783","messageId":"7vr75p4ojt.fsf@assigned-by-dhcp.cox.net","threadId":"3464","inReplyTo":"Pine.LNX.4.64.0602261213340.22647@g5.osdl.org","subject":"Re: the war on trailing whitespace","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-02-27T00:45:42Z","receivedAt":"2006-02-27T00:45:42Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> Personally, I don't mind whitespace that much. In particular, I _suspect_ \n> I often have empty lines like\n>\n> \tint i;\n> \t\n> \ti = 10;\n>\n> where the \"empty\" line actually has the same indentation as the lines \n> around it. Is that wrong? Perhaps.\n\nYes, you do, and I hand-fixed one a couple of minutes ago ;-).\n\nRegarding git-apply change, I suspect warn_on_whitespace should\nnot squelch itself after the first one, and error_on_whitespace\nshould not die instantly. The sample pre-applypatch hook (it\nwas missing code to figure out where GIT_DIR was so it never\nworked as shipped; corrected in \"master\") shows line numbers of\nsuspicious lines from the files being patched. They can be\nmanually fixed up, and then \"git am --resolved\", if the\nintegrator is in a better mood.\n\nThe error messages from pre-commit/pre-applypatch hook mimic the\nway compiler errors are spit out, so that it works well in Emacs\ncompilation buffer -- doing C-x ` (next-error) takes you the\nline the error appears and lets you edit it.\n"},{"id":"16789","messageId":"7v8xrx4kf8.fsf_-_@assigned-by-dhcp.cox.net","threadId":"3464","inReplyTo":"7vr75p4ojt.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] apply --whitespace fixes and enhancements.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-02-27T02:14:51Z","receivedAt":"2006-02-27T02:14:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"In addition to fixing obvious command line parsing bugs in the\nprevious round, this changes the following:\n\n * Adds \"--whitespace=strip\". This applies after stripping the\n new trailing whitespaces introduced to the patch.\n\n * The output error message format is changed to say\n \"patch-filename:linenumber:contents of the line\". This makes\n it similar to typical compiler error message format, and\n helps C-x ` (next-error) in Emacs compilation buffer.\n\n * --whitespace=error and --whitespace=warn do not stop at the\n first error. We might want to limit the output to say first\n 20 such lines to prevent cluttering, but on the other hand if\n you are willing to hand-fix after inspecting them, getting\n everything with a single run might be easier to work with.\n After all, somebody has to do the clean-up work somewhere.\n\nSigned-off-by: Junio C Hamano \n\n---\n\n Junio C Hamano writes:\n\n > Regarding git-apply change, I suspect warn_on_whitespace should\n > not squelch itself after the first one, and error_on_whitespace\n > should not die instantly. The sample pre-applypatch hook (it\n > was missing code to figure out where GIT_DIR was so it never\n > worked as shipped; corrected in \"master\") shows line numbers of\n > suspicious lines from the files being patched. They can be\n > manually fixed up, and then \"git am --resolved\", if the\n > integrator is in a better mood.\n >\n > The error messages from pre-commit/pre-applypatch hook mimic the\n > way compiler errors are spit out, so that it works well in Emacs\n > compilation buffer -- doing C-x ` (next-error) takes you the\n > line the error appears and lets you edit it.\n\n apply.c | 77 ++++++++++++++++++++++++++++++++++++++++++++-------------------\n 1 files changed, 54 insertions(+), 23 deletions(-)\n\ne0af70a72d4115c32a1f9b91f1cf4556bbd014b6\ndiff --git a/apply.c b/apply.c\nindex e7b3dca..7dbbeb4 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -37,8 +37,11 @@ static const char apply_usage[] =\n static enum whitespace_eol {\n \tnowarn,\n \twarn_on_whitespace,\n-\terror_on_whitespace\n+\terror_on_whitespace,\n+\tstrip_and_apply,\n } new_whitespace = nowarn;\n+static int whitespace_error = 0;\n+static const char *patch_input_file = NULL;\n \n /*\n * For \"diff-stat\" like behaviour, we keep track of the biggest change\n@@ -823,19 +826,17 @@ static int parse_fragment(char *line, un\n \t\tcase '+':\n \t\t\t/*\n \t\t\t * We know len is at least two, since we have a '+' and\n-\t\t\t * we checked that the last character was a '\\n' above\n+\t\t\t * we checked that the last character was a '\\n' above.\n+\t\t\t * That is, an addition of an empty line would check\n+\t\t\t * the '+' here. Sneaky...\n \t\t\t */\n-\t\t\tif (isspace(line[len-2])) {\n-\t\t\t\tswitch (new_whitespace) {\n-\t\t\t\tcase nowarn:\n-\t\t\t\t\tbreak;\n-\t\t\t\tcase warn_on_whitespace:\n-\t\t\t\t\tnew_whitespace = nowarn;\t/* Just once */\n-\t\t\t\t\terror(\"Added whitespace at end of line at line %d\", linenr);\n-\t\t\t\t\tbreak;\n-\t\t\t\tcase error_on_whitespace:\n-\t\t\t\t\tdie(\"Added whitespace at end of line at line %d\", linenr);\n-\t\t\t\t}\n+\t\t\tif ((new_whitespace != nowarn) &&\n+\t\t\t isspace(line[len-2])) {\n+\t\t\t\tfprintf(stderr, \"Added whitespace\\n\");\n+\t\t\t\tfprintf(stderr, \"%s:%d:%.*s\\n\",\n+\t\t\t\t\tpatch_input_file,\n+\t\t\t\t\tlinenr, len-2, line+1);\n+\t\t\t\twhitespace_error = 1;\n \t\t\t}\n \t\t\tadded++;\n \t\t\tnewlines--;\n@@ -1114,6 +1115,27 @@ struct buffer_desc {\n \tunsigned long alloc;\n };\n \n+static int apply_line(char *output, const char *patch, int plen)\n+{\n+\t/* plen is number of bytes to be copied from patch,\n+\t * starting at patch+1 (patch[0] is '+'). Typically\n+\t * patch[plen] is '\\n'.\n+\t */\n+\tint add_nl_to_tail = 0;\n+\tif ((new_whitespace == strip_and_apply) &&\n+\t 1 < plen && isspace(patch[plen-1])) {\n+\t\tif (patch[plen] == '\\n')\n+\t\t\tadd_nl_to_tail = 1;\n+\t\tplen--;\n+\t\twhile (0 < plen && isspace(patch[plen]))\n+\t\t\tplen--;\n+\t}\n+\tmemcpy(output, patch + 1, plen);\n+\tif (add_nl_to_tail)\n+\t\toutput[plen++] = '\\n';\n+\treturn plen;\n+}\n+\n static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag)\n {\n \tchar *buf = desc->buffer;\n@@ -1149,10 +1171,9 @@ static int apply_one_fragment(struct buf\n \t\t\t\tbreak;\n \t\t/* Fall-through for ' ' */\n \t\tcase '+':\n-\t\t\tif (*patch != '+' || !no_add) {\n-\t\t\t\tmemcpy(new + newsize, patch + 1, plen);\n-\t\t\t\tnewsize += plen;\n-\t\t\t}\n+\t\t\tif (*patch != '+' || !no_add)\n+\t\t\t\tnewsize += apply_line(new + newsize, patch,\n+\t\t\t\t\t\t plen);\n \t\t\tbreak;\n \t\tcase '@': case '\\\\':\n \t\t\t/* Ignore it, we already handled it */\n@@ -1721,7 +1742,7 @@ static int use_patch(struct patch *p)\n \treturn 1;\n }\n \n-static int apply_patch(int fd)\n+static int apply_patch(int fd, const char *filename)\n {\n \tint newfd;\n \tunsigned long offset, size;\n@@ -1729,6 +1750,7 @@ static int apply_patch(int fd)\n \tstruct patch *list = NULL, **listp = &list;\n \tint skipped_patch = 0;\n \n+\tpatch_input_file = filename;\n \tif (!buffer)\n \t\treturn -1;\n \toffset = 0;\n@@ -1755,6 +1777,9 @@ static int apply_patch(int fd)\n \t}\n \n \tnewfd = -1;\n+\tif (whitespace_error && (new_whitespace == error_on_whitespace))\n+\t\tapply = 0;\n+\n \twrite_index = check_index && apply;\n \tif (write_index)\n \t\tnewfd = hold_index_file_for_update(&cache_file, get_index_file());\n@@ -1801,7 +1826,7 @@ int main(int argc, char **argv)\n \t\tint fd;\n \n \t\tif (!strcmp(arg, \"-\")) {\n-\t\t\tapply_patch(0);\n+\t\t\tapply_patch(0, \"\");\n \t\t\tread_stdin = 0;\n \t\t\tcontinue;\n \t\t}\n@@ -1862,14 +1887,18 @@ int main(int argc, char **argv)\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strncmp(arg, \"--whitespace=\", 13)) {\n-\t\t\tif (strcmp(arg+13, \"warn\")) {\n+\t\t\tif (!strcmp(arg+13, \"warn\")) {\n \t\t\t\tnew_whitespace = warn_on_whitespace;\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (strcmp(arg+13, \"error\")) {\n+\t\t\tif (!strcmp(arg+13, \"error\")) {\n \t\t\t\tnew_whitespace = error_on_whitespace;\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strcmp(arg+13, \"strip\")) {\n+\t\t\t\tnew_whitespace = strip_and_apply;\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\tdie(\"unrecognixed whitespace option '%s'\", arg+13);\n \t\t}\n \n@@ -1885,10 +1914,12 @@ int main(int argc, char **argv)\n \t\tif (fd < 0)\n \t\t\tusage(apply_usage);\n \t\tread_stdin = 0;\n-\t\tapply_patch(fd);\n+\t\tapply_patch(fd, arg);\n \t\tclose(fd);\n \t}\n \tif (read_stdin)\n-\t\tapply_patch(0);\n+\t\tapply_patch(0, \"\");\n+\tif (whitespace_error && new_whitespace == error_on_whitespace)\n+\t\treturn 1;\n \treturn 0;\n }\n-- \n1.2.3.gac5f\n"},{"id":"16790","messageId":"1141008633.7593.13.camel@homer","threadId":"3464","inReplyTo":"20060226202617.GH7851@redhat.com","subject":"Re: the war on trailing whitespace","fromName":"MIke Galbraith","fromEmail":"efault@gmx.de","sentAt":"2006-02-27T02:50:33Z","receivedAt":"2006-02-27T02:50:33Z","isPatch":false,"sender":{"key":"efault@gmx.de","avatar":null},"body":"On Sun, 2006-02-26 at 15:26 -0500, Dave Jones wrote:\n\n> I think I have the same anal-retentive problem Andrew has, because I have ..\n> \n> highlight RedundantSpaces term=standout ctermbg=red guibg=red\n> match RedundantSpaces /\\s\\+$\\| \\+\\ze\\t/\n> \n> in my .vimrc, which highlights this (and other trailing whitespace) as\n> a big red blob. I do this in part for the same reason Andrew does,\n> so that when someone sends me a diff with a zillion spaces at the EOL,\n> it screams at me, I spot them, and chop them out.\n\nDang, I'm _not_ perfect after all ;-). I just tried this on a patch of\nmine, and up popped a few angry red blobs even though I try to be\ncareful. Perhaps someone should add more pet peeve thingies to it and\npost it to lkml, or put it some place people can be pointed at.\n\n\t-Mike\n"},{"id":"16800","messageId":"Pine.LNX.4.63.0602271004130.5937@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"3464","inReplyTo":"1141008633.7593.13.camel@homer","subject":"Re: the war on trailing whitespace","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-02-27T09:07:05Z","receivedAt":"2006-02-27T09:07:05Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nthere is a good reason not to enable the no-whitespace-at-eol checking in \npre-commit by default (at least for *all* files) for git development:\n\n\tPython.\n\nJust do a \"/ $\" in git-merge-recursive.py. These whitespaces are not an \nerror, but a syntactic *requirement*.\n\nHth,\nDscho\n"},{"id":"16802","messageId":"20060227011832.78359f0a.akpm@osdl.org","threadId":"3464","inReplyTo":"Pine.LNX.4.63.0602271004130.5937@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: the war on trailing whitespace","fromName":"Andrew Morton","fromEmail":"akpm@osdl.org","sentAt":"2006-02-27T09:18:32Z","receivedAt":"2006-02-27T09:18:32Z","isPatch":false,"sender":{"key":"akpm@osdl.org","avatar":null},"body":"Johannes Schindelin wrote:\n>\n> Hi,\n> \n> there is a good reason not to enable the no-whitespace-at-eol checking in \n> pre-commit by default (at least for *all* files) for git development:\n> \n> \tPython.\n\nThat's not a good reason. People will discover that git has started\nshouting at them and they'll work out how to make it stop.\n\nThe probem is getting C users to turn the check on, not in getting python\nusers to turn it off.\n"},{"id":"16803","messageId":"94fc236b0602270326s3079d737l102d5728d59f0c98@mail.gmail.com","threadId":"3464","inReplyTo":"Pine.LNX.4.63.0602271004130.5937@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: the war on trailing whitespace","fromName":"Adrien Beau","fromEmail":"adrienbeau@gmail.com","sentAt":"2006-02-27T11:26:34Z","receivedAt":"2006-02-27T11:26:34Z","isPatch":false,"sender":{"key":"adrienbeau@gmail.com","avatar":null},"body":"On 2/27/06, Johannes Schindelin wrote:\n>\n> there is a good reason not to enable the no-whitespace-at-eol checking in\n> pre-commit by default (at least for *all* files) for git development:\n>\n> Python.\n>\n> Just do a \"/ $\" in git-merge-recursive.py. These whitespaces are not an\n> error, but a syntactic *requirement*.\n\nNo, they aren't.\n\nA logical line that contains only spaces and tabs is ignored by\nPython. (All the \"dirty\" lines in git-merge-recursive.py are such\nlines.)\n\nBesides, spaces, tabs and newlines can be used interchangeably to\nseparate tokens, so trailing whitespace is never *required*.\n\nHope this helps,\n\nAdrien\n"},{"id":"16804","messageId":"4402E56D.4010606@op5.se","threadId":"3464","inReplyTo":"94fc236b0602270326s3079d737l102d5728d59f0c98@mail.gmail.com","subject":"Re: the war on trailing whitespace","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2006-02-27T11:41:33Z","receivedAt":"2006-02-27T11:41:33Z","isPatch":false,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Adrien Beau wrote:\n> On 2/27/06, Johannes Schindelin wrote:\n> \n>>there is a good reason not to enable the no-whitespace-at-eol checking in\n>>pre-commit by default (at least for *all* files) for git development:\n>>\n>> Python.\n>>\n>>Just do a \"/ $\" in git-merge-recursive.py. These whitespaces are not an\n>>error, but a syntactic *requirement*.\n> \n> \n> No, they aren't.\n> \n> A logical line that contains only spaces and tabs is ignored by\n> Python. (All the \"dirty\" lines in git-merge-recursive.py are such\n> lines.)\n> \n\nI think the question is whether completely empty lines are also ignored \nby Python, or if they start a new block of code. Whatever the case, it \nmust hold true for both 2.3 and 2.4.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"16805","messageId":"Pine.LNX.4.63.0602271255340.21502@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"3464","inReplyTo":"94fc236b0602270326s3079d737l102d5728d59f0c98@mail.gmail.com","subject":"Re: the war on trailing whitespace","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-02-27T11:55:59Z","receivedAt":"2006-02-27T11:55:59Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\n\nOn Mon, 27 Feb 2006, Adrien Beau wrote:\n\n> A logical line that contains only spaces and tabs is ignored by\n> Python. (All the \"dirty\" lines in git-merge-recursive.py are such\n> lines.)\n> \n> Hope this helps,\n\nIt does.\n\nThanks,\nDscho\n"},{"id":"16814","messageId":"20060227133124.GA8794@informatik.uni-freiburg.de","threadId":"3464","inReplyTo":"4402E56D.4010606@op5.se","subject":"Re: the war on trailing whitespace","fromName":"Uwe Zeisberger","fromEmail":"zeisberg@informatik.uni-freiburg.de","sentAt":"2006-02-27T13:31:24Z","receivedAt":"2006-02-27T13:31:24Z","isPatch":false,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"Hello,\n\nAndreas Ericsson wrote:\n> I think the question is whether completely empty lines are also ignored \n> by Python, or if they start a new block of code. Whatever the case, it \n> must hold true for both 2.3 and 2.4.\nsee\n\thttp://www.python.org/doc/2.2.3/ref/blank-lines.html\n\thttp://www.python.org/doc/2.3.5/ref/blank-lines.html\n\thttp://www.python.org/doc/2.4.2/ref/blank-lines.html\n\nBest regards\nUwe\n\n-- \nUwe Zeisberger\n\nhttp://www.google.com/search?q=gravity+on+earth%3D\n"},{"id":"16815","messageId":"4403086F.5040704@op5.se","threadId":"3464","inReplyTo":"20060227133124.GA8794@informatik.uni-freiburg.de","subject":"Re: the war on trailing whitespace","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2006-02-27T14:10:55Z","receivedAt":"2006-02-27T14:10:55Z","isPatch":false,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Uwe Zeisberger wrote:\n> Hello,\n> \n> Andreas Ericsson wrote:\n> \n>>I think the question is whether completely empty lines are also ignored \n>>by Python, or if they start a new block of code. Whatever the case, it \n>>must hold true for both 2.3 and 2.4.\n> \n> see\n> \thttp://www.python.org/doc/2.2.3/ref/blank-lines.html\n> \thttp://www.python.org/doc/2.3.5/ref/blank-lines.html\n> \thttp://www.python.org/doc/2.4.2/ref/blank-lines.html\n> \n\nSo in essence, a multi-line statement is closed when a completely empty \nline is found, which means that making git internals recognize and strip \nsuch lines will result in Python code never being manageable by git.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"16816","messageId":"20060227143147.GA12196@brainysmurf.cs.umu.se","threadId":"3464","inReplyTo":"4403086F.5040704@op5.se","subject":"Re: the war on trailing whitespace","fromName":"Peter Hagervall","fromEmail":"hager@cs.umu.se","sentAt":"2006-02-27T14:31:47Z","receivedAt":"2006-02-27T14:31:47Z","isPatch":false,"sender":{"key":"hager@cs.umu.se","avatar":null},"body":"On Mon, Feb 27, 2006 at 03:10:55PM +0100, Andreas Ericsson wrote:\n> So in essence, a multi-line statement is closed when a completely empty \n> line is found, which means that making git internals recognize and strip \n> such lines will result in Python code never being manageable by git.\n> \n\nI believe completely empty lines are to be left untouched. The war is on\ntrailing whitespace.\n\n/ Peter\n\n-- \nPeter Hagervall......................email: hager@cs.umu.se\nDepartment of Computing Science........tel: +46(0)90 786 7018\nUniversity of Umeå, SE-901 87 Umeå.....fax: +46(0)90 786 6126\n"},{"id":"16817","messageId":"Pine.LNX.4.63.0602271539160.4371@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"3464","inReplyTo":"20060227143147.GA12196@brainysmurf.cs.umu.se","subject":"Re: the war on trailing whitespace","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-02-27T14:40:51Z","receivedAt":"2006-02-27T14:40:51Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 27 Feb 2006, Peter Hagervall wrote:\n\n> On Mon, Feb 27, 2006 at 03:10:55PM +0100, Andreas Ericsson wrote:\n> > So in essence, a multi-line statement is closed when a completely empty \n> > line is found, which means that making git internals recognize and strip \n> > such lines will result in Python code never being manageable by git.\n> > \n> \n> I believe completely empty lines are to be left untouched. The war is on\n> trailing whitespace.\n\nExactly. That is what Andreas is saying: if you change a line which \nconsists only of trailing whitespace to an empty line, it breaks python \n(or formatting).\n\nHth,\nDscho\n"},{"id":"16821","messageId":"86wtfgzv0m.fsf@blue.stonehenge.com","threadId":"3464","inReplyTo":"Pine.LNX.4.63.0602271539160.4371@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: the war on trailing whitespace","fromName":"Randal L. Schwartz","fromEmail":"merlyn@stonehenge.com","sentAt":"2006-02-27T15:22:33Z","receivedAt":"2006-02-27T15:22:33Z","isPatch":false,"sender":{"key":"merlyn@stonehenge.com","avatar":"https://gravatar.com/avatar/dc528d210743ff0333e6213f9ee7b33b23f1b7bc1f3c5a8c2d819074ecd7ab19?d=mp&s=160"},"body":">>>>> \"Johannes\" == Johannes Schindelin writes:\n\nJohannes> Exactly. That is what Andreas is saying: if you change a line which \nJohannes> consists only of trailing whitespace to an empty line, it breaks python \nJohannes> (or formatting).\n\n[insert standard rant about Python treating invisible things significantly,\nwhich is the prime counter-rant to people saying Perl looks like line noise]\n\n[chuckling]\n\n-- \nRandal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095\n \nPerl/Unix/security consulting, Technical writing, Comedy, etc. etc.\nSee PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!\n"},{"id":"16825","messageId":"200602271708.24656.Josef.Weidendorfer@gmx.de","threadId":"3464","inReplyTo":"4403086F.5040704@op5.se","subject":"Re: the war on trailing whitespace","fromName":"Josef Weidendorfer","fromEmail":"josef.weidendorfer@gmx.de","sentAt":"2006-02-27T16:08:24Z","receivedAt":"2006-02-27T16:08:24Z","isPatch":false,"sender":{"key":"josef.weidendorfer@gmx.de","avatar":null},"body":"On Monday 27 February 2006 15:10, you wrote:\n> So in essence, a multi-line statement is closed when a completely empty \n> line is found,\n\nAs I read this reference, such handling of completely empty lines is\ndone only in interactive mode, ie. when python is attached to a terminal...\nGit is about storing python scripts, and not \"interactive input\"; therefore,\nthis seems to be a non-issue.\n\nI just checked it. The following, with a completely empty line 3, is working\nas expected, and not looping on an empty statement:\n===================\na = ['cat', 'window', 'defenestrate']\nfor x in a:\n\n print x, len(x)\n===================\n\nJosef\n\nPS: I am not a python programmer...\n"},{"id":"16827","messageId":"94fc236b0602270822v1922b15dw9f7c09351210e08f@mail.gmail.com","threadId":"3464","inReplyTo":"4403086F.5040704@op5.se","subject":"Re: the war on trailing whitespace","fromName":"Adrien Beau","fromEmail":"adrienbeau@gmail.com","sentAt":"2006-02-27T16:22:59Z","receivedAt":"2006-02-27T16:22:59Z","isPatch":false,"sender":{"key":"adrienbeau@gmail.com","avatar":null},"body":"On 2/27/06, Andreas Ericsson wrote:\n>\n> So in essence, a multi-line statement is closed when a completely empty\n> line is found, which means that making git internals recognize and strip\n> such lines will result in Python code never being manageable by git.\n\nIncorrect. This is only the case in the *interactive* interpreter in\nthe standard implementation. For source code in general, quoting the\nPython Reference Manual:\n\n\"A logical line that contains only spaces, tabs, formfeeds and\npossibly a comment, is ignored (i.e., no NEWLINE token is generated).\"\n\nSo such lines, whether completely empty or only apparently so (i.e.\ndirty), are ignored, and can be safely cleaned-up.\n\nAdrien\n"},{"id":"16829","messageId":"20060227163709.GA12538@informatik.uni-freiburg.de","threadId":"3464","inReplyTo":"4403086F.5040704@op5.se","subject":"Re: the war on trailing whitespace","fromName":"Uwe Zeisberger","fromEmail":"zeisberg@informatik.uni-freiburg.de","sentAt":"2006-02-27T16:37:09Z","receivedAt":"2006-02-27T16:37:09Z","isPatch":false,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"Andreas Ericsson wrote:\n> Uwe Zeisberger wrote:\n> >Hello,\n> >\n> >Andreas Ericsson wrote:\n> >\n> >>I think the question is whether completely empty lines are also ignored \n> >>by Python, or if they start a new block of code. Whatever the case, it \n> >>must hold true for both 2.3 and 2.4.\n> >\n> >see\n> >\thttp://www.python.org/doc/2.2.3/ref/blank-lines.html\n> >\thttp://www.python.org/doc/2.3.5/ref/blank-lines.html\n> >\thttp://www.python.org/doc/2.4.2/ref/blank-lines.html\n> >\n> \n> So in essence, a multi-line statement is closed when a completely empty \n> line is found,\nWrong.\n\nA logical line that contains only spaces, tabs, formfeeds and possibly a\ncomment, is ignored (i.e., no NEWLINE token is generated). During\ninteractive input of statements, handling of a blank line may differ\ndepending on the implementation of the read-eval-print loop. In the\nstandard implementation, an entirely blank logical line (i.e. one\ncontaining not even whitespace or a comment) terminates a multi-line\nstatement.\n\nTo translate that to python:\n\n if not interactive:\n a line only containing whitespace is ignored.\n else:\n if standard implementation:\n empty line terminates multi-line statement\n else:\n dependent on implementation\n\ni.e. In scripts, lines containing only (zero or more) whitespaces are\nignored.\n\nhth\nUwe\n\n-- \nUwe Zeisberger\n\nSet the I_WANT_A_BROKEN_PS environment variable to force BSD syntax ...\n\t-- manpage of procps\n"},{"id":"16830","messageId":"44032BBE.6090608@op5.se","threadId":"3464","inReplyTo":"20060227163709.GA12538@informatik.uni-freiburg.de","subject":"Re: the war on trailing whitespace","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2006-02-27T16:41:34Z","receivedAt":"2006-02-27T16:41:34Z","isPatch":false,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Uwe Zeisberger wrote:\n> Andreas Ericsson wrote:\n> \n>>Uwe Zeisberger wrote:\n>>\n>>>Hello,\n>>>\n>>>Andreas Ericsson wrote:\n>>>\n>>>\n>>>>I think the question is whether completely empty lines are also ignored \n>>>>by Python, or if they start a new block of code. Whatever the case, it \n>>>>must hold true for both 2.3 and 2.4.\n>>>\n>>>see\n>>>\thttp://www.python.org/doc/2.2.3/ref/blank-lines.html\n>>>\thttp://www.python.org/doc/2.3.5/ref/blank-lines.html\n>>>\thttp://www.python.org/doc/2.4.2/ref/blank-lines.html\n>>>\n>>\n>>So in essence, a multi-line statement is closed when a completely empty \n>>line is found,\n> \n> Wrong.\n> \n> i.e. In scripts, lines containing only (zero or more) whitespaces are\n> ignored.\n> \n\nI stand corrected. Thrice, even. Voi, voi... :)\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"16861","messageId":"7vhd6kxuea.fsf@assigned-by-dhcp.cox.net","threadId":"3464","inReplyTo":"20060227011832.78359f0a.akpm@osdl.org","subject":"Re: the war on trailing whitespace","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-02-27T23:18:53Z","receivedAt":"2006-02-27T23:18:53Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andrew Morton writes:\n\n> That's not a good reason. People will discover that git has started\n> shouting at them and they'll work out how to make it stop.\n>\n> The problem is getting C users to turn the check on, not in getting python\n> users to turn it off.\n\nThis whitespace policy should be at least per-project (people\nworking on both kernel and other things may have legitimate\nreason to want trailing whitespace in the other project), so we\nwould need some configurability; the problem is *both*.\n\nWe could do one of two things, at least.\n\n - I modify the git-apply that is in the \"next\" branch further\n to make --whitespace=error the default, and push it out. You\n convince people who feed things to you to update to *that*\n version or later.\n\n - I already have the added whitespace detection hook (a fixed\n one that actually matches what I use) shipped with git. You\n convince people who feed things to you to update to *that*\n version or later, and to enable that hook.\n\nI think you are arguing for the first one. I am reluctant to do\nso because it would not help by itself *anyway*. In any case\nyou need to convince people who feed things to you to do\nsomething to prevent later changes fed to you from being\ncontaminated with trailing whitespaces.\n\nHaving said that, I have a third solution, which consists of two\npatches that come on top of what are already in \"next\" branch:\n\n - apply: squelch excessive errors and --whitespace=error-all\n - apply --whitespace: configuration option.\n\nWith these, git-apply used by git-applymbox and git-am would\nrefuse to apply a patch that adds trailing whitespaces, when the\nper-repository configuration is set like this:\n\n [apply]\n whitespace = error\n\n(Alternatively,\n\n\t$ git repo-config apply.whitespace error\n\nwould set these lines there for you).\n\nI think there are three kinds of git users.\n\n * Linus, you, and the kernel subsystem maintainers. The\n whitespace policy with this version of git-apply (with the\n configuration option set to apply.whitespace=error) gives\n would help these people by enforcing the SubmittingPatches\n and your \"perfect patch\" requirements.\n\n * People who feed patches to the above people. They are helped\n by enabling the pre-commit hook that comes with git to\n conform to the kernel whitespace policy -- they need to be\n educated to do so.\n\n * People outside of kernel community, using git in projects to\n which the kernel whitespace policy does not have any\n relevance.\n\nWhile I do consider the kernel folks a lot more important\ncustomers than other users, I have to take flak from the third\nkind of users, and to them, authority by Linus or you does not\nweigh as much as the first two classes of people. Making the\ndefault to --whitespace=error means that you are making me\njustify this kernel project policy as something applicable to\nprojects outside the kernel. That is simply not fair to me.\n\nYou have to convince people you work with to update to at least\nto this version anyway, so I do not think it is too much to ask\nfrom you, while you are at it, to tell the higher echelon folks\nto do:\n\n\t$ git repo-config apply.whitespace error\n\nin their repositories (and/or set that in their templates so new\nrepositories created with git-init-db would inherit it).\n"},{"id":"16862","messageId":"44038B73.4030004@bigpond.net.au","threadId":"3464","inReplyTo":"7vhd6kxuea.fsf@assigned-by-dhcp.cox.net","subject":"Re: the war on trailing whitespace","fromName":"Peter Williams","fromEmail":"pwil3058@bigpond.net.au","sentAt":"2006-02-27T23:29:55Z","receivedAt":"2006-02-27T23:29:55Z","isPatch":false,"sender":{"key":"pwil3058@bigpond.net.au","avatar":"https://gravatar.com/avatar/fc711ad9d9890c01c6ede5a99a62a8b8ca82a3eef0def8ef98e0ec7da7f99367?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> Andrew Morton writes:\n> \n> \n>>That's not a good reason. People will discover that git has started\n>>shouting at them and they'll work out how to make it stop.\n>>\n>>The problem is getting C users to turn the check on, not in getting python\n>>users to turn it off.\n> \n> \n> This whitespace policy should be at least per-project (people\n> working on both kernel and other things may have legitimate\n> reason to want trailing whitespace in the other project),\n\nI'd be interested to hear these reasons. My experience is that people \ndon't put trailing white space in deliberately (or even tolerate it if \nthey notice it) and it's usually there as a side effect of the way their \ntext editor works (and that's also the reason that they don't usually \nnotice it). But if my experience is misleading me and there are valid \nreasons for having trailing white space I'm genuinely interested in \nknowing what they are.\n\n> so we\n> would need some configurability; the problem is *both*.\n> \n> We could do one of two things, at least.\n> \n> - I modify the git-apply that is in the \"next\" branch further\n> to make --whitespace=error the default, and push it out. You\n> convince people who feed things to you to update to *that*\n> version or later.\n> \n> - I already have the added whitespace detection hook (a fixed\n> one that actually matches what I use) shipped with git. You\n> convince people who feed things to you to update to *that*\n> version or later, and to enable that hook.\n> \n> I think you are arguing for the first one. I am reluctant to do\n> so because it would not help by itself *anyway*. In any case\n> you need to convince people who feed things to you to do\n> something to prevent later changes fed to you from being\n> contaminated with trailing whitespaces.\n> \n> Having said that, I have a third solution, which consists of two\n> patches that come on top of what are already in \"next\" branch:\n> \n> - apply: squelch excessive errors and --whitespace=error-all\n> - apply --whitespace: configuration option.\n> \n> With these, git-apply used by git-applymbox and git-am would\n> refuse to apply a patch that adds trailing whitespaces, when the\n> per-repository configuration is set like this:\n> \n> [apply]\n> whitespace = error\n> \n> (Alternatively,\n> \n> \t$ git repo-config apply.whitespace error\n> \n> would set these lines there for you).\n> \n> I think there are three kinds of git users.\n> \n> * Linus, you, and the kernel subsystem maintainers. The\n> whitespace policy with this version of git-apply (with the\n> configuration option set to apply.whitespace=error) gives\n> would help these people by enforcing the SubmittingPatches\n> and your \"perfect patch\" requirements.\n> \n> * People who feed patches to the above people. They are helped\n> by enabling the pre-commit hook that comes with git to\n> conform to the kernel whitespace policy -- they need to be\n> educated to do so.\n> \n> * People outside of kernel community, using git in projects to\n> which the kernel whitespace policy does not have any\n> relevance.\n> \n> While I do consider the kernel folks a lot more important\n> customers than other users, I have to take flak from the third\n> kind of users, and to them, authority by Linus or you does not\n> weigh as much as the first two classes of people. Making the\n> default to --whitespace=error means that you are making me\n> justify this kernel project policy as something applicable to\n> projects outside the kernel. That is simply not fair to me.\n> \n> You have to convince people you work with to update to at least\n> to this version anyway, so I do not think it is too much to ask\n> from you, while you are at it, to tell the higher echelon folks\n> to do:\n> \n> \t$ git repo-config apply.whitespace error\n> \n> in their repositories (and/or set that in their templates so new\n> repositories created with git-init-db would inherit it).\n> \n> -\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n\n\n-- \nPeter Williams pwil3058@bigpond.net.au\n\n\"Learning, n. The kind of ignorance distinguishing the studious.\"\n -- Ambrose Bierce\n"},{"id":"16863","messageId":"20060227153702.375db751.akpm@osdl.org","threadId":"3464","inReplyTo":"7vhd6kxuea.fsf@assigned-by-dhcp.cox.net","subject":"Re: the war on trailing whitespace","fromName":"Andrew Morton","fromEmail":"akpm@osdl.org","sentAt":"2006-02-27T23:37:02Z","receivedAt":"2006-02-27T23:37:02Z","isPatch":false,"sender":{"key":"akpm@osdl.org","avatar":null},"body":"Junio C Hamano wrote:\n>\n> tell the higher echelon folks\n> to do:\n> \n> \t$ git repo-config apply.whitespace error\n> \n> in their repositories\n\nThat might be reasonable, some might object to tiny amounts of extra work..\n\nIn my setup, trailing whitespace purely causes warnings. But with a\nquilt-style thing, it's trivial to unapply the patch, strip it, reapply.\n\ngit is different - I'd imagine that if warnings were generated while an\nmbox was being applied, it's too much hassle to go and unapply the mbox,\nfix the diffs up and then apply the mbox again.\n\nI'd suggest that git have options to a) generate trailing-whitespace\nwarnings, b) generate trailing-whitespace errors and c) strip trailing\nwhitespace while applying. And that the as-shipped default be a).\n"},{"id":"16865","messageId":"7vbqwswdf7.fsf@assigned-by-dhcp.cox.net","threadId":"3464","inReplyTo":"44038B73.4030004@bigpond.net.au","subject":"Re: the war on trailing whitespace","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-02-28T00:10:52Z","receivedAt":"2006-02-28T00:10:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Peter Williams writes:\n\n>> This whitespace policy should be at least per-project (people\n>> working on both kernel and other things may have legitimate\n>> reason to want trailing whitespace in the other project),\n>\n> I'd be interested to hear these reasons. My experience is that people\n> don't put trailing white space in deliberately (or even tolerate it if\n> they notice it) and it's usually there as a side effect of the way\n> their text editor works (and that's also the reason that they don't\n> usually notice it). But if my experience is misleading me and there\n> are valid reasons for having trailing white space I'm genuinely\n> interested in knowing what they are.\n\nFor example:\n\n\thttp://compsoc.dur.ac.uk/whitespace/\n\nJokes aside, I can imagine people want to keep format=flowed\ntext messages (i.e. not programming language source code) under\ngit control. Maybe pulling and pushing would be the default\nmode of operation for those people, so what git-apply does would\nnot be in the picture for those people, but who knows.\n\nOne way to find it out is to push out a strict one and see who\nscreams ;-), but the point is I am reluctant to make a stricter\npolicy the default, thinking, but not knowing as a fact, that it\nis good enough for everybody.\n"},{"id":"16871","messageId":"7vfym4uvyd.fsf@assigned-by-dhcp.cox.net","threadId":"3464","inReplyTo":"7vhd6kxuea.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH 1/3] apply: squelch excessive errors and --whitespace=error-all","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-02-28T01:13:30Z","receivedAt":"2006-02-28T01:13:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This by default makes --whitespace=warn, error, and strip to\nwarn only the first 5 additions of trailing whitespaces. A new\noption --whitespace=error-all can be used to view all of them\nbefore applying.\n\nSigned-off-by: Junio C Hamano \n\n---\n\n * This is already in \"next\".\n\n apply.c | 53 +++++++++++++++++++++++++++++++++++++++++++++--------\n 1 files changed, 45 insertions(+), 8 deletions(-)\n\nfc96b7c9ba5034a408d508c663a96a15b8f8729c\ndiff --git a/apply.c b/apply.c\nindex 7dbbeb4..8139d83 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -41,6 +41,8 @@ static enum whitespace_eol {\n \tstrip_and_apply,\n } new_whitespace = nowarn;\n static int whitespace_error = 0;\n+static int squelch_whitespace_errors = 5;\n+static int applied_after_stripping = 0;\n static const char *patch_input_file = NULL;\n \n /*\n@@ -832,11 +834,16 @@ static int parse_fragment(char *line, un\n \t\t\t */\n \t\t\tif ((new_whitespace != nowarn) &&\n \t\t\t isspace(line[len-2])) {\n-\t\t\t\tfprintf(stderr, \"Added whitespace\\n\");\n-\t\t\t\tfprintf(stderr, \"%s:%d:%.*s\\n\",\n-\t\t\t\t\tpatch_input_file,\n-\t\t\t\t\tlinenr, len-2, line+1);\n-\t\t\t\twhitespace_error = 1;\n+\t\t\t\twhitespace_error++;\n+\t\t\t\tif (squelch_whitespace_errors &&\n+\t\t\t\t squelch_whitespace_errors <\n+\t\t\t\t whitespace_error)\n+\t\t\t\t\t;\n+\t\t\t\telse {\n+\t\t\t\t\tfprintf(stderr, \"Adds trailing whitespace.\\n%s:%d:%.*s\\n\",\n+\t\t\t\t\t\tpatch_input_file,\n+\t\t\t\t\t\tlinenr, len-2, line+1);\n+\t\t\t\t}\n \t\t\t}\n \t\t\tadded++;\n \t\t\tnewlines--;\n@@ -1129,6 +1136,7 @@ static int apply_line(char *output, cons\n \t\tplen--;\n \t\twhile (0 < plen && isspace(patch[plen]))\n \t\t\tplen--;\n+\t\tapplied_after_stripping++;\n \t}\n \tmemcpy(output, patch + 1, plen);\n \tif (add_nl_to_tail)\n@@ -1895,11 +1903,16 @@ int main(int argc, char **argv)\n \t\t\t\tnew_whitespace = error_on_whitespace;\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strcmp(arg+13, \"error-all\")) {\n+\t\t\t\tnew_whitespace = error_on_whitespace;\n+\t\t\t\tsquelch_whitespace_errors = 0;\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\tif (!strcmp(arg+13, \"strip\")) {\n \t\t\t\tnew_whitespace = strip_and_apply;\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tdie(\"unrecognixed whitespace option '%s'\", arg+13);\n+\t\t\tdie(\"unrecognized whitespace option '%s'\", arg+13);\n \t\t}\n \n \t\tif (check_index && prefix_length < 0) {\n@@ -1919,7 +1932,31 @@ int main(int argc, char **argv)\n \t}\n \tif (read_stdin)\n \t\tapply_patch(0, \"\");\n-\tif (whitespace_error && new_whitespace == error_on_whitespace)\n-\t\treturn 1;\n+\tif (whitespace_error) {\n+\t\tif (squelch_whitespace_errors &&\n+\t\t squelch_whitespace_errors < whitespace_error) {\n+\t\t\tint squelched =\n+\t\t\t\twhitespace_error - squelch_whitespace_errors;\n+\t\t\tfprintf(stderr, \"warning: squelched %d whitespace error%s\\n\",\n+\t\t\t\tsquelched,\n+\t\t\t\tsquelched == 1 ? \"\" : \"s\");\n+\t\t}\n+\t\tif (new_whitespace == error_on_whitespace)\n+\t\t\tdie(\"%d line%s add%s trailing whitespaces.\",\n+\t\t\t whitespace_error,\n+\t\t\t whitespace_error == 1 ? \"\" : \"s\",\n+\t\t\t whitespace_error == 1 ? \"s\" : \"\");\n+\t\tif (applied_after_stripping)\n+\t\t\tfprintf(stderr, \"warning: %d line%s applied after\"\n+\t\t\t\t\" stripping trailing whitespaces.\\n\",\n+\t\t\t\tapplied_after_stripping,\n+\t\t\t\tapplied_after_stripping == 1 ? \"\" : \"s\");\n+\t\telse if (whitespace_error)\n+\t\t\tfprintf(stderr, \"warning: %d line%s add%s trailing\"\n+\t\t\t\t\" whitespaces.\\n\",\n+\t\t\t\twhitespace_error,\n+\t\t\t\twhitespace_error == 1 ? \"\" : \"s\",\n+\t\t\t\twhitespace_error == 1 ? \"s\" : \"\");\n+\t}\n \treturn 0;\n }\n-- \n1.2.3.gbfea\n"},{"id":"16873","messageId":"7vacccuvxz.fsf@assigned-by-dhcp.cox.net","threadId":"3464","inReplyTo":"7vhd6kxuea.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH 2/3] apply --whitespace: configuration option.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-02-28T01:13:44Z","receivedAt":"2006-02-28T01:13:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The new configuration option apply.whitespace can take one of\n\"warn\", \"error\", \"error-all\", or \"strip\". When git-apply is run\nto apply the patch to the index, they are used as the default\nvalue if there is no command line --whitespace option.\n\nAndrew can now tell people who feed him git trees to update to\nthis version and say:\n\n\tgit repo-config apply.whitespace error\n\nSigned-off-by: Junio C Hamano \n\n---\n * Already in \"next\".\n\n apply.c | 72 ++++++++++++++++++++++++++++++++++++++-------------------\n cache.h | 2 ++\n environment.c | 1 +\n 3 files changed, 51 insertions(+), 24 deletions(-)\n\n2ae1c53b51ff78b13cc8abf8e9798a12140b7638\ndiff --git a/apply.c b/apply.c\nindex 8139d83..a5cdd8e 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -35,16 +35,42 @@ static const char apply_usage[] =\n \"git-apply [--stat] [--numstat] [--summary] [--check] [--index] [--apply] [--no-add] [--index-info] [--allow-binary-replacement] [-z] [-pNUM] ...\";\n \n static enum whitespace_eol {\n-\tnowarn,\n+\tnowarn_whitespace,\n \twarn_on_whitespace,\n \terror_on_whitespace,\n-\tstrip_and_apply,\n-} new_whitespace = nowarn;\n+\tstrip_whitespace,\n+} new_whitespace = nowarn_whitespace;\n static int whitespace_error = 0;\n static int squelch_whitespace_errors = 5;\n static int applied_after_stripping = 0;\n static const char *patch_input_file = NULL;\n \n+static void parse_whitespace_option(const char *option)\n+{\n+\tif (!option) {\n+\t\tnew_whitespace = nowarn_whitespace;\n+\t\treturn;\n+\t}\n+\tif (!strcmp(option, \"warn\")) {\n+\t\tnew_whitespace = warn_on_whitespace;\n+\t\treturn;\n+\t}\n+\tif (!strcmp(option, \"error\")) {\n+\t\tnew_whitespace = error_on_whitespace;\n+\t\treturn;\n+\t}\n+\tif (!strcmp(option, \"error-all\")) {\n+\t\tnew_whitespace = error_on_whitespace;\n+\t\tsquelch_whitespace_errors = 0;\n+\t\treturn;\n+\t}\n+\tif (!strcmp(option, \"strip\")) {\n+\t\tnew_whitespace = strip_whitespace;\n+\t\treturn;\n+\t}\n+\tdie(\"unrecognized whitespace option '%s'\", option);\n+}\n+\n /*\n * For \"diff-stat\" like behaviour, we keep track of the biggest change\n * we've seen, and the longest filename. That allows us to do simple\n@@ -832,7 +858,7 @@ static int parse_fragment(char *line, un\n \t\t\t * That is, an addition of an empty line would check\n \t\t\t * the '+' here. Sneaky...\n \t\t\t */\n-\t\t\tif ((new_whitespace != nowarn) &&\n+\t\t\tif ((new_whitespace != nowarn_whitespace) &&\n \t\t\t isspace(line[len-2])) {\n \t\t\t\twhitespace_error++;\n \t\t\t\tif (squelch_whitespace_errors &&\n@@ -1129,7 +1155,7 @@ static int apply_line(char *output, cons\n \t * patch[plen] is '\\n'.\n \t */\n \tint add_nl_to_tail = 0;\n-\tif ((new_whitespace == strip_and_apply) &&\n+\tif ((new_whitespace == strip_whitespace) &&\n \t 1 < plen && isspace(patch[plen-1])) {\n \t\tif (patch[plen] == '\\n')\n \t\t\tadd_nl_to_tail = 1;\n@@ -1824,10 +1850,21 @@ static int apply_patch(int fd, const cha\n \treturn 0;\n }\n \n+static int git_apply_config(const char *var, const char *value)\n+{\n+\tif (!strcmp(var, \"apply.whitespace\")) {\n+\t\tapply_default_whitespace = strdup(value);\n+\t\treturn 0;\n+\t}\n+\treturn git_default_config(var, value);\n+}\n+\n+\n int main(int argc, char **argv)\n {\n \tint i;\n \tint read_stdin = 1;\n+\tconst char *whitespace_option = NULL;\n \n \tfor (i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n@@ -1895,30 +1932,17 @@ int main(int argc, char **argv)\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strncmp(arg, \"--whitespace=\", 13)) {\n-\t\t\tif (!strcmp(arg+13, \"warn\")) {\n-\t\t\t\tnew_whitespace = warn_on_whitespace;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg+13, \"error\")) {\n-\t\t\t\tnew_whitespace = error_on_whitespace;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg+13, \"error-all\")) {\n-\t\t\t\tnew_whitespace = error_on_whitespace;\n-\t\t\t\tsquelch_whitespace_errors = 0;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tif (!strcmp(arg+13, \"strip\")) {\n-\t\t\t\tnew_whitespace = strip_and_apply;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tdie(\"unrecognized whitespace option '%s'\", arg+13);\n+\t\t\twhitespace_option = arg + 13;\n+\t\t\tparse_whitespace_option(arg + 13);\n+\t\t\tcontinue;\n \t\t}\n \n \t\tif (check_index && prefix_length < 0) {\n \t\t\tprefix = setup_git_directory();\n \t\t\tprefix_length = prefix ? strlen(prefix) : 0;\n-\t\t\tgit_config(git_default_config);\n+\t\t\tgit_config(git_apply_config);\n+\t\t\tif (!whitespace_option && apply_default_whitespace)\n+\t\t\t\tparse_whitespace_option(apply_default_whitespace);\n \t\t}\n \t\tif (0 < prefix_length)\n \t\t\targ = prefix_filename(prefix, prefix_length, arg);\ndiff --git a/cache.h b/cache.h\nindex 58eec00..0d3b244 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -161,11 +161,13 @@ extern int hold_index_file_for_update(st\n extern int commit_index_file(struct cache_file *);\n extern void rollback_index_file(struct cache_file *);\n \n+/* Environment bits from configuration mechanism */\n extern int trust_executable_bit;\n extern int assume_unchanged;\n extern int only_use_symrefs;\n extern int diff_rename_limit_default;\n extern int shared_repository;\n+extern const char *apply_default_whitespace;\n \n #define GIT_REPO_VERSION 0\n extern int repository_format_version;\ndiff --git a/environment.c b/environment.c\nindex 251e53c..16c08f0 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -17,6 +17,7 @@ int only_use_symrefs = 0;\n int repository_format_version = 0;\n char git_commit_encoding[MAX_ENCODING_LENGTH] = \"utf-8\";\n int shared_repository = 0;\n+const char *apply_default_whitespace = NULL;\n \n static char *git_dir, *git_object_dir, *git_index_file, *git_refs_dir,\n \t*git_graft_file;\n-- \n1.2.3.gbfea\n"},{"id":"16874","messageId":"7v4q2kuvxk.fsf@assigned-by-dhcp.cox.net","threadId":"3464","inReplyTo":"7vhd6kxuea.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH 3/3] git-apply --whitespace=nowarn","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-02-28T01:13:59Z","receivedAt":"2006-02-28T01:13:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andrew insists --whitespace=warn should be the default, and I\ntend to agree. This introduces --whitespace=warn, so if your\nproject policy is more lenient, you can squelch them by having\napply.whitespace=nowarn in your configuration file.\n\nSigned-off-by: Junio C Hamano \n\n---\n * Not in \"next\" but will be shortly.\n\n apply.c | 6 +++++-\n 1 files changed, 5 insertions(+), 1 deletions(-)\n\n114b085dd7b82c3ca74760c896e86c425127cf76\ndiff --git a/apply.c b/apply.c\nindex a5cdd8e..d5cb5b1 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -39,7 +39,7 @@ static enum whitespace_eol {\n \twarn_on_whitespace,\n \terror_on_whitespace,\n \tstrip_whitespace,\n-} new_whitespace = nowarn_whitespace;\n+} new_whitespace = warn_on_whitespace;\n static int whitespace_error = 0;\n static int squelch_whitespace_errors = 5;\n static int applied_after_stripping = 0;\n@@ -55,6 +55,10 @@ static void parse_whitespace_option(cons\n \t\tnew_whitespace = warn_on_whitespace;\n \t\treturn;\n \t}\n+\tif (!strcmp(option, \"nowarn\")) {\n+\t\tnew_whitespace = nowarn_whitespace;\n+\t\treturn;\n+\t}\n \tif (!strcmp(option, \"error\")) {\n \t\tnew_whitespace = error_on_whitespace;\n \t\treturn;\n-- \n1.2.3.gbfea\n"},{"id":"16879","messageId":"4403C303.70602@gmail.com","threadId":"3464","inReplyTo":"7v4q2kuvxk.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 3/3] git-apply --whitespace=nowarn","fromName":"A Large Angry SCM","fromEmail":"gitzilla@gmail.com","sentAt":"2006-02-28T03:26:59Z","receivedAt":"2006-02-28T03:26:59Z","isPatch":true,"sender":{"key":"gitzilla@gmail.com","avatar":"https://gravatar.com/avatar/354625c442439908ff3dd99757dee330e29e9df7847472384faf7a00add247fb?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> Andrew insists --whitespace=warn should be the default, and I\n> tend to agree. This introduces --whitespace=warn, so if your\n> project policy is more lenient, you can squelch them by having\n> apply.whitespace=nowarn in your configuration file.\n> \n> Signed-off-by: Junio C Hamano \n\nI think this is wrong. The default policy of, non-security, tools SHOULD \n be the least restrictive and most flexible.\n"},{"id":"16882","messageId":"7vy7zwt6hq.fsf@assigned-by-dhcp.cox.net","threadId":"3464","inReplyTo":"4403C303.70602@gmail.com","subject":"Re: [PATCH 3/3] git-apply --whitespace=nowarn","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-02-28T05:08:49Z","receivedAt":"2006-02-28T05:08:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"A Large Angry SCM writes:\n\n> Junio C Hamano wrote:\n>> Andrew insists --whitespace=warn should be the default, and I\n>> tend to agree. This introduces --whitespace=warn, so if your\n>> project policy is more lenient, you can squelch them by having\n>> apply.whitespace=nowarn in your configuration file.\n>> Signed-off-by: Junio C Hamano \n>\n> I think this is wrong. The default policy of, non-security, tools\n> SHOULD be the least restrictive and most flexible.\n\nThe reason is that --whitespace=warn does not refuse to apply\nbut spits out some warning messages. Admittedly, going from\nzero, any amount of new warning message makes it infinitely more\nchatty, but then people can squelch it by having the\nconfiguration option \"apply.whitespace = nowarn\".\n"},{"id":"16895","messageId":"7vu0ajonfu.fsf_-_@assigned-by-dhcp.cox.net","threadId":"3464","inReplyTo":"20060227153702.375db751.akpm@osdl.org","subject":"[PATCH] git-apply: war on whitespace -- finishing touches.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-02-28T09:13:57Z","receivedAt":"2006-02-28T09:13:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andrew Morton writes:\n\n> I'd suggest that git have options to a) generate trailing-whitespace\n> warnings, b) generate trailing-whitespace errors and c) strip trailing\n> whitespace while applying. And that the as-shipped default be a).\n\nI've done this and will be pushing it out to \"master\" branch on\nmy next git day (Wednesday, west coast US); \"maint\" branch will\nhave the same for v1.2.4 sometime by the end of this week.\n\nThere is one thing. By making --whitespace=warn the default,\nthe diffstat output people would see after \"git pull\" would also\nshow the warning message. I personally do not think this is a\nproblem (you will know how dirty a tree you are merging into\nyour tree), but it might not be a bad idea to explicitly squelch\nit by making it not to warn when we are not applying.\n\n-- >8 --\n\nThis changes the default --whitespace policy to nowarn when we\nare only getting --stat, --summary etc. IOW when not applying\nthe patch. When applying the patch, the default is warn (spit\nout warning message but apply the patch).\n\nSigned-off-by: Junio C Hamano \n\n---\n\n apply.c | 11 +++++++++++\n 1 files changed, 11 insertions(+), 0 deletions(-)\n\nf21d6726150ec4219e94ea605f27a4cd58eb3d99\ndiff --git a/apply.c b/apply.c\nindex c4ff418..9deb206 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -75,6 +75,15 @@ static void parse_whitespace_option(cons\n \tdie(\"unrecognized whitespace option '%s'\", option);\n }\n \n+static void set_default_whitespace_mode(const char *whitespace_option)\n+{\n+\tif (!whitespace_option && !apply_default_whitespace) {\n+\t\tnew_whitespace = (apply\n+\t\t\t\t ? warn_on_whitespace\n+\t\t\t\t : nowarn_whitespace);\n+\t}\n+}\n+\n /*\n * For \"diff-stat\" like behaviour, we keep track of the biggest change\n * we've seen, and the longest filename. That allows us to do simple\n@@ -1955,9 +1964,11 @@ int main(int argc, char **argv)\n \t\tif (fd < 0)\n \t\t\tusage(apply_usage);\n \t\tread_stdin = 0;\n+\t\tset_default_whitespace_mode(whitespace_option);\n \t\tapply_patch(fd, arg);\n \t\tclose(fd);\n \t}\n+\tset_default_whitespace_mode(whitespace_option);\n \tif (read_stdin)\n \t\tapply_patch(0, \"\");\n \tif (whitespace_error) {\n-- \n1.2.3.g1da2\n"},{"id":"16896","messageId":"440414D6.8050407@op5.se","threadId":"3464","inReplyTo":"7vacccuvxz.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/3] apply --whitespace: configuration option.","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2006-02-28T09:16:06Z","receivedAt":"2006-02-28T09:16:06Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> The new configuration option apply.whitespace can take one of\n> \"warn\", \"error\", \"error-all\", or \"strip\". When git-apply is run\n> to apply the patch to the index, they are used as the default\n> value if there is no command line --whitespace option.\n> \n\nI would think \"warn-all\" would be the logical thing, since \"error\" \neither breaks out early or prints all warnings before denying the patch \nanyway.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"16897","messageId":"7vzmkbn7qx.fsf@assigned-by-dhcp.cox.net","threadId":"3464","inReplyTo":"440414D6.8050407@op5.se","subject":"Re: [PATCH 2/3] apply --whitespace: configuration option.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-02-28T09:38:14Z","receivedAt":"2006-02-28T09:38:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andreas Ericsson writes:\n\n> Junio C Hamano wrote:\n>> The new configuration option apply.whitespace can take one of\n>> \"warn\", \"error\", \"error-all\", or \"strip\". When git-apply is run\n>> to apply the patch to the index, they are used as the default\n>> value if there is no command line --whitespace option.\n>\n> I would think \"warn-all\" would be the logical thing, since \"error\"\n> either breaks out early or prints all warnings before denying the\n> patch anyway.\n\nActually there is some thinking behind why I did not do warn-all.\nI did consider it at first but rejected.\n\n * If you are a busy top echelon person but cares about tree\n cleanliness, --whitespace=error is good enough. The patch is\n rejected on WS basis whether it introduces one such trailing\n WS or hundreds. The patch is returned to the submitter and\n the tree remains clean.\n\n * --whitespace=warn-all, if existed, would apply the patch\n _anyway_, so if you notice you got warnings, and if that\n bothers you enough that you would want to do something about\n it, you will have to rewind the HEAD, fix up .dotest/patch\n and reapply. This means you are willing to clean up other\n peoples' patches.\n\n * But if you are that kind of person, --whitespace=error-all is\n a better choice for you. Your tree stays clean and you do\n not have to rewind. Instead, you get all the errors you can\n go through with your editor (e.g. Emacs users can use C-x `;\n I hope vim users have similar macros) and fix things.\n\n * --whitespace=warn would show some, but not all, so that you\n can continue while making a mental note to scold the patch\n submitter to be careful the next time. You chose \"warn\" to\n apply the patch anyway, so there is no point showing the full\n extent of damage -- the damage is already done to your tree.\n\n * --whitespace=strip is for people who care about cleanliness,\n who wants to be nice to the submitters, but not nice enough\n to educate them. They do not want to fix things by hand.\n Instead they have the tool to do the fixing for them.\n\nThe last one is somewhat risky, and the output may need to be\nexamined carefully depending on the contents (e.g. programming\nlanguage) the project is dealing with.\n"},{"id":"16898","messageId":"44041C05.3030103@op5.se","threadId":"3464","inReplyTo":"7vzmkbn7qx.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/3] apply --whitespace: configuration option.","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2006-02-28T09:46:45Z","receivedAt":"2006-02-28T09:46:45Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> Andreas Ericsson writes:\n> \n> \n>>Junio C Hamano wrote:\n>>\n>>>The new configuration option apply.whitespace can take one of\n>>>\"warn\", \"error\", \"error-all\", or \"strip\". When git-apply is run\n>>>to apply the patch to the index, they are used as the default\n>>>value if there is no command line --whitespace option.\n>>\n>>I would think \"warn-all\" would be the logical thing, since \"error\"\n>>either breaks out early or prints all warnings before denying the\n>>patch anyway.\n> \n> \n> Actually there is some thinking behind why I did not do warn-all.\n> I did consider it at first but rejected.\n> \n> * If you are a busy top echelon person but cares about tree\n> cleanliness, --whitespace=error is good enough. The patch is\n> rejected on WS basis whether it introduces one such trailing\n> WS or hundreds. The patch is returned to the submitter and\n> the tree remains clean.\n> \n> * --whitespace=warn-all, if existed, would apply the patch\n> _anyway_, so if you notice you got warnings, and if that\n> bothers you enough that you would want to do something about\n> it, you will have to rewind the HEAD, fix up .dotest/patch\n> and reapply. This means you are willing to clean up other\n> peoples' patches.\n> \n> * But if you are that kind of person, --whitespace=error-all is\n> a better choice for you. Your tree stays clean and you do\n> not have to rewind. Instead, you get all the errors you can\n> go through with your editor (e.g. Emacs users can use C-x `;\n> I hope vim users have similar macros) and fix things.\n> \n\nGood Thinking. Thanks for explaining.\n\n> \n> The last one is somewhat risky, and the output may need to be\n> examined carefully depending on the contents (e.g. programming\n> language) the project is dealing with.\n> \n> \n\necho Makefile >> .git/no-ws-strip\necho '*.[ch]' >> .git/ws-strip\n\nPerhaps not viable, and probably stupid as well. Mixed content repos \nwould likely just keep the 'warn' policy.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"}]}