git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/9] stash show: learn --include-untracked and --only-untracked
@ 2021-02-02  9:31 Denton Liu
  2021-02-02  9:33 ` Denton Liu
                   ` (10 more replies)
  0 siblings, 11 replies; 46+ messages in thread
From: Denton Liu @ 2021-02-02  9:31 UTC (permalink / raw)
  To: Git Mailing List

A blindspot that I've noticed in git is that it's not possible to
properly view a stash entry that has untracked files via `git stash
show`. Teach `git stash show --include-untracked` which should do this.
In addition, this series also teaches `--only-untracked` and the
`stash.showIncludeUntracked` config option.

The first seven patches of this series are just some clean up that I've
done prior to working (because it bothers me). The remaining two patches
should be the meat of the change.

Denton Liu (9):
  git-stash.txt: be explicit about subcommand options
  t3905: remove spaces after redirect operators
  t3905: move all commands into test cases
  t3905: remove nested git in command substitution
  t3905: replace test -s with test_file_not_empty
  t3905: use test_cmp() to check file contents
  stash: declare ref_stash as an array
  stash show: teach --include-tracked and --only-untracked
  stash show: learn stash.showIncludeUntracked

 Documentation/config/stash.txt         |   5 +
 Documentation/git-stash.txt            |  22 +-
 builtin/stash.c                        |  30 ++-
 contrib/completion/git-completion.bash |   2 +-
 t/t3905-stash-include-untracked.sh     | 278 +++++++++++++++++--------
 5 files changed, 235 insertions(+), 102 deletions(-)

-- 
2.30.0.478.g8a0d178c01


^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH 0/9] stash show: learn --include-untracked and --only-untracked
  2021-02-02  9:31 [PATCH 0/9] stash show: learn --include-untracked and --only-untracked Denton Liu
@ 2021-02-02  9:33 ` Denton Liu
  2021-02-02  9:33 ` [PATCH 1/9] git-stash.txt: be explicit about subcommand options Denton Liu
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 46+ messages in thread
From: Denton Liu @ 2021-02-02  9:33 UTC (permalink / raw)
  To: Git Mailing List

A blindspot that I've noticed in git is that it's not possible to
properly view a stash entry that has untracked files via `git stash
show`. Teach `git stash show --include-untracked` which should do this.
In addition, this series also teaches `--only-untracked` and the
`stash.showIncludeUntracked` config option.

The first seven patches of this series are just some clean up that I've
done prior to working (because it bothers me). The remaining two patches
should be the meat of the change.

Denton Liu (9):
  git-stash.txt: be explicit about subcommand options
  t3905: remove spaces after redirect operators
  t3905: move all commands into test cases
  t3905: remove nested git in command substitution
  t3905: replace test -s with test_file_not_empty
  t3905: use test_cmp() to check file contents
  stash: declare ref_stash as an array
  stash show: teach --include-tracked and --only-untracked
  stash show: learn stash.showIncludeUntracked

 Documentation/config/stash.txt         |   5 +
 Documentation/git-stash.txt            |  22 +-
 builtin/stash.c                        |  30 ++-
 contrib/completion/git-completion.bash |   2 +-
 t/t3905-stash-include-untracked.sh     | 278 +++++++++++++++++--------
 5 files changed, 235 insertions(+), 102 deletions(-)

-- 
2.30.0.478.g8a0d178c01


^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH 1/9] git-stash.txt: be explicit about subcommand options
  2021-02-02  9:31 [PATCH 0/9] stash show: learn --include-untracked and --only-untracked Denton Liu
  2021-02-02  9:33 ` Denton Liu
@ 2021-02-02  9:33 ` Denton Liu
  2021-02-02 17:37   ` Eric Sunshine
  2021-02-02  9:33 ` [PATCH 2/9] t3905: remove spaces after redirect operators Denton Liu
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 46+ messages in thread
From: Denton Liu @ 2021-02-02  9:33 UTC (permalink / raw)
  To: Git Mailing List

Currently, the options for the `list` and `show` subcommands are just
listed as `<options>`. This seems to imply, from a cursory glance at the
summary, that they take the stash options listed below. However, reading
more carefully, we see that they take log options and diff options
respectively.

Make it more obvious that they take log and diff options by explicitly
stating this in the subcommand summary.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 Documentation/git-stash.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 31f1beb65b..46ee37b35a 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -67,7 +67,7 @@ save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q
 	Instead, all non-option arguments are concatenated to form the stash
 	message.
 
-list [<options>]::
+list [<log options>]::
 
 	List the stash entries that you currently have.  Each 'stash entry' is
 	listed with its name (e.g. `stash@{0}` is the latest entry, `stash@{1}` is
@@ -83,7 +83,7 @@ stash@{1}: On master: 9cc0589... Add git-stash
 The command takes options applicable to the 'git log'
 command to control what is shown and how. See linkgit:git-log[1].
 
-show [<options>] [<stash>]::
+show [<diff options>] [<stash>]::
 
 	Show the changes recorded in the stash entry as a diff between the
 	stashed contents and the commit back when the stash entry was first
-- 
2.30.0.478.g8a0d178c01


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 2/9] t3905: remove spaces after redirect operators
  2021-02-02  9:31 [PATCH 0/9] stash show: learn --include-untracked and --only-untracked Denton Liu
  2021-02-02  9:33 ` Denton Liu
  2021-02-02  9:33 ` [PATCH 1/9] git-stash.txt: be explicit about subcommand options Denton Liu
@ 2021-02-02  9:33 ` Denton Liu
  2021-02-02  9:33 ` [PATCH 3/9] t3905: move all commands into test cases Denton Liu
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 46+ messages in thread
From: Denton Liu @ 2021-02-02  9:33 UTC (permalink / raw)
  To: Git Mailing List

For shell scripts, the usual convention is for there to be no space
after redirection operators, (e.g. `>file`, not `> file`). Remove these
spaces wherever they appear.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t3905-stash-include-untracked.sh | 40 +++++++++++++++---------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
index f075c7f1f3..1d416944b7 100755
--- a/t/t3905-stash-include-untracked.sh
+++ b/t/t3905-stash-include-untracked.sh
@@ -8,16 +8,16 @@ test_description='Test git stash --include-untracked'
 . ./test-lib.sh
 
 test_expect_success 'stash save --include-untracked some dirty working directory' '
-	echo 1 > file &&
+	echo 1 >file &&
 	git add file &&
 	test_tick &&
 	git commit -m initial &&
-	echo 2 > file &&
+	echo 2 >file &&
 	git add file &&
-	echo 3 > file &&
+	echo 3 >file &&
 	test_tick &&
-	echo 1 > file2 &&
-	echo 1 > HEAD &&
+	echo 1 >file2 &&
+	echo 1 >HEAD &&
 	mkdir untracked &&
 	echo untracked >untracked/untracked &&
 	git stash --include-untracked &&
@@ -25,7 +25,7 @@ test_expect_success 'stash save --include-untracked some dirty working directory
 	git diff-index --cached --quiet HEAD
 '
 
-cat > expect <<EOF
+cat >expect <<EOF
 ?? actual
 ?? expect
 EOF
@@ -37,7 +37,7 @@ test_expect_success 'stash save --include-untracked cleaned the untracked files'
 
 tracked=$(git rev-parse --short $(echo 1 | git hash-object --stdin))
 untracked=$(git rev-parse --short $(echo untracked | git hash-object --stdin))
-cat > expect.diff <<EOF
+cat >expect.diff <<EOF
 diff --git a/HEAD b/HEAD
 new file mode 100644
 index 0000000..$tracked
@@ -60,7 +60,7 @@ index 0000000..$untracked
 @@ -0,0 +1 @@
 +untracked
 EOF
-cat > expect.lstree <<EOF
+cat >expect.lstree <<EOF
 HEAD
 file2
 untracked
@@ -85,7 +85,7 @@ test_expect_success 'stash save --patch --all fails' '
 
 git clean --force --quiet
 
-cat > expect <<EOF
+cat >expect <<EOF
  M file
 ?? HEAD
 ?? actual
@@ -105,14 +105,14 @@ test_expect_success 'stash pop after save --include-untracked leaves files untra
 git clean --force --quiet -d
 
 test_expect_success 'stash save -u dirty index' '
-	echo 4 > file3 &&
+	echo 4 >file3 &&
 	git add file3 &&
 	test_tick &&
 	git stash -u
 '
 
 blob=$(git rev-parse --short $(echo 4 | git hash-object --stdin))
-cat > expect <<EOF
+cat >expect <<EOF
 diff --git a/file3 b/file3
 new file mode 100644
 index 0000000..$blob
@@ -128,12 +128,12 @@ test_expect_success 'stash save --include-untracked dirty index got stashed' '
 	test_cmp expect actual
 '
 
-git reset > /dev/null
+git reset >/dev/null
 
 # Must direct output somewhere where it won't be considered an untracked file
 test_expect_success 'stash save --include-untracked -q is quiet' '
-	echo 1 > file5 &&
-	git stash save --include-untracked --quiet > .git/stash-output.out 2>&1 &&
+	echo 1 >file5 &&
+	git stash save --include-untracked --quiet >.git/stash-output.out 2>&1 &&
 	test_line_count = 0 .git/stash-output.out &&
 	rm -f .git/stash-output.out
 '
@@ -141,7 +141,7 @@ test_expect_success 'stash save --include-untracked -q is quiet' '
 test_expect_success 'stash save --include-untracked removed files' '
 	rm -f file &&
 	git stash save --include-untracked &&
-	echo 1 > expect &&
+	echo 1 >expect &&
 	test_cmp expect file
 '
 
@@ -152,14 +152,14 @@ test_expect_success 'stash save --include-untracked removed files got stashed' '
 	test_path_is_missing file
 '
 
-cat > .gitignore <<EOF
+cat >.gitignore <<EOF
 .gitignore
 ignored
 ignored.d/
 EOF
 
 test_expect_success 'stash save --include-untracked respects .gitignore' '
-	echo ignored > ignored &&
+	echo ignored >ignored &&
 	mkdir ignored.d &&
 	echo ignored >ignored.d/untracked &&
 	git stash -u &&
@@ -169,7 +169,7 @@ test_expect_success 'stash save --include-untracked respects .gitignore' '
 '
 
 test_expect_success 'stash save -u can stash with only untracked files different' '
-	echo 4 > file4 &&
+	echo 4 >file4 &&
 	git stash -u &&
 	test_path_is_missing file4
 '
@@ -214,7 +214,7 @@ test_expect_success 'stash push with $IFS character' '
 	test_path_is_file bar
 '
 
