{"thread":{"id":"11026","subject":"[PATCH] Use --no-color option on git log commands.","startedAt":"2007-11-26T22:04:28Z","lastAt":"2007-12-01T06:10:35Z","messageCount":10,"participants":["Pascal Obry","Junio C Hamano","Johannes Schindelin","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"61002","messageId":"474B42EC.1000408@wanadoo.fr","threadId":"11026","inReplyTo":null,"subject":"[PATCH] Use --no-color option on git log commands.","fromName":"Pascal Obry","fromEmail":"pascal.obry@wanadoo.fr","sentAt":"2007-11-26T22:04:28Z","receivedAt":"2007-11-26T22:04:28Z","isPatch":true,"sender":{"key":"pascal@obry.net","avatar":"https://avatars.githubusercontent.com/u/467069?v=4"},"body":"\nWhen colors are activated on the repository the git log output\nwill contain control characters to set/reset the colors. This\nmakes list_stash() fails as the sed regular expression does not\nmatch the color control characters. Also use --no-color when\ncomputing the head on create_stash() procedure.\n---\n git-stash.sh | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 534eb16..cde9767 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -37,7 +37,7 @@ create_stash () {\n # state of the base commit\n if b_commit=$(git rev-parse --verify HEAD)\n then\n- head=$(git log --abbrev-commit --pretty=oneline -n 1 HEAD)\n+ head=$(git log --no-color --abbrev-commit\n--pretty=oneline -n 1 HEAD)\n else\n die \"You do not have the initial commit yet\"\n fi\n@@ -108,7 +108,7 @@ have_stash () {\n\n list_stash () {\n have_stash || return 0\n- git log --pretty=oneline -g \"$@\" $ref_stash |\n+ git log --no-color --pretty=oneline -g \"$@\" $ref_stash |\n sed -n -e 's/^[.0-9a-f]* refs\\///p'\n }\n\n--\n1.5.3.6.959.g1ab5\n\n-- \n\n--|------------------------------------------------------\n--| Pascal Obry Team-Ada Member\n--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE\n--|------------------------------------------------------\n--| http://www.obry.net\n--| \"The best way to travel is by means of imagination\"\n--|\n--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595\n"},{"id":"61003","messageId":"7vr6icej23.fsf@gitster.siamese.dyndns.org","threadId":"11026","inReplyTo":"474B42EC.1000408@wanadoo.fr","subject":"Re: [PATCH] Use --no-color option on git log commands.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-26T22:30:28Z","receivedAt":"2007-11-26T22:30:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pascal Obry writes:\n\n> When colors are activated on the repository the git log output\n> will contain control characters to set/reset the colors.\n\nThe patch is good as belt-and-suspender, thanks.\n\nBut I suspect that we should make 'true' to mean 'auto' someday in\ngit_config_colorbool(). Crazy people can set 'always' if they really\nwanted to, but most normal people would not want color unless the output\ngoes to the terminal, I would think.\n\nSomething like this, perhaps...\n\n---\n color.c | 25 ++++++++++++-------------\n 1 files changed, 12 insertions(+), 13 deletions(-)\n\ndiff --git a/color.c b/color.c\nindex 09d82ee..060d3cf 100644\n--- a/color.c\n+++ b/color.c\n@@ -118,21 +118,24 @@ bad:\n \n int git_config_colorbool(const char *var, const char *value)\n {\n-\tif (!value)\n-\t\treturn 1;\n-\tif (!strcasecmp(value, \"auto\")) {\n-\t\tif (isatty(1) || (pager_in_use && pager_use_color)) {\n-\t\t\tchar *term = getenv(\"TERM\");\n-\t\t\tif (term && strcmp(term, \"dumb\"))\n-\t\t\t\treturn 1;\n-\t\t}\n-\t\treturn 0;\n-\t}\n-\tif (!strcasecmp(value, \"never\"))\n- \t\treturn 0;\n-\tif (!strcasecmp(value, \"always\"))\n-\t\treturn 1;\n-\treturn git_config_bool(var, value);\n+\tif (value) {\n+\t\tif (!strcasecmp(value, \"never\"))\n+\t\t\treturn 0;\n+\t\tif (!strcasecmp(value, \"always\"))\n+\t\t\treturn 1;\n+\t\tif (!strcasecmp(value, \"auto\"))\n+\t\t\tgoto auto;\n+ \t}\n+\tif (!git_config_bool(var, value))\n+ \t\treturn 0;\n+auto:\n+\t/* any normal truth value defaults to 'auto' */\n+\tif (isatty(1) || (pager_in_use && pager_use_color)) {\n+\t\tchar *term = getenv(\"TERM\");\n+\t\tif (term && strcmp(term, \"dumb\"))\n+\t\t\treturn 1;\n+\t}\n+\treturn 0;\n }\n \n static int color_vprintf(const char *color, const char *fmt,\n"},{"id":"61114","messageId":"474C60FA.4040302@wanadoo.fr","threadId":"11026","inReplyTo":"7vr6icej23.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Use --no-color option on git log commands.","fromName":"Pascal Obry","fromEmail":"pascal.obry@wanadoo.fr","sentAt":"2007-11-27T18:24:58Z","receivedAt":"2007-11-27T18:24:58Z","isPatch":true,"sender":{"key":"pascal@obry.net","avatar":"https://avatars.githubusercontent.com/u/467069?v=4"},"body":"Junio C Hamano a écrit :\n> The patch is good as belt-and-suspender, thanks.\n\nOk.\n\n> But I suspect that we should make 'true' to mean 'auto' someday in\n> git_config_colorbool(). Crazy people can set 'always' if they really\n> wanted to, but most normal people would not want color unless the output\n> goes to the terminal, I would think.\n\nI definitely agree. I add it set to true, using auto instead I do not\nhave the problem. Anyway I still think that it is good to apply my patch\nto completely avoid such issues.\n\nPascal.\n\n-- \n\n--|------------------------------------------------------\n--| Pascal Obry Team-Ada Member\n--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE\n--|------------------------------------------------------\n--| http://www.obry.net\n--| \"The best way to travel is by means of imagination\"\n--|\n--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595\n"},{"id":"61196","messageId":"7vr6ib9dwx.fsf@gitster.siamese.dyndns.org","threadId":"11026","inReplyTo":"474C60FA.4040302@wanadoo.fr","subject":"Re: [PATCH] Use --no-color option on git log commands.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-28T04:45:02Z","receivedAt":"2007-11-28T04:45:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pascal Obry writes:\n\n> Junio C Hamano a écrit :\n>> The patch is good as belt-and-suspender, thanks.\n>\n> Ok.\n>\n>> But I suspect that we should make 'true' to mean 'auto' someday in\n>> git_config_colorbool(). Crazy people can set 'always' if they really\n>> wanted to, but most normal people would not want color unless the output\n>> goes to the terminal, I would think.\n>\n> I definitely agree. I add it set to true, using auto instead I do not\n> have the problem. Anyway I still think that it is good to apply my patch\n> to completely avoid such issues.\n\nYes, that is what I said.\n\nExcept that the patch is severely whitespace damaged, and the message\nlack a sign-off.\n\nI fixed them up by hand, so no need to resend.\n"},{"id":"61178","messageId":"7vd4tuakzj.fsf_-_@gitster.siamese.dyndns.org","threadId":"11026","inReplyTo":"7vr6icej23.fsf@gitster.siamese.dyndns.org","subject":"[PATCH/RFC] \"color.diff = true\" is not \"always\" anymore.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-28T07:26:56Z","receivedAt":"2007-11-28T07:26:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Too many people got burned by setting color.diff and color.status to\ntrue when they really should have set it to \"auto\".\n\nThis makes only \"always\" to do the unconditional colorization, and\nchange the meaning of \"true\" to the same as \"auto\": colorize only when\nwe are talking to a terminal.\n\nSigned-off-by: Junio C Hamano \n---\n\n * This is definitely a backward incompatible change, but I think it is\n only in a good way. Are there people who have \"color.* = true\" and\n do mean it? If we do this, they need to change their configuration\n and use \"always\", but I suspect there is no sane workflow that wants\n the color escape code in files (e.g. \"git log >file\") or pipes\n (e.g. \"git diff | grep foo\") by default, in which case this won't\n hurt anybody and would help countless normal people who were bitten\n by the mistaken meaning originally chosen for \"true\".\n\n color.c | 32 +++++++++++++++++++-------------\n 1 files changed, 19 insertions(+), 13 deletions(-)\n\ndiff --git a/color.c b/color.c\nindex 124ba33..97cfbda 100644\n--- a/color.c\n+++ b/color.c\n@@ -118,21 +118,27 @@ bad:\n \n int git_config_colorbool(const char *var, const char *value)\n {\n-\tif (!value)\n-\t\treturn 1;\n-\tif (!strcasecmp(value, \"auto\")) {\n-\t\tif (isatty(1) || (pager_in_use && pager_use_color)) {\n-\t\t\tchar *term = getenv(\"TERM\");\n-\t\t\tif (term && strcmp(term, \"dumb\"))\n-\t\t\t\treturn 1;\n-\t\t}\n-\t\treturn 0;\n+\tif (value) {\n+\t\tif (!strcasecmp(value, \"never\"))\n+\t\t\treturn 0;\n+\t\tif (!strcasecmp(value, \"always\"))\n+\t\t\treturn 1;\n+\t\tif (!strcasecmp(value, \"auto\"))\n+\t\t\tgoto auto_color;\n \t}\n-\tif (!strcasecmp(value, \"never\"))\n+\n+\t/* Missing or explicit false to turn off colorization */\n+\tif (!git_config_bool(var, value))\n \t\treturn 0;\n-\tif (!strcasecmp(value, \"always\"))\n-\t\treturn 1;\n-\treturn git_config_bool(var, value);\n+\n+\t/* any normal truth value defaults to 'auto' */\n+ auto_color:\n+\tif (isatty(1) || (pager_in_use && pager_use_color)) {\n+\t\tchar *term = getenv(\"TERM\");\n+\t\tif (term && strcmp(term, \"dumb\"))\n+\t\t\treturn 1;\n+\t}\n+\treturn 0;\n }\n \n static int color_vfprintf(FILE *fp, const char *color, const char *fmt,\n-- \n1.5.3.6.2039.g0495\n"},{"id":"61181","messageId":"Pine.LNX.4.64.0711281313070.27959@racer.site","threadId":"11026","inReplyTo":"7vd4tuakzj.fsf_-_@gitster.siamese.dyndns.org","subject":"Re: [PATCH/RFC] \"color.diff = true\" is not \"always\" anymore.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-28T13:13:26Z","receivedAt":"2007-11-28T13:13:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 27 Nov 2007, Junio C Hamano wrote:\n\n> * This is definitely a backward incompatible change, but I think it is\n> only in a good way.\n\nI think so, too.\n\nThanks,\nDscho\n"},{"id":"61274","messageId":"20071128190439.GA11396@coredump.intra.peff.net","threadId":"11026","inReplyTo":"7vd4tuakzj.fsf_-_@gitster.siamese.dyndns.org","subject":"Re: [PATCH/RFC] \"color.diff = true\" is not \"always\" anymore.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-28T19:04:39Z","receivedAt":"2007-11-28T19:04:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 27, 2007 at 11:26:56PM -0800, Junio C Hamano wrote:\n\n> Too many people got burned by setting color.diff and color.status to\n> true when they really should have set it to \"auto\".\n> \n> This makes only \"always\" to do the unconditional colorization, and\n> change the meaning of \"true\" to the same as \"auto\": colorize only when\n> we are talking to a terminal.\n\nI think this is a good change. However, there needs to be a matching\nchange for all scripts which read the color.* variables (git-svn is the\nonly one now, I think, but Dan's git-add--interactive patch does the\nsame thing).\n\nIt would be nice to have a \"git config --colorbool\" option, but it has\nthe unfortunate problem that the stdout of \"git config\" is piped back to\nthe caller, so the isatty check is meaningless (and the \"pager in use\"\nis similarly tricky). Perhaps it should go in Git.pm, so it at least\nonly needs to be written once.\n\n-Peff\n"},{"id":"61544","messageId":"7v4pf39m4j.fsf@gitster.siamese.dyndns.org","threadId":"11026","inReplyTo":"20071128190439.GA11396@coredump.intra.peff.net","subject":"Re: [PATCH/RFC] \"color.diff = true\" is not \"always\" anymore.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-01T02:36:44Z","receivedAt":"2007-12-01T02:36:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> It would be nice to have a \"git config --colorbool\" option, but it has\n> the unfortunate problem that the stdout of \"git config\" is piped back to\n> the caller, so the isatty check is meaningless (and the \"pager in use\"\n> is similarly tricky). Perhaps it should go in Git.pm, so it at least\n> only needs to be written once.\n\nAbout the isatty(3) check, you do not have to use the stdout to report\nthe result, though. IOW, you could use the exit code from the command.\n"},{"id":"61557","messageId":"20071201041549.GB30725@coredump.intra.peff.net","threadId":"11026","inReplyTo":"7v4pf39m4j.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH/RFC] \"color.diff = true\" is not \"always\" anymore.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-12-01T04:15:49Z","receivedAt":"2007-12-01T04:15:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 30, 2007 at 06:36:44PM -0800, Junio C Hamano wrote:\n\n> > It would be nice to have a \"git config --colorbool\" option, but it has\n> > the unfortunate problem that the stdout of \"git config\" is piped back to\n> > the caller, so the isatty check is meaningless (and the \"pager in use\"\n> > is similarly tricky). Perhaps it should go in Git.pm, so it at least\n> > only needs to be written once.\n> \n> About the isatty(3) check, you do not have to use the stdout to report\n> the result, though. IOW, you could use the exit code from the command.\n\nI thought about that, but it feels a little wrong since it is so unlike\nall of the other interfaces to git-config. Still, I would consider doing\nit if there weren't other issues (like knowing when a pager is in use).\nAt some point it becomes more complex than simply having the 5-10 lines\nnecessary to do the check in perl.\n\n-Peff\n"},{"id":"61561","messageId":"7vd4tr6j38.fsf@gitster.siamese.dyndns.org","threadId":"11026","inReplyTo":"20071201041549.GB30725@coredump.intra.peff.net","subject":"Re: [PATCH/RFC] \"color.diff = true\" is not \"always\" anymore.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-01T06:10:35Z","receivedAt":"2007-12-01T06:10:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Fri, Nov 30, 2007 at 06:36:44PM -0800, Junio C Hamano wrote:\n>\n>> > It would be nice to have a \"git config --colorbool\" option, but it has\n>> > the unfortunate problem that the stdout of \"git config\" is piped back to\n>> > the caller, so the isatty check is meaningless (and the \"pager in use\"\n>> > is similarly tricky). Perhaps it should go in Git.pm, so it at least\n>> > only needs to be written once.\n>> \n>> About the isatty(3) check, you do not have to use the stdout to report\n>> the result, though. IOW, you could use the exit code from the command.\n>\n> I thought about that, but it feels a little wrong since it is so unlike\n> all of the other interfaces to git-config.\n\nYeah, that is why I did not seriously suggest it. The message you were\nresponding to was sitting in my \"I do not know if this should go out\"\nbox for a few days and was sent out purely by accident ;-)\n"}]}