{"thread":{"id":"55748","subject":"[PATCH] stash show: use stash.showIncludeUntracked even when diff options given","startedAt":"2021-05-21T10:38:41Z","lastAt":"2021-05-22T08:56:37Z","messageCount":2,"participants":["Denton Liu","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"425199","messageId":"e2e3574b25099a627d03447f5ace64b0e56c2ad1.1621593333.git.liu.denton@gmail.com","threadId":"55748","inReplyTo":null,"subject":"[PATCH] stash show: use stash.showIncludeUntracked even when diff options given","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2021-05-21T10:37:47Z","receivedAt":"2021-05-21T10:38:41Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"If options pertaining to how the diff is displayed is provided to\n`git stash show`, the command will ignore the stash.showIncludeUntracked\nconfiguration variable, defaulting to not showing any untracked files.\nThis is unintuitive behaviour since the format of the diff output and\nwhether or not to display untracked files are orthogonal.\n\nUse stash.showIncludeUntracked even when diff options are given. Of\ncourse, this is still overridable via the command-line options.\n\nUpdate the documentation to explicitly say which configuration variables\nwill be overridden when a diff options are given.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\nThis patch is a follow-up to [0]. This patch is based on top of\n'dl/stash-show-untracked-fixup'.\n\n[0]: https://lore.kernel.org/git/76dfa90a32ae926f7477d5966109f81441eb2783.1621325684.git.liu.denton@gmail.com/\n\n Documentation/config/stash.txt     | 6 +++---\n Documentation/git-stash.txt        | 6 ++++--\n builtin/stash.c                    | 5 +----\n t/t3905-stash-include-untracked.sh | 2 ++\n 4 files changed, 10 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/config/stash.txt b/Documentation/config/stash.txt\nindex 413f907cba..9ed775281f 100644\n--- a/Documentation/config/stash.txt\n+++ b/Documentation/config/stash.txt\n@@ -6,9 +6,9 @@ stash.useBuiltin::\n \tremaining users that setting this now does nothing.\n \n stash.showIncludeUntracked::\n-\tIf this is set to true, the `git stash show` command without an\n-\toption will show the untracked files of a stash entry.  Defaults to\n-\tfalse. See description of 'show' command in linkgit:git-stash[1].\n+\tIf this is set to true, the `git stash show` command will show\n+\tthe untracked files of a stash entry.  Defaults to false. See\n+\tdescription of 'show' command in linkgit:git-stash[1].\n \n stash.showPatch::\n \tIf this is set to true, the `git stash show` command without an\ndiff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\nindex a8c8c32f1e..be6084ccef 100644\n--- a/Documentation/git-stash.txt\n+++ b/Documentation/git-stash.txt\n@@ -91,8 +91,10 @@ show [-u|--include-untracked|--only-untracked] [<diff-options>] [<stash>]::\n \tBy default, the command shows the diffstat, but it will accept any\n \tformat known to 'git diff' (e.g., `git stash show -p stash@{1}`\n \tto view the second most recent entry in patch form).\n-\tYou can use stash.showIncludeUntracked, stash.showStat, and\n-\tstash.showPatch config variables to change the default behavior.\n+\tIf no `<diff-option>` is provided, the default behavior will be given\n+\tby the `stash.showStat`, and `stash.showPatch` config variables. You\n+\tcan also use `stash.showIncludeUntracked` to set whether\n+\t`--include-untracked` is enabled by default.\n \n pop [--index] [-q|--quiet] [<stash>]::\n \ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex 82e4829d44..864b6c1416 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -831,7 +831,7 @@ static int show_stash(int argc, const char **argv, const char *prefix)\n \t\tUNTRACKED_NONE,\n \t\tUNTRACKED_INCLUDE,\n \t\tUNTRACKED_ONLY\n-\t} show_untracked = UNTRACKED_NONE;\n+\t} show_untracked = show_include_untracked ? UNTRACKED_INCLUDE : UNTRACKED_NONE;\n \tstruct option options[] = {\n \t\tOPT_SET_INT('u', \"include-untracked\", &show_untracked,\n \t\t\t    N_(\"include untracked files in the stash\"),\n@@ -874,9 +874,6 @@ static int show_stash(int argc, const char **argv, const char *prefix)\n \t\tif (show_patch)\n \t\t\trev.diffopt.output_format |= DIFF_FORMAT_PATCH;\n \n-\t\tif (show_include_untracked)\n-\t\t\tshow_untracked = UNTRACKED_INCLUDE;\n-\n \t\tif (!show_stat && !show_patch) {\n \t\t\tfree_stash_info(&info);\n \t\t\treturn 0;\ndiff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh\nindex 1c9765928d..f7fafcd447 100755\n--- a/t/t3905-stash-include-untracked.sh\n+++ b/t/t3905-stash-include-untracked.sh\n@@ -333,6 +333,8 @@ test_expect_success 'stash show --include-untracked shows untracked files' '\n \tgit stash show -p --include-untracked >actual &&\n \ttest_cmp expect actual &&\n \tgit stash show --include-untracked -p >actual &&\n+\ttest_cmp expect actual &&\n+\tgit -c stash.showIncludeUntracked=true stash show -p >actual &&\n \ttest_cmp expect actual\n '\n \n-- \n2.32.0.rc0.171.g09c0ee21fe\n\n"},{"id":"425292","messageId":"xmqqbl93qh8h.fsf@gitster.g","threadId":"55748","inReplyTo":"e2e3574b25099a627d03447f5ace64b0e56c2ad1.1621593333.git.liu.denton@gmail.com","subject":"Re: [PATCH] stash show: use stash.showIncludeUntracked even when diff options given","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-22T08:56:30Z","receivedAt":"2021-05-22T08:56:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu <liu.denton@gmail.com> writes:\n\n> If options pertaining to how the diff is displayed is provided to\n> `git stash show`, the command will ignore the stash.showIncludeUntracked\n> configuration variable, defaulting to not showing any untracked files.\n> This is unintuitive behaviour since the format of the diff output and\n> whether or not to display untracked files are orthogonal.\n>\n> Use stash.showIncludeUntracked even when diff options are given. Of\n> course, this is still overridable via the command-line options.\n>\n> Update the documentation to explicitly say which configuration variables\n> will be overridden when a diff options are given.\n>\n> Signed-off-by: Denton Liu <liu.denton@gmail.com>\n> ---\n> This patch is a follow-up to [0]. This patch is based on top of\n> 'dl/stash-show-untracked-fixup'.\n\nIt does make sense to keep \"what to show\" and \"how to show them\"\northogonal.  It seems that not enough thoguht went into the topic\nbefore it got merged to 'next', which is quite sad.\n\n> diff --git a/Documentation/config/stash.txt b/Documentation/config/stash.txt\n> index 413f907cba..9ed775281f 100644\n> --- a/Documentation/config/stash.txt\n> +++ b/Documentation/config/stash.txt\n> @@ -6,9 +6,9 @@ stash.useBuiltin::\n>  \tremaining users that setting this now does nothing.\n>  \n>  stash.showIncludeUntracked::\n> -\tIf this is set to true, the `git stash show` command without an\n> -\toption will show the untracked files of a stash entry.  Defaults to\n> -\tfalse. See description of 'show' command in linkgit:git-stash[1].\n> +\tIf this is set to true, the `git stash show` command will show\n> +\tthe untracked files of a stash entry.  Defaults to false. See\n> +\tdescription of 'show' command in linkgit:git-stash[1].\n\nOK.\n\n> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\n> index a8c8c32f1e..be6084ccef 100644\n> --- a/Documentation/git-stash.txt\n> +++ b/Documentation/git-stash.txt\n> @@ -91,8 +91,10 @@ show [-u|--include-untracked|--only-untracked] [<diff-options>] [<stash>]::\n>  \tBy default, the command shows the diffstat, but it will accept any\n>  \tformat known to 'git diff' (e.g., `git stash show -p stash@{1}`\n>  \tto view the second most recent entry in patch form).\n> +\tIf no `<diff-option>` is provided, the default behavior will be given\n> +\tby the `stash.showStat`, and `stash.showPatch` config variables. You\n> +\tcan also use `stash.showIncludeUntracked` to set whether\n> +\t`--include-untracked` is enabled by default.\n\nOK.\n\n> diff --git a/builtin/stash.c b/builtin/stash.c\n> index 82e4829d44..864b6c1416 100644\n> --- a/builtin/stash.c\n> +++ b/builtin/stash.c\n> @@ -831,7 +831,7 @@ static int show_stash(int argc, const char **argv, const char *prefix)\n>  \t\tUNTRACKED_NONE,\n>  \t\tUNTRACKED_INCLUDE,\n>  \t\tUNTRACKED_ONLY\n> -\t} show_untracked = UNTRACKED_NONE;\n> +\t} show_untracked = show_include_untracked ? UNTRACKED_INCLUDE : UNTRACKED_NONE;\n\nOK.  We initialize this to what the config said...\n\n>  \tstruct option options[] = {\n>  \t\tOPT_SET_INT('u', \"include-untracked\", &show_untracked,\n>  \t\t\t    N_(\"include untracked files in the stash\"),\n> @@ -874,9 +874,6 @@ static int show_stash(int argc, const char **argv, const char *prefix)\n>  \t\tif (show_patch)\n>  \t\t\trev.diffopt.output_format |= DIFF_FORMAT_PATCH;\n>  \n> -\t\tif (show_include_untracked)\n> -\t\t\tshow_untracked = UNTRACKED_INCLUDE;\n> -\n\n... without limiting the defaulting only to the case where\nrevision_args.nr==1 (no options are given).\n\n>  \t\tif (!show_stat && !show_patch) {\n>  \t\t\tfree_stash_info(&info);\n>  \t\t\treturn 0;\n\nAs this is a fix to a part of a new feature that was broken from day\none, let's take it and fast-track.\n\nThanks.\n\n> diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh\n> index 1c9765928d..f7fafcd447 100755\n> --- a/t/t3905-stash-include-untracked.sh\n> +++ b/t/t3905-stash-include-untracked.sh\n> @@ -333,6 +333,8 @@ test_expect_success 'stash show --include-untracked shows untracked files' '\n>  \tgit stash show -p --include-untracked >actual &&\n>  \ttest_cmp expect actual &&\n>  \tgit stash show --include-untracked -p >actual &&\n> +\ttest_cmp expect actual &&\n> +\tgit -c stash.showIncludeUntracked=true stash show -p >actual &&\n>  \ttest_cmp expect actual\n>  '\n"}]}