-cat > .gitignore <<EOF
+cat >.gitignore <<EOF
 ignored
 ignored.d/*
 EOF
@@ -224,7 +224,7 @@ test_expect_success 'stash previously ignored file' '
 	git add .gitignore &&
 	git commit -m "Add .gitignore" &&
 	>ignored.d/foo &&
-	echo "!ignored.d/foo" >> .gitignore &&
+	echo "!ignored.d/foo" >>.gitignore &&
 	git stash save --include-untracked &&
 	test_path_is_missing ignored.d/foo &&
 	git stash pop &&
-- 
2.30.0.478.g8a0d178c01


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 3/9] t3905: move all commands into test cases
  2021-02-02  9:31 [PATCH 0/9] stash show: learn --include-untracked and --only-untracked Denton Liu
                   ` (2 preceding siblings ...)
  2021-02-02  9:33 ` [PATCH 2/9] t3905: remove spaces after redirect operators Denton Liu
@ 2021-02-02  9:33 ` Denton Liu
  2021-02-02 21:41   ` Junio C Hamano
  2021-02-02  9:33 ` [PATCH 4/9] t3905: remove nested git in command substitution Denton Liu
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 46+ messages in thread
From: Denton Liu @ 2021-02-02  9:33 UTC (permalink / raw)
  To: Git Mailing List

In order to modernize the tests, move commands that currently run
outside of test cases into a test case. Where possible, clean up files
that are produced using test_when_finished() but in the case where files
persist over multiple test cases, create a new test case to perform
cleanup.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t3905-stash-include-untracked.sh | 147 +++++++++++++++--------------
 1 file changed, 75 insertions(+), 72 deletions(-)

diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
index 1d416944b7..892a2c8057 100755
--- a/t/t3905-stash-include-untracked.sh
+++ b/t/t3905-stash-include-untracked.sh
@@ -25,48 +25,48 @@ test_expect_success 'stash save --include-untracked some dirty working directory
 	git diff-index --cached --quiet HEAD
 '
 
-cat >expect <<EOF
-?? actual
-?? expect
-EOF
-
 test_expect_success 'stash save --include-untracked cleaned the untracked files' '
+	cat >expect <<-EOF &&
+	?? actual
+	?? expect
+	EOF
+
 	git status --porcelain >actual &&
 	test_cmp expect actual
 '
 
-tracked=$(git rev-parse --short $(echo 1 | git hash-object --stdin))
-untracked=$(git rev-parse --short $(echo untracked | git hash-object --stdin))
-cat >expect.diff <<EOF
-diff --git a/HEAD b/HEAD
-new file mode 100644
-index 0000000..$tracked
---- /dev/null
-+++ b/HEAD
-@@ -0,0 +1 @@
-+1
-diff --git a/file2 b/file2
-new file mode 100644
-index 0000000..$tracked
---- /dev/null
-+++ b/file2
-@@ -0,0 +1 @@
-+1
-diff --git a/untracked/untracked b/untracked/untracked
-new file mode 100644
-index 0000000..$untracked
---- /dev/null
-+++ b/untracked/untracked
-@@ -0,0 +1 @@
-+untracked
-EOF
-cat >expect.lstree <<EOF
-HEAD
-file2
-untracked
-EOF
-
 test_expect_success 'stash save --include-untracked stashed the untracked files' '
+	tracked=$(git rev-parse --short $(echo 1 | git hash-object --stdin)) &&
+	untracked=$(git rev-parse --short $(echo untracked | git hash-object --stdin)) &&
+	cat >expect.diff <<-EOF &&
+	diff --git a/HEAD b/HEAD
+	new file mode 100644
+	index 0000000..$tracked
+	--- /dev/null
+	+++ b/HEAD
+	@@ -0,0 +1 @@
+	+1
+	diff --git a/file2 b/file2
+	new file mode 100644
+	index 0000000..$tracked
+	--- /dev/null
+	+++ b/file2
+	@@ -0,0 +1 @@
+	+1
+	diff --git a/untracked/untracked b/untracked/untracked
+	new file mode 100644
+	index 0000000..$untracked
+	--- /dev/null
+	+++ b/untracked/untracked
+	@@ -0,0 +1 @@
+	+untracked
+	EOF
+	cat >expect.lstree <<-EOF &&
+	HEAD
+	file2
+	untracked
+	EOF
+
 	test_path_is_missing file2 &&
 	test_path_is_missing untracked &&
 	test_path_is_missing HEAD &&
@@ -83,18 +83,21 @@ test_expect_success 'stash save --patch --all fails' '
 	test_must_fail git stash --patch --all
 '
 
-git clean --force --quiet
+test_expect_success 'clean up untracked/untracked file to prepare for next tests' '
+	git clean --force --quiet
 
-cat >expect <<EOF
- M file
-?? HEAD
-?? actual
-?? expect
-?? file2
-?? untracked/
-EOF
+'
 
 test_expect_success 'stash pop after save --include-untracked leaves files untracked again' '
+	cat >expect <<-EOF &&
+	 M file
+	?? HEAD
+	?? actual
+	?? expect
+	?? file2
+	?? untracked/
+	EOF
+
 	git stash pop &&
 	git status --porcelain >actual &&
 	test_cmp expect actual &&
@@ -102,7 +105,9 @@ test_expect_success 'stash pop after save --include-untracked leaves files untra
 	test untracked = "$(cat untracked/untracked)"
 '
 
-git clean --force --quiet -d
+test_expect_success 'clean up untracked/ directory to prepare for next tests' '
+	git clean --force --quiet -d
+'
 
 test_expect_success 'stash save -u dirty index' '
 	echo 4 >file3 &&
@@ -111,25 +116,24 @@ test_expect_success 'stash save -u dirty index' '
 	git stash -u
 '
 
-blob=$(git rev-parse --short $(echo 4 | git hash-object --stdin))
-cat >expect <<EOF
-diff --git a/file3 b/file3
-new file mode 100644
-index 0000000..$blob
---- /dev/null
-+++ b/file3
-@@ -0,0 +1 @@
-+4
-EOF
-
 test_expect_success 'stash save --include-untracked dirty index got stashed' '
+	blob=$(git rev-parse --short $(echo 4 | git hash-object --stdin)) &&
+	cat >expect <<-EOF &&
+	diff --git a/file3 b/file3
+	new file mode 100644
+	index 0000000..$blob
+	--- /dev/null
+	+++ b/file3
+	@@ -0,0 +1 @@
+	+4
+	EOF
+
 	git stash pop --index &&
+	test_when_finished "git reset" &&
 	git diff --cached >actual &&
 	test_cmp expect actual
 '
 
-git reset >/dev/null
-
 # Must direct output somewhere where it won't be considered an untracked file
 test_expect_success 'stash save --include-untracked -q is quiet' '
 	echo 1 >file5 &&
@@ -142,23 +146,22 @@ test_expect_success 'stash save --include-untracked removed files' '
 	rm -f file &&
 	git stash save --include-untracked &&
 	echo 1 >expect &&
+	test_when_finished "rm -f expect" &&
 	test_cmp expect file
 '
 
-rm -f expect
-
 test_expect_success 'stash save --include-untracked removed files got stashed' '
 	git stash pop &&
 	test_path_is_missing file
 '
 
-cat >.gitignore <<EOF
-.gitignore
-ignored
-ignored.d/
-EOF
-
 test_expect_success 'stash save --include-untracked respects .gitignore' '
+	cat >.gitignore <<-EOF &&
+	.gitignore
+	ignored
+	ignored.d/
+	EOF
+
 	echo ignored >ignored &&
 	mkdir ignored.d &&
 	echo ignored >ignored.d/untracked &&
@@ -214,12 +217,12 @@ test_expect_success 'stash push with $IFS character' '
 	test_path_is_file bar
 '
 
-cat >.gitignore <<EOF
-ignored
-ignored.d/*
-EOF
-
 test_expect_success 'stash previously ignored file' '
+	cat >.gitignore <<-EOF &&
+	ignored
+	ignored.d/*
+	EOF
+
 	git reset HEAD &&
 	git add .gitignore &&
 	git commit -m "Add .gitignore" &&
-- 
2.30.0.478.g8a0d178c01


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 4/9] t3905: remove nested git in command substitution
  2021-02-02  9:31 [PATCH 0/9] stash show: learn --include-untracked and --only-untracked Denton Liu
                   ` (3 preceding siblings ...)
  2021-02-02  9:33 ` [PATCH 3/9] t3905: move all commands into test cases Denton Liu
@ 2021-02-02  9:33 ` Denton Liu
  2021-02-02  9:33 ` [PATCH 5/9] t3905: replace test -s with test_file_not_empty Denton Liu
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 46+ messages in thread
From: Denton Liu @ 2021-02-02  9:33 UTC (permalink / raw)
  To: Git Mailing List

If a git command in a nested command substitution fails, it will be
silently ignored since only the return code of the outer command
substitutions is reported. Factor out nested command substitutions so
that the error codes of those commands are reported.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t3905-stash-include-untracked.sh | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
index 892a2c8057..f008e5d945 100755
--- a/t/t3905-stash-include-untracked.sh
+++ b/t/t3905-stash-include-untracked.sh
@@ -36,8 +36,10 @@ test_expect_success 'stash save --include-untracked cleaned the untracked files'
 '
 
 test_expect_success 'stash save --include-untracked stashed the untracked files' '
-	tracked=$(git rev-parse --short $(echo 1 | git hash-object --stdin)) &&
-	untracked=$(git rev-parse --short $(echo untracked | git hash-object --stdin)) &&
+	one_blob=$(echo 1 | git hash-object --stdin) &&
+	tracked=$(git rev-parse --short "$one_blob") &&
+	untracked_blob=$(echo untracked | git hash-object --stdin) &&
+	untracked=$(git rev-parse --short "$untracked_blob") &&
 	cat >expect.diff <<-EOF &&
 	diff --git a/HEAD b/HEAD
 	new file mode 100644
@@ -117,7 +119,8 @@ test_expect_success 'stash save -u dirty index' '
 '
 
 test_expect_success 'stash save --include-untracked dirty index got stashed' '
-	blob=$(git rev-parse --short $(echo 4 | git hash-object --stdin)) &&
+	four_blob=$(echo 4 | git hash-object --stdin) &&
+	blob=$(git rev-parse --short "$four_blob") &&
 	cat >expect <<-EOF &&
 	diff --git a/file3 b/file3
 	new file mode 100644
-- 
2.30.0.478.g8a0d178c01


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 5/9] t3905: replace test -s with test_file_not_empty
  2021-02-02  9:31 [PATCH 0/9] stash show: learn --include-untracked and --only-untracked Denton Liu
                   ` (4 preceding siblings ...)
  2021-02-02  9:33 ` [PATCH 4/9] t3905: remove nested git in command substitution Denton Liu
@ 2021-02-02  9:33 ` Denton Liu
  2021-02-02  9:33 ` [PATCH 6/9] t3905: use test_cmp() to check file contents Denton Liu
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 46+ messages in thread
From: Denton Liu @ 2021-02-02  9:33 UTC (permalink / raw)
  To: Git Mailing List

In order to modernize the test script, replace `test -s` with
test_file_not_empty(), which provides better diagnostic output in the
case of failure.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t3905-stash-include-untracked.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
index f008e5d945..c87ac24042 100755
--- a/t/t3905-stash-include-untracked.sh
+++ b/t/t3905-stash-include-untracked.sh
@@ -169,9 +169,9 @@ test_expect_success 'stash save --include-untracked respects .gitignore' '
 	mkdir ignored.d &&
 	echo ignored >ignored.d/untracked &&
 	git stash -u &&
-	test -s ignored &&
-	test -s ignored.d/untracked &&
-	test -s .gitignore
+	test_file_not_empty ignored &&
+	test_file_not_empty ignored.d/untracked &&
+	test_file_not_empty .gitignore
 '
 
 test_expect_success 'stash save -u can stash with only untracked files different' '
@@ -189,9 +189,9 @@ test_expect_success 'stash save --all does not respect .gitignore' '
 
 test_expect_success 'stash save --all is stash poppable' '
 	git stash pop &&
-	test -s ignored &&
-	test -s ignored.d/untracked &&
-	test -s .gitignore
+	test_file_not_empty ignored &&
+	test_file_not_empty ignored.d/untracked &&
+	test_file_not_empty .gitignore
 '
 
 test_expect_success 'stash push --include-untracked with pathspec' '
-- 
2.30.0.478.g8a0d178c01


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 6/9] t3905: use test_cmp() to check file contents
  2021-02-02  9:31 [PATCH 0/9] stash show: learn --include-untracked and --only-untracked Denton Liu
                   ` (5 preceding siblings ...)
  2021-02-02  9:33 ` [PATCH 5/9] t3905: replace test -s with test_file_not_empty Denton Liu
@ 2021-02-02  9:33 ` Denton Liu
  2021-02-02  9:33 ` [PATCH 7/9] stash: declare ref_stash as an array Denton Liu
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 46+ messages in thread
From: Denton Liu @ 2021-02-02  9:33 UTC (permalink / raw)
  To: Git Mailing List

Modernize the script by doing file content comparisons using test_cmp()
instead of `test x = "$(cat file)"`.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t3905-stash-include-untracked.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
index c87ac24042..b26a97aef4 100755
--- a/t/t3905-stash-include-untracked.sh
+++ b/t/t3905-stash-include-untracked.sh
@@ -103,8 +103,10 @@ test_expect_success 'stash pop after save --include-untracked leaves files untra
 	git stash pop &&
 	git status --porcelain >actual &&
 	test_cmp expect actual &&
-	test "1" = "$(cat file2)" &&
-	test untracked = "$(cat untracked/untracked)"
+	echo 1 >expect_file2 &&
+	test_cmp expect_file2 file2 &&
+	echo untracked >untracked_expect &&
+	test_cmp untracked_expect untracked/untracked
 '
 
 test_expect_success 'clean up untracked/ directory to prepare for next tests' '
-- 
2.30.0.478.g8a0d178c01


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 7/9] stash: declare ref_stash as an array
  2021-02-02  9:31 [PATCH 0/9] stash show: learn --include-untracked and --only-untracked Denton Liu
                   ` (6 preceding siblings ...)
  2021-02-02  9:33 ` [PATCH 6/9] t3905: use test_cmp() to check file contents Denton Liu
@ 2021-02-02  9:33 ` Denton Liu
  2021-02-02 21:46   ` Junio C Hamano
  2021-02-02  9:33 ` [PATCH 8/9] stash show: teach --include-tracked and --only-untracked Denton Liu
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 46+ messages in thread
From: Denton Liu @ 2021-02-02  9:33 UTC (permalink / raw)
  To: Git Mailing List

Save sizeof(const char *) bytes by declaring ref_stash as an array
instead of having a redundant pointer to an array.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 builtin/stash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/stash.c b/builtin/stash.c
index 9bc85f91cd..6f2b58f6ab 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -87,7 +87,7 @@ static const char * const git_stash_save_usage[] = {
 	NULL
 };
 
-static const char *ref_stash = "refs/stash";
+static const char ref_stash[] = "refs/stash";
 static struct strbuf stash_index_path = STRBUF_INIT;
 
 /*
-- 
2.30.0.478.g8a0d178c01


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 8/9] stash show: teach --include-tracked and --only-untracked
  2021-02-02  9:31 [PATCH 0/9] stash show: learn --include-untracked and --only-untracked Denton Liu
                   ` (7 preceding siblings ...)
  2021-02-02  9:33 ` [PATCH 7/9] stash: declare ref_stash as an array Denton Liu
@ 2021-02-02  9:33 ` Denton Liu
  2021-02-02 21:56   ` Junio C Hamano
  2021-02-02  9:33 ` [PATCH 9/9] stash show: learn stash.showIncludeUntracked Denton Liu
  2021-02-09  7:28 ` [PATCH v2 0/9] stash show: learn --include-untracked and --only-untracked Denton Liu
  10 siblings, 1 reply; 46+ messages in thread
From: Denton Liu @ 2021-02-02  9:33 UTC (permalink / raw)
  To: Git Mailing List

Stash entries can be made with untracked files via
`git stash push --include-untracked`. However, because the untracked
files are stored in the third parent of the stash entry and not the
stash entry itself, running `git stash show` does not include the
untracked files as part of the diff.

Teach stash the --include-tracked option, which also displays the
untracked files in a stash entry from the third parent (if it exists).
Do this by just concatenating the diff of the third parent against an
empty tree. One limitation of this is that it would be possible to
manually craft a stash entry which would present duplicate entries in
the diff by duplicating a file in the stash and in the third parent.
This seems like an instance of "Doctor, it hurts when I do this! So
don't do that!" so this can be written off.

Also, teach stash the --only-untracked option which only shows the
untracked files of a stash entry. This is similar to `git show stash^3`
but it is nice to provide a convenient abstraction for it so that users
do not have to think about the underlying implementation.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 Documentation/git-stash.txt            | 16 +++--
 builtin/stash.c                        | 20 +++++-
 contrib/completion/git-completion.bash |  2 +-
 t/t3905-stash-include-untracked.sh     | 84 ++++++++++++++++++++++++++
 4 files changed, 116 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 46ee37b35a..a9956e5c51 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -83,7 +83,7 @@ stash@{1}: On master: 9cc0589... Add git-stash
 The command takes options applicable to the 'git log'
 command to control what is shown and how. See linkgit:git-log[1].
 
-show [<diff options>] [<stash>]::
+show [-u|--include-untracked|--only-untracked] [<diff options>] [<stash>]::
 
 	Show the changes recorded in the stash entry as a diff between the
 	stashed contents and the commit back when the stash entry was first
@@ -160,10 +160,18 @@ up with `git clean`.
 
 -u::
 --include-untracked::
-	This option is only valid for `push` and `save` commands.
+--no-include-untracked::
+	When used with the `push` and `save` commands,
+	all untracked files are also stashed and then cleaned up with
+	`git clean`.
 +
-All untracked files are also stashed and then cleaned up with
-`git clean`.
+When used with the `show` command, show the untracked files in the stash
+entry as part of the diff.
+
+--only-untracked::
+	This option is only valid for the `show` command.
++
+Show only the untracked files in the stash entry as part of the diff.
 
 --index::
 	This option is only valid for `pop` and `apply` commands.
diff --git a/builtin/stash.c b/builtin/stash.c
index 6f2b58f6ab..7a8770676b 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -795,7 +795,18 @@ static int show_stash(int argc, const char **argv, const char *prefix)
 	struct rev_info rev;
 	struct strvec stash_args = STRVEC_INIT;
 	struct strvec revision_args = STRVEC_INIT;
+	enum {
+		UNTRACKED_NONE,
+		UNTRACKED_INCLUDE,
+		UNTRACKED_ONLY
+	} show_untracked = UNTRACKED_NONE;
 	struct option options[] = {
+		OPT_SET_INT('u', "include-untracked", &show_untracked,
+			    N_("include untracked files in the stash"),
+			    UNTRACKED_INCLUDE),
+		OPT_SET_INT_F(0, "only-untracked", &show_untracked,
+			      N_("only show untracked files in the stash"),
+			      UNTRACKED_ONLY, PARSE_OPT_NONEG),
 		OPT_END()
 	};
 
@@ -803,6 +814,10 @@ static int show_stash(int argc, const char **argv, const char *prefix)
 	git_config(git_diff_ui_config, NULL);
 	init_revisions(&rev, prefix);
 
+	argc = parse_options(argc, argv, prefix, options, git_stash_show_usage,
+			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |
+			     PARSE_OPT_KEEP_DASHDASH);
+
 	strvec_push(&revision_args, argv[0]);
 	for (i = 1; i < argc; i++) {
 		if (argv[i][0] != '-')
@@ -845,7 +860,10 @@ static int show_stash(int argc, const char **argv, const char *prefix)
 
 	rev.diffopt.flags.recursive = 1;
 	setup_diff_pager(&rev.diffopt);
-	diff_tree_oid(&info.b_commit, &info.w_commit, "", &rev.diffopt);
+	if (show_untracked != UNTRACKED_ONLY)
+		diff_tree_oid(&info.b_commit, &info.w_commit, "", &rev.diffopt);
+	if (show_untracked != UNTRACKED_NONE && info.has_u)
+		diff_root_tree_oid(&info.u_tree, "", &rev.diffopt);
 	log_tree_diff_flush(&rev);
 
 	free_stash_info(&info);
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 4b1f4264a6..64ef6ffa21 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3051,7 +3051,7 @@ _git_stash ()
 			__gitcomp "--name-status --oneline --patch-with-stat"
 			;;
 		show,--*)
-			__gitcomp "$__git_diff_common_options"
+			__gitcomp "--include-untracked --only-untracked $__git_diff_common_options"
 			;;
 		branch,--*)
 			;;
diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
index b26a97aef4..978bc97baf 100755
--- a/t/t3905-stash-include-untracked.sh
+++ b/t/t3905-stash-include-untracked.sh
@@ -297,4 +297,88 @@ test_expect_success 'stash -u with globs' '
 	test_path_is_missing untracked.txt
 '
 
+test_expect_success 'stash show --include-untracked shows untracked files' '
+	git reset --hard &&
+	git clean -xf &&
+	>untracked &&
+	>tracked &&
+	git add tracked &&
+	git stash -u &&
+
+	cat >expect <<-EOF &&
+	 tracked   | 0
+	 untracked | 0
+	 2 files changed, 0 insertions(+), 0 deletions(-)
+	EOF
+	git stash show --include-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show -u >actual &&
+	test_cmp expect actual &&
+	git stash show --no-include-untracked --include-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show --only-untracked --include-untracked >actual &&
+	test_cmp expect actual &&
+
+	cat >expect <<-EOF &&
+	diff --git a/tracked b/tracked
+	new file mode 100644
+	index 0000000..e69de29
+	diff --git a/untracked b/untracked
+	new file mode 100644
+	index 0000000..e69de29
+	EOF
+	git stash show -p --include-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show --include-untracked -p >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'stash show --only-untracked only shows untracked files' '
+	git reset --hard &&
+	git clean -xf &&
+	>untracked &&
+	>tracked &&
+	git add tracked &&
+	git stash -u &&
+
+	cat >expect <<-EOF &&
+	 untracked | 0
+	 1 file changed, 0 insertions(+), 0 deletions(-)
+	EOF
+	git stash show --only-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show --no-include-untracked --only-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show --include-untracked --only-untracked >actual &&
+	test_cmp expect actual &&
+
+	cat >expect <<-EOF &&
+	diff --git a/untracked b/untracked
+	new file mode 100644
+	index 0000000..e69de29
+	EOF
+	git stash show -p --only-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show --only-untracked -p >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'stash show --no-include-untracked cancels --{include,show}-untracked' '
+	git reset --hard &&
+	git clean -xf &&
+	>untracked &&
+	>tracked &&
+	git add tracked &&
+	git stash -u &&
+
+	cat >expect <<-EOF &&
+	 tracked | 0
+	 1 file changed, 0 insertions(+), 0 deletions(-)
+	EOF
+	git stash show --only-untracked --no-include-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show --include-untracked --no-include-untracked >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.30.0.478.g8a0d178c01


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH 9/9] stash show: learn stash.showIncludeUntracked
  2021-02-02  9:31 [PATCH 0/9] stash show: learn --include-untracked and --only-untracked Denton Liu
                   ` (8 preceding siblings ...)
  2021-02-02  9:33 ` [PATCH 8/9] stash show: teach --include-tracked and --only-untracked Denton Liu
@ 2021-02-02  9:33 ` Denton Liu
  2021-02-09  7:28 ` [PATCH v2 0/9] stash show: learn --include-untracked and --only-untracked Denton Liu
  10 siblings, 0 replies; 46+ messages in thread
From: Denton Liu @ 2021-02-02  9:33 UTC (permalink / raw)
  To: Git Mailing List

The previous commit teaches `git stash show --include-untracked`. It
may be desirable for a user to be able to always enable the
--include-untracked behavior. Teach the stash.showIncludeUntracked
config option which allows users to do this in a similar manner to
stash.showPatch.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 Documentation/config/stash.txt     | 5 +++++
 Documentation/git-stash.txt        | 4 ++--
 builtin/stash.c                    | 8 ++++++++
 t/t3905-stash-include-untracked.sh | 2 ++
 4 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/Documentation/config/stash.txt b/Documentation/config/stash.txt
index 00eb35434e..413f907cba 100644
--- a/Documentation/config/stash.txt
+++ b/Documentation/config/stash.txt
@@ -5,6 +5,11 @@ stash.useBuiltin::
 	is always used. Setting this will emit a warning, to alert any
 	remaining users that setting this now does nothing.
 
+stash.showIncludeUntracked::
+	If this is set to true, the `git stash show` command without an
+	option will show the untracked files of a stash entry.  Defaults to
+	false. See description of 'show' command in linkgit:git-stash[1].
+
 stash.showPatch::
 	If this is set to true, the `git stash show` command without an
 	option will show the stash entry in patch form.  Defaults to false.
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index a9956e5c51..e18c1deb97 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -91,8 +91,8 @@ show [-u|--include-untracked|--only-untracked] [<diff options>] [<stash>]::
 	By default, the command shows the diffstat, but it will accept any
 	format known to 'git diff' (e.g., `git stash show -p stash@{1}`
 	to view the second most recent entry in patch form).
-	You can use stash.showStat and/or stash.showPatch config variables
-	to change the default behavior.
+	You can use stash.showIncludeUntracked, stash.showStat, and
+	stash.showPatch config variables to change the default behavior.
 
 pop [--index] [-q|--quiet] [<stash>]::
 
diff --git a/builtin/stash.c b/builtin/stash.c
index 7a8770676b..784fb518be 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -768,6 +768,7 @@ static int list_stash(int argc, const char **argv, const char *prefix)
 
 static int show_stat = 1;
 static int show_patch;
+static int show_include_untracked;
 static int use_legacy_stash;
 
 static int git_stash_config(const char *var, const char *value, void *cb)
@@ -780,6 +781,10 @@ static int git_stash_config(const char *var, const char *value, void *cb)
 		show_patch = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "stash.showincludeuntracked")) {
+		show_include_untracked = git_config_bool(var, value);
+		return 0;
+	}
 	if (!strcmp(var, "stash.usebuiltin")) {
 		use_legacy_stash = !git_config_bool(var, value);
 		return 0;
@@ -842,6 +847,9 @@ static int show_stash(int argc, const char **argv, const char *prefix)
 		if (show_patch)
 			rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
 
+		if (show_include_untracked)
+			show_untracked = UNTRACKED_INCLUDE;
+
 		if (!show_stat && !show_patch) {
 			free_stash_info(&info);
 			return 0;
diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
index 978bc97baf..8bcd4c5ca8 100755
--- a/t/t3905-stash-include-untracked.sh
+++ b/t/t3905-stash-include-untracked.sh
@@ -318,6 +318,8 @@ test_expect_success 'stash show --include-untracked shows untracked files' '
 	test_cmp expect actual &&
 	git stash show --only-untracked --include-untracked >actual &&
 	test_cmp expect actual &&
+	git -c stash.showIncludeUntracked=true stash show >actual &&
+	test_cmp expect actual &&
 
 	cat >expect <<-EOF &&
 	diff --git a/tracked b/tracked
-- 
2.30.0.478.g8a0d178c01


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* Re: [PATCH 1/9] git-stash.txt: be explicit about subcommand options
  2021-02-02  9:33 ` [PATCH 1/9] git-stash.txt: be explicit about subcommand options Denton Liu
@ 2021-02-02 17:37   ` Eric Sunshine
  0 siblings, 0 replies; 46+ messages in thread
From: Eric Sunshine @ 2021-02-02 17:37 UTC (permalink / raw)
  To: Denton Liu; +Cc: Git Mailing List

On Tue, Feb 2, 2021 at 4:36 AM Denton Liu <liu.denton@gmail.com> wrote:
> Currently, the options for the `list` and `show` subcommands are just
> listed as `<options>`. This seems to imply, from a cursory glance at the
> summary, that they take the stash options listed below. However, reading
> more carefully, we see that they take log options and diff options
> respectively.
>
> Make it more obvious that they take log and diff options by explicitly
> stating this in the subcommand summary.
>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
> @@ -67,7 +67,7 @@ save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q
> -list [<options>]::
> +list [<log options>]::
>
> -show [<options>] [<stash>]::
> +show [<diff options>] [<stash>]::

I might suggest that it is more common to hyphenate these words than
to separate them with spaces:

    list [<log-options>]::
    show [<diff-options>] [<stash>]::

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 3/9] t3905: move all commands into test cases
  2021-02-02  9:33 ` [PATCH 3/9] t3905: move all commands into test cases Denton Liu
@ 2021-02-02 21:41   ` Junio C Hamano
  0 siblings, 0 replies; 46+ messages in thread
From: Junio C Hamano @ 2021-02-02 21:41 UTC (permalink / raw)
  To: Denton Liu; +Cc: Git Mailing List

Denton Liu <liu.denton@gmail.com> writes:

>  test_expect_success 'stash save --include-untracked stashed the untracked files' '
> +	tracked=$(git rev-parse --short $(echo 1 | git hash-object --stdin)) &&
> +	untracked=$(git rev-parse --short $(echo untracked | git hash-object --stdin)) &&

Not a new issue introduced by this patch, but

 * these will fail if blobs that record "1\n" and "untracked\n" do
   not exist in the repository already, because the hash-object
   command lacks the "-w" option.

 * the reason why they do not fail is because there are these blobs
   already; grabbing them using extended SHA-1 expression may be
   simpler to read, e.g.

	tracked=$(git rev-parse --short HEAD:file)

 * even if it is not trivial to get to such a blob object, it
   probably is easier to read the test if a file that has the
   desired contents in it is used, not an "echo", e.g.

	untracked=$(git rev-parse --short $(git hash-object -w untracked/untracked))

We may want to clean these up someday, but it does not have to be
part of this topic (#leftoverbits).

> +	cat >expect.diff <<-EOF &&
> +	diff --git a/HEAD b/HEAD
> +	new file mode 100644
> +	index 0000000..$tracked
> +	--- /dev/null
> +	+++ b/HEAD
> +	@@ -0,0 +1 @@
> +	+1
> +	diff --git a/file2 b/file2
> +	new file mode 100644
> +	index 0000000..$tracked
> +	--- /dev/null
> +	+++ b/file2
> +	@@ -0,0 +1 @@
> +	+1
> +	diff --git a/untracked/untracked b/untracked/untracked
> +	new file mode 100644
> +	index 0000000..$untracked
> +	--- /dev/null
> +	+++ b/untracked/untracked
> +	@@ -0,0 +1 @@
> +	+untracked
> +	EOF


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 7/9] stash: declare ref_stash as an array
  2021-02-02  9:33 ` [PATCH 7/9] stash: declare ref_stash as an array Denton Liu
@ 2021-02-02 21:46   ` Junio C Hamano
  0 siblings, 0 replies; 46+ messages in thread
From: Junio C Hamano @ 2021-02-02 21:46 UTC (permalink / raw)
  To: Denton Liu; +Cc: Git Mailing List

Denton Liu <liu.denton@gmail.com> writes:

> Save sizeof(const char *) bytes by declaring ref_stash as an array
> instead of having a redundant pointer to an array.

One important prerequisite for this rewrite to work is that nobody
in the code repoints ref_stash pointer to point at a different
string, which has been the case for the past number of years and
will continue to be so.

Looks good.

> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
>  builtin/stash.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/stash.c b/builtin/stash.c
> index 9bc85f91cd..6f2b58f6ab 100644
> --- a/builtin/stash.c
> +++ b/builtin/stash.c
> @@ -87,7 +87,7 @@ static const char * const git_stash_save_usage[] = {
>  	NULL
>  };
>  
> -static const char *ref_stash = "refs/stash";
> +static const char ref_stash[] = "refs/stash";
>  static struct strbuf stash_index_path = STRBUF_INIT;
>  
>  /*

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH 8/9] stash show: teach --include-tracked and --only-untracked
  2021-02-02  9:33 ` [PATCH 8/9] stash show: teach --include-tracked and --only-untracked Denton Liu
@ 2021-02-02 21:56   ` Junio C Hamano
  0 siblings, 0 replies; 46+ messages in thread
From: Junio C Hamano @ 2021-02-02 21:56 UTC (permalink / raw)
  To: Denton Liu; +Cc: Git Mailing List

Denton Liu <liu.denton@gmail.com> writes:

> Stash entries can be made with untracked files via
> `git stash push --include-untracked`. However, because the untracked
> files are stored in the third parent of the stash entry and not the
> stash entry itself, running `git stash show` does not include the
> untracked files as part of the diff.
>
> Teach stash the --include-tracked option, which also displays the

Is that "tracked" or "untracked"?

> untracked files in a stash entry from the third parent (if it exists).
> Do this by just concatenating the diff of the third parent against an
> empty tree. One limitation of this is that it would be possible to
> manually craft a stash entry which would present duplicate entries in
> the diff by duplicating a file in the stash and in the third parent.

In other words, a broken "stash" that cannot have be taken with
"stash save -u" may show nonsense?  I wouldn't be so worried about
it, as long as we won't crash in "git stash show".

But a larger downside is that you will have to see all diffs from
the tracked paths from A to Z before you start seeing untracked
paths from A to Z, which is not what people would expect how
"include" behave---it is more like "append as afterthought".  If we
cannot do a good job showing both in a sensible way, I'd rather not
to see us introduce such an incomplete "--include-untracked" option
until we can do so.

The "only-untracked" one does not have such problem, so it is
probably a good feature to add at this moment, though.

This is not in the scope of this topic, but I wonder if it people
want to have the "--only-untracked" option on the "stash apply"
command.

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH v2 0/9] stash show: learn --include-untracked and --only-untracked
  2021-02-02  9:31 [PATCH 0/9] stash show: learn --include-untracked and --only-untracked Denton Liu
                   ` (9 preceding siblings ...)
  2021-02-02  9:33 ` [PATCH 9/9] stash show: learn stash.showIncludeUntracked Denton Liu
@ 2021-02-09  7:28 ` Denton Liu
  2021-02-09  7:28   ` [PATCH v2 1/9] git-stash.txt: be explicit about subcommand options Denton Liu
                     ` (9 more replies)
  10 siblings, 10 replies; 46+ messages in thread
From: Denton Liu @ 2021-02-09  7:28 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Eric Sunshine

A blindspot that I've noticed in git is that it's not possible to
properly view a stash entry that has untracked files via `git stash
show`. Teach `git stash show --include-untracked` which should do this.
In addition, this series also teaches `--only-untracked` and the
`stash.showIncludeUntracked` config option.

The first seven patches of this series are just some clean up that I've
done prior to working (because it bothers me). The remaining two patches
should be the meat of the change.

Changes since v1:

* Add a dash for <log-options> and <diff-options>

* Fix the misspelling of --include-untracked in a commit message

* Change the approach from concatenating diffs to using `git read-tree`

Denton Liu (9):
  git-stash.txt: be explicit about subcommand options
  t3905: remove spaces after redirect operators
  t3905: move all commands into test cases
  t3905: remove nested git in command substitution
  t3905: replace test -s with test_file_not_empty
  t3905: use test_cmp() to check file contents
  stash: declare ref_stash as an array
  stash show: teach --include-untracked and --only-untracked
  stash show: learn stash.showIncludeUntracked

 Documentation/config/stash.txt         |   5 +
 Documentation/git-stash.txt            |  22 +-
 builtin/stash.c                        |  87 +++++++-
 contrib/completion/git-completion.bash |   2 +-
 t/t3905-stash-include-untracked.sh     | 278 +++++++++++++++++--------
 5 files changed, 292 insertions(+), 102 deletions(-)

Range-diff against v1:
 1:  17675b9e4c !  1:  5697f14f1c git-stash.txt: be explicit about subcommand options
    @@ Documentation/git-stash.txt: save [-p|--patch] [-k|--[no-]keep-index] [-u|--incl
      	message.
      
     -list [<options>]::
    -+list [<log options>]::
    ++list [<log-options>]::
      
      	List the stash entries that you currently have.  Each 'stash entry' is
      	listed with its name (e.g. `stash@{0}` is the latest entry, `stash@{1}` is
    @@ Documentation/git-stash.txt: stash@{1}: On master: 9cc0589... Add git-stash
      command to control what is shown and how. See linkgit:git-log[1].
      
     -show [<options>] [<stash>]::
    -+show [<diff options>] [<stash>]::
    ++show [<diff-options>] [<stash>]::
      
      	Show the changes recorded in the stash entry as a diff between the
      	stashed contents and the commit back when the stash entry was first
 2:  0de324e3bc =  2:  45ed17bfe2 t3905: remove spaces after redirect operators
 3:  519840b1a2 =  3:  5bda09b4bd t3905: move all commands into test cases
 4:  4b72d39e01 =  4:  57c21e2461 t3905: remove nested git in command substitution
 5:  7fe27ab620 =  5:  2530883b6c t3905: replace test -s with test_file_not_empty
 6:  4a5dd83ff4 =  6:  80194bcfa5 t3905: use test_cmp() to check file contents
 7:  b5f22de3fc =  7:  2f03d38b36 stash: declare ref_stash as an array
 8:  c2375d1fc6 !  8:  88d4791259 stash show: teach --include-tracked and --only-untracked
    @@ Metadata
     Author: Denton Liu <liu.denton@gmail.com>
     
      ## Commit message ##
    -    stash show: teach --include-tracked and --only-untracked
    +    stash show: teach --include-untracked and --only-untracked
     
         Stash entries can be made with untracked files via
         `git stash push --include-untracked`. However, because the untracked
    @@ Commit message
         stash entry itself, running `git stash show` does not include the
         untracked files as part of the diff.
     
    -    Teach stash the --include-tracked option, which also displays the
    +    Teach stash the --include-untracked option, which also displays the
         untracked files in a stash entry from the third parent (if it exists).
    -    Do this by just concatenating the diff of the third parent against an
    -    empty tree. One limitation of this is that it would be possible to
    -    manually craft a stash entry which would present duplicate entries in
    -    the diff by duplicating a file in the stash and in the third parent.
    -    This seems like an instance of "Doctor, it hurts when I do this! So
    -    don't do that!" so this can be written off.
    +    Do this via something like
    +
    +            GIT_INDEX_FILE=... git read-tree stash stash^3
    +
    +    and diffing the resulting tree object against the stash base.
    +
    +    One improvement that this could use for the future is performing the
    +    action without writing anything to disk as one would expect this to be a
    +    read-only operation. This can be fixed in the future, however.
    +
    +    Another limitation of this is that it would be possible to manually
    +    craft a stash entry where duplicate untracked files in the stash entry
    +    will mask tracked files. This seems like an instance of "Doctor, it
    +    hurts when I do this! So don't do that!" so this can be written off.
     
         Also, teach stash the --only-untracked option which only shows the
         untracked files of a stash entry. This is similar to `git show stash^3`
    @@ Documentation/git-stash.txt: stash@{1}: On master: 9cc0589... Add git-stash
      The command takes options applicable to the 'git log'
      command to control what is shown and how. See linkgit:git-log[1].
      
    --show [<diff options>] [<stash>]::
    -+show [-u|--include-untracked|--only-untracked] [<diff options>] [<stash>]::
    +-show [<diff-options>] [<stash>]::
    ++show [-u|--include-untracked|--only-untracked] [<diff-options>] [<stash>]::
      
      	Show the changes recorded in the stash entry as a diff between the
      	stashed contents and the commit back when the stash entry was first
    @@ Documentation/git-stash.txt: up with `git clean`.
      	This option is only valid for `pop` and `apply` commands.
     
      ## builtin/stash.c ##
    +@@ builtin/stash.c: static int git_stash_config(const char *var, const char *value, void *cb)
    + 	return git_diff_basic_config(var, value, cb);
    + }
    + 
    ++static int merge_track_untracked(struct object_id *result, const struct stash_info *info)
    ++{
    ++	int ret = 0;
    ++	struct index_state istate = { NULL };
    ++	struct child_process cp_read_tree = CHILD_PROCESS_INIT;
    ++
    ++	if (!info->has_u) {
    ++		oidcpy(result, &info->w_commit);
    ++		return 0;
    ++	}
    ++
    ++	/*
    ++	 * TODO: is there a way of doing this all in-memory without writing
    ++	 * anything to disk?
    ++	 */
    ++	remove_path(stash_index_path.buf);
    ++
    ++	cp_read_tree.git_cmd = 1;
    ++	strvec_push(&cp_read_tree.args, "read-tree");
    ++	strvec_push(&cp_read_tree.args, oid_to_hex(&info->w_commit));
    ++	strvec_push(&cp_read_tree.args, oid_to_hex(&info->u_tree));
    ++	strvec_pushf(&cp_read_tree.env_array, "GIT_INDEX_FILE=%s",
    ++		     stash_index_path.buf);
    ++
    ++	if (run_command(&cp_read_tree)) {
    ++		ret = -1;
    ++		goto done;
    ++	}
    ++
    ++	if (write_index_as_tree(result, &istate, stash_index_path.buf, 0,
    ++				NULL)) {
    ++		ret = -1;
    ++		goto done;
    ++	}
    ++
    ++done:
    ++	discard_index(&istate);
    ++	remove_path(stash_index_path.buf);
    ++	return ret;
    ++}
    ++
    + static int show_stash(int argc, const char **argv, const char *prefix)
    + {
    + 	int i;
     @@ builtin/stash.c: static int show_stash(int argc, const char **argv, const char *prefix)
      	struct rev_info rev;
      	struct strvec stash_args = STRVEC_INIT;
      	struct strvec revision_args = STRVEC_INIT;
    ++	struct object_id *before = NULL;
    ++	struct object_id *after = NULL;
    ++	struct object_id untracked_merged_tree;
     +	enum {
     +		UNTRACKED_NONE,
     +		UNTRACKED_INCLUDE,
    @@ builtin/stash.c: static int show_stash(int argc, const char **argv, const char *
      	rev.diffopt.flags.recursive = 1;
      	setup_diff_pager(&rev.diffopt);
     -	diff_tree_oid(&info.b_commit, &info.w_commit, "", &rev.diffopt);
    -+	if (show_untracked != UNTRACKED_ONLY)
    -+		diff_tree_oid(&info.b_commit, &info.w_commit, "", &rev.diffopt);
    -+	if (show_untracked != UNTRACKED_NONE && info.has_u)
    -+		diff_root_tree_oid(&info.u_tree, "", &rev.diffopt);
    ++	switch (show_untracked) {
    ++	case UNTRACKED_NONE:
    ++		before = &info.b_commit;
    ++		after = &info.w_commit;
    ++		break;
    ++	case UNTRACKED_ONLY:
    ++		before = NULL;
    ++		after = &info.u_tree;
    ++		break;
    ++	case UNTRACKED_INCLUDE:
    ++		if (merge_track_untracked(&untracked_merged_tree, &info) < 0)
    ++			die(_("unable merge stash index with untracked files index"));
    ++		before = &info.b_commit;
    ++		after = &untracked_merged_tree;
    ++		break;
    ++	}
    ++	diff_tree_oid(before, after, "", &rev.diffopt);
      	log_tree_diff_flush(&rev);
      
      	free_stash_info(&info);
 9:  2c5d5d9dd4 !  9:  ac4019f47e stash show: learn stash.showIncludeUntracked
    @@ Documentation/config/stash.txt: stash.useBuiltin::
      	option will show the stash entry in patch form.  Defaults to false.
     
      ## Documentation/git-stash.txt ##
    -@@ Documentation/git-stash.txt: show [-u|--include-untracked|--only-untracked] [<diff options>] [<stash>]::
    +@@ Documentation/git-stash.txt: show [-u|--include-untracked|--only-untracked] [<diff-options>] [<stash>]::
      	By default, the command shows the diffstat, but it will accept any
      	format known to 'git diff' (e.g., `git stash show -p stash@{1}`
      	to view the second most recent entry in patch form).
-- 
2.30.0.478.g8a0d178c01


^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH v2 1/9] git-stash.txt: be explicit about subcommand options
  2021-02-09  7:28 ` [PATCH v2 0/9] stash show: learn --include-untracked and --only-untracked Denton Liu
@ 2021-02-09  7:28   ` Denton Liu
  2021-02-10  7:17     ` Junio C Hamano
  2021-02-09  7:28   ` [PATCH v2 2/9] t3905: remove spaces after redirect operators Denton Liu
                     ` (8 subsequent siblings)
  9 siblings, 1 reply; 46+ messages in thread
From: Denton Liu @ 2021-02-09  7:28 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Eric Sunshine

Currently, the options for the `list` and `show` subcommands are just
listed as `<options>`. This seems to imply, from a cursory glance at the
summary, that they take the stash options listed below. However, reading
more carefully, we see that they take log options and diff options
respectively.

Make it more obvious that they take log and diff options by explicitly
stating this in the subcommand summary.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 Documentation/git-stash.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 31f1beb65b..04e55eb826 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -67,7 +67,7 @@ save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q
 	Instead, all non-option arguments are concatenated to form the stash
 	message.
 
-list [<options>]::
+list [<log-options>]::
 
 	List the stash entries that you currently have.  Each 'stash entry' is
 	listed with its name (e.g. `stash@{0}` is the latest entry, `stash@{1}` is
@@ -83,7 +83,7 @@ stash@{1}: On master: 9cc0589... Add git-stash
 The command takes options applicable to the 'git log'
 command to control what is shown and how. See linkgit:git-log[1].
 
-show [<options>] [<stash>]::
+show [<diff-options>] [<stash>]::
 
 	Show the changes recorded in the stash entry as a diff between the
 	stashed contents and the commit back when the stash entry was first
-- 
2.30.0.478.g8a0d178c01


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH v2 2/9] t3905: remove spaces after redirect operators
  2021-02-09  7:28 ` [PATCH v2 0/9] stash show: learn --include-untracked and --only-untracked Denton Liu
  2021-02-09  7:28   ` [PATCH v2 1/9] git-stash.txt: be explicit about subcommand options Denton Liu
@ 2021-02-09  7:28   ` Denton Liu
  2021-02-10  7:18     ` Junio C Hamano
  2021-02-09  7:28   ` [PATCH v2 3/9] t3905: move all commands into test cases Denton Liu
                     ` (7 subsequent siblings)
  9 siblings, 1 reply; 46+ messages in thread
From: Denton Liu @ 2021-02-09  7:28 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Eric Sunshine

For shell scripts, the usual convention is for there to be no space
after redirection operators, (e.g. `>file`, not `> file`). Remove these
spaces wherever they appear.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t3905-stash-include-untracked.sh | 40 +++++++++++++++---------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
index f075c7f1f3..1d416944b7 100755
--- a/t/t3905-stash-include-untracked.sh
+++ b/t/t3905-stash-include-untracked.sh
@@ -8,16 +8,16 @@ test_description='Test git stash --include-untracked'
 . ./test-lib.sh
 
 test_expect_success 'stash save --include-untracked some dirty working directory' '
-	echo 1 > file &&
+	echo 1 >file &&
 	git add file &&
 	test_tick &&
 	git commit -m initial &&
-	echo 2 > file &&
+	echo 2 >file &&
 	git add file &&
-	echo 3 > file &&
+	echo 3 >file &&
 	test_tick &&
-	echo 1 > file2 &&
-	echo 1 > HEAD &&
+	echo 1 >file2 &&
+	echo 1 >HEAD &&
 	mkdir untracked &&
 	echo untracked >untracked/untracked &&
 	git stash --include-untracked &&
@@ -25,7 +25,7 @@ test_expect_success 'stash save --include-untracked some dirty working directory
 	git diff-index --cached --quiet HEAD
 '
 
-cat > expect <<EOF
+cat >expect <<EOF
 ?? actual
 ?? expect
 EOF
@@ -37,7 +37,7 @@ test_expect_success 'stash save --include-untracked cleaned the untracked files'
 
 tracked=$(git rev-parse --short $(echo 1 | git hash-object --stdin))
 untracked=$(git rev-parse --short $(echo untracked | git hash-object --stdin))
-cat > expect.diff <<EOF
+cat >expect.diff <<EOF
 diff --git a/HEAD b/HEAD
 new file mode 100644
 index 0000000..$tracked
@@ -60,7 +60,7 @@ index 0000000..$untracked
 @@ -0,0 +1 @@
 +untracked
 EOF
-cat > expect.lstree <<EOF
+cat >expect.lstree <<EOF
 HEAD
 file2
 untracked
@@ -85,7 +85,7 @@ test_expect_success 'stash save --patch --all fails' '
 
 git clean --force --quiet
 
-cat > expect <<EOF
+cat >expect <<EOF
  M file
 ?? HEAD
 ?? actual
@@ -105,14 +105,14 @@ test_expect_success 'stash pop after save --include-untracked leaves files untra
 git clean --force --quiet -d
 
 test_expect_success 'stash save -u dirty index' '
-	echo 4 > file3 &&
+	echo 4 >file3 &&
 	git add file3 &&
 	test_tick &&
 	git stash -u
 '
 
 blob=$(git rev-parse --short $(echo 4 | git hash-object --stdin))
-cat > expect <<EOF
+cat >expect <<EOF
 diff --git a/file3 b/file3
 new file mode 100644
 index 0000000..$blob
@@ -128,12 +128,12 @@ test_expect_success 'stash save --include-untracked dirty index got stashed' '
 	test_cmp expect actual
 '
 
-git reset > /dev/null
+git reset >/dev/null
 
 # Must direct output somewhere where it won't be considered an untracked file
 test_expect_success 'stash save --include-untracked -q is quiet' '
-	echo 1 > file5 &&
-	git stash save --include-untracked --quiet > .git/stash-output.out 2>&1 &&
+	echo 1 >file5 &&
+	git stash save --include-untracked --quiet >.git/stash-output.out 2>&1 &&
 	test_line_count = 0 .git/stash-output.out &&
 	rm -f .git/stash-output.out
 '
@@ -141,7 +141,7 @@ test_expect_success 'stash save --include-untracked -q is quiet' '
 test_expect_success 'stash save --include-untracked removed files' '
 	rm -f file &&
 	git stash save --include-untracked &&
-	echo 1 > expect &&
+	echo 1 >expect &&
 	test_cmp expect file
 '
 
@@ -152,14 +152,14 @@ test_expect_success 'stash save --include-untracked removed files got stashed' '
 	test_path_is_missing file
 '
 
-cat > .gitignore <<EOF
+cat >.gitignore <<EOF
 .gitignore
 ignored
 ignored.d/
 EOF
 
 test_expect_success 'stash save --include-untracked respects .gitignore' '
-	echo ignored > ignored &&
+	echo ignored >ignored &&
 	mkdir ignored.d &&
 	echo ignored >ignored.d/untracked &&
 	git stash -u &&
@@ -169,7 +169,7 @@ test_expect_success 'stash save --include-untracked respects .gitignore' '
 '
 
 test_expect_success 'stash save -u can stash with only untracked files different' '
-	echo 4 > file4 &&
+	echo 4 >file4 &&
 	git stash -u &&
 	test_path_is_missing file4
 '
@@ -214,7 +214,7 @@ test_expect_success 'stash push with $IFS character' '
 	test_path_is_file bar
 '
 
-cat > .gitignore <<EOF
+cat >.gitignore <<EOF
 ignored
 ignored.d/*
 EOF
@@ -224,7 +224,7 @@ test_expect_success 'stash previously ignored file' '
 	git add .gitignore &&
 	git commit -m "Add .gitignore" &&
 	>ignored.d/foo &&
-	echo "!ignored.d/foo" >> .gitignore &&
+	echo "!ignored.d/foo" >>.gitignore &&
 	git stash save --include-untracked &&
 	test_path_is_missing ignored.d/foo &&
 	git stash pop &&
-- 
2.30.0.478.g8a0d178c01


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH v2 3/9] t3905: move all commands into test cases
  2021-02-09  7:28 ` [PATCH v2 0/9] stash show: learn --include-untracked and --only-untracked Denton Liu
  2021-02-09  7:28   ` [PATCH v2 1/9] git-stash.txt: be explicit about subcommand options Denton Liu
  2021-02-09  7:28   ` [PATCH v2 2/9] t3905: remove spaces after redirect operators Denton Liu
@ 2021-02-09  7:28   ` Denton Liu
  2021-02-09  7:28   ` [PATCH v2 4/9] t3905: remove nested git in command substitution Denton Liu
                     ` (6 subsequent siblings)
  9 siblings, 0 replies; 46+ messages in thread
From: Denton Liu @ 2021-02-09  7:28 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Eric Sunshine

In order to modernize the tests, move commands that currently run
outside of test cases into a test case. Where possible, clean up files
that are produced using test_when_finished() but in the case where files
persist over multiple test cases, create a new test case to perform
cleanup.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t3905-stash-include-untracked.sh | 147 +++++++++++++++--------------
 1 file changed, 75 insertions(+), 72 deletions(-)

diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
index 1d416944b7..892a2c8057 100755
--- a/t/t3905-stash-include-untracked.sh
+++ b/t/t3905-stash-include-untracked.sh
@@ -25,48 +25,48 @@ test_expect_success 'stash save --include-untracked some dirty working directory
 	git diff-index --cached --quiet HEAD
 '
 
-cat >expect <<EOF
-?? actual
-?? expect
-EOF
-
 test_expect_success 'stash save --include-untracked cleaned the untracked files' '
+	cat >expect <<-EOF &&
+	?? actual
+	?? expect
+	EOF
+
 	git status --porcelain >actual &&
 	test_cmp expect actual
 '
 
-tracked=$(git rev-parse --short $(echo 1 | git hash-object --stdin))
-untracked=$(git rev-parse --short $(echo untracked | git hash-object --stdin))
-cat >expect.diff <<EOF
-diff --git a/HEAD b/HEAD
-new file mode 100644
-index 0000000..$tracked
---- /dev/null
-+++ b/HEAD
-@@ -0,0 +1 @@
-+1
-diff --git a/file2 b/file2
-new file mode 100644
-index 0000000..$tracked
---- /dev/null
-+++ b/file2
-@@ -0,0 +1 @@
-+1
-diff --git a/untracked/untracked b/untracked/untracked
-new file mode 100644
-index 0000000..$untracked
---- /dev/null
-+++ b/untracked/untracked
-@@ -0,0 +1 @@
-+untracked
-EOF
-cat >expect.lstree <<EOF
-HEAD
-file2
-untracked
-EOF
-
 test_expect_success 'stash save --include-untracked stashed the untracked files' '
+	tracked=$(git rev-parse --short $(echo 1 | git hash-object --stdin)) &&
+	untracked=$(git rev-parse --short $(echo untracked | git hash-object --stdin)) &&
+	cat >expect.diff <<-EOF &&
+	diff --git a/HEAD b/HEAD
+	new file mode 100644
+	index 0000000..$tracked
+	--- /dev/null
+	+++ b/HEAD
+	@@ -0,0 +1 @@
+	+1
+	diff --git a/file2 b/file2
+	new file mode 100644
+	index 0000000..$tracked
+	--- /dev/null
+	+++ b/file2
+	@@ -0,0 +1 @@
+	+1
+	diff --git a/untracked/untracked b/untracked/untracked
+	new file mode 100644
+	index 0000000..$untracked
+	--- /dev/null
+	+++ b/untracked/untracked
+	@@ -0,0 +1 @@
+	+untracked
+	EOF
+	cat >expect.lstree <<-EOF &&
+	HEAD
+	file2
+	untracked
+	EOF
+
 	test_path_is_missing file2 &&
 	test_path_is_missing untracked &&
 	test_path_is_missing HEAD &&
@@ -83,18 +83,21 @@ test_expect_success 'stash save --patch --all fails' '
 	test_must_fail git stash --patch --all
 '
 
-git clean --force --quiet
+test_expect_success 'clean up untracked/untracked file to prepare for next tests' '
+	git clean --force --quiet
 
-cat >expect <<EOF
- M file
-?? HEAD
-?? actual
-?? expect
-?? file2
-?? untracked/
-EOF
+'
 
 test_expect_success 'stash pop after save --include-untracked leaves files untracked again' '
+	cat >expect <<-EOF &&
+	 M file
+	?? HEAD
+	?? actual
+	?? expect
+	?? file2
+	?? untracked/
+	EOF
+
 	git stash pop &&
 	git status --porcelain >actual &&
 	test_cmp expect actual &&
@@ -102,7 +105,9 @@ test_expect_success 'stash pop after save --include-untracked leaves files untra
 	test untracked = "$(cat untracked/untracked)"
 '
 
-git clean --force --quiet -d
+test_expect_success 'clean up untracked/ directory to prepare for next tests' '
+	git clean --force --quiet -d
+'
 
 test_expect_success 'stash save -u dirty index' '
 	echo 4 >file3 &&
@@ -111,25 +116,24 @@ test_expect_success 'stash save -u dirty index' '
 	git stash -u
 '
 
-blob=$(git rev-parse --short $(echo 4 | git hash-object --stdin))
-cat >expect <<EOF
-diff --git a/file3 b/file3
-new file mode 100644
-index 0000000..$blob
---- /dev/null
-+++ b/file3
-@@ -0,0 +1 @@
-+4
-EOF
-
 test_expect_success 'stash save --include-untracked dirty index got stashed' '
+	blob=$(git rev-parse --short $(echo 4 | git hash-object --stdin)) &&
+	cat >expect <<-EOF &&
+	diff --git a/file3 b/file3
+	new file mode 100644
+	index 0000000..$blob
+	--- /dev/null
+	+++ b/file3
+	@@ -0,0 +1 @@
+	+4
+	EOF
+
 	git stash pop --index &&
+	test_when_finished "git reset" &&
 	git diff --cached >actual &&
 	test_cmp expect actual
 '
 
-git reset >/dev/null
-
 # Must direct output somewhere where it won't be considered an untracked file
 test_expect_success 'stash save --include-untracked -q is quiet' '
 	echo 1 >file5 &&
@@ -142,23 +146,22 @@ test_expect_success 'stash save --include-untracked removed files' '
 	rm -f file &&
 	git stash save --include-untracked &&
 	echo 1 >expect &&
+	test_when_finished "rm -f expect" &&
 	test_cmp expect file
 '
 
-rm -f expect
-
 test_expect_success 'stash save --include-untracked removed files got stashed' '
 	git stash pop &&
 	test_path_is_missing file
 '
 
-cat >.gitignore <<EOF
-.gitignore
-ignored
-ignored.d/
-EOF
-
 test_expect_success 'stash save --include-untracked respects .gitignore' '
+	cat >.gitignore <<-EOF &&
+	.gitignore
+	ignored
+	ignored.d/
+	EOF
+
 	echo ignored >ignored &&
 	mkdir ignored.d &&
 	echo ignored >ignored.d/untracked &&
@@ -214,12 +217,12 @@ test_expect_success 'stash push with $IFS character' '
 	test_path_is_file bar
 '
 
-cat >.gitignore <<EOF
-ignored
-ignored.d/*
-EOF
-
 test_expect_success 'stash previously ignored file' '
+	cat >.gitignore <<-EOF &&
+	ignored
+	ignored.d/*
+	EOF
+
 	git reset HEAD &&
 	git add .gitignore &&
 	git commit -m "Add .gitignore" &&
-- 
2.30.0.478.g8a0d178c01


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH v2 4/9] t3905: remove nested git in command substitution
  2021-02-09  7:28 ` [PATCH v2 0/9] stash show: learn --include-untracked and --only-untracked Denton Liu
                     ` (2 preceding siblings ...)
  2021-02-09  7:28   ` [PATCH v2 3/9] t3905: move all commands into test cases Denton Liu
@ 2021-02-09  7:28   ` Denton Liu
  2021-02-10  7:24     ` Junio C Hamano
  2021-02-09  7:28   ` [PATCH v2 5/9] t3905: replace test -s with test_file_not_empty Denton Liu
                     ` (5 subsequent siblings)
  9 siblings, 1 reply; 46+ messages in thread
From: Denton Liu @ 2021-02-09  7:28 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Eric Sunshine

If a git command in a nested command substitution fails, it will be
silently ignored since only the return code of the outer command
substitutions is reported. Factor out nested command substitutions so
that the error codes of those commands are reported.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t3905-stash-include-untracked.sh | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
index 892a2c8057..f008e5d945 100755
--- a/t/t3905-stash-include-untracked.sh
+++ b/t/t3905-stash-include-untracked.sh
@@ -36,8 +36,10 @@ test_expect_success 'stash save --include-untracked cleaned the untracked files'
 '
 
 test_expect_success 'stash save --include-untracked stashed the untracked files' '
-	tracked=$(git rev-parse --short $(echo 1 | git hash-object --stdin)) &&
-	untracked=$(git rev-parse --short $(echo untracked | git hash-object --stdin)) &&
+	one_blob=$(echo 1 | git hash-object --stdin) &&
+	tracked=$(git rev-parse --short "$one_blob") &&
+	untracked_blob=$(echo untracked | git hash-object --stdin) &&
+	untracked=$(git rev-parse --short "$untracked_blob") &&
 	cat >expect.diff <<-EOF &&
 	diff --git a/HEAD b/HEAD
 	new file mode 100644
@@ -117,7 +119,8 @@ test_expect_success 'stash save -u dirty index' '
 '
 
 test_expect_success 'stash save --include-untracked dirty index got stashed' '
-	blob=$(git rev-parse --short $(echo 4 | git hash-object --stdin)) &&
+	four_blob=$(echo 4 | git hash-object --stdin) &&
+	blob=$(git rev-parse --short "$four_blob") &&
 	cat >expect <<-EOF &&
 	diff --git a/file3 b/file3
 	new file mode 100644
-- 
2.30.0.478.g8a0d178c01


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH v2 5/9] t3905: replace test -s with test_file_not_empty
  2021-02-09  7:28 ` [PATCH v2 0/9] stash show: learn --include-untracked and --only-untracked Denton Liu
                     ` (3 preceding siblings ...)
  2021-02-09  7:28   ` [PATCH v2 4/9] t3905: remove nested git in command substitution Denton Liu
@ 2021-02-09  7:28   ` Denton Liu
  2021-02-09  7:28   ` [PATCH v2 6/9] t3905: use test_cmp() to check file contents Denton Liu
                     ` (4 subsequent siblings)
  9 siblings, 0 replies; 46+ messages in thread
From: Denton Liu @ 2021-02-09  7:28 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Eric Sunshine

In order to modernize the test script, replace `test -s` with
test_file_not_empty(), which provides better diagnostic output in the
case of failure.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t3905-stash-include-untracked.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
index f008e5d945..c87ac24042 100755
--- a/t/t3905-stash-include-untracked.sh
+++ b/t/t3905-stash-include-untracked.sh
@@ -169,9 +169,9 @@ test_expect_success 'stash save --include-untracked respects .gitignore' '
 	mkdir ignored.d &&
 	echo ignored >ignored.d/untracked &&
 	git stash -u &&
-	test -s ignored &&
-	test -s ignored.d/untracked &&
-	test -s .gitignore
+	test_file_not_empty ignored &&
+	test_file_not_empty ignored.d/untracked &&
+	test_file_not_empty .gitignore
 '
 
 test_expect_success 'stash save -u can stash with only untracked files different' '
@@ -189,9 +189,9 @@ test_expect_success 'stash save --all does not respect .gitignore' '
 
 test_expect_success 'stash save --all is stash poppable' '
 	git stash pop &&
-	test -s ignored &&
-	test -s ignored.d/untracked &&
-	test -s .gitignore
+	test_file_not_empty ignored &&
+	test_file_not_empty ignored.d/untracked &&
+	test_file_not_empty .gitignore
 '
 
 test_expect_success 'stash push --include-untracked with pathspec' '
-- 
2.30.0.478.g8a0d178c01


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH v2 6/9] t3905: use test_cmp() to check file contents
  2021-02-09  7:28 ` [PATCH v2 0/9] stash show: learn --include-untracked and --only-untracked Denton Liu
                     ` (4 preceding siblings ...)
  2021-02-09  7:28   ` [PATCH v2 5/9] t3905: replace test -s with test_file_not_empty Denton Liu
@ 2021-02-09  7:28   ` Denton Liu
  2021-02-09  7:28   ` [PATCH v2 7/9] stash: declare ref_stash as an array Denton Liu
                     ` (3 subsequent siblings)
  9 siblings, 0 replies; 46+ messages in thread
