{"thread":{"id":"40196","subject":"[PATCH] stash: Add stash.showFlag config variable","startedAt":"2015-08-27T13:52:08Z","lastAt":"2015-08-29T15:19:43Z","messageCount":9,"participants":["Namhyung Kim","SZEDER Gábor","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"268792","messageId":"1440683528-11725-1-git-send-email-namhyung@gmail.com","threadId":"40196","inReplyTo":null,"subject":"[PATCH] stash: Add stash.showFlag config variable","fromName":"Namhyung Kim","fromEmail":"namhyung@gmail.com","sentAt":"2015-08-27T13:52:08Z","receivedAt":"2015-08-27T13:52:08Z","isPatch":true,"sender":{"key":"namhyung@gmail.com","avatar":"https://avatars.githubusercontent.com/u/48503?v=4"},"body":"Some users might want to see diff (patch) output always rather than\ndiffstat when [s]he runs 'git stash show'.  Although this can be done\nwith adding -p option, it'd be better to provide a config option to\ncontrol this behavior IMHO.\n\nSigned-off-by: Namhyung Kim <namhyung@gmail.com>\n---\n Documentation/config.txt    | 5 +++++\n Documentation/git-stash.txt | 1 +\n git-stash.sh                | 8 +++++++-\n 3 files changed, 13 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex f5d15ff..bbadae6 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2567,6 +2567,11 @@ status.submoduleSummary::\n \tsubmodule summary' command, which shows a similar output but does\n \tnot honor these settings.\n \n+stash.showFlag::\n+\tThe default option to pass to `git stash show` when no option is\n+\tgiven. The default is '--stat'.  See description of 'show' command\n+\tin linkgit:git-stash[1].\n+\n submodule.<name>.path::\n submodule.<name>.url::\n \tThe path within this project and URL for a submodule. These\ndiff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\nindex 375213f..e00f67e 100644\n--- a/Documentation/git-stash.txt\n+++ b/Documentation/git-stash.txt\n@@ -95,6 +95,7 @@ show [<stash>]::\n \tshows the latest one. By default, the command shows the diffstat, but\n \tit will accept any format known to 'git diff' (e.g., `git stash show\n \t-p stash@{1}` to view the second most recent stash in patch form).\n+\tYou can use stash.showflag config variable to change this behavior.\n \n pop [--index] [-q|--quiet] [<stash>]::\n \ndiff --git a/git-stash.sh b/git-stash.sh\nindex 1d5ba7a..8432435 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -33,6 +33,12 @@ else\n        reset_color=\n fi\n \n+if git config --get stash.showflag > /dev/null 2> /dev/null; then\n+\tshow_flag=$(git config --get stash.showflag)\n+else\n+\tshow_flag=--stat\n+fi\n+\n no_changes () {\n \tgit diff-index --quiet --cached HEAD --ignore-submodules -- &&\n \tgit diff-files --quiet --ignore-submodules &&\n@@ -305,7 +311,7 @@ show_stash () {\n \tALLOW_UNKNOWN_FLAGS=t\n \tassert_stash_like \"$@\"\n \n-\tgit diff ${FLAGS:---stat} $b_commit $w_commit\n+\tgit diff ${FLAGS:-${show_flag}} $b_commit $w_commit\n }\n \n show_help () {\n-- \n2.5.0\n"},{"id":"268793","messageId":"1440688825-1303-1-git-send-email-szeder@ira.uka.de","threadId":"40196","inReplyTo":"1440683528-11725-1-git-send-email-namhyung@gmail.com","subject":"Re: [PATCH] stash: Add stash.showFlag config variable","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-08-27T15:20:25Z","receivedAt":"2015-08-27T15:20:25Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Hi,\n\nI haven't made up my mind about this feature yet, but have a few\ncomments about its implementation.\n\n> diff --git a/git-stash.sh b/git-stash.sh\n> index 1d5ba7a..8432435 100755\n> --- a/git-stash.sh\n> +++ b/git-stash.sh\n> @@ -33,6 +33,12 @@ else\n>         reset_color=\n>  fi\n>  \n> +if git config --get stash.showflag > /dev/null 2> /dev/null; then\n> +\tshow_flag=$(git config --get stash.showflag)\n> +else\n> +\tshow_flag=--stat\n> +fi\n> +\n\nForking and executing processes are costly on some important platforms\nwe care about, so we should strive to avoid them whenever possible.\n\n - This hunk runs the the exact same 'git config' command twice.  Run it\n   only once, perhaps something like this:\n\n     show_flag=$(git config --get stash.showflag || echo --stat)\n\n   (I hope there are no obscure crazy 'echo' implemtations out there\n   that might barf on the unknown option '--stat'...)\n\n - It runs 'git config' in the main code path, i.e. even for subcommands\n   other than 'show'.  Run it only for 'git stash show'.\n\n - This config setting is not relevant if there were options given on the\n   command line.  Run it only if there are no options given, i.e. when\n   $FLAGS is empty.\n\n\n>  no_changes () {\n>  \tgit diff-index --quiet --cached HEAD --ignore-submodules -- &&\n>  \tgit diff-files --quiet --ignore-submodules &&\n> @@ -305,7 +311,7 @@ show_stash () {\n>  \tALLOW_UNKNOWN_FLAGS=t\n>  \tassert_stash_like \"$@\"\n>  \n> -\tgit diff ${FLAGS:---stat} $b_commit $w_commit\n> +\tgit diff ${FLAGS:-${show_flag}} $b_commit $w_commit\n>  }\n>  \n>  show_help () {\n> -- \n> 2.5.0\n"},{"id":"268796","messageId":"CAM9d7chUf=srU060Q4+qQ4mFBaXmRL0yQ1Ns4UeWcDj62CFoYg@mail.gmail.com","threadId":"40196","inReplyTo":"1440688825-1303-1-git-send-email-szeder@ira.uka.de","subject":"Re: [PATCH] stash: Add stash.showFlag config variable","fromName":"Namhyung Kim","fromEmail":"namhyung@gmail.com","sentAt":"2015-08-27T15:36:35Z","receivedAt":"2015-08-27T15:36:35Z","isPatch":true,"sender":{"key":"namhyung@gmail.com","avatar":"https://avatars.githubusercontent.com/u/48503?v=4"},"body":"Hi,\n\nOn Fri, Aug 28, 2015 at 12:20 AM, SZEDER Gábor <szeder@ira.uka.de> wrote:\n> Hi,\n>\n> I haven't made up my mind about this feature yet, but have a few\n> comments about its implementation.\n\nThanks for taking your time!\n\n>\n>> diff --git a/git-stash.sh b/git-stash.sh\n>> index 1d5ba7a..8432435 100755\n>> --- a/git-stash.sh\n>> +++ b/git-stash.sh\n>> @@ -33,6 +33,12 @@ else\n>>         reset_color=\n>>  fi\n>>\n>> +if git config --get stash.showflag > /dev/null 2> /dev/null; then\n>> +     show_flag=$(git config --get stash.showflag)\n>> +else\n>> +     show_flag=--stat\n>> +fi\n>> +\n>\n> Forking and executing processes are costly on some important platforms\n> we care about, so we should strive to avoid them whenever possible.\n>\n>  - This hunk runs the the exact same 'git config' command twice.  Run it\n>    only once, perhaps something like this:\n>\n>      show_flag=$(git config --get stash.showflag || echo --stat)\n>\n>    (I hope there are no obscure crazy 'echo' implemtations out there\n>    that might barf on the unknown option '--stat'...)\n\nWhat about `echo \"--stat\"` then?\n\n>\n>  - It runs 'git config' in the main code path, i.e. even for subcommands\n>    other than 'show'.  Run it only for 'git stash show'.\n>\n>  - This config setting is not relevant if there were options given on the\n>    command line.  Run it only if there are no options given, i.e. when\n>    $FLAGS is empty.\n\nFair enough.  I'll resend v2.\n\nThanks,\nNamhyung\n\n\n>\n>\n>>  no_changes () {\n>>       git diff-index --quiet --cached HEAD --ignore-submodules -- &&\n>>       git diff-files --quiet --ignore-submodules &&\n>> @@ -305,7 +311,7 @@ show_stash () {\n>>       ALLOW_UNKNOWN_FLAGS=t\n>>       assert_stash_like \"$@\"\n>>\n>> -     git diff ${FLAGS:---stat} $b_commit $w_commit\n>> +     git diff ${FLAGS:-${show_flag}} $b_commit $w_commit\n>>  }\n>>\n>>  show_help () {\n>> --\n>> 2.5.0\n"},{"id":"268807","messageId":"CAPig+cQmTS5rRkfh1in9qR4MyP1_y9vNar7U4H3uayK6Vixa7w@mail.gmail.com","threadId":"40196","inReplyTo":"CAM9d7chUf=srU060Q4+qQ4mFBaXmRL0yQ1Ns4UeWcDj62CFoYg@mail.gmail.com","subject":"Re: [PATCH] stash: Add stash.showFlag config variable","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-08-28T00:16:35Z","receivedAt":"2015-08-28T00:16:35Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Aug 27, 2015 at 11:36 AM, Namhyung Kim <namhyung@gmail.com> wrote:\n> On Fri, Aug 28, 2015 at 12:20 AM, SZEDER Gábor <szeder@ira.uka.de> wrote:\n>>  - This hunk runs the the exact same 'git config' command twice.  Run it\n>>    only once, perhaps something like this:\n>>\n>>      show_flag=$(git config --get stash.showflag || echo --stat)\n>>\n>>    (I hope there are no obscure crazy 'echo' implemtations out there\n>>    that might barf on the unknown option '--stat'...)\n>\n> What about `echo \"--stat\"` then?\n\nAdding quotes around --stat won't buy you anything since the shell\nwill have removed the quotes by the time the argument is passed to\necho, so an \"obscure crazy\" 'echo' will still see --stat as an option.\n\nPOSIX states that printf should take no options, so:\n\n    printf --stat\n\nshould be safe, but some implementations do process options (and will\ncomplain about the unknown --stat option), therefore, best would be:\n\n    printf '%s' --stat\n"},{"id":"268808","messageId":"xmqq614043u0.fsf@gitster.mtv.corp.google.com","threadId":"40196","inReplyTo":"1440683528-11725-1-git-send-email-namhyung@gmail.com","subject":"Re: [PATCH] stash: Add stash.showFlag config variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-28T01:08:39Z","receivedAt":"2015-08-28T01:08:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Namhyung Kim <namhyung@gmail.com> writes:\n\n> +stash.showFlag::\n> +\tThe default option to pass to `git stash show` when no option is\n> +\tgiven. The default is '--stat'.  See description of 'show' command\n> +\tin linkgit:git-stash[1].\n\nDoesn't the same discussion in $gmane/275752 apply here?  By\ndesigning the configuration variable in a sloppy way, this change\nwill force us to spawn \"git diff\" via the shell forever, even after\nsomebody ports \"git stash\" to C.\n\nWhich is not great.\n\nPerhaps a pair of new booleans\n\n - stash.showStat (defaults to true but you can turn it off)\n - stash.showPatch (defaults to false but you can turn it on)\n\nor something along that line might be sufficient and more palatable.\n\nI dunno.\n\n\n[Footnote]\n\n*1* Besides, showFlag is a strange configuration variable name.  I\nthought that by setting it to true, you are making \"git stash\"\ncommand to somehow show some kind of a flag when it does its\noperation ;-).\n"},{"id":"268819","messageId":"20150828014742.GA17656@sejong","threadId":"40196","inReplyTo":"CAPig+cQmTS5rRkfh1in9qR4MyP1_y9vNar7U4H3uayK6Vixa7w@mail.gmail.com","subject":"Re: [PATCH] stash: Add stash.showFlag config variable","fromName":"Namhyung Kim","fromEmail":"namhyung@gmail.com","sentAt":"2015-08-28T01:47:42Z","receivedAt":"2015-08-28T01:47:42Z","isPatch":true,"sender":{"key":"namhyung@gmail.com","avatar":"https://avatars.githubusercontent.com/u/48503?v=4"},"body":"Hi,\n\nOn Thu, Aug 27, 2015 at 08:16:35PM -0400, Eric Sunshine wrote:\n> On Thu, Aug 27, 2015 at 11:36 AM, Namhyung Kim <namhyung@gmail.com> wrote:\n> > On Fri, Aug 28, 2015 at 12:20 AM, SZEDER Gábor <szeder@ira.uka.de> wrote:\n> >>  - This hunk runs the the exact same 'git config' command twice.  Run it\n> >>    only once, perhaps something like this:\n> >>\n> >>      show_flag=$(git config --get stash.showflag || echo --stat)\n> >>\n> >>    (I hope there are no obscure crazy 'echo' implemtations out there\n> >>    that might barf on the unknown option '--stat'...)\n> >\n> > What about `echo \"--stat\"` then?\n> \n> Adding quotes around --stat won't buy you anything since the shell\n> will have removed the quotes by the time the argument is passed to\n> echo, so an \"obscure crazy\" 'echo' will still see --stat as an option.\n> \n> POSIX states that printf should take no options, so:\n> \n>     printf --stat\n> \n> should be safe, but some implementations do process options (and will\n> complain about the unknown --stat option), therefore, best would be:\n> \n>     printf '%s' --stat\n\nThat's good to know.  I'll change it that way.\n\nThanks for your review!\nNamhyung\n"},{"id":"268820","messageId":"20150828015433.GB17656@sejong","threadId":"40196","inReplyTo":"xmqq614043u0.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] stash: Add stash.showFlag config variable","fromName":"Namhyung Kim","fromEmail":"namhyung@gmail.com","sentAt":"2015-08-28T01:54:33Z","receivedAt":"2015-08-28T01:54:33Z","isPatch":true,"sender":{"key":"namhyung@gmail.com","avatar":"https://avatars.githubusercontent.com/u/48503?v=4"},"body":"Hi,\n\nOn Thu, Aug 27, 2015 at 06:08:39PM -0700, Junio C Hamano wrote:\n> Namhyung Kim <namhyung@gmail.com> writes:\n> \n> > +stash.showFlag::\n> > +\tThe default option to pass to `git stash show` when no option is\n> > +\tgiven. The default is '--stat'.  See description of 'show' command\n> > +\tin linkgit:git-stash[1].\n> \n> Doesn't the same discussion in $gmane/275752 apply here?  By\n> designing the configuration variable in a sloppy way, this change\n> will force us to spawn \"git diff\" via the shell forever, even after\n> somebody ports \"git stash\" to C.\n> \n> Which is not great.\n\nI see.\n\n\n> \n> Perhaps a pair of new booleans\n> \n>  - stash.showStat (defaults to true but you can turn it off)\n>  - stash.showPatch (defaults to false but you can turn it on)\n> \n> or something along that line might be sufficient and more palatable.\n\nHmm.. I agree with you, but I don't know what we should do if both of\nthe options were off.  Just run 'git diff' with no option is ok to you?\n\n> \n> I dunno.\n\n:)\n\n> \n> \n> [Footnote]\n> \n> *1* Besides, showFlag is a strange configuration variable name.  I\n> thought that by setting it to true, you are making \"git stash\"\n> command to somehow show some kind of a flag when it does its\n> operation ;-).\n\nI admit that it's a bad name.  My naming sense is always horrible.. ;-p\n\nThanks for the review!\nNamhyung\n"},{"id":"268858","messageId":"xmqqy4gv1dyr.fsf@gitster.mtv.corp.google.com","threadId":"40196","inReplyTo":"20150828015433.GB17656@sejong","subject":"Re: [PATCH] stash: Add stash.showFlag config variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-28T18:10:20Z","receivedAt":"2015-08-28T18:10:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Namhyung Kim <namhyung@gmail.com> writes:\n\n>> Perhaps a pair of new booleans\n>> \n>>  - stash.showStat (defaults to true but you can turn it off)\n>>  - stash.showPatch (defaults to false but you can turn it on)\n>> \n>> or something along that line might be sufficient and more palatable.\n>\n> Hmm.. I agree with you, but I don't know what we should do if both of\n> the options were off.  Just run 'git diff' with no option is ok to you?\n\nIf the user does not want stat or patch, then not running anything\nwould be more appropriate, don't you think?\n"},{"id":"268905","messageId":"CAM9d7cg4k=H9GY=CihuHdyH1yj-z4EUkKsWrDO0DXKx3QwDejw@mail.gmail.com","threadId":"40196","inReplyTo":"xmqqy4gv1dyr.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] stash: Add stash.showFlag config variable","fromName":"Namhyung Kim","fromEmail":"namhyung@gmail.com","sentAt":"2015-08-29T15:19:43Z","receivedAt":"2015-08-29T15:19:43Z","isPatch":true,"sender":{"key":"namhyung@gmail.com","avatar":"https://avatars.githubusercontent.com/u/48503?v=4"},"body":"Hi,\n\nOn Sat, Aug 29, 2015 at 3:10 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Namhyung Kim <namhyung@gmail.com> writes:\n>\n>>> Perhaps a pair of new booleans\n>>>\n>>>  - stash.showStat (defaults to true but you can turn it off)\n>>>  - stash.showPatch (defaults to false but you can turn it on)\n>>>\n>>> or something along that line might be sufficient and more palatable.\n>>\n>> Hmm.. I agree with you, but I don't know what we should do if both of\n>> the options were off.  Just run 'git diff' with no option is ok to you?\n>\n> If the user does not want stat or patch, then not running anything\n> would be more appropriate, don't you think?\n\nAh, ok. :)  I'll change that way and send v3 soon!\n\nThanks,\nNamhyung\n"}]}