{"thread":{"id":"41699","subject":"Tabs in commit messages - de-tabify option in strbuf_stripspace()?","startedAt":"2016-03-16T00:16:57Z","lastAt":"2016-03-20T13:29:48Z","messageCount":17,"participants":["Linus Torvalds","Stefan Beller","Randall S. Becker","Junio C Hamano","Jeff King","Marc Branchaud","Duy Nguyen","Matthieu Moy"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"280856","messageId":"CA+55aFzHMp4hiCp7+2Yxy=KNQ_rBru3RM-pghXUPtoxr_L+w2w@mail.gmail.com","threadId":"41699","inReplyTo":null,"subject":"Tabs in commit messages - de-tabify option in strbuf_stripspace()?","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2016-03-16T00:16:57Z","receivedAt":"2016-03-16T00:16:57Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"So I end up doing this manually when I notice, but I was wondering ig\nmaybe git could just have an option to \"git am\" and friends to\nde-tabify the commit message.\n\nIt's particularly noticeable when people line things up using tabs\n(for the kernel, it's often things like \"cpu1 does X, cpu2 does Y\"),\nand then when you do \"git log\" it looks like a unholy mess, because\nthe 4-char indentation of the log message ends up causing those things\nto not line up at all after all.\n\nThe natural thing to do would be to pass in a \"tab size\" parameter to\nstrbuf_stripspace(), and default it to 0 (for no change), but have\nsome way to let people say \"expand tabs to spaces at 8-character\ntab-stops\" or similar (but let people use different tab-stops if they\nwant).\n\nDo people hate that idea? I may not get around to it for a while (it's\nthe kernel merge window right now), but I can write the patch\neventually - I just wanted to do an RFC first.\n\n                Linus\n"},{"id":"280857","messageId":"CAGZ79kaGp==-On6HBf5X+jr+0d6mWmHQbsPSBQoRG0S=cqddQA@mail.gmail.com","threadId":"41699","inReplyTo":"CA+55aFzHMp4hiCp7+2Yxy=KNQ_rBru3RM-pghXUPtoxr_L+w2w@mail.gmail.com","subject":"Re: Tabs in commit messages - de-tabify option in strbuf_stripspace()?","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-03-16T00:23:32Z","receivedAt":"2016-03-16T00:23:32Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Mar 15, 2016 at 5:16 PM, Linus Torvalds\n<torvalds@linux-foundation.org> wrote:\n> Do people hate that idea? I may not get around to it for a while (it's\n> the kernel merge window right now), but I can write the patch\n> eventually - I just wanted to do an RFC first.\n\nCould you point at some example to better understand the problem?\n\nThanks,\nStefan\n\n>\n>                 Linus\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"},{"id":"280858","messageId":"009801d17f1b$a4c3e080$ee4ba180$@nexbridge.com","threadId":"41699","inReplyTo":"CA+55aFzHMp4hiCp7+2Yxy=KNQ_rBru3RM-pghXUPtoxr_L+w2w@mail.gmail.com","subject":"RE: Tabs in commit messages - de-tabify option in strbuf_stripspace()?","fromName":"Randall S. Becker","fromEmail":"rsbecker@nexbridge.com","sentAt":"2016-03-16T00:34:41Z","receivedAt":"2016-03-16T00:34:41Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On March 15, 2016 8:17 PM Linus Torvalds wrote:\n> So I end up doing this manually when I notice, but I was wondering ig maybe\n> git could just have an option to \"git am\" and friends to de-tabify the commit\n> message.\n> \n> It's particularly noticeable when people line things up using tabs (for the\n> kernel, it's often things like \"cpu1 does X, cpu2 does Y\"), and then when you\n> do \"git log\" it looks like a unholy mess, because the 4-char indentation of the\n> log message ends up causing those things to not line up at all after all.\n> \n> The natural thing to do would be to pass in a \"tab size\" parameter to\n> strbuf_stripspace(), and default it to 0 (for no change), but have some way to\n> let people say \"expand tabs to spaces at 8-character tab-stops\" or similar\n> (but let people use different tab-stops if they want).\n> \n> Do people hate that idea? I may not get around to it for a while (it's the\n> kernel merge window right now), but I can write the patch eventually - I just\n> wanted to do an RFC first.\n\nSpeaking partly as a consumer of the comments and partly as someone who generates the commits through APIs, I would ask that the commit tab handling semantic be more formalized than just tab size to strbuf_stripspace(). While it might seem a bit unfair to have to worry about non-git git clients, the detabbing can impact the other commit implementers (e.g., SourceTree, EGit, JGit, and the raft of process automation bits out there using JGit for cool stuff). Personally, I would prefer to have a normalized behaviour so that any bit of automation building a commit message would have a specific definition to go to (and hopefully comply with) in order to properly format the message for posterity and across all consumers. It might also be useful to have some ability to be presentation-compatible \n with legacy commits (done after this type of enhancement) so that a reasonable presentation can be done for those 8 year old commits that still have embedded tabs. Personally, I don't encourage tabs in commits myself and do see the value of this, but is this really restricted just to git am?\n\nJust my $0.02,\n\nRandall\n\n-- Brief whoami: NonStop&UNIX developer since approximately UNIX(421664400)/NonStop(211288444200000000)\n-- In my real life, I talk too much.\n"},{"id":"280859","messageId":"xmqq4mc76yji.fsf@gitster.mtv.corp.google.com","threadId":"41699","inReplyTo":"CA+55aFzHMp4hiCp7+2Yxy=KNQ_rBru3RM-pghXUPtoxr_L+w2w@mail.gmail.com","subject":"Re: Tabs in commit messages - de-tabify option in strbuf_stripspace()?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-16T00:45:21Z","receivedAt":"2016-03-16T00:45:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> Do people hate that idea? I may not get around to it for a while (it's\n> the kernel merge window right now), but I can write the patch\n> eventually - I just wanted to do an RFC first.\n\nWouldn't it be nicer to do this kind of thing at the output side?  A\nstupid way would be to have an option to indent the log text with a\ntab instead of 4-spaces, but a more sensible way would be to keep\nthe visual 4-space-indent and do the expand-tab for each line of\ntext.\n\nThat way, your viewing of existing commits that use 8-space HT to\nalign (and worse yet, mixture of 8-space HT and 8 spaces and assume\nthe end result would align in the output) would become more pleasant\nwithout you having to run filter-branch ;-)\n"},{"id":"280860","messageId":"CA+55aFw5AWz4m64Akz-xzuzbmqiWVpLQU_twoeWq3kkgfSjMWQ@mail.gmail.com","threadId":"41699","inReplyTo":"CAGZ79kaGp==-On6HBf5X+jr+0d6mWmHQbsPSBQoRG0S=cqddQA@mail.gmail.com","subject":"Re: Tabs in commit messages - de-tabify option in strbuf_stripspace()?","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2016-03-16T00:45:58Z","receivedAt":"2016-03-16T00:45:58Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Tue, Mar 15, 2016 at 5:23 PM, Stefan Beller <sbeller@google.com> wrote:\n>\n> Could you point at some example to better understand the problem?\n\nSo in the kernel repo, I just randomly looked for tabs that show this\nproblem, and take for example commit\nff9a9b4c4334b53b52ee9279f30bd5dd92ea9bdd.\n\nYou can see how the original lined up, by doing\n\n    git show --pretty=email ff9a9b4c4334\n\nbecause the email format doesn't indent the commit message. But if you do just\n\n    git show ff9a9b4c4334\n\nand get the usual indentation, you'll see things not line up at all.\n\nIn case you don't want to bother with the kernel repo, here's what it\nlooks like:\n\nemail format:\n\n--- snip snip 8< ---\nA microbenchmark calling an invalid syscall number 10 million\ntimes in a row speeds up an additional 30% over the numbers\nwith just the previous patches, for a total speedup of about\n40% over 4.4 and 4.5-rc1.\n\nRun times for the microbenchmark:\n\n 4.4                            3.8 seconds\n 4.5-rc1                        3.7 seconds\n 4.5-rc1 + first patch          3.3 seconds\n 4.5-rc1 + first 3 patches      3.1 seconds\n 4.5-rc1 + all patches          2.3 seconds\n\nA non-NOHZ_FULL cpu (not the housekeeping CPU):\n\n all kernels                    1.86 seconds\n--- snip snip 8< ---\n\nNormal \"git show\" format:\n\n--- snip snip 8< ---\n    A microbenchmark calling an invalid syscall number 10 million\n    times in a row speeds up an additional 30% over the numbers\n    with just the previous patches, for a total speedup of about\n    40% over 4.4 and 4.5-rc1.\n\n    Run times for the microbenchmark:\n\n     4.4                                3.8 seconds\n     4.5-rc1                    3.7 seconds\n     4.5-rc1 + first patch              3.3 seconds\n     4.5-rc1 + first 3 patches  3.1 seconds\n     4.5-rc1 + all patches              2.3 seconds\n\n    A non-NOHZ_FULL cpu (not the housekeeping CPU):\n\n     all kernels                        1.86 seconds\n--- snip snip 8< ---\n\nwhich hopefully clarifies.\n\nIn the above case, it really isn't very annoying. It's just slightly\nugly. In some other cases, it can get quite hard to see what's up, but\nthe ones that come through me I actually tend to try to edit, so many\nof them have been corrected.\n\nFor other examples (again, in the kernel), look at 19b2c30d3cce, or\n0dc8c730c98a.\n\n                Linus\n"},{"id":"280861","messageId":"CA+55aFyXXHNrJW56A_DKkmrmGpWxeUd6row_ja3bzqhs_yswhw@mail.gmail.com","threadId":"41699","inReplyTo":"xmqq4mc76yji.fsf@gitster.mtv.corp.google.com","subject":"Re: Tabs in commit messages - de-tabify option in strbuf_stripspace()?","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2016-03-16T00:48:46Z","receivedAt":"2016-03-16T00:48:46Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Tue, Mar 15, 2016 at 5:45 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Wouldn't it be nicer to do this kind of thing at the output side?  A\n> stupid way would be to have an option to indent the log text with a\n> tab instead of 4-spaces, but a more sensible way would be to keep\n> the visual 4-space-indent and do the expand-tab for each line of\n> text.\n\nYes, that would certainly work for me too. It's just the \"ascii boxes\ndon't line up\" that is problematic..\n\n(Also people including example source code etc, where the first tab\nlooks wildly different from the second one)\n\n            Linus\n"},{"id":"280863","messageId":"CAGZ79kZihaftwwmY23mZ_i4H6vv2Z9r=LC68M0MMD1o2h2Z4Sw@mail.gmail.com","threadId":"41699","inReplyTo":"CA+55aFyXXHNrJW56A_DKkmrmGpWxeUd6row_ja3bzqhs_yswhw@mail.gmail.com","subject":"Re: Tabs in commit messages - de-tabify option in strbuf_stripspace()?","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-03-16T01:00:17Z","receivedAt":"2016-03-16T01:00:17Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Mar 15, 2016 at 5:48 PM, Linus Torvalds\n<torvalds@linux-foundation.org> wrote:\n> On Tue, Mar 15, 2016 at 5:45 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>>\n>> Wouldn't it be nicer to do this kind of thing at the output side?  A\n>> stupid way would be to have an option to indent the log text with a\n>> tab instead of 4-spaces, but a more sensible way would be to keep\n>> the visual 4-space-indent and do the expand-tab for each line of\n>> text.\n>\n> Yes, that would certainly work for me too. It's just the \"ascii boxes\n> don't line up\" that is problematic..\n\nI would also rather side to correct the display side rather than the\napplying side. [I still want to send and apply patches with git written in\nthe white space language ;]\n\nInstead of converting to whitespaces in Git, we could make use of the\nset_tabs capability for ttys and setup the terminal for having tabs align\nto 12,+8,+8,+8... such that it looks like an indented terminal.\nThat way we would also preserve the tabs in case you'd want to\ncopy the message as is.\n\nThanks,\nStefan\n\n>\n> (Also people including example source code etc, where the first tab\n> looks wildly different from the second one)\n>\n>             Linus\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"},{"id":"280864","messageId":"CAGZ79kZtAm1M=9CGDGxPdecXEuNEQcbpQb3FNj9=Py0VE2UrKQ@mail.gmail.com","threadId":"41699","inReplyTo":"CAGZ79kZihaftwwmY23mZ_i4H6vv2Z9r=LC68M0MMD1o2h2Z4Sw@mail.gmail.com","subject":"Re: Tabs in commit messages - de-tabify option in strbuf_stripspace()?","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-03-16T01:02:03Z","receivedAt":"2016-03-16T01:02:03Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Mar 15, 2016 at 6:00 PM, Stefan Beller <sbeller@google.com> wrote:\n>\n> Instead of converting to whitespaces in Git, we could make use of the\n> set_tabs capability for ttys and setup the terminal for having tabs align\n> to 12,+8,+8,+8...\n\nOr rather read in the existing tabs configuration and shift it by a constant.\n"},{"id":"280870","messageId":"CA+55aFw8roOTTuFknzh3zRkCbgaMGmtxMRk-ctBdJ8Wsu041bA@mail.gmail.com","threadId":"41699","inReplyTo":"CA+55aFyXXHNrJW56A_DKkmrmGpWxeUd6row_ja3bzqhs_yswhw@mail.gmail.com","subject":"Re: Tabs in commit messages - de-tabify option in strbuf_stripspace()?","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2016-03-16T04:05:01Z","receivedAt":"2016-03-16T04:05:01Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Tue, Mar 15, 2016 at 5:48 PM, Linus Torvalds\n<torvalds@linux-foundation.org> wrote:\n> On Tue, Mar 15, 2016 at 5:45 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>>\n>> Wouldn't it be nicer to do this kind of thing at the output side?  A\n>> stupid way would be to have an option to indent the log text with a\n>> tab instead of 4-spaces, but a more sensible way would be to keep\n>> the visual 4-space-indent and do the expand-tab for each line of\n>> text.\n>\n> Yes, that would certainly work for me too. It's just the \"ascii boxes\n> don't line up\" that is problematic..\n\nOk, that actually turns out to be pretty easy.\n\nHere's a first try at it. It does tab expansion only for the cases\nthat indent the commit message, so for things like \"pretty=email\" it\nmakes no difference at all.\n\nHowever, it hardcodes the hardtab size to 8, and makes this all\nunconditional. That's how a vt100 terminal works, but fancer terminals\nmay allow you set actual tab-stops, and if you're in emacs you can\nprobably do just about anything)\n\nComments? This does make the kernel commit 0dc8c730c98a look fine, so\nit would make me happy.\n\nI can write a commit log etc if people think this is ok, but right now\nthis is just a silly raw patch in the attachement..\n\n               Linus\n\n\n pretty.c | 20 +++++++++++++++++++-\n 1 file changed, 19 insertions(+), 1 deletion(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex 92b2870a7eab..dcd6105d1eb2 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1652,8 +1652,26 @@ void pp_remainder(struct pretty_print_context *pp,\n \t\tfirst = 0;\n \n \t\tstrbuf_grow(sb, linelen + indent + 20);\n-\t\tif (indent)\n+\t\tif (indent) {\n+\t\t\tint i, last = 0, pos = 0;\n+\n \t\t\tstrbuf_addchars(sb, ' ', indent);\n+\t\t\tfor (i = 0; i < linelen; i++) {\n+\t\t\t\tint expand;\n+\t\t\t\tif (line[i] != '\\t')\n+\t\t\t\t\tcontinue;\n+\t\t\t\tstrbuf_add(sb, line+last, i-last);\n+\t\t\t\tpos += i-last;\n+\t\t\t\texpand = (pos + 8) & ~7;\n+\t\t\t\tstrbuf_addchars(sb, ' ', expand - pos);\n+\t\t\t\tpos = expand;\n+\t\t\t\tlast = i+1;\n+\t\t\t}\n+\n+\t\t\t// Handle the tail non-tab content\n+\t\t\tline += last;\n+\t\t\tlinelen -= last;\n+\t\t}\n \t\tstrbuf_add(sb, line, linelen);\n \t\tstrbuf_addch(sb, '\\n');\n \t}\n"},{"id":"280871","messageId":"xmqqzitz58si.fsf@gitster.mtv.corp.google.com","threadId":"41699","inReplyTo":"CA+55aFw8roOTTuFknzh3zRkCbgaMGmtxMRk-ctBdJ8Wsu041bA@mail.gmail.com","subject":"Re: Tabs in commit messages - de-tabify option in strbuf_stripspace()?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-16T04:46:53Z","receivedAt":"2016-03-16T04:46:53Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> Here's a first try at it. It does tab expansion only for the cases\n> that indent the commit message, so for things like \"pretty=email\" it\n> makes no difference at all.\n\nIt also ignores that byte counts of non-HT bytes may not necessarily\nmatch display columns.  There is utf8_width() function exported from\nutf8.c for that purpose.\n\nI think it is fine to make it default for the pretty=medium, but it\nwould be nicer to introduce a new command line option --no-untabify\nto optionally disable the expansion.\n\n>  pretty.c | 20 +++++++++++++++++++-\n>  1 file changed, 19 insertions(+), 1 deletion(-)\n>\n> diff --git a/pretty.c b/pretty.c\n> index 92b2870a7eab..dcd6105d1eb2 100644\n> --- a/pretty.c\n> +++ b/pretty.c\n> @@ -1652,8 +1652,26 @@ void pp_remainder(struct pretty_print_context *pp,\n>  \t\tfirst = 0;\n>  \n>  \t\tstrbuf_grow(sb, linelen + indent + 20);\n> -\t\tif (indent)\n> +\t\tif (indent) {\n> +\t\t\tint i, last = 0, pos = 0;\n> +\n>  \t\t\tstrbuf_addchars(sb, ' ', indent);\n> +\t\t\tfor (i = 0; i < linelen; i++) {\n> +\t\t\t\tint expand;\n> +\t\t\t\tif (line[i] != '\\t')\n> +\t\t\t\t\tcontinue;\n> +\t\t\t\tstrbuf_add(sb, line+last, i-last);\n> +\t\t\t\tpos += i-last;\n> +\t\t\t\texpand = (pos + 8) & ~7;\n> +\t\t\t\tstrbuf_addchars(sb, ' ', expand - pos);\n> +\t\t\t\tpos = expand;\n> +\t\t\t\tlast = i+1;\n> +\t\t\t}\n> +\n> +\t\t\t// Handle the tail non-tab content\n> +\t\t\tline += last;\n> +\t\t\tlinelen -= last;\n> +\t\t}\n>  \t\tstrbuf_add(sb, line, linelen);\n>  \t\tstrbuf_addch(sb, '\\n');\n>  \t}\n"},{"id":"280873","messageId":"CA+55aFyFHmmoHMgRkuBqNUho=tiJ=VwxHWzcGw3pRjr+aGS7ZQ@mail.gmail.com","threadId":"41699","inReplyTo":"xmqqzitz58si.fsf@gitster.mtv.corp.google.com","subject":"Re: Tabs in commit messages - de-tabify option in strbuf_stripspace()?","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2016-03-16T04:57:16Z","receivedAt":"2016-03-16T04:57:16Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Tue, Mar 15, 2016 at 9:46 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> It also ignores that byte counts of non-HT bytes may not necessarily\n> match display columns.  There is utf8_width() function exported from\n> utf8.c for that purpose.\n\nHmm. I did that to now make it horribly slower. Doing the\nper-character widths really does horrible things for performance.\n\nThat said, I think I can make it do the fast thing for lines that have\nno tabs at all, which is the big bulk of it. So it would do the width\ncalculations only in the rare \"yes, I found a tab\" case.\n\nI already wrote it in a way that non-tab lines end up just looping\nover the line looking for a tab and then fall back to the old code.\n\nI might just have to make that behavior a bit more explicit. Let me\nstare at it a bit, but it won't happen until tomorrow, I think.\n\n                 Linus\n"},{"id":"280875","messageId":"20160316051748.GA3524@sigill.intra.peff.net","threadId":"41699","inReplyTo":"CA+55aFyFHmmoHMgRkuBqNUho=tiJ=VwxHWzcGw3pRjr+aGS7ZQ@mail.gmail.com","subject":"Re: Tabs in commit messages - de-tabify option in strbuf_stripspace()?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-16T05:17:49Z","receivedAt":"2016-03-16T05:17:49Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 15, 2016 at 09:57:16PM -0700, Linus Torvalds wrote:\n\n> On Tue, Mar 15, 2016 at 9:46 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> >\n> > It also ignores that byte counts of non-HT bytes may not necessarily\n> > match display columns.  There is utf8_width() function exported from\n> > utf8.c for that purpose.\n> \n> Hmm. I did that to now make it horribly slower. Doing the\n> per-character widths really does horrible things for performance.\n> \n> That said, I think I can make it do the fast thing for lines that have\n> no tabs at all, which is the big bulk of it. So it would do the width\n> calculations only in the rare \"yes, I found a tab\" case.\n> \n> I already wrote it in a way that non-tab lines end up just looping\n> over the line looking for a tab and then fall back to the old code.\n> \n> I might just have to make that behavior a bit more explicit. Let me\n> stare at it a bit, but it won't happen until tomorrow, I think.\n\nI wondered about performance when reading yours, and measured a straight\n\"git log >/dev/null\" on the kernel at about 3% slower (best-of-five and\naverage). We can get most of that back by sticking a\n\n    memchr(line, '\\t', linelen);\n\nin front and skipping the loop if it returns NULL. You can also\nimplement your loop in terms of memchr() calls to skip to the next tab,\nbut I don't know if it's worth it. It's really only worth spending time\noptimizing the non-tab case (and even then, only worth it for trivial\nthings like \"use the already optimized-as-hell memchr\").\n\n-Peff\n"},{"id":"280903","messageId":"56E96D61.6060007@xiplink.com","threadId":"41699","inReplyTo":"CAGZ79kZtAm1M=9CGDGxPdecXEuNEQcbpQb3FNj9=Py0VE2UrKQ@mail.gmail.com","subject":"Re: Tabs in commit messages - de-tabify option in strbuf_stripspace()?","fromName":"Marc Branchaud","fromEmail":"marcnarc@xiplink.com","sentAt":"2016-03-16T14:27:45Z","receivedAt":"2016-03-16T14:27:45Z","isPatch":false,"sender":{"key":"marcnarc@xiplink.com","avatar":"https://avatars.githubusercontent.com/u/14980203?v=4"},"body":"On 16-03-15 09:02 PM, Stefan Beller wrote:\n> On Tue, Mar 15, 2016 at 6:00 PM, Stefan Beller <sbeller@google.com> wrote:\n>>\n>> Instead of converting to whitespaces in Git, we could make use of the\n>> set_tabs capability for ttys and setup the terminal for having tabs align\n>> to 12,+8,+8,+8...\n> \n> Or rather read in the existing tabs configuration and shift it by a constant.\n\nCould this also help with diff output, where the leading + or - mars the\nindentation in a similar way?\n\nThat opens a bit of a deeper problem, because not all the files in a single\nrepo necessarily use the same tab size.\n\n\t\tM.\n"},{"id":"280906","messageId":"CACsJy8B08c5ydNL0e8fvaG7_4UEFqsCR4a0beuvDmYEmPDr78Q@mail.gmail.com","threadId":"41699","inReplyTo":"56E96D61.6060007@xiplink.com","subject":"Re: Tabs in commit messages - de-tabify option in strbuf_stripspace()?","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-03-16T14:50:11Z","receivedAt":"2016-03-16T14:50:11Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Mar 16, 2016 at 9:27 PM, Marc Branchaud <marcnarc@xiplink.com> wrote:\n> On 16-03-15 09:02 PM, Stefan Beller wrote:\n>> On Tue, Mar 15, 2016 at 6:00 PM, Stefan Beller <sbeller@google.com> wrote:\n>>>\n>>> Instead of converting to whitespaces in Git, we could make use of the\n>>> set_tabs capability for ttys and setup the terminal for having tabs align\n>>> to 12,+8,+8,+8...\n>>\n>> Or rather read in the existing tabs configuration and shift it by a constant.\n>\n> Could this also help with diff output, where the leading + or - mars the\n> indentation in a similar way?\n\nYes, please! Even if we only activate this when stdout is tty. Much\nbetter output.\n\n> That opens a bit of a deeper problem, because not all the files in a single\n> repo necessarily use the same tab size.\n\nMaybe we can pass --tab-width=xx to git?\n-- \nDuy\n"},{"id":"280911","messageId":"CA+55aFwFsBKZZeqhBDf_YXG6vrSrvfFVCsRc0mkFUiDS9Rd+QA@mail.gmail.com","threadId":"41699","inReplyTo":"56E96D61.6060007@xiplink.com","subject":"Re: Tabs in commit messages - de-tabify option in strbuf_stripspace()?","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2016-03-16T16:15:39Z","receivedAt":"2016-03-16T16:15:39Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Wed, Mar 16, 2016 at 7:27 AM, Marc Branchaud <marcnarc@xiplink.com> wrote:\n>\n> Could this also help with diff output, where the leading + or - mars the\n> indentation in a similar way?\n\nI don't think that's a good idea at least by default, since then it\nwill break things like running diff in emacs capture mode.\n\nSo even if you're in a terminal, you can't assume that you can munge\nthe output too much.\n\nOf course, if colorization is on, you might as well pretty-print the\ndiff by indenting things properly too, since the end result isn't\ngoing to be used as a _diff_.\n\n              Linus\n"},{"id":"280918","messageId":"xmqqfuvq5pz7.fsf@gitster.mtv.corp.google.com","threadId":"41699","inReplyTo":"CA+55aFwFsBKZZeqhBDf_YXG6vrSrvfFVCsRc0mkFUiDS9Rd+QA@mail.gmail.com","subject":"Re: Tabs in commit messages - de-tabify option in strbuf_stripspace()?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-16T16:47:56Z","receivedAt":"2016-03-16T16:47:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> On Wed, Mar 16, 2016 at 7:27 AM, Marc Branchaud <marcnarc@xiplink.com> wrote:\n>>\n>> Could this also help with diff output, where the leading + or - mars the\n>> indentation in a similar way?\n>\n> I don't think that's a good idea at least by default, since then it\n> will break things like running diff in emacs capture mode.\n>\n> So even if you're in a terminal, you can't assume that you can munge\n> the output too much.\n>\n> Of course, if colorization is on, you might as well pretty-print the\n> diff by indenting things properly too, since the end result isn't\n> going to be used as a _diff_.\n\nI agree that I will never use such an end result as a diff, but I\nmay still be tempted to cut-and-paste individual lines after '^+'\nwhen resurrecting a WIP topic that does not rebase very well, so I\nagree with you that the output shouldn't be munged by default even\nthough I think it is OK to have an option..\n"},{"id":"281277","messageId":"vpqwpoxcm5v.fsf@anie.imag.fr","threadId":"41699","inReplyTo":"56E96D61.6060007@xiplink.com","subject":"Re: Tabs in commit messages - de-tabify option in strbuf_stripspace()?","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-03-20T13:29:48Z","receivedAt":"2016-03-20T13:29:48Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Marc Branchaud <marcnarc@xiplink.com> writes:\n\n> On 16-03-15 09:02 PM, Stefan Beller wrote:\n>> On Tue, Mar 15, 2016 at 6:00 PM, Stefan Beller <sbeller@google.com> wrote:\n>>>\n>>> Instead of converting to whitespaces in Git, we could make use of the\n>>> set_tabs capability for ttys and setup the terminal for having tabs align\n>>> to 12,+8,+8,+8...\n>> \n>> Or rather read in the existing tabs configuration and shift it by a constant.\n>\n> Could this also help with diff output, where the leading + or - mars the\n> indentation in a similar way?\n\nHardly: the same pager would be used to display the commit message (with\na 4-space prefix) and the diff (with a 1-char +/- prefix). So the\noffsets in these two places would need to be different, and I don't\nthink we can change that on the fly.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"}]}