From: Denton Liu @ 2021-02-09  7:28 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Eric Sunshine

Modernize the script by doing file content comparisons using test_cmp()
instead of `test x = "$(cat file)"`.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t3905-stash-include-untracked.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
index c87ac24042..b26a97aef4 100755
--- a/t/t3905-stash-include-untracked.sh
+++ b/t/t3905-stash-include-untracked.sh
@@ -103,8 +103,10 @@ test_expect_success 'stash pop after save --include-untracked leaves files untra
 	git stash pop &&
 	git status --porcelain >actual &&
 	test_cmp expect actual &&
-	test "1" = "$(cat file2)" &&
-	test untracked = "$(cat untracked/untracked)"
+	echo 1 >expect_file2 &&
+	test_cmp expect_file2 file2 &&
+	echo untracked >untracked_expect &&
+	test_cmp untracked_expect untracked/untracked
 '
 
 test_expect_success 'clean up untracked/ directory to prepare for next tests' '
-- 
2.30.0.478.g8a0d178c01


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH v2 7/9] stash: declare ref_stash as an array
  2021-02-09  7:28 ` [PATCH v2 0/9] stash show: learn --include-untracked and --only-untracked Denton Liu
                     ` (5 preceding siblings ...)
  2021-02-09  7:28   ` [PATCH v2 6/9] t3905: use test_cmp() to check file contents Denton Liu
@ 2021-02-09  7:28   ` Denton Liu
  2021-02-10  7:28     ` Junio C Hamano
  2021-02-09  7:28   ` [PATCH v2 8/9] stash show: teach --include-untracked and --only-untracked Denton Liu
                     ` (2 subsequent siblings)
  9 siblings, 1 reply; 46+ messages in thread
From: Denton Liu @ 2021-02-09  7:28 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Eric Sunshine

Save sizeof(const char *) bytes by declaring ref_stash as an array
instead of having a redundant pointer to an array.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 builtin/stash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/stash.c b/builtin/stash.c
index 9bc85f91cd..6f2b58f6ab 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -87,7 +87,7 @@ static const char * const git_stash_save_usage[] = {
 	NULL
 };
 
-static const char *ref_stash = "refs/stash";
+static const char ref_stash[] = "refs/stash";
 static struct strbuf stash_index_path = STRBUF_INIT;
 
 /*
-- 
2.30.0.478.g8a0d178c01


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH v2 8/9] stash show: teach --include-untracked and --only-untracked
  2021-02-09  7:28 ` [PATCH v2 0/9] stash show: learn --include-untracked and --only-untracked Denton Liu
                     ` (6 preceding siblings ...)
  2021-02-09  7:28   ` [PATCH v2 7/9] stash: declare ref_stash as an array Denton Liu
