{"thread":{"id":"45489","subject":"[PATCH v2] log: if --decorate is not given, default to --decorate=auto","startedAt":"2017-03-24T05:46:44Z","lastAt":"2017-03-30T21:55:05Z","messageCount":6,"participants":["Alex Henrie","Junio C Hamano","Jonathan Nieder","Jeff King"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"315229","messageId":"20170324054631.21622-1-alexhenrie24@gmail.com","threadId":"45489","inReplyTo":null,"subject":"[PATCH v2] log: if --decorate is not given, default to --decorate=auto","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2017-03-24T05:46:31Z","receivedAt":"2017-03-24T05:46:44Z","isPatch":true,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>\n---\n builtin/log.c  |  9 ++++++++-\n t/t4202-log.sh | 10 +++++++++-\n 2 files changed, 17 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 281af8c1e..d755a5960 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -52,6 +52,11 @@ struct line_opt_callback_data {\n \tstruct string_list args;\n };\n \n+static int auto_decoration_style(void)\n+{\n+\treturn (isatty(1) || pager_in_use()) ? DECORATE_SHORT_REFS : 0;\n+}\n+\n static int parse_decoration_style(const char *var, const char *value)\n {\n \tswitch (git_config_maybe_bool(var, value)) {\n@@ -67,7 +72,7 @@ static int parse_decoration_style(const char *var, const char *value)\n \telse if (!strcmp(value, \"short\"))\n \t\treturn DECORATE_SHORT_REFS;\n \telse if (!strcmp(value, \"auto\"))\n-\t\treturn (isatty(1) || pager_in_use()) ? DECORATE_SHORT_REFS : 0;\n+\t\treturn auto_decoration_style();\n \treturn -1;\n }\n \n@@ -405,6 +410,8 @@ static int git_log_config(const char *var, const char *value, void *cb)\n \t\tif (decoration_style < 0)\n \t\t\tdecoration_style = 0; /* maybe warn? */\n \t\treturn 0;\n+\t} else {\n+\t\tdecoration_style = auto_decoration_style();\n \t}\n \tif (!strcmp(var, \"log.showroot\")) {\n \t\tdefault_show_root = git_config_bool(var, value);\ndiff --git a/t/t4202-log.sh b/t/t4202-log.sh\nindex 48b55bfd2..f57799071 100755\n--- a/t/t4202-log.sh\n+++ b/t/t4202-log.sh\n@@ -4,6 +4,7 @@ test_description='git log'\n \n . ./test-lib.sh\n . \"$TEST_DIRECTORY/lib-gpg.sh\"\n+. \"$TEST_DIRECTORY/lib-terminal.sh\"\n \n test_expect_success setup '\n \n@@ -520,7 +521,7 @@ test_expect_success 'log --graph with merge' '\n '\n \n test_expect_success 'log.decorate configuration' '\n-\tgit log --oneline >expect.none &&\n+\tgit log --oneline --no-decorate >expect.none &&\n \tgit log --oneline --decorate >expect.short &&\n \tgit log --oneline --decorate=full >expect.full &&\n \n@@ -576,6 +577,13 @@ test_expect_success 'log.decorate configuration' '\n \n '\n \n+test_expect_success TTY 'log output on a TTY' '\n+\tgit log --oneline --decorate >expect.short &&\n+\n+\ttest_terminal git log --oneline >actual &&\n+\ttest_cmp expect.short actual\n+'\n+\n test_expect_success 'reflog is expected format' '\n \tgit log -g --abbrev-commit --pretty=oneline >expect &&\n \tgit reflog >actual &&\n-- \n2.12.0\n\n"},{"id":"315285","messageId":"xmqq8tnu8te2.fsf@gitster.mtv.corp.google.com","threadId":"45489","inReplyTo":"20170324054631.21622-1-alexhenrie24@gmail.com","subject":"Re: [PATCH v2] log: if --decorate is not given, default to --decorate=auto","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-24T17:41:09Z","receivedAt":"2017-03-24T17:41:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Henrie <alexhenrie24@gmail.com> writes:\n\n> +test_expect_success TTY 'log output on a TTY' '\n> +\tgit log --oneline --decorate >expect.short &&\n> +\n> +\ttest_terminal git log --oneline >actual &&\n> +\ttest_cmp expect.short actual\n> +'\n> +\n\nNice.  I didn't know test_terminal was so easy to use ;-)\n\nLooks good.  Will queue.\n\nThanks.\n"},{"id":"315301","messageId":"20170324183825.GD31294@aiede.mtv.corp.google.com","threadId":"45489","inReplyTo":"20170324054631.21622-1-alexhenrie24@gmail.com","subject":"Re: [PATCH v2] log: if --decorate is not given, default to --decorate=auto","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-03-24T18:38:25Z","receivedAt":"2017-03-24T18:38:41Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Alex Henrie wrote:\n\n> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>\n> ---\n>  builtin/log.c  |  9 ++++++++-\n>  t/t4202-log.sh | 10 +++++++++-\n>  2 files changed, 17 insertions(+), 2 deletions(-)\n\nNice.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n"},{"id":"315801","messageId":"xmqqa88263qw.fsf@gitster.mtv.corp.google.com","threadId":"45489","inReplyTo":"20170324183825.GD31294@aiede.mtv.corp.google.com","subject":"Re: [PATCH v2] log: if --decorate is not given, default to --decorate=auto","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-30T18:03:51Z","receivedAt":"2017-03-30T18:03:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"With the \"--decorate=auto\" option becoming the default for \"git\nlog\", \"git tbdiff\" will be broken.\n\nThe configuration variable has been already there, so in that sense\nthis is not a new breakage (tbdiff wouldn't have worked well for\nthose with configured default).  A fix is trivial (attached).\n\nI suspect that Alex's change may uncover similar breakages in\npeople's scripts.  Perhaps the topic should be cooked a bit longer\nthan other topics on 'next'?\n\n\n git-tbdiff.py | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-tbdiff.py b/git-tbdiff.py\nindex ccf7a0b..ae7cbb5 100755\n--- a/git-tbdiff.py\n+++ b/git-tbdiff.py\n@@ -75,7 +75,7 @@ def read_patches(rev_list_args):\n     series = []\n     diffs = {}\n     p = subprocess.Popen(['git', 'log', '--no-color', '-p', '--no-merges',\n-                          '--reverse', '--date-order']\n+                          '--no-decorate', '--reverse', '--date-order']\n                          + rev_list_args,\n                          stdout=subprocess.PIPE)\n     sha1 = None\n"},{"id":"315837","messageId":"20170330213358.qjsobjcbons66skf@sigill.intra.peff.net","threadId":"45489","inReplyTo":"xmqqa88263qw.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2] log: if --decorate is not given, default to --decorate=auto","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-30T21:33:58Z","receivedAt":"2017-03-30T21:34:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 30, 2017 at 11:03:51AM -0700, Junio C Hamano wrote:\n\n> With the \"--decorate=auto\" option becoming the default for \"git\n> log\", \"git tbdiff\" will be broken.\n> \n> The configuration variable has been already there, so in that sense\n> this is not a new breakage (tbdiff wouldn't have worked well for\n> those with configured default).  A fix is trivial (attached).\n> \n> I suspect that Alex's change may uncover similar breakages in\n> people's scripts.  Perhaps the topic should be cooked a bit longer\n> than other topics on 'next'?\n\nI'm confused. I thought \"auto\" would kick in only when we are outputting\nto a terminal. Or is the problem that the \"is it a terminal\" check is\nfooled by $GIT_PAGER_IN_USE, because you are running \"git -p tbdiff\"?\n\nIf so, this is the symptom of a more general problem, which is that\na script outputting to a pager will have confused sub-processes, who do\nnot know if their pipe is the pager one or not. Perhaps it is time to\nresurrect my patch from:\n\n  http://public-inbox.org/git/20150810052353.GB15441@sigill.intra.peff.net/\n\nI think it would need a Windows-specific variant, but the general idea\nis sound.\n\n-Peff\n\nPS I've been running git-tbdiff occasionally for years with log.decorate\n   set to \"true\". I wonder why I haven't noticed any problems.\n"},{"id":"315838","messageId":"xmqq1ste2zwu.fsf@gitster.mtv.corp.google.com","threadId":"45489","inReplyTo":"20170330213358.qjsobjcbons66skf@sigill.intra.peff.net","subject":"Re: [PATCH v2] log: if --decorate is not given, default to --decorate=auto","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-30T21:54:57Z","receivedAt":"2017-03-30T21:55:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Mar 30, 2017 at 11:03:51AM -0700, Junio C Hamano wrote:\n>\n>> With the \"--decorate=auto\" option becoming the default for \"git\n>> log\", \"git tbdiff\" will be broken.\n>> ...\n> I'm confused. I thought \"auto\" would kick in only when we are outputting\n> to a terminal. Or is the problem that the \"is it a terminal\" check is\n> fooled by $GIT_PAGER_IN_USE, because you are running \"git -p tbdiff\"?\n\nInteresting.  Yes, I do use \n\n    [pager]\n\ttbdiff\n\nin my ~/.gitconfig file.\n\n    $ git tbdiff ..@{-1} @{-1}..\n\nis one of the most frequently used commands in my ~/.bash_history\nthese days [*1*].  I by accident has been running the 'master'\nversion (not my private edition 'jch' that is a bit ahead of 'next')\nfor the past few weeks, and I just switched back to using the 'jch'\nversion so that I can say\n\n    $ git tbdiff ..- -..\n\ninstead, and that is when I noticed we broke \"tbdiff\".\n\n> If so, this is the symptom of a more general problem, which is that\n> a script outputting to a pager will have confused sub-processes, who do\n> not know if their pipe is the pager one or not. Perhaps it is time to\n> resurrect my patch from:\n>\n>   http://public-inbox.org/git/20150810052353.GB15441@sigill.intra.peff.net/\n>\n> I think it would need a Windows-specific variant, but the general idea\n> is sound.\n\nYes, that might be necessary.\n\n\n[Footnote]\n\n*1* The general flow to accept a reroll of a topic \"au/topic\" goes\n    like this:\n\n    $ git checkout au/topic\n    $ git log master.. ;# to remind me what it was about\n    $ git checkout master... ;# to go back to the original base\n    $ Meta/CP ./+au-topic.mbox ;# run checkpatch\n    $ git am -s3c\n    $ git tbdiff ..@{-1} @{-1}..\n\n    Then if the initial N patches are identical, e.g. when the\n    output of tbdiff begins like this:\n\n     1: f6d8dfd8b6 =  1: d681cf5ada do not check odb_mkstemp return value for errors\n     2: 52dcad2c2e =  2: abf30edce4 odb_mkstemp: write filename into strbuf\n     3: 033d6ae6cb =  3: 38fceca547 odb_mkstemp: use git_path_buf\n     4: 55e3179076 !  4: 344267b632 diff: avoid fixed-size buffer for patch-ids\n        @@ ... @@\n\n    $ git rebase --onto 033d6ae6cb 38fceca547\n    $ git tbdiff ..@{-1} @{-1}..\n\n    That way, I can preserve the author and committer timestamps of\n    the earlier part that did not change.\n"}]}