@ 2021-02-09  7:28   ` Denton Liu
  2021-02-10  7:53     ` Junio C Hamano
  2021-02-09  7:28   ` [PATCH v2 9/9] stash show: learn stash.showIncludeUntracked Denton Liu
  2021-02-16  7:11   ` [PATCH v3 0/2] stash show: learn --include-untracked and --only-untracked Denton Liu
  9 siblings, 1 reply; 46+ messages in thread
From: Denton Liu @ 2021-02-09  7:28 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Eric Sunshine

Stash entries can be made with untracked files via
`git stash push --include-untracked`. However, because the untracked
files are stored in the third parent of the stash entry and not the
stash entry itself, running `git stash show` does not include the
untracked files as part of the diff.

Teach stash the --include-untracked option, which also displays the
untracked files in a stash entry from the third parent (if it exists).
Do this via something like

	GIT_INDEX_FILE=... git read-tree stash stash^3

and diffing the resulting tree object against the stash base.

One improvement that this could use for the future is performing the
action without writing anything to disk as one would expect this to be a
read-only operation. This can be fixed in the future, however.

Another limitation of this is that it would be possible to manually
craft a stash entry where duplicate untracked files in the stash entry
will mask tracked files. This seems like an instance of "Doctor, it
hurts when I do this! So don't do that!" so this can be written off.

Also, teach stash the --only-untracked option which only shows the
untracked files of a stash entry. This is similar to `git show stash^3`
but it is nice to provide a convenient abstraction for it so that users
do not have to think about the underlying implementation.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 Documentation/git-stash.txt            | 16 +++--
 builtin/stash.c                        | 77 ++++++++++++++++++++++-
 contrib/completion/git-completion.bash |  2 +-
 t/t3905-stash-include-untracked.sh     | 84 ++++++++++++++++++++++++++
 4 files changed, 173 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 04e55eb826..9d4b9f0b5c 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -83,7 +83,7 @@ stash@{1}: On master: 9cc0589... Add git-stash
 The command takes options applicable to the 'git log'
 command to control what is shown and how. See linkgit:git-log[1].
 
-show [<diff-options>] [<stash>]::
+show [-u|--include-untracked|--only-untracked] [<diff-options>] [<stash>]::
 
 	Show the changes recorded in the stash entry as a diff between the
 	stashed contents and the commit back when the stash entry was first
@@ -160,10 +160,18 @@ up with `git clean`.
 
 -u::
 --include-untracked::
-	This option is only valid for `push` and `save` commands.
+--no-include-untracked::
+	When used with the `push` and `save` commands,
+	all untracked files are also stashed and then cleaned up with
+	`git clean`.
 +
-All untracked files are also stashed and then cleaned up with
-`git clean`.
+When used with the `show` command, show the untracked files in the stash
+entry as part of the diff.
+
+--only-untracked::
+	This option is only valid for the `show` command.
++
+Show only the untracked files in the stash entry as part of the diff.
 
 --index::
 	This option is only valid for `pop` and `apply` commands.
diff --git a/builtin/stash.c b/builtin/stash.c
index 6f2b58f6ab..f7220fad56 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -787,6 +787,47 @@ static int git_stash_config(const char *var, const char *value, void *cb)
 	return git_diff_basic_config(var, value, cb);
 }
 
+static int merge_track_untracked(struct object_id *result, const struct stash_info *info)
+{
+	int ret = 0;
+	struct index_state istate = { NULL };
+	struct child_process cp_read_tree = CHILD_PROCESS_INIT;
+
+	if (!info->has_u) {
+		oidcpy(result, &info->w_commit);
+		return 0;
+	}
+
+	/*
+	 * TODO: is there a way of doing this all in-memory without writing
+	 * anything to disk?
+	 */
+	remove_path(stash_index_path.buf);
+
+	cp_read_tree.git_cmd = 1;
+	strvec_push(&cp_read_tree.args, "read-tree");
+	strvec_push(&cp_read_tree.args, oid_to_hex(&info->w_commit));
+	strvec_push(&cp_read_tree.args, oid_to_hex(&info->u_tree));
+	strvec_pushf(&cp_read_tree.env_array, "GIT_INDEX_FILE=%s",
+		     stash_index_path.buf);
+
+	if (run_command(&cp_read_tree)) {
+		ret = -1;
+		goto done;
+	}
+
+	if (write_index_as_tree(result, &istate, stash_index_path.buf, 0,
+				NULL)) {
+		ret = -1;
+		goto done;
+	}
+
+done:
+	discard_index(&istate);
+	remove_path(stash_index_path.buf);
+	return ret;
+}
+
 static int show_stash(int argc, const char **argv, const char *prefix)
 {
 	int i;
@@ -795,7 +836,21 @@ static int show_stash(int argc, const char **argv, const char *prefix)
 	struct rev_info rev;
 	struct strvec stash_args = STRVEC_INIT;
 	struct strvec revision_args = STRVEC_INIT;
+	struct object_id *before = NULL;
+	struct object_id *after = NULL;
+	struct object_id untracked_merged_tree;
+	enum {
+		UNTRACKED_NONE,
+		UNTRACKED_INCLUDE,
+		UNTRACKED_ONLY
+	} show_untracked = UNTRACKED_NONE;
 	struct option options[] = {
+		OPT_SET_INT('u', "include-untracked", &show_untracked,
+			    N_("include untracked files in the stash"),
+			    UNTRACKED_INCLUDE),
+		OPT_SET_INT_F(0, "only-untracked", &show_untracked,
+			      N_("only show untracked files in the stash"),
+			      UNTRACKED_ONLY, PARSE_OPT_NONEG),
 		OPT_END()
 	};
 
@@ -803,6 +858,10 @@ static int show_stash(int argc, const char **argv, const char *prefix)
 	git_config(git_diff_ui_config, NULL);
 	init_revisions(&rev, prefix);
 
+	argc = parse_options(argc, argv, prefix, options, git_stash_show_usage,
+			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |
+			     PARSE_OPT_KEEP_DASHDASH);
+
 	strvec_push(&revision_args, argv[0]);
 	for (i = 1; i < argc; i++) {
 		if (argv[i][0] != '-')
@@ -845,7 +904,23 @@ static int show_stash(int argc, const char **argv, const char *prefix)
 
 	rev.diffopt.flags.recursive = 1;
 	setup_diff_pager(&rev.diffopt);
-	diff_tree_oid(&info.b_commit, &info.w_commit, "", &rev.diffopt);
+	switch (show_untracked) {
+	case UNTRACKED_NONE:
+		before = &info.b_commit;
+		after = &info.w_commit;
+		break;
+	case UNTRACKED_ONLY:
+		before = NULL;
+		after = &info.u_tree;
+		break;
+	case UNTRACKED_INCLUDE:
+		if (merge_track_untracked(&untracked_merged_tree, &info) < 0)
+			die(_("unable merge stash index with untracked files index"));
+		before = &info.b_commit;
+		after = &untracked_merged_tree;
+		break;
+	}
+	diff_tree_oid(before, after, "", &rev.diffopt);
 	log_tree_diff_flush(&rev);
 
 	free_stash_info(&info);
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 4b1f4264a6..64ef6ffa21 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3051,7 +3051,7 @@ _git_stash ()
 			__gitcomp "--name-status --oneline --patch-with-stat"
 			;;
 		show,--*)
-			__gitcomp "$__git_diff_common_options"
+			__gitcomp "--include-untracked --only-untracked $__git_diff_common_options"
 			;;
 		branch,--*)
 			;;
diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
index b26a97aef4..978bc97baf 100755
--- a/t/t3905-stash-include-untracked.sh
+++ b/t/t3905-stash-include-untracked.sh
@@ -297,4 +297,88 @@ test_expect_success 'stash -u with globs' '
 	test_path_is_missing untracked.txt
 '
 
+test_expect_success 'stash show --include-untracked shows untracked files' '
+	git reset --hard &&
+	git clean -xf &&
+	>untracked &&
+	>tracked &&
+	git add tracked &&
+	git stash -u &&
+
+	cat >expect <<-EOF &&
+	 tracked   | 0
+	 untracked | 0
+	 2 files changed, 0 insertions(+), 0 deletions(-)
+	EOF
+	git stash show --include-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show -u >actual &&
+	test_cmp expect actual &&
+	git stash show --no-include-untracked --include-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show --only-untracked --include-untracked >actual &&
+	test_cmp expect actual &&
+
+	cat >expect <<-EOF &&
+	diff --git a/tracked b/tracked
+	new file mode 100644
+	index 0000000..e69de29
+	diff --git a/untracked b/untracked
+	new file mode 100644
+	index 0000000..e69de29
+	EOF
+	git stash show -p --include-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show --include-untracked -p >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'stash show --only-untracked only shows untracked files' '
+	git reset --hard &&
+	git clean -xf &&
+	>untracked &&
+	>tracked &&
+	git add tracked &&
+	git stash -u &&
+
+	cat >expect <<-EOF &&
+	 untracked | 0
+	 1 file changed, 0 insertions(+), 0 deletions(-)
+	EOF
+	git stash show --only-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show --no-include-untracked --only-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show --include-untracked --only-untracked >actual &&
+	test_cmp expect actual &&
+
+	cat >expect <<-EOF &&
+	diff --git a/untracked b/untracked
+	new file mode 100644
+	index 0000000..e69de29
+	EOF
+	git stash show -p --only-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show --only-untracked -p >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'stash show --no-include-untracked cancels --{include,show}-untracked' '
+	git reset --hard &&
+	git clean -xf &&
+	>untracked &&
+	>tracked &&
+	git add tracked &&
+	git stash -u &&
+
+	cat >expect <<-EOF &&
+	 tracked | 0
+	 1 file changed, 0 insertions(+), 0 deletions(-)
+	EOF
+	git stash show --only-untracked --no-include-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show --include-untracked --no-include-untracked >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.30.0.478.g8a0d178c01


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH v2 9/9] stash show: learn stash.showIncludeUntracked
  2021-02-09  7:28 ` [PATCH v2 0/9] stash show: learn --include-untracked and --only-untracked Denton Liu
                     ` (7 preceding siblings ...)
  2021-02-09  7:28   ` [PATCH v2 8/9] stash show: teach --include-untracked and --only-untracked Denton Liu
@ 2021-02-09  7:28   ` Denton Liu
  2021-02-16  7:11   ` [PATCH v3 0/2] stash show: learn --include-untracked and --only-untracked Denton Liu
  9 siblings, 0 replies; 46+ messages in thread
From: Denton Liu @ 2021-02-09  7:28 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Eric Sunshine

The previous commit teaches `git stash show --include-untracked`. It
may be desirable for a user to be able to always enable the
--include-untracked behavior. Teach the stash.showIncludeUntracked
config option which allows users to do this in a similar manner to
stash.showPatch.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 Documentation/config/stash.txt     | 5 +++++
 Documentation/git-stash.txt        | 4 ++--
 builtin/stash.c                    | 8 ++++++++
 t/t3905-stash-include-untracked.sh | 2 ++
 4 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/Documentation/config/stash.txt b/Documentation/config/stash.txt
index 00eb35434e..413f907cba 100644
--- a/Documentation/config/stash.txt
+++ b/Documentation/config/stash.txt
@@ -5,6 +5,11 @@ stash.useBuiltin::
 	is always used. Setting this will emit a warning, to alert any
 	remaining users that setting this now does nothing.
 
+stash.showIncludeUntracked::
+	If this is set to true, the `git stash show` command without an
+	option will show the untracked files of a stash entry.  Defaults to
+	false. See description of 'show' command in linkgit:git-stash[1].
+
 stash.showPatch::
 	If this is set to true, the `git stash show` command without an
 	option will show the stash entry in patch form.  Defaults to false.
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 9d4b9f0b5c..d004e9abdb 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -91,8 +91,8 @@ show [-u|--include-untracked|--only-untracked] [<diff-options>] [<stash>]::
 	By default, the command shows the diffstat, but it will accept any
 	format known to 'git diff' (e.g., `git stash show -p stash@{1}`
 	to view the second most recent entry in patch form).
-	You can use stash.showStat and/or stash.showPatch config variables
-	to change the default behavior.
+	You can use stash.showIncludeUntracked, stash.showStat, and
+	stash.showPatch config variables to change the default behavior.
 
 pop [--index] [-q|--quiet] [<stash>]::
 
diff --git a/builtin/stash.c b/builtin/stash.c
index f7220fad56..53428e9e64 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -768,6 +768,7 @@ static int list_stash(int argc, const char **argv, const char *prefix)
 
 static int show_stat = 1;
 static int show_patch;
+static int show_include_untracked;
 static int use_legacy_stash;
 
 static int git_stash_config(const char *var, const char *value, void *cb)
@@ -780,6 +781,10 @@ static int git_stash_config(const char *var, const char *value, void *cb)
 		show_patch = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "stash.showincludeuntracked")) {
+		show_include_untracked = git_config_bool(var, value);
+		return 0;
+	}
 	if (!strcmp(var, "stash.usebuiltin")) {
 		use_legacy_stash = !git_config_bool(var, value);
 		return 0;
@@ -886,6 +891,9 @@ static int show_stash(int argc, const char **argv, const char *prefix)
 		if (show_patch)
 			rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
 
+		if (show_include_untracked)
+			show_untracked = UNTRACKED_INCLUDE;
+
 		if (!show_stat && !show_patch) {
 			free_stash_info(&info);
 			return 0;
diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
index 978bc97baf..8bcd4c5ca8 100755
--- a/t/t3905-stash-include-untracked.sh
+++ b/t/t3905-stash-include-untracked.sh
@@ -318,6 +318,8 @@ test_expect_success 'stash show --include-untracked shows untracked files' '
 	test_cmp expect actual &&
 	git stash show --only-untracked --include-untracked >actual &&
 	test_cmp expect actual &&
+	git -c stash.showIncludeUntracked=true stash show >actual &&
+	test_cmp expect actual &&
 
 	cat >expect <<-EOF &&
 	diff --git a/tracked b/tracked
-- 
2.30.0.478.g8a0d178c01


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* Re: [PATCH v2 1/9] git-stash.txt: be explicit about subcommand options
  2021-02-09  7:28   ` [PATCH v2 1/9] git-stash.txt: be explicit about subcommand options Denton Liu
@ 2021-02-10  7:17     ` Junio C Hamano
  2021-02-11 19:07       ` [PATCH] fixup! " Denton Liu
  0 siblings, 1 reply; 46+ messages in thread
From: Junio C Hamano @ 2021-02-10  7:17 UTC (permalink / raw)
  To: Denton Liu; +Cc: Git Mailing List, Eric Sunshine

Denton Liu <liu.denton@gmail.com> writes:

> Currently, the options for the `list` and `show` subcommands are just
> listed as `<options>`. This seems to imply, from a cursory glance at the
> summary, that they take the stash options listed below. However, reading
> more carefully, we see that they take log options and diff options
> respectively.
>
> Make it more obvious that they take log and diff options by explicitly
> stating this in the subcommand summary.

Makes sense.

> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
> index 31f1beb65b..04e55eb826 100644
> --- a/Documentation/git-stash.txt
> +++ b/Documentation/git-stash.txt
> @@ -67,7 +67,7 @@ save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q
>  	Instead, all non-option arguments are concatenated to form the stash
>  	message.
>  
> -list [<options>]::
> +list [<log-options>]::
>  
>  	List the stash entries that you currently have.  Each 'stash entry' is
>  	listed with its name (e.g. `stash@{0}` is the latest entry, `stash@{1}` is
> @@ -83,7 +83,7 @@ stash@{1}: On master: 9cc0589... Add git-stash
>  The command takes options applicable to the 'git log'
>  command to control what is shown and how. See linkgit:git-log[1].
>  
> -show [<options>] [<stash>]::
> +show [<diff-options>] [<stash>]::
>  
>  	Show the changes recorded in the stash entry as a diff between the
>  	stashed contents and the commit back when the stash entry was first

This makes me wonder if we should also update the SYNOPSIS that
talks about the "list" and "show" operations taking a generic
"<options>", though.

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH v2 2/9] t3905: remove spaces after redirect operators
  2021-02-09  7:28   ` [PATCH v2 2/9] t3905: remove spaces after redirect operators Denton Liu
@ 2021-02-10  7:18     ` Junio C Hamano
  0 siblings, 0 replies; 46+ messages in thread
From: Junio C Hamano @ 2021-02-10  7:18 UTC (permalink / raw)
  To: Denton Liu; +Cc: Git Mailing List, Eric Sunshine

Denton Liu <liu.denton@gmail.com> writes:

> For shell scripts, the usual convention is for there to be no space
> after redirection operators, (e.g. `>file`, not `> file`). Remove these
> spaces wherever they appear.

OK.

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH v2 4/9] t3905: remove nested git in command substitution
  2021-02-09  7:28   ` [PATCH v2 4/9] t3905: remove nested git in command substitution Denton Liu
@ 2021-02-10  7:24     ` Junio C Hamano
  0 siblings, 0 replies; 46+ messages in thread
From: Junio C Hamano @ 2021-02-10  7:24 UTC (permalink / raw)
  To: Denton Liu; +Cc: Git Mailing List, Eric Sunshine

Denton Liu <liu.denton@gmail.com> writes:

> If a git command in a nested command substitution fails, it will be
> silently ignored since only the return code of the outer command
> substitutions is reported. Factor out nested command substitutions so
> that the error codes of those commands are reported.

Locally this makes sense.  We may want to clean it up further by
using existing blob objects, instead of using hash-objects, but that
is for some other day.

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH v2 7/9] stash: declare ref_stash as an array
  2021-02-09  7:28   ` [PATCH v2 7/9] stash: declare ref_stash as an array Denton Liu
@ 2021-02-10  7:28     ` Junio C Hamano
  0 siblings, 0 replies; 46+ messages in thread
From: Junio C Hamano @ 2021-02-10  7:28 UTC (permalink / raw)
  To: Denton Liu; +Cc: Git Mailing List, Eric Sunshine

Denton Liu <liu.denton@gmail.com> writes:

> Save sizeof(const char *) bytes by declaring ref_stash as an array
> instead of having a redundant pointer to an array.
>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
>  builtin/stash.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

OK, up to this point there is nothing new that can be controversial
in design.  I suspect that we may want to fast-track these early
clean-up steps as a separate topic rather quickly while the new
feature(s) are worked out.

Thanks.

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH v2 8/9] stash show: teach --include-untracked and --only-untracked
  2021-02-09  7:28   ` [PATCH v2 8/9] stash show: teach --include-untracked and --only-untracked Denton Liu
@ 2021-02-10  7:53     ` Junio C Hamano
  2021-02-16  3:15       ` Denton Liu
  0 siblings, 1 reply; 46+ messages in thread
From: Junio C Hamano @ 2021-02-10  7:53 UTC (permalink / raw)
  To: Denton Liu; +Cc: Git Mailing List, Eric Sunshine

Denton Liu <liu.denton@gmail.com> writes:

> Stash entries can be made with untracked files via
> `git stash push --include-untracked`. However, because the untracked
> files are stored in the third parent of the stash entry and not the
> stash entry itself, running `git stash show` does not include the
> untracked files as part of the diff.

> Teach stash the --include-untracked option, which also displays the
> untracked files in a stash entry from the third parent (if it exists).

A few points:

 - "Teach stash the --include-untracked option"?  (some part of)
   "stash" knows --include-untracked already.  Perhaps "Teach 'stash
   show' the '--include-untracked' option"?

 - "which also displays"?  Let's spell it out that untracked paths
   are shown in addition to what.  "With this option, untracked
   paths recorded in the third-parent (if exists) are shown, in
   addition to the paths whose modifications between the stash base
   and the working tree are stashed", or something like that,
   perhaps?

> Do this via something like
>
> 	GIT_INDEX_FILE=... git read-tree stash stash^3
>
> and diffing the resulting tree object against the stash base.

That explains the implementation, but does not make it clear what
the implementation wants to achieve.  So we read the tree from stash
(i.e. working tree) into a temporary index, and then overlay the
tree of stash^3 (i.e. untracked) on top---which means the resulting
"index" has the state of the working tree plus the untracked cruft
in it.  And comparing that with "stash base" (by the way is that a
term well understood?  I borrowed it for the above review comment,
which shows that there certainly is need for such a term) would show
the diff between the "HEAD" and the state that would have result if
you were to do an "git add ." in the working tree.  OK.

> One improvement that this could use for the future is performing the
> action without writing anything to disk as one would expect this to be a
> read-only operation. This can be fixed in the future, however.

Is it so difficult that we have to delay the fix for "the future"?
After reading two trees into an in-core index, without writing it
out to any file, all that remains to be done is just a matter of
running diff-lib.c::do_diff_cache(), no?  I must be missing something.q

> Another limitation of this is that it would be possible to manually
> craft a stash entry where duplicate untracked files in the stash entry
> will mask tracked files. This seems like an instance of "Doctor, it
> hurts when I do this! So don't do that!" so this can be written off.

Well, when you read the second tree into the in-core index to
overlay what you read from the working tree state, you can certainly
report the collision and error it out.

> Also, teach stash the --only-untracked option which only shows the
> untracked files of a stash entry. This is similar to `git show stash^3`
> but it is nice to provide a convenient abstraction for it so that users
> do not have to think about the underlying implementation.

OK.

>  -u::
>  --include-untracked::
> -	This option is only valid for `push` and `save` commands.
> +--no-include-untracked::
> +	When used with the `push` and `save` commands,
> +	all untracked files are also stashed and then cleaned up with
> +	`git clean`.

Back when "--include-untracked" was there and no "--only-untracked"
existed, it made sense for the former to squat on a short-and-sweet
"-u".  Now it comes back to bite us ;-)

>  +
> -All untracked files are also stashed and then cleaned up with
> -`git clean`.
> +When used with the `show` command, show the untracked files in the stash
> +entry as part of the diff.

OK.

> +--only-untracked::
> +	This option is only valid for the `show` command.
> ++
> +Show only the untracked files in the stash entry as part of the diff.

OK.

> diff --git a/builtin/stash.c b/builtin/stash.c
> index 6f2b58f6ab..f7220fad56 100644
> --- a/builtin/stash.c
> +++ b/builtin/stash.c
> @@ -787,6 +787,47 @@ static int git_stash_config(const char *var, const char *value, void *cb)
>  	return git_diff_basic_config(var, value, cb);
>  }
>  
> +static int merge_track_untracked(struct object_id *result, const struct stash_info *info)
> +{
> +	int ret = 0;
> +	struct index_state istate = { NULL };
> +	struct child_process cp_read_tree = CHILD_PROCESS_INIT;
> +
> +	if (!info->has_u) {
> +		oidcpy(result, &info->w_commit);
> +		return 0;
> +	}
> +
> +	/*
> +	 * TODO: is there a way of doing this all in-memory without writing
> +	 * anything to disk?
> +	 */

Of course.  Read and study what read-tree does, which boils down to
a call to unpack_trees() without .merge option.

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH] fixup! git-stash.txt: be explicit about subcommand options
  2021-02-10  7:17     ` Junio C Hamano
@ 2021-02-11 19:07       ` Denton Liu
  0 siblings, 0 replies; 46+ messages in thread
From: Denton Liu @ 2021-02-11 19:07 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Eric Sunshine

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 Documentation/git-stash.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 04e55eb826..f1197d641b 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -8,8 +8,8 @@ git-stash - Stash the changes in a dirty working directory away
 SYNOPSIS
 --------
 [verse]
-'git stash' list [<options>]
-'git stash' show [<options>] [<stash>]
+'git stash' list [<log-options>]
+'git stash' show [<diff-options>] [<stash>]
 'git stash' drop [-q|--quiet] [<stash>]
 'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]
 'git stash' branch <branchname> [<stash>]
-- 
2.30.0.478.g8a0d178c01


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* Re: [PATCH v2 8/9] stash show: teach --include-untracked and --only-untracked
  2021-02-10  7:53     ` Junio C Hamano
@ 2021-02-16  3:15       ` Denton Liu
  2021-02-16  6:42         ` Junio C Hamano
  0 siblings, 1 reply; 46+ messages in thread
From: Denton Liu @ 2021-02-16  3:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List, Eric Sunshine

Hi Junio,

On Tue, Feb 09, 2021 at 11:53:06PM -0800, Junio C Hamano wrote:
> > Do this via something like
> >
> > 	GIT_INDEX_FILE=... git read-tree stash stash^3
> >
> > and diffing the resulting tree object against the stash base.
> 
> That explains the implementation, but does not make it clear what
> the implementation wants to achieve.  So we read the tree from stash
> (i.e. working tree) into a temporary index, and then overlay the
> tree of stash^3 (i.e. untracked) on top---which means the resulting
> "index" has the state of the working tree plus the untracked cruft
> in it.  And comparing that with "stash base" (by the way is that a
> term well understood?  I borrowed it for the above review comment,
> which shows that there certainly is need for such a term) would show

I'm not sure if it's a well-understood term but I can't think of any
other meanings for the term so it doesn't seem very ambiguous.

> the diff between the "HEAD" and the state that would have result if
> you were to do an "git add ." in the working tree.  OK.
> 
> > One improvement that this could use for the future is performing the
> > action without writing anything to disk as one would expect this to be a
> > read-only operation. This can be fixed in the future, however.
> 
> Is it so difficult that we have to delay the fix for "the future"?
> After reading two trees into an in-core index, without writing it
> out to any file, all that remains to be done is just a matter of
> running diff-lib.c::do_diff_cache(), no?  I must be missing something.q

No, I don't think it's difficult. It's just my inexperience with this
area of the code.

> > Another limitation of this is that it would be possible to manually
> > craft a stash entry where duplicate untracked files in the stash entry
> > will mask tracked files. This seems like an instance of "Doctor, it
> > hurts when I do this! So don't do that!" so this can be written off.
> 
> Well, when you read the second tree into the in-core index to
> overlay what you read from the working tree state, you can certainly
> report the collision and error it out.

I'll send out my revised patch later today and I was unable to figure
out an easy way of doing this.

Thanks,
Denton

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH v2 8/9] stash show: teach --include-untracked and --only-untracked
  2021-02-16  3:15       ` Denton Liu
@ 2021-02-16  6:42         ` Junio C Hamano
  0 siblings, 0 replies; 46+ messages in thread
From: Junio C Hamano @ 2021-02-16  6:42 UTC (permalink / raw)
  To: Denton Liu; +Cc: Git Mailing List, Eric Sunshine

Denton Liu <liu.denton@gmail.com> writes:

> Hi Junio,
>
> On Tue, Feb 09, 2021 at 11:53:06PM -0800, Junio C Hamano wrote:
>> > Do this via something like
>> >
>> > 	GIT_INDEX_FILE=... git read-tree stash stash^3
>> >
>> > and diffing the resulting tree object against the stash base.
>> 
>> That explains the implementation, but does not make it clear what
>> the implementation wants to achieve.  So we read the tree from stash
>> (i.e. working tree) into a temporary index, and then overlay the
>> tree of stash^3 (i.e. untracked) on top---which means the resulting
>> "index" has the state of the working tree plus the untracked cruft
>> in it.  And comparing that with "stash base" (by the way is that a
>> term well understood?  I borrowed it for the above review comment,
>> which shows that there certainly is need for such a term) would show
>
> I'm not sure if it's a well-understood term but I can't think of any
> other meanings for the term so it doesn't seem very ambiguous.

Thanks.  I was hoping to hear either "Yes, glossary defines it like
this" or "I believe it is an unambiguous good term; let's add it to
the glossary".

> I'll send out my revised patch later today and I was unable to figure
> out an easy way of doing this.

OK.


^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH v3 0/2] stash show: learn --include-untracked and --only-untracked
  2021-02-09  7:28 ` [PATCH v2 0/9] stash show: learn --include-untracked and --only-untracked Denton Liu
                     ` (8 preceding siblings ...)
  2021-02-09  7:28   ` [PATCH v2 9/9] stash show: learn stash.showIncludeUntracked Denton Liu
@ 2021-02-16  7:11   ` Denton Liu
  2021-02-16  7:11     ` [PATCH v3 1/2] stash show: teach " Denton Liu
                       ` (2 more replies)
  9 siblings, 3 replies; 46+ messages in thread
From: Denton Liu @ 2021-02-16  7:11 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Eric Sunshine

A blindspot that I've noticed in git is that it's not possible to
properly view a stash entry that has untracked files via `git stash
show`. Teach `git stash show --include-untracked` which should do this.
In addition, this series also teaches `--only-untracked` and the
`stash.showIncludeUntracked` config option.

This series is based on 'dl/stash-cleanup'.

Changes since v2:

* Base this series on top of 'dl/stash-cleanup'

* Attempt to replicate the read-tree code to merge the untracked tree

Changes since v1:

* Add a dash for <log-options> and <diff-options>

* Fix the misspelling of --include-untracked in a commit message

* Change the approach from concatenating diffs to using `git read-tree`

Denton Liu (2):
  stash show: teach --include-untracked and --only-untracked
  stash show: learn stash.showIncludeUntracked

 Documentation/config/stash.txt         |  5 ++
 Documentation/git-stash.txt            | 22 ++++---
 builtin/stash.c                        | 61 +++++++++++++++++-
 contrib/completion/git-completion.bash |  2 +-
 t/t3905-stash-include-untracked.sh     | 86 ++++++++++++++++++++++++++
 5 files changed, 167 insertions(+), 9 deletions(-)

Range-diff against v2:
1:  88d4791259 ! 1:  85b81f2f06 stash show: teach --include-untracked and --only-untracked
    @@ Commit message
         stash entry itself, running `git stash show` does not include the
         untracked files as part of the diff.
     
    -    Teach stash the --include-untracked option, which also displays the
    -    untracked files in a stash entry from the third parent (if it exists).
    -    Do this via something like
    +    With --include-untracked, untracked paths, which are recorded in the
    +    third-parent if it exists, are shown in addition to the paths that have
    +    modifications between the stash base and the working tree in the stash.
     
    -            GIT_INDEX_FILE=... git read-tree stash stash^3
    -
    -    and diffing the resulting tree object against the stash base.
    -
    -    One improvement that this could use for the future is performing the
    -    action without writing anything to disk as one would expect this to be a
    -    read-only operation. This can be fixed in the future, however.
    -
    -    Another limitation of this is that it would be possible to manually
    -    craft a stash entry where duplicate untracked files in the stash entry
    -    will mask tracked files. This seems like an instance of "Doctor, it
    -    hurts when I do this! So don't do that!" so this can be written off.
    +    One limitation of this is that it would be possible to manually craft a
    +    stash entry where duplicate untracked files in the stash entry will mask
    +    tracked files. This seems like an instance of "Doctor, it hurts when I
    +    do this! So don't do that!" so this can be written off.
     
         Also, teach stash the --only-untracked option which only shows the
         untracked files of a stash entry. This is similar to `git show stash^3`
         but it is nice to provide a convenient abstraction for it so that users
         do not have to think about the underlying implementation.
     
    +
    + ## Notes ##
    +    I am not familiar with the read-tree code so this attempt at replicating
    +    the read-tree code may in diff_include_untracked() may be incorrect
    +    (particularly the use of the_index?).
    +
    +    Also, I could not figure out how to make unpack_trees() error out in the
    +    case where untracked tree entry contains duplicate entries with the
    +    worktree entry.
    +
      ## Documentation/git-stash.txt ##
    +@@ Documentation/git-stash.txt: SYNOPSIS
    + --------
    + [verse]
    + 'git stash' list [<log-options>]
    +-'git stash' show [<diff-options>] [<stash>]
    ++'git stash' show [-u|--include-untracked|--only-untracked] [<diff-options>] [<stash>]
    + 'git stash' drop [-q|--quiet] [<stash>]
    + 'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]
    + 'git stash' branch <branchname> [<stash>]
     @@ Documentation/git-stash.txt: stash@{1}: On master: 9cc0589... Add git-stash
      The command takes options applicable to the 'git log'
      command to control what is shown and how. See linkgit:git-log[1].
    @@ builtin/stash.c: static int git_stash_config(const char *var, const char *value,
      	return git_diff_basic_config(var, value, cb);
      }
      
    -+static int merge_track_untracked(struct object_id *result, const struct stash_info *info)
    ++static void diff_include_untracked(const struct stash_info *info, struct diff_options *diff_opt)
     +{
    -+	int ret = 0;
    -+	struct index_state istate = { NULL };
    -+	struct child_process cp_read_tree = CHILD_PROCESS_INIT;
    ++	const struct object_id *oid[] = { &info->w_commit, &info->u_tree };
    ++	struct tree *tree[ARRAY_SIZE(oid)];
    ++	struct tree_desc tree_desc[ARRAY_SIZE(oid)];
    ++	struct unpack_trees_options unpack_tree_opt = { 0 };
    ++	int i;
     +
    -+	if (!info->has_u) {
    -+		oidcpy(result, &info->w_commit);
    -+		return 0;
    ++	for (i = 0; i < ARRAY_SIZE(oid); i++) {
    ++		tree[i] = parse_tree_indirect(oid[i]);
    ++		if (parse_tree(tree[i]) < 0)
    ++			die(_("failed to parse tree"));
    ++		init_tree_desc(&tree_desc[i], tree[i]->buffer, tree[i]->size);
     +	}
     +
    -+	/*
    -+	 * TODO: is there a way of doing this all in-memory without writing
    -+	 * anything to disk?
    -+	 */
    -+	remove_path(stash_index_path.buf);
    ++	unpack_tree_opt.head_idx = -1;
    ++	unpack_tree_opt.src_index = &the_index;
    ++	unpack_tree_opt.dst_index = &the_index;
    ++	unpack_tree_opt.fn = twoway_merge;
     +
    -+	cp_read_tree.git_cmd = 1;
    -+	strvec_push(&cp_read_tree.args, "read-tree");
    -+	strvec_push(&cp_read_tree.args, oid_to_hex(&info->w_commit));
    -+	strvec_push(&cp_read_tree.args, oid_to_hex(&info->u_tree));
    -+	strvec_pushf(&cp_read_tree.env_array, "GIT_INDEX_FILE=%s",
    -+		     stash_index_path.buf);
    ++	if (unpack_trees(ARRAY_SIZE(tree_desc), tree_desc, &unpack_tree_opt))
    ++		die(_("failed to unpack trees"));
     +
    -+	if (run_command(&cp_read_tree)) {
    -+		ret = -1;
    -+		goto done;
    -+	}
    -+
    -+	if (write_index_as_tree(result, &istate, stash_index_path.buf, 0,
    -+				NULL)) {
    -+		ret = -1;
    -+		goto done;
    -+	}
    -+
    -+done:
    -+	discard_index(&istate);
    -+	remove_path(stash_index_path.buf);
    -+	return ret;
    ++	do_diff_cache(&info->b_commit, diff_opt);
     +}
     +
      static int show_stash(int argc, const char **argv, const char *prefix)
    @@ builtin/stash.c: static int show_stash(int argc, const char **argv, const char *
      	struct rev_info rev;
      	struct strvec stash_args = STRVEC_INIT;
      	struct strvec revision_args = STRVEC_INIT;
    -+	struct object_id *before = NULL;
    -+	struct object_id *after = NULL;
    -+	struct object_id untracked_merged_tree;
     +	enum {
     +		UNTRACKED_NONE,
     +		UNTRACKED_INCLUDE,
    @@ builtin/stash.c: static int show_stash(int argc, const char **argv, const char *
     -	diff_tree_oid(&info.b_commit, &info.w_commit, "", &rev.diffopt);
     +	switch (show_untracked) {
     +	case UNTRACKED_NONE:
    -+		before = &info.b_commit;
    -+		after = &info.w_commit;
    ++		diff_tree_oid(&info.b_commit, &info.w_commit, "", &rev.diffopt);
     +		break;
     +	case UNTRACKED_ONLY:
    -+		before = NULL;
    -+		after = &info.u_tree;
    ++		diff_root_tree_oid(&info.u_tree, "", &rev.diffopt);
     +		break;
     +	case UNTRACKED_INCLUDE:
    -+		if (merge_track_untracked(&untracked_merged_tree, &info) < 0)
    -+			die(_("unable merge stash index with untracked files index"));
    -+		before = &info.b_commit;
    -+		after = &untracked_merged_tree;
    ++		diff_include_untracked(&info, &rev.diffopt);
     +		break;
     +	}
    -+	diff_tree_oid(before, after, "", &rev.diffopt);
      	log_tree_diff_flush(&rev);
      
      	free_stash_info(&info);
2:  ac4019f47e = 2:  d19d07ec27 stash show: learn stash.showIncludeUntracked
-- 
2.30.0.478.g8a0d178c01


^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH v3 1/2] stash show: teach --include-untracked and --only-untracked
  2021-02-16  7:11   ` [PATCH v3 0/2] stash show: learn --include-untracked and --only-untracked Denton Liu
@ 2021-02-16  7:11     ` Denton Liu
  2021-02-16 20:22       ` Junio C Hamano
  2021-02-17  2:31       ` Junio C Hamano
  2021-02-16  7:11     ` [PATCH v3 2/2] stash show: learn stash.showIncludeUntracked Denton Liu
  2021-03-03 11:16     ` [PATCH v4 0/2] stash show: learn --include-untracked and --only-untracked Denton Liu
  2 siblings, 2 replies; 46+ messages in thread
From: Denton Liu @ 2021-02-16  7:11 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Eric Sunshine

Stash entries can be made with untracked files via
`git stash push --include-untracked`. However, because the untracked
files are stored in the third parent of the stash entry and not the
stash entry itself, running `git stash show` does not include the
untracked files as part of the diff.

With --include-untracked, untracked paths, which are recorded in the
third-parent if it exists, are shown in addition to the paths that have
modifications between the stash base and the working tree in the stash.

One limitation of this is that it would be possible to manually craft a
stash entry where duplicate untracked files in the stash entry will mask
tracked files. This seems like an instance of "Doctor, it hurts when I
do this! So don't do that!" so this can be written off.

Also, teach stash the --only-untracked option which only shows the
untracked files of a stash entry. This is similar to `git show stash^3`
but it is nice to provide a convenient abstraction for it so that users
do not have to think about the underlying implementation.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---

Notes:
    I am not familiar with the read-tree code so this attempt at replicating
    the read-tree code may in diff_include_untracked() may be incorrect
    (particularly the use of the_index?).
    
    Also, I could not figure out how to make unpack_trees() error out in the
    case where untracked tree entry contains duplicate entries with the
    worktree entry.

 Documentation/git-stash.txt            | 18 ++++--
 builtin/stash.c                        | 53 +++++++++++++++-
 contrib/completion/git-completion.bash |  2 +-
 t/t3905-stash-include-untracked.sh     | 84 ++++++++++++++++++++++++++
 4 files changed, 150 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index f1197d641b..8eeb60feb1 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -9,7 +9,7 @@ SYNOPSIS
 --------
 [verse]
 'git stash' list [<log-options>]
-'git stash' show [<diff-options>] [<stash>]
+'git stash' show [-u|--include-untracked|--only-untracked] [<diff-options>] [<stash>]
 'git stash' drop [-q|--quiet] [<stash>]
 'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]
 'git stash' branch <branchname> [<stash>]
@@ -83,7 +83,7 @@ stash@{1}: On master: 9cc0589... Add git-stash
 The command takes options applicable to the 'git log'
 command to control what is shown and how. See linkgit:git-log[1].
 
-show [<diff-options>] [<stash>]::
+show [-u|--include-untracked|--only-untracked] [<diff-options>] [<stash>]::
 
 	Show the changes recorded in the stash entry as a diff between the
 	stashed contents and the commit back when the stash entry was first
@@ -160,10 +160,18 @@ up with `git clean`.
 
 -u::
 --include-untracked::
-	This option is only valid for `push` and `save` commands.
+--no-include-untracked::
+	When used with the `push` and `save` commands,
+	all untracked files are also stashed and then cleaned up with
+	`git clean`.
 +
-All untracked files are also stashed and then cleaned up with
-`git clean`.
+When used with the `show` command, show the untracked files in the stash
+entry as part of the diff.
+
+--only-untracked::
+	This option is only valid for the `show` command.
++
+Show only the untracked files in the stash entry as part of the diff.
 
 --index::
 	This option is only valid for `pop` and `apply` commands.
diff --git a/builtin/stash.c b/builtin/stash.c
index 6f2b58f6ab..417ed2b4a1 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -787,6 +787,32 @@ static int git_stash_config(const char *var, const char *value, void *cb)
 	return git_diff_basic_config(var, value, cb);
 }
 
+static void diff_include_untracked(const struct stash_info *info, struct diff_options *diff_opt)
+{
+	const struct object_id *oid[] = { &info->w_commit, &info->u_tree };
+	struct tree *tree[ARRAY_SIZE(oid)];
+	struct tree_desc tree_desc[ARRAY_SIZE(oid)];
+	struct unpack_trees_options unpack_tree_opt = { 0 };
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(oid); i++) {
+		tree[i] = parse_tree_indirect(oid[i]);
+		if (parse_tree(tree[i]) < 0)
+			die(_("failed to parse tree"));
+		init_tree_desc(&tree_desc[i], tree[i]->buffer, tree[i]->size);
+	}
+
+	unpack_tree_opt.head_idx = -1;
+	unpack_tree_opt.src_index = &the_index;
+	unpack_tree_opt.dst_index = &the_index;
+	unpack_tree_opt.fn = twoway_merge;
+
+	if (unpack_trees(ARRAY_SIZE(tree_desc), tree_desc, &unpack_tree_opt))
+		die(_("failed to unpack trees"));
+
+	do_diff_cache(&info->b_commit, diff_opt);
+}
+
 static int show_stash(int argc, const char **argv, const char *prefix)
 {
 	int i;
@@ -795,7 +821,18 @@ static int show_stash(int argc, const char **argv, const char *prefix)
 	struct rev_info rev;
 	struct strvec stash_args = STRVEC_INIT;
 	struct strvec revision_args = STRVEC_INIT;
+	enum {
+		UNTRACKED_NONE,
+		UNTRACKED_INCLUDE,
+		UNTRACKED_ONLY
+	} show_untracked = UNTRACKED_NONE;
 	struct option options[] = {
+		OPT_SET_INT('u', "include-untracked", &show_untracked,
+			    N_("include untracked files in the stash"),
+			    UNTRACKED_INCLUDE),
+		OPT_SET_INT_F(0, "only-untracked", &show_untracked,
+			      N_("only show untracked files in the stash"),
+			      UNTRACKED_ONLY, PARSE_OPT_NONEG),
 		OPT_END()
 	};
 
@@ -803,6 +840,10 @@ static int show_stash(int argc, const char **argv, const char *prefix)
 	git_config(git_diff_ui_config, NULL);
 	init_revisions(&rev, prefix);
 
+	argc = parse_options(argc, argv, prefix, options, git_stash_show_usage,
+			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |
+			     PARSE_OPT_KEEP_DASHDASH);
+
 	strvec_push(&revision_args, argv[0]);
 	for (i = 1; i < argc; i++) {
 		if (argv[i][0] != '-')
@@ -845,7 +886,17 @@ static int show_stash(int argc, const char **argv, const char *prefix)
 
 	rev.diffopt.flags.recursive = 1;
 	setup_diff_pager(&rev.diffopt);
-	diff_tree_oid(&info.b_commit, &info.w_commit, "", &rev.diffopt);
+	switch (show_untracked) {
+	case UNTRACKED_NONE:
+		diff_tree_oid(&info.b_commit, &info.w_commit, "", &rev.diffopt);
+		break;
+	case UNTRACKED_ONLY:
+		diff_root_tree_oid(&info.u_tree, "", &rev.diffopt);
+		break;
+	case UNTRACKED_INCLUDE:
+		diff_include_untracked(&info, &rev.diffopt);
+		break;
+	}
 	log_tree_diff_flush(&rev);
 
 	free_stash_info(&info);
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 4b1f4264a6..64ef6ffa21 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3051,7 +3051,7 @@ _git_stash ()
 			__gitcomp "--name-status --oneline --patch-with-stat"
 			;;
 		show,--*)
-			__gitcomp "$__git_diff_common_options"
+			__gitcomp "--include-untracked --only-untracked $__git_diff_common_options"
 			;;
 		branch,--*)
 			;;
diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
index b26a97aef4..978bc97baf 100755
--- a/t/t3905-stash-include-untracked.sh
+++ b/t/t3905-stash-include-untracked.sh
@@ -297,4 +297,88 @@ test_expect_success 'stash -u with globs' '
 	test_path_is_missing untracked.txt
 '
 
+test_expect_success 'stash show --include-untracked shows untracked files' '
+	git reset --hard &&
+	git clean -xf &&
+	>untracked &&
+	>tracked &&
+	git add tracked &&
+	git stash -u &&
+
+	cat >expect <<-EOF &&
+	 tracked   | 0
+	 untracked | 0
+	 2 files changed, 0 insertions(+), 0 deletions(-)
+	EOF
+	git stash show --include-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show -u >actual &&
+	test_cmp expect actual &&
+	git stash show --no-include-untracked --include-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show --only-untracked --include-untracked >actual &&
+	test_cmp expect actual &&
+
+	cat >expect <<-EOF &&
+	diff --git a/tracked b/tracked
+	new file mode 100644
+	index 0000000..e69de29
+	diff --git a/untracked b/untracked
+	new file mode 100644
+	index 0000000..e69de29
+	EOF
+	git stash show -p --include-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show --include-untracked -p >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'stash show --only-untracked only shows untracked files' '
+	git reset --hard &&
+	git clean -xf &&
+	>untracked &&
+	>tracked &&
+	git add tracked &&
+	git stash -u &&
+
+	cat >expect <<-EOF &&
+	 untracked | 0
+	 1 file changed, 0 insertions(+), 0 deletions(-)
+	EOF
+	git stash show --only-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show --no-include-untracked --only-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show --include-untracked --only-untracked >actual &&
+	test_cmp expect actual &&
+
+	cat >expect <<-EOF &&
+	diff --git a/untracked b/untracked
+	new file mode 100644
+	index 0000000..e69de29
+	EOF
+	git stash show -p --only-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show --only-untracked -p >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'stash show --no-include-untracked cancels --{include,show}-untracked' '
+	git reset --hard &&
+	git clean -xf &&
+	>untracked &&
+	>tracked &&
+	git add tracked &&
+	git stash -u &&
+
+	cat >expect <<-EOF &&
+	 tracked | 0
+	 1 file changed, 0 insertions(+), 0 deletions(-)
+	EOF
+	git stash show --only-untracked --no-include-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show --include-untracked --no-include-untracked >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.30.0.478.g8a0d178c01


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH v3 2/2] stash show: learn stash.showIncludeUntracked
  2021-02-16  7:11   ` [PATCH v3 0/2] stash show: learn --include-untracked and --only-untracked Denton Liu
  2021-02-16  7:11     ` [PATCH v3 1/2] stash show: teach " Denton Liu
@ 2021-02-16  7:11     ` Denton Liu
  2021-03-03 11:16     ` [PATCH v4 0/2] stash show: learn --include-untracked and --only-untracked Denton Liu
  2 siblings, 0 replies; 46+ messages in thread
From: Denton Liu @ 2021-02-16  7:11 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Eric Sunshine

The previous commit teaches `git stash show --include-untracked`. It
may be desirable for a user to be able to always enable the
--include-untracked behavior. Teach the stash.showIncludeUntracked
config option which allows users to do this in a similar manner to
stash.showPatch.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 Documentation/config/stash.txt     | 5 +++++
 Documentation/git-stash.txt        | 4 ++--
 builtin/stash.c                    | 8 ++++++++
 t/t3905-stash-include-untracked.sh | 2 ++
 4 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/Documentation/config/stash.txt b/Documentation/config/stash.txt
index 00eb35434e..413f907cba 100644
--- a/Documentation/config/stash.txt
+++ b/Documentation/config/stash.txt
@@ -5,6 +5,11 @@ stash.useBuiltin::
 	is always used. Setting this will emit a warning, to alert any
 	remaining users that setting this now does nothing.
 
+stash.showIncludeUntracked::
+	If this is set to true, the `git stash show` command without an
+	option will show the untracked files of a stash entry.  Defaults to
+	false. See description of 'show' command in linkgit:git-stash[1].
+
 stash.showPatch::
 	If this is set to true, the `git stash show` command without an
 	option will show the stash entry in patch form.  Defaults to false.
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 8eeb60feb1..a8c8c32f1e 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -91,8 +91,8 @@ show [-u|--include-untracked|--only-untracked] [<diff-options>] [<stash>]::
 	By default, the command shows the diffstat, but it will accept any
 	format known to 'git diff' (e.g., `git stash show -p stash@{1}`
 	to view the second most recent entry in patch form).
-	You can use stash.showStat and/or stash.showPatch config variables
-	to change the default behavior.
+	You can use stash.showIncludeUntracked, stash.showStat, and
+	stash.showPatch config variables to change the default behavior.
 
 pop [--index] [-q|--quiet] [<stash>]::
 
diff --git a/builtin/stash.c b/builtin/stash.c
index 417ed2b4a1..c788a3e236 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -768,6 +768,7 @@ static int list_stash(int argc, const char **argv, const char *prefix)
 
 static int show_stat = 1;
 static int show_patch;
+static int show_include_untracked;
 static int use_legacy_stash;
 
 static int git_stash_config(const char *var, const char *value, void *cb)
@@ -780,6 +781,10 @@ static int git_stash_config(const char *var, const char *value, void *cb)
 		show_patch = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "stash.showincludeuntracked")) {
+		show_include_untracked = git_config_bool(var, value);
+		return 0;
+	}
 	if (!strcmp(var, "stash.usebuiltin")) {
 		use_legacy_stash = !git_config_bool(var, value);
 		return 0;
@@ -868,6 +873,9 @@ static int show_stash(int argc, const char **argv, const char *prefix)
 		if (show_patch)
 			rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
 
+		if (show_include_untracked)
+			show_untracked = UNTRACKED_INCLUDE;
+
 		if (!show_stat && !show_patch) {
 			free_stash_info(&info);
 			return 0;
diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
index 978bc97baf..8bcd4c5ca8 100755
--- a/t/t3905-stash-include-untracked.sh
+++ b/t/t3905-stash-include-untracked.sh
@@ -318,6 +318,8 @@ test_expect_success 'stash show --include-untracked shows untracked files' '
 	test_cmp expect actual &&
 	git stash show --only-untracked --include-untracked >actual &&
 	test_cmp expect actual &&
+	git -c stash.showIncludeUntracked=true stash show >actual &&
+	test_cmp expect actual &&
 
 	cat >expect <<-EOF &&
 	diff --git a/tracked b/tracked
-- 
2.30.0.478.g8a0d178c01


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* Re: [PATCH v3 1/2] stash show: teach --include-untracked and --only-untracked
  2021-02-16  7:11     ` [PATCH v3 1/2] stash show: teach " Denton Liu
@ 2021-02-16 20:22       ` Junio C Hamano
  2021-02-17 11:18         ` Denton Liu
  2021-02-17  2:31       ` Junio C Hamano
  1 sibling, 1 reply; 46+ messages in thread
From: Junio C Hamano @ 2021-02-16 20:22 UTC (permalink / raw)
  To: Denton Liu; +Cc: Git Mailing List, Eric Sunshine

Denton Liu <liu.denton@gmail.com> writes:

> +static void diff_include_untracked(const struct stash_info *info, struct diff_options *diff_opt)
> +{
> +	const struct object_id *oid[] = { &info->w_commit, &info->u_tree };
> +	struct tree *tree[ARRAY_SIZE(oid)];
> +	struct tree_desc tree_desc[ARRAY_SIZE(oid)];
> +	struct unpack_trees_options unpack_tree_opt = { 0 };
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(oid); i++) {
> +		tree[i] = parse_tree_indirect(oid[i]);
> +		if (parse_tree(tree[i]) < 0)
> +			die(_("failed to parse tree"));
> +		init_tree_desc(&tree_desc[i], tree[i]->buffer, tree[i]->size);
> +	}
> +
> +	unpack_tree_opt.head_idx = -1;
> +	unpack_tree_opt.src_index = &the_index;
> +	unpack_tree_opt.dst_index = &the_index;
> +	unpack_tree_opt.fn = twoway_merge;

OK, it looks like this was borrowed from read_tree implementation
for reading two trees into the index, sort-of, but is a bit funny.

The setting of .fn to twoway_merge is misleading.  The .fn callback
is to be used when .merge is set (otherwise nothing should call it
inside unpack-trees.c), but nobody seems to set opt.merge to true.

> +	if (unpack_trees(ARRAY_SIZE(tree_desc), tree_desc, &unpack_tree_opt))
> +		die(_("failed to unpack trees"));
> +
> +	do_diff_cache(&info->b_commit, diff_opt);
> +}

Nice to see that it was just a simple matter of programming ;-)



 builtin/stash.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git c/builtin/stash.c w/builtin/stash.c
index c788a3e236..7e0204bd8a 100644
--- c/builtin/stash.c
+++ w/builtin/stash.c
@@ -807,10 +807,11 @@ static void diff_include_untracked(const struct stash_info *info, struct diff_op
 		init_tree_desc(&tree_desc[i], tree[i]->buffer, tree[i]->size);
 	}
 
+	/* mimic "git read-tree W U" without "-m" */
 	unpack_tree_opt.head_idx = -1;
 	unpack_tree_opt.src_index = &the_index;
 	unpack_tree_opt.dst_index = &the_index;
-	unpack_tree_opt.fn = twoway_merge;
+	unpack_tree_opt.fn = NULL;
 
 	if (unpack_trees(ARRAY_SIZE(tree_desc), tree_desc, &unpack_tree_opt))
 		die(_("failed to unpack trees"));

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* Re: [PATCH v3 1/2] stash show: teach --include-untracked and --only-untracked
  2021-02-16  7:11     ` [PATCH v3 1/2] stash show: teach " Denton Liu
  2021-02-16 20:22       ` Junio C Hamano
@ 2021-02-17  2:31       ` Junio C Hamano
  1 sibling, 0 replies; 46+ messages in thread
From: Junio C Hamano @ 2021-02-17  2:31 UTC (permalink / raw)
  To: Denton Liu; +Cc: Git Mailing List, Eric Sunshine

Denton Liu <liu.denton@gmail.com> writes:

> diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
> index b26a97aef4..978bc97baf 100755
> --- a/t/t3905-stash-include-untracked.sh
> +++ b/t/t3905-stash-include-untracked.sh
> @@ -297,4 +297,88 @@ test_expect_success 'stash -u with globs' '
>  	test_path_is_missing untracked.txt
>  '
>  
> +test_expect_success 'stash show --include-untracked shows untracked files' '
> + ...
> +	cat >expect <<-EOF &&
> +	diff --git a/tracked b/tracked
> +	new file mode 100644
> +	index 0000000..e69de29
> +	diff --git a/untracked b/untracked
> +	new file mode 100644
> +	index 0000000..e69de29
> +	EOF

We'd need this on top.

Thanks.

----- >8 ---------- >8 ---------- >8 ---------- >8 -----
Subject: [PATCH] SQUASH???

 t/t3905-stash-include-untracked.sh | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
index 8bcd4c5ca8..a706ab80a5 100755
--- a/t/t3905-stash-include-untracked.sh
+++ b/t/t3905-stash-include-untracked.sh
@@ -303,6 +303,7 @@ test_expect_success 'stash show --include-untracked shows untracked files' '
 	>untracked &&
 	>tracked &&
 	git add tracked &&
+	empty_blob_oid=$(git rev-parse --short :tracked) &&
 	git stash -u &&
 
 	cat >expect <<-EOF &&
@@ -324,10 +325,10 @@ test_expect_success 'stash show --include-untracked shows untracked files' '
 	cat >expect <<-EOF &&
 	diff --git a/tracked b/tracked
 	new file mode 100644
-	index 0000000..e69de29
+	index 0000000..$empty_blob_oid
 	diff --git a/untracked b/untracked
 	new file mode 100644
-	index 0000000..e69de29
+	index 0000000..$empty_blob_oid
 	EOF
 	git stash show -p --include-untracked >actual &&
 	test_cmp expect actual &&
@@ -341,6 +342,7 @@ test_expect_success 'stash show --only-untracked only shows untracked files' '
 	>untracked &&
 	>tracked &&
 	git add tracked &&
+	empty_blob_oid=$(git rev-parse --short :tracked) &&
 	git stash -u &&
 
 	cat >expect <<-EOF &&
@@ -357,7 +359,7 @@ test_expect_success 'stash show --only-untracked only shows untracked files' '
 	cat >expect <<-EOF &&
 	diff --git a/untracked b/untracked
 	new file mode 100644
-	index 0000000..e69de29
+	index 0000000..$empty_blob_oid
 	EOF
 	git stash show -p --only-untracked >actual &&
 	test_cmp expect actual &&
-- 
2.30.1-684-g310d03ec1a


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* Re: [PATCH v3 1/2] stash show: teach --include-untracked and --only-untracked
  2021-02-16 20:22       ` Junio C Hamano
@ 2021-02-17 11:18         ` Denton Liu
  2021-02-17 17:50           ` Junio C Hamano
  0 siblings, 1 reply; 46+ messages in thread
From: Denton Liu @ 2021-02-17 11:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List, Eric Sunshine

Hi Junio,

On Tue, Feb 16, 2021 at 12:22:52PM -0800, Junio C Hamano wrote:
>  builtin/stash.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git c/builtin/stash.c w/builtin/stash.c
> index c788a3e236..7e0204bd8a 100644
> --- c/builtin/stash.c
> +++ w/builtin/stash.c
> @@ -807,10 +807,11 @@ static void diff_include_untracked(const struct stash_info *info, struct diff_op
>  		init_tree_desc(&tree_desc[i], tree[i]->buffer, tree[i]->size);
>  	}
>  
> +	/* mimic "git read-tree W U" without "-m" */
>  	unpack_tree_opt.head_idx = -1;
>  	unpack_tree_opt.src_index = &the_index;
>  	unpack_tree_opt.dst_index = &the_index;
> -	unpack_tree_opt.fn = twoway_merge;
> +	unpack_tree_opt.fn = NULL;

Perhaps it would be even more clear if we just removed this line
entirely, otherwise it may give future readers a false impression that
.fn is significant in any way.

Aside from that, both of your SQUASH??? commits look good to me. Thanks
for tying up the loose ends.

-Denton

>  	if (unpack_trees(ARRAY_SIZE(tree_desc), tree_desc, &unpack_tree_opt))
>  		die(_("failed to unpack trees"));

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH v3 1/2] stash show: teach --include-untracked and --only-untracked
  2021-02-17 11:18         ` Denton Liu
@ 2021-02-17 17:50           ` Junio C Hamano
  0 siblings, 0 replies; 46+ messages in thread
From: Junio C Hamano @ 2021-02-17 17:50 UTC (permalink / raw)
  To: Denton Liu; +Cc: Git Mailing List, Eric Sunshine

Denton Liu <liu.denton@gmail.com> writes:

> Hi Junio,
>
> On Tue, Feb 16, 2021 at 12:22:52PM -0800, Junio C Hamano wrote:
>>  builtin/stash.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git c/builtin/stash.c w/builtin/stash.c
>> index c788a3e236..7e0204bd8a 100644
>> --- c/builtin/stash.c
>> +++ w/builtin/stash.c
>> @@ -807,10 +807,11 @@ static void diff_include_untracked(const struct stash_info *info, struct diff_op
>>  		init_tree_desc(&tree_desc[i], tree[i]->buffer, tree[i]->size);
>>  	}
>>  
>> +	/* mimic "git read-tree W U" without "-m" */
>>  	unpack_tree_opt.head_idx = -1;
>>  	unpack_tree_opt.src_index = &the_index;
>>  	unpack_tree_opt.dst_index = &the_index;
>> -	unpack_tree_opt.fn = twoway_merge;
>> +	unpack_tree_opt.fn = NULL;
>
> Perhaps it would be even more clear if we just removed this line
> entirely, otherwise it may give future readers a false impression that
> .fn is significant in any way.

Assignment of something concrete like "twoway_merge" to .fn when it
is a no-op was misleading, but between NULL or uninitialized, both
state clear that it is not used, so I am OK with either.

> Aside from that, both of your SQUASH??? commits look good to me. Thanks
> for tying up the loose ends.

We may want to write a custom unpack_trees() callback for this, and
use it here to catch "this third tree claims to be untracked, but
why does it have an entry that overlaps and/or D/F-conflicts with
the entry from the working tree side?" that you talked about in the
log message, but I think it is better to leave it for future [*], as
the feature should already be usable in its current shape.

Thanks.


[Footnote]

 * Yes, I didn't say "we can leave it", but said "it is better to
   leave it"; as long as we do not declare victory too early and end
   up shipping a half-baked unusable mess, I think it is better to
   wrap a topic early and plan to make it nicer in a future
   follow-up.

   To encourage such future refinements, however, you may add a
   NEEDSWORK comment in the code as a reminder for our future
   selves.

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH v4 0/2] stash show: learn --include-untracked and --only-untracked
  2021-02-16  7:11   ` [PATCH v3 0/2] stash show: learn --include-untracked and --only-untracked Denton Liu
  2021-02-16  7:11     ` [PATCH v3 1/2] stash show: teach " Denton Liu
  2021-02-16  7:11     ` [PATCH v3 2/2] stash show: learn stash.showIncludeUntracked Denton Liu
@ 2021-03-03 11:16     ` Denton Liu
  2021-03-03 11:16       ` [PATCH v4 1/2] stash show: teach " Denton Liu
                         ` (2 more replies)
  2 siblings, 3 replies; 46+ messages in thread
From: Denton Liu @ 2021-03-03 11:16 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Eric Sunshine

A blindspot that I've noticed in git is that it's not possible to
properly view a stash entry that has untracked files via `git stash
show`. Teach `git stash show --include-untracked` which should do this.
In addition, this series also teaches `--only-untracked` and the
`stash.showIncludeUntracked` config option.

This series is based on 'dl/stash-cleanup'.

Changes since v3:

* Incorporate Junio's SQUASH??? commits

* Implement a custom unpack_trees() callback to detect the case where
  there are duplicate entries in worktree and untracked commits

Changes since v2:

* Base this series on top of 'dl/stash-cleanup'

* Attempt to replicate the read-tree code to merge the untracked tree

Changes since v1:

* Add a dash for <log-options> and <diff-options>

* Fix the misspelling of --include-untracked in a commit message

* Change the approach from concatenating diffs to using `git read-tree`

Denton Liu (2):
  stash show: teach --include-untracked and --only-untracked
  stash show: learn stash.showIncludeUntracked

 Documentation/config/stash.txt         |   5 ++
 Documentation/git-stash.txt            |  22 +++--
 builtin/stash.c                        |  62 +++++++++++++-
 contrib/completion/git-completion.bash |   2 +-
 t/t3905-stash-include-untracked.sh     | 108 +++++++++++++++++++++++++
 unpack-trees.c                         |  22 +++++
 unpack-trees.h                         |   2 +
 7 files changed, 214 insertions(+), 9 deletions(-)

Range-diff against v3:
1:  85b81f2f06 ! 1:  af3757135b stash show: teach --include-untracked and --only-untracked
    @@ Commit message
         third-parent if it exists, are shown in addition to the paths that have
         modifications between the stash base and the working tree in the stash.
     
    -    One limitation of this is that it would be possible to manually craft a
    -    stash entry where duplicate untracked files in the stash entry will mask
    -    tracked files. This seems like an instance of "Doctor, it hurts when I
    -    do this! So don't do that!" so this can be written off.
    +    It is possible to manually craft a malformed stash entry where duplicate
    +    untracked files in the stash entry will mask tracked files. We detect
    +    and error out in that case via a custom unpack_trees() callback:
    +    stash_worktree_untracked_merge().
     
         Also, teach stash the --only-untracked option which only shows the
         untracked files of a stash entry. This is similar to `git show stash^3`
         but it is nice to provide a convenient abstraction for it so that users
         do not have to think about the underlying implementation.
     
    -
    - ## Notes ##
    -    I am not familiar with the read-tree code so this attempt at replicating
    -    the read-tree code may in diff_include_untracked() may be incorrect
    -    (particularly the use of the_index?).
    -
    -    Also, I could not figure out how to make unpack_trees() error out in the
    -    case where untracked tree entry contains duplicate entries with the
    -    worktree entry.
    -
      ## Documentation/git-stash.txt ##
     @@ Documentation/git-stash.txt: SYNOPSIS
      --------
    @@ builtin/stash.c: static int git_stash_config(const char *var, const char *value,
     +	unpack_tree_opt.head_idx = -1;
     +	unpack_tree_opt.src_index = &the_index;
     +	unpack_tree_opt.dst_index = &the_index;
    -+	unpack_tree_opt.fn = twoway_merge;
    ++	unpack_tree_opt.merge = 1;
    ++	unpack_tree_opt.fn = stash_worktree_untracked_merge;
     +
     +	if (unpack_trees(ARRAY_SIZE(tree_desc), tree_desc, &unpack_tree_opt))
     +		die(_("failed to unpack trees"));
    @@ t/t3905-stash-include-untracked.sh: test_expect_success 'stash -u with globs' '
     +	>untracked &&
     +	>tracked &&
     +	git add tracked &&
    ++	empty_blob_oid=$(git rev-parse --short :tracked) &&
     +	git stash -u &&
     +
     +	cat >expect <<-EOF &&
    @@ t/t3905-stash-include-untracked.sh: test_expect_success 'stash -u with globs' '
     +	cat >expect <<-EOF &&
     +	diff --git a/tracked b/tracked
     +	new file mode 100644
    -+	index 0000000..e69de29
    ++	index 0000000..$empty_blob_oid
     +	diff --git a/untracked b/untracked
     +	new file mode 100644
    -+	index 0000000..e69de29
    ++	index 0000000..$empty_blob_oid
     +	EOF
     +	git stash show -p --include-untracked >actual &&
     +	test_cmp expect actual &&
    @@ t/t3905-stash-include-untracked.sh: test_expect_success 'stash -u with globs' '
     +	>untracked &&
     +	>tracked &&
     +	git add tracked &&
    ++	empty_blob_oid=$(git rev-parse --short :tracked) &&
     +	git stash -u &&
     +
     +	cat >expect <<-EOF &&
    @@ t/t3905-stash-include-untracked.sh: test_expect_success 'stash -u with globs' '
     +	cat >expect <<-EOF &&
     +	diff --git a/untracked b/untracked
     +	new file mode 100644
    -+	index 0000000..e69de29
    ++	index 0000000..$empty_blob_oid
     +	EOF
     +	git stash show -p --only-untracked >actual &&
     +	test_cmp expect actual &&
    @@ t/t3905-stash-include-untracked.sh: test_expect_success 'stash -u with globs' '
     +	git stash show --include-untracked --no-include-untracked >actual &&
     +	test_cmp expect actual
     +'
    ++
    ++test_expect_success 'stash show --include-untracked errors on duplicate files' '
    ++	git reset --hard &&
    ++	git clean -xf &&
    ++	>tracked &&
    ++	git add tracked &&
    ++	tree=$(git write-tree) &&
    ++	i_commit=$(git commit-tree -p HEAD -m "index on any-branch" "$tree") &&
    ++	test_when_finished "rm -f untracked_index" &&
    ++	u_commit=$(
    ++		GIT_INDEX_FILE="untracked_index" &&
    ++		export GIT_INDEX_FILE &&
    ++		git update-index --add tracked &&
    ++		u_tree=$(git write-tree) &&
    ++		git commit-tree -m "untracked files on any-branch" "$u_tree"
    ++	) &&
    ++	w_commit=$(git commit-tree -p HEAD -p "$i_commit" -p "$u_commit" -m "WIP on any-branch" "$tree") &&
    ++	test_must_fail git stash show --include-untracked "$w_commit" 2>err &&
    ++	test_i18ngrep "worktree and untracked commit have duplicate entries: tracked" err
    ++'
     +
      test_done
    +
    + ## unpack-trees.c ##
    +@@ unpack-trees.c: int oneway_merge(const struct cache_entry * const *src,
    + 	}
    + 	return merged_entry(a, old, o);
    + }
    ++
    ++/*
    ++ * Merge worktree and untracked entries in a stash entry.
    ++ *
    ++ * Ignore all index entries. Collapse remaining trees but make sure that they
    ++ * don't have any conflicting files.
    ++ */
    ++int stash_worktree_untracked_merge(const struct cache_entry * const *src,
    ++				   struct unpack_trees_options *o)
    ++{
    ++	const struct cache_entry *worktree = src[1];
    ++	const struct cache_entry *untracked = src[2];
    ++
    ++	if (o->merge_size != 2)
    ++		BUG("invalid merge_size: %d", o->merge_size);
    ++
    ++	if (worktree && untracked)
    ++		return error(_("worktree and untracked commit have duplicate entries: %s"),
    ++			     super_prefixed(worktree->name));
    ++
    ++	return merged_entry(worktree ? worktree : untracked, NULL, o);
    ++}
    +
    + ## unpack-trees.h ##
    +@@ unpack-trees.h: int bind_merge(const struct cache_entry * const *src,
    + 	       struct unpack_trees_options *o);
    + int oneway_merge(const struct cache_entry * const *src,
    + 		 struct unpack_trees_options *o);
    ++int stash_worktree_untracked_merge(const struct cache_entry * const *src,
    ++				   struct unpack_trees_options *o);
    + 
    + #endif
2:  d19d07ec27 = 2:  3480086f1d stash show: learn stash.showIncludeUntracked
-- 
2.31.0.rc1.228.gb75b4e4ce2


^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH v4 1/2] stash show: teach --include-untracked and --only-untracked
  2021-03-03 11:16     ` [PATCH v4 0/2] stash show: learn --include-untracked and --only-untracked Denton Liu
@ 2021-03-03 11:16       ` Denton Liu
  2021-03-03 11:16       ` [PATCH v4 2/2] stash show: learn stash.showIncludeUntracked Denton Liu
  2021-03-04  0:38       ` [PATCH v4 0/2] stash show: learn --include-untracked and --only-untracked Junio C Hamano
  2 siblings, 0 replies; 46+ messages in thread
From: Denton Liu @ 2021-03-03 11:16 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Eric Sunshine

Stash entries can be made with untracked files via
`git stash push --include-untracked`. However, because the untracked
files are stored in the third parent of the stash entry and not the
stash entry itself, running `git stash show` does not include the
untracked files as part of the diff.

With --include-untracked, untracked paths, which are recorded in the
third-parent if it exists, are shown in addition to the paths that have
modifications between the stash base and the working tree in the stash.

It is possible to manually craft a malformed stash entry where duplicate
untracked files in the stash entry will mask tracked files. We detect
and error out in that case via a custom unpack_trees() callback:
stash_worktree_untracked_merge().

Also, teach stash the --only-untracked option which only shows the
untracked files of a stash entry. This is similar to `git show stash^3`
but it is nice to provide a convenient abstraction for it so that users
do not have to think about the underlying implementation.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 Documentation/git-stash.txt            |  18 +++--
 builtin/stash.c                        |  54 ++++++++++++-
 contrib/completion/git-completion.bash |   2 +-
 t/t3905-stash-include-untracked.sh     | 106 +++++++++++++++++++++++++
 unpack-trees.c                         |  22 +++++
 unpack-trees.h                         |   2 +
 6 files changed, 197 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index f1197d641b..8eeb60feb1 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -9,7 +9,7 @@ SYNOPSIS
 --------
 [verse]
 'git stash' list [<log-options>]
-'git stash' show [<diff-options>] [<stash>]
+'git stash' show [-u|--include-untracked|--only-untracked] [<diff-options>] [<stash>]
 'git stash' drop [-q|--quiet] [<stash>]
 'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]
 'git stash' branch <branchname> [<stash>]
@@ -83,7 +83,7 @@ stash@{1}: On master: 9cc0589... Add git-stash
 The command takes options applicable to the 'git log'
 command to control what is shown and how. See linkgit:git-log[1].
 
-show [<diff-options>] [<stash>]::
+show [-u|--include-untracked|--only-untracked] [<diff-options>] [<stash>]::
 
 	Show the changes recorded in the stash entry as a diff between the
 	stashed contents and the commit back when the stash entry was first
@@ -160,10 +160,18 @@ up with `git clean`.
 
 -u::
 --include-untracked::
-	This option is only valid for `push` and `save` commands.
+--no-include-untracked::
+	When used with the `push` and `save` commands,
+	all untracked files are also stashed and then cleaned up with
+	`git clean`.
 +
-All untracked files are also stashed and then cleaned up with
-`git clean`.
+When used with the `show` command, show the untracked files in the stash
+entry as part of the diff.
+
+--only-untracked::
+	This option is only valid for the `show` command.
++
+Show only the untracked files in the stash entry as part of the diff.
 
 --index::
 	This option is only valid for `pop` and `apply` commands.
diff --git a/builtin/stash.c b/builtin/stash.c
index 6f2b58f6ab..9b7a541cd0 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -787,6 +787,33 @@ static int git_stash_config(const char *var, const char *value, void *cb)
 	return git_diff_basic_config(var, value, cb);
 }
 
+static void diff_include_untracked(const struct stash_info *info, struct diff_options *diff_opt)
+{
+	const struct object_id *oid[] = { &info->w_commit, &info->u_tree };
+	struct tree *tree[ARRAY_SIZE(oid)];
+	struct tree_desc tree_desc[ARRAY_SIZE(oid)];
+	struct unpack_trees_options unpack_tree_opt = { 0 };
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(oid); i++) {
+		tree[i] = parse_tree_indirect(oid[i]);
+		if (parse_tree(tree[i]) < 0)
+			die(_("failed to parse tree"));
+		init_tree_desc(&tree_desc[i], tree[i]->buffer, tree[i]->size);
+	}
+
+	unpack_tree_opt.head_idx = -1;
+	unpack_tree_opt.src_index = &the_index;
+	unpack_tree_opt.dst_index = &the_index;
+	unpack_tree_opt.merge = 1;
+	unpack_tree_opt.fn = stash_worktree_untracked_merge;
+
+	if (unpack_trees(ARRAY_SIZE(tree_desc), tree_desc, &unpack_tree_opt))
+		die(_("failed to unpack trees"));
+
+	do_diff_cache(&info->b_commit, diff_opt);
+}
+
 static int show_stash(int argc, const char **argv, const char *prefix)
 {
 	int i;
@@ -795,7 +822,18 @@ static int show_stash(int argc, const char **argv, const char *prefix)
 	struct rev_info rev;
 	struct strvec stash_args = STRVEC_INIT;
 	struct strvec revision_args = STRVEC_INIT;
+	enum {
+		UNTRACKED_NONE,
+		UNTRACKED_INCLUDE,
+		UNTRACKED_ONLY
+	} show_untracked = UNTRACKED_NONE;
 	struct option options[] = {
+		OPT_SET_INT('u', "include-untracked", &show_untracked,
+			    N_("include untracked files in the stash"),
+			    UNTRACKED_INCLUDE),
+		OPT_SET_INT_F(0, "only-untracked", &show_untracked,
+			      N_("only show untracked files in the stash"),
+			      UNTRACKED_ONLY, PARSE_OPT_NONEG),
 		OPT_END()
 	};
 
@@ -803,6 +841,10 @@ static int show_stash(int argc, const char **argv, const char *prefix)
 	git_config(git_diff_ui_config, NULL);
 	init_revisions(&rev, prefix);
 
+	argc = parse_options(argc, argv, prefix, options, git_stash_show_usage,
+			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |
+			     PARSE_OPT_KEEP_DASHDASH);
+
 	strvec_push(&revision_args, argv[0]);
 	for (i = 1; i < argc; i++) {
 		if (argv[i][0] != '-')
@@ -845,7 +887,17 @@ static int show_stash(int argc, const char **argv, const char *prefix)
 
 	rev.diffopt.flags.recursive = 1;
 	setup_diff_pager(&rev.diffopt);
-	diff_tree_oid(&info.b_commit, &info.w_commit, "", &rev.diffopt);
+	switch (show_untracked) {
+	case UNTRACKED_NONE:
+		diff_tree_oid(&info.b_commit, &info.w_commit, "", &rev.diffopt);
+		break;
+	case UNTRACKED_ONLY:
+		diff_root_tree_oid(&info.u_tree, "", &rev.diffopt);
+		break;
+	case UNTRACKED_INCLUDE:
+		diff_include_untracked(&info, &rev.diffopt);
+		break;
+	}
 	log_tree_diff_flush(&rev);
 
 	free_stash_info(&info);
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 4b1f4264a6..64ef6ffa21 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3051,7 +3051,7 @@ _git_stash ()
 			__gitcomp "--name-status --oneline --patch-with-stat"
 			;;
 		show,--*)
-			__gitcomp "$__git_diff_common_options"
+			__gitcomp "--include-untracked --only-untracked $__git_diff_common_options"
 			;;
 		branch,--*)
 			;;
diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
index b26a97aef4..08ceef6411 100755
--- a/t/t3905-stash-include-untracked.sh
+++ b/t/t3905-stash-include-untracked.sh
@@ -297,4 +297,110 @@ test_expect_success 'stash -u with globs' '
 	test_path_is_missing untracked.txt
 '
 
+test_expect_success 'stash show --include-untracked shows untracked files' '
+	git reset --hard &&
+	git clean -xf &&
+	>untracked &&
+	>tracked &&
+	git add tracked &&
+	empty_blob_oid=$(git rev-parse --short :tracked) &&
+	git stash -u &&
+
+	cat >expect <<-EOF &&
+	 tracked   | 0
+	 untracked | 0
+	 2 files changed, 0 insertions(+), 0 deletions(-)
+	EOF
+	git stash show --include-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show -u >actual &&
+	test_cmp expect actual &&
+	git stash show --no-include-untracked --include-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show --only-untracked --include-untracked >actual &&
+	test_cmp expect actual &&
+
+	cat >expect <<-EOF &&
+	diff --git a/tracked b/tracked
+	new file mode 100644
+	index 0000000..$empty_blob_oid
+	diff --git a/untracked b/untracked
+	new file mode 100644
+	index 0000000..$empty_blob_oid
+	EOF
+	git stash show -p --include-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show --include-untracked -p >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'stash show --only-untracked only shows untracked files' '
+	git reset --hard &&
+	git clean -xf &&
+	>untracked &&
+	>tracked &&
+	git add tracked &&
+	empty_blob_oid=$(git rev-parse --short :tracked) &&
+	git stash -u &&
+
+	cat >expect <<-EOF &&
+	 untracked | 0
+	 1 file changed, 0 insertions(+), 0 deletions(-)
+	EOF
+	git stash show --only-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show --no-include-untracked --only-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show --include-untracked --only-untracked >actual &&
+	test_cmp expect actual &&
+
+	cat >expect <<-EOF &&
+	diff --git a/untracked b/untracked
+	new file mode 100644
+	index 0000000..$empty_blob_oid
+	EOF
+	git stash show -p --only-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show --only-untracked -p >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'stash show --no-include-untracked cancels --{include,show}-untracked' '
+	git reset --hard &&
+	git clean -xf &&
+	>untracked &&
+	>tracked &&
+	git add tracked &&
+	git stash -u &&
+
+	cat >expect <<-EOF &&
+	 tracked | 0
+	 1 file changed, 0 insertions(+), 0 deletions(-)
+	EOF
+	git stash show --only-untracked --no-include-untracked >actual &&
+	test_cmp expect actual &&
+	git stash show --include-untracked --no-include-untracked >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'stash show --include-untracked errors on duplicate files' '
+	git reset --hard &&
+	git clean -xf &&
+	>tracked &&
+	git add tracked &&
+	tree=$(git write-tree) &&
+	i_commit=$(git commit-tree -p HEAD -m "index on any-branch" "$tree") &&
+	test_when_finished "rm -f untracked_index" &&
+	u_commit=$(
+		GIT_INDEX_FILE="untracked_index" &&
+		export GIT_INDEX_FILE &&
+		git update-index --add tracked &&
+		u_tree=$(git write-tree) &&
+		git commit-tree -m "untracked files on any-branch" "$u_tree"
+	) &&
+	w_commit=$(git commit-tree -p HEAD -p "$i_commit" -p "$u_commit" -m "WIP on any-branch" "$tree") &&
+	test_must_fail git stash show --include-untracked "$w_commit" 2>err &&
+	test_i18ngrep "worktree and untracked commit have duplicate entries: tracked" err
+'
+
 test_done
diff --git a/unpack-trees.c b/unpack-trees.c
index af6e9b9c2f..dde8c320c5 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -2566,3 +2566,25 @@ int oneway_merge(const struct cache_entry * const *src,
 	}
 	return merged_entry(a, old, o);
 }
+
+/*
+ * Merge worktree and untracked entries in a stash entry.
+ *
+ * Ignore all index entries. Collapse remaining trees but make sure that they
+ * don't have any conflicting files.
+ */
+int stash_worktree_untracked_merge(const struct cache_entry * const *src,
+				   struct unpack_trees_options *o)
+{
+	const struct cache_entry *worktree = src[1];
+	const struct cache_entry *untracked = src[2];
+
+	if (o->merge_size != 2)
+		BUG("invalid merge_size: %d", o->merge_size);
+
+	if (worktree && untracked)
+		return error(_("worktree and untracked commit have duplicate entries: %s"),
+			     super_prefixed(worktree->name));
+
+	return merged_entry(worktree ? worktree : untracked, NULL, o);
+}
diff --git a/unpack-trees.h b/unpack-trees.h
index 2e87875b15..2d88b19dca 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -114,5 +114,7 @@ int bind_merge(const struct cache_entry * const *src,
 	       struct unpack_trees_options *o);
 int oneway_merge(const struct cache_entry * const *src,
 		 struct unpack_trees_options *o);
+int stash_worktree_untracked_merge(const struct cache_entry * const *src,
+				   struct unpack_trees_options *o);
 
 #endif
-- 
2.31.0.rc1.228.gb75b4e4ce2


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH v4 2/2] stash show: learn stash.showIncludeUntracked
  2021-03-03 11:16     ` [PATCH v4 0/2] stash show: learn --include-untracked and --only-untracked Denton Liu
  2021-03-03 11:16       ` [PATCH v4 1/2] stash show: teach " Denton Liu
@ 2021-03-03 11:16       ` Denton Liu
  2021-03-04  0:38       ` [PATCH v4 0/2] stash show: learn --include-untracked and --only-untracked Junio C Hamano
  2 siblings, 0 replies; 46+ messages in thread
From: Denton Liu @ 2021-03-03 11:16 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano, Eric Sunshine

The previous commit teaches `git stash show --include-untracked`. It
may be desirable for a user to be able to always enable the
--include-untracked behavior. Teach the stash.showIncludeUntracked
config option which allows users to do this in a similar manner to
stash.showPatch.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 Documentation/config/stash.txt     | 5 +++++
 Documentation/git-stash.txt        | 4 ++--
 builtin/stash.c                    | 8 ++++++++
 t/t3905-stash-include-untracked.sh | 2 ++
 4 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/Documentation/config/stash.txt b/Documentation/config/stash.txt
index 00eb35434e..413f907cba 100644
--- a/Documentation/config/stash.txt
+++ b/Documentation/config/stash.txt
@@ -5,6 +5,11 @@ stash.useBuiltin::
 	is always used. Setting this will emit a warning, to alert any
 	remaining users that setting this now does nothing.
 
+stash.showIncludeUntracked::
+	If this is set to true, the `git stash show` command without an
+	option will show the untracked files of a stash entry.  Defaults to
+	false. See description of 'show' command in linkgit:git-stash[1].
+
 stash.showPatch::
 	If this is set to true, the `git stash show` command without an
 	option will show the stash entry in patch form.  Defaults to false.
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 8eeb60feb1..a8c8c32f1e 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -91,8 +91,8 @@ show [-u|--include-untracked|--only-untracked] [<diff-options>] [<stash>]::
 	By default, the command shows the diffstat, but it will accept any
 	format known to 'git diff' (e.g., `git stash show -p stash@{1}`
 	to view the second most recent entry in patch form).
-	You can use stash.showStat and/or stash.showPatch config variables
-	to change the default behavior.
+	You can use stash.showIncludeUntracked, stash.showStat, and
+	stash.showPatch config variables to change the default behavior.
 
 pop [--index] [-q|--quiet] [<stash>]::
 
diff --git a/builtin/stash.c b/builtin/stash.c
index 9b7a541cd0..8922a1240c 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -768,6 +768,7 @@ static int list_stash(int argc, const char **argv, const char *prefix)
 
 static int show_stat = 1;
 static int show_patch;
+static int show_include_untracked;
 static int use_legacy_stash;
 
 static int git_stash_config(const char *var, const char *value, void *cb)
@@ -780,6 +781,10 @@ static int git_stash_config(const char *var, const char *value, void *cb)
 		show_patch = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "stash.showincludeuntracked")) {
+		show_include_untracked = git_config_bool(var, value);
+		return 0;
+	}
 	if (!strcmp(var, "stash.usebuiltin")) {
 		use_legacy_stash = !git_config_bool(var, value);
 		return 0;
@@ -869,6 +874,9 @@ static int show_stash(int argc, const char **argv, const char *prefix)
 		if (show_patch)
 			rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
 
+		if (show_include_untracked)
+			show_untracked = UNTRACKED_INCLUDE;
+
 		if (!show_stat && !show_patch) {
 			free_stash_info(&info);
 			return 0;
diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
index 08ceef6411..b470db7ef7 100755
--- a/t/t3905-stash-include-untracked.sh
+++ b/t/t3905-stash-include-untracked.sh
@@ -319,6 +319,8 @@ test_expect_success 'stash show --include-untracked shows untracked files' '
 	test_cmp expect actual &&
 	git stash show --only-untracked --include-untracked >actual &&
 	test_cmp expect actual &&
+	git -c stash.showIncludeUntracked=true stash show >actual &&
+	test_cmp expect actual &&
 
 	cat >expect <<-EOF &&
 	diff --git a/tracked b/tracked
-- 
2.31.0.rc1.228.gb75b4e4ce2


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* Re: [PATCH v4 0/2] stash show: learn --include-untracked and --only-untracked
  2021-03-03 11:16     ` [PATCH v4 0/2] stash show: learn --include-untracked and --only-untracked Denton Liu
  2021-03-03 11:16       ` [PATCH v4 1/2] stash show: teach " Denton Liu
  2021-03-03 11:16       ` [PATCH v4 2/2] stash show: learn stash.showIncludeUntracked Denton Liu
@ 2021-03-04  0:38       ` Junio C Hamano
  2021-03-04  1:33         ` Denton Liu
  2 siblings, 1 reply; 46+ messages in thread
From: Junio C Hamano @ 2021-03-04  0:38 UTC (permalink / raw)
  To: Denton Liu; +Cc: Git Mailing List, Eric Sunshine

Denton Liu <liu.denton@gmail.com> writes:

> A blindspot that I've noticed in git is that it's not possible to
> properly view a stash entry that has untracked files via `git stash
> show`. Teach `git stash show --include-untracked` which should do this.
> In addition, this series also teaches `--only-untracked` and the
> `stash.showIncludeUntracked` config option.
>
> This series is based on 'dl/stash-cleanup'.
>
> Changes since v3:
>
> * Incorporate Junio's SQUASH??? commits
>
> * Implement a custom unpack_trees() callback to detect the case where
>   there are duplicate entries in worktree and untracked commits

I actually expected the latter enhancement to be done outside the
scope of this series.  I briefly looked at the callback but I am not
convinced that it is correct (e.g. how do you notice and barf when
the untracked tree records foo/bar.txt and the index or the working
tree records foo as a file?).

Thanks.

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH v4 0/2] stash show: learn --include-untracked and --only-untracked
  2021-03-04  0:38       ` [PATCH v4 0/2] stash show: learn --include-untracked and --only-untracked Junio C Hamano
@ 2021-03-04  1:33         ` Denton Liu
  2021-03-04  1:42           ` Denton Liu
  0 siblings, 1 reply; 46+ messages in thread
From: Denton Liu @ 2021-03-04  1:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List, Eric Sunshine

Hi Junio,

On Wed, Mar 03, 2021 at 04:38:31PM -0800, Junio C Hamano wrote:
> Denton Liu <liu.denton@gmail.com> writes:
> 
> > A blindspot that I've noticed in git is that it's not possible to
> > properly view a stash entry that has untracked files via `git stash
> > show`. Teach `git stash show --include-untracked` which should do this.
> > In addition, this series also teaches `--only-untracked` and the
> > `stash.showIncludeUntracked` config option.
> >
> > This series is based on 'dl/stash-cleanup'.
> >
> > Changes since v3:
> >
> > * Incorporate Junio's SQUASH??? commits
> >
> > * Implement a custom unpack_trees() callback to detect the case where
> >   there are duplicate entries in worktree and untracked commits
> 
> I actually expected the latter enhancement to be done outside the
> scope of this series.  I briefly looked at the callback but I am not
> convinced that it is correct (e.g. how do you notice and barf when
> the untracked tree records foo/bar.txt and the index or the working
> tree records foo as a file?).

From my testing, the conflict is detected just fine. The following
test-case should confirm it:

-- >8 --
From: Denton Liu <liu.denton@gmail.com>
Subject: [PATCH] fixup! stash show: teach --include-untracked and --only-untracked

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t3905-stash-include-untracked.sh | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
index b470db7ef7..470aa65b44 100755
--- a/t/t3905-stash-include-untracked.sh
+++ b/t/t3905-stash-include-untracked.sh
@@ -405,4 +405,27 @@ test_expect_success 'stash show --include-untracked errors on duplicate files' '
 	test_i18ngrep "worktree and untracked commit have duplicate entries: tracked" err
 '
 
+test_expect_success 'stash show --include-untracked errors on directory/file conflict' '
+	git reset --hard &&
+	git clean -xf &&
+	>tracked &&
+	git add tracked &&
+	tree=$(git write-tree) &&
+	i_commit=$(git commit-tree -p HEAD -m "index on any-branch" "$tree") &&
+	test_when_finished "rm -f untracked_index" &&
+	u_commit=$(
+		GIT_INDEX_FILE="untracked_index" &&
+		export GIT_INDEX_FILE &&
+		rm tracked &&
+		mkdir tracked &&
+		>tracked/file &&
+		git update-index --add tracked/file &&
+		u_tree=$(git write-tree) &&
+		git commit-tree -m "untracked files on any-branch" "$u_tree"
+	) &&
+	w_commit=$(git commit-tree -p HEAD -p "$i_commit" -p "$u_commit" -m "WIP on any-branch" "$tree") &&
+	test_must_fail git stash show --include-untracked "$w_commit" 2>err &&
+	test_i18ngrep "worktree and untracked commit have duplicate entries: tracked" err
+'
+
 test_done
-- 
2.31.0.rc1.228.gb75b4e4ce2


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* Re: [PATCH v4 0/2] stash show: learn --include-untracked and --only-untracked
  2021-03-04  1:33         ` Denton Liu
@ 2021-03-04  1:42           ` Denton Liu
  0 siblings, 0 replies; 46+ messages in thread
From: Denton Liu @ 2021-03-04  1:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List, Eric Sunshine

On Wed, Mar 03, 2021 at 05:33:17PM -0800, Denton Liu wrote:
> Hi Junio,
> 
> On Wed, Mar 03, 2021 at 04:38:31PM -0800, Junio C Hamano wrote:
> > Denton Liu <liu.denton@gmail.com> writes:
> > 
> > > A blindspot that I've noticed in git is that it's not possible to
> > > properly view a stash entry that has untracked files via `git stash
> > > show`. Teach `git stash show --include-untracked` which should do this.
> > > In addition, this series also teaches `--only-untracked` and the
> > > `stash.showIncludeUntracked` config option.
> > >
> > > This series is based on 'dl/stash-cleanup'.
> > >
> > > Changes since v3:
> > >
> > > * Incorporate Junio's SQUASH??? commits
> > >
> > > * Implement a custom unpack_trees() callback to detect the case where
> > >   there are duplicate entries in worktree and untracked commits
> > 
> > I actually expected the latter enhancement to be done outside the
> > scope of this series.

I decided to squash it into the current series because we have a lot of
time until it will be merged (since it's definitely not going in now
during the rc-period). If you'd like, though, I can reroll this series
with the callback as a commit on top.

> > I briefly looked at the callback but I am not
> > convinced that it is correct (e.g. how do you notice and barf when
> > the untracked tree records foo/bar.txt and the index or the working
> > tree records foo as a file?).
> 
> From my testing, the conflict is detected just fine. The following
> test-case should confirm it:
> 
> -- >8 --
> From: Denton Liu <liu.denton@gmail.com>
> Subject: [PATCH] fixup! stash show: teach --include-untracked and --only-untracked
> 
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
>  t/t3905-stash-include-untracked.sh | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh
> index b470db7ef7..470aa65b44 100755
> --- a/t/t3905-stash-include-untracked.sh
> +++ b/t/t3905-stash-include-untracked.sh
> @@ -405,4 +405,27 @@ test_expect_success 'stash show --include-untracked errors on duplicate files' '
>  	test_i18ngrep "worktree and untracked commit have duplicate entries: tracked" err
>  '
>  
> +test_expect_success 'stash show --include-untracked errors on directory/file conflict' '
> +	git reset --hard &&
> +	git clean -xf &&
> +	>tracked &&
> +	git add tracked &&
> +	tree=$(git write-tree) &&
> +	i_commit=$(git commit-tree -p HEAD -m "index on any-branch" "$tree") &&
> +	test_when_finished "rm -f untracked_index" &&
> +	u_commit=$(
> +		GIT_INDEX_FILE="untracked_index" &&
> +		export GIT_INDEX_FILE &&
> +		rm tracked &&
> +		mkdir tracked &&
> +		>tracked/file &&
> +		git update-index --add tracked/file &&
> +		u_tree=$(git write-tree) &&
> +		git commit-tree -m "untracked files on any-branch" "$u_tree"
> +	) &&
> +	w_commit=$(git commit-tree -p HEAD -p "$i_commit" -p "$u_commit" -m "WIP on any-branch" "$tree") &&
> +	test_must_fail git stash show --include-untracked "$w_commit" 2>err &&
> +	test_i18ngrep "worktree and untracked commit have duplicate entries: tracked" err
> +'
> +
>  test_done
> -- 
> 2.31.0.rc1.228.gb75b4e4ce2
> 

^ permalink raw reply	[flat|nested] 46+ messages in thread

end of thread, other threads:[~2021-03-04  1:43 UTC | newest]

Thread overview: 46+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-02  9:31 [PATCH 0/9] stash show: learn --include-untracked and --only-untracked Denton Liu
2021-02-02  9:33 ` Denton Liu
2021-02-02  9:33 ` [PATCH 1/9] git-stash.txt: be explicit about subcommand options Denton Liu
2021-02-02 17:37   ` Eric Sunshine
2021-02-02  9:33 ` [PATCH 2/9] t3905: remove spaces after redirect operators Denton Liu
2021-02-02  9:33 ` [PATCH 3/9] t3905: move all commands into test cases Denton Liu
2021-02-02 21:41   ` Junio C Hamano
2021-02-02  9:33 ` [PATCH 4/9] t3905: remove nested git in command substitution Denton Liu
2021-02-02  9:33 ` [PATCH 5/9] t3905: replace test -s with test_file_not_empty Denton Liu
2021-02-02  9:33 ` [PATCH 6/9] t3905: use test_cmp() to check file contents Denton Liu
2021-02-02  9:33 ` [PATCH 7/9] stash: declare ref_stash as an array Denton Liu
2021-02-02 21:46   ` Junio C Hamano
2021-02-02  9:33 ` [PATCH 8/9] stash show: teach --include-tracked and --only-untracked Denton Liu
2021-02-02 21:56   ` Junio C Hamano
2021-02-02  9:33 ` [PATCH 9/9] stash show: learn stash.showIncludeUntracked Denton Liu
2021-02-09  7:28 ` [PATCH v2 0/9] stash show: learn --include-untracked and --only-untracked Denton Liu
2021-02-09  7:28   ` [PATCH v2 1/9] git-stash.txt: be explicit about subcommand options Denton Liu
2021-02-10  7:17     ` Junio C Hamano
2021-02-11 19:07       ` [PATCH] fixup! " Denton Liu
2021-02-09  7:28   ` [PATCH v2 2/9] t3905: remove spaces after redirect operators Denton Liu
2021-02-10  7:18     ` Junio C Hamano
2021-02-09  7:28   ` [PATCH v2 3/9] t3905: move all commands into test cases Denton Liu
2021-02-09  7:28   ` [PATCH v2 4/9] t3905: remove nested git in command substitution Denton Liu
2021-02-10  7:24     ` Junio C Hamano
2021-02-09  7:28   ` [PATCH v2 5/9] t3905: replace test -s with test_file_not_empty Denton Liu
2021-02-09  7:28   ` [PATCH v2 6/9] t3905: use test_cmp() to check file contents Denton Liu
2021-02-09  7:28   ` [PATCH v2 7/9] stash: declare ref_stash as an array Denton Liu
2021-02-10  7:28     ` Junio C Hamano
2021-02-09  7:28   ` [PATCH v2 8/9] stash show: teach --include-untracked and --only-untracked Denton Liu
2021-02-10  7:53     ` Junio C Hamano
2021-02-16  3:15       ` Denton Liu
2021-02-16  6:42         ` Junio C Hamano
2021-02-09  7:28   ` [PATCH v2 9/9] stash show: learn stash.showIncludeUntracked Denton Liu
2021-02-16  7:11   ` [PATCH v3 0/2] stash show: learn --include-untracked and --only-untracked Denton Liu
2021-02-16  7:11     ` [PATCH v3 1/2] stash show: teach " Denton Liu
2021-02-16 20:22       ` Junio C Hamano
2021-02-17 11:18         ` Denton Liu
2021-02-17 17:50           ` Junio C Hamano
2021-02-17  2:31       ` Junio C Hamano
2021-02-16  7:11     ` [PATCH v3 2/2] stash show: learn stash.showIncludeUntracked Denton Liu
2021-03-03 11:16     ` [PATCH v4 0/2] stash show: learn --include-untracked and --only-untracked Denton Liu
2021-03-03 11:16       ` [PATCH v4 1/2] stash show: teach " Denton Liu
2021-03-03 11:16       ` [PATCH v4 2/2] stash show: learn stash.showIncludeUntracked Denton Liu
2021-03-04  0:38       ` [PATCH v4 0/2] stash show: learn --include-untracked and --only-untracked Junio C Hamano
2021-03-04  1:33         ` Denton Liu
2021-03-04  1:42           ` Denton Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).