diff mbox series

[8/9] stash show: teach --include-tracked and --only-untracked

Message ID c2375d1fc6c5fe7a36d2f868b172e83749d67b45.1612258145.git.liu.denton@gmail.com (mailing list archive)
State Superseded
Headers show
Series stash show: learn --include-untracked and --only-untracked | expand

Commit Message

Denton Liu Feb. 2, 2021, 9:33 a.m. UTC
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(-)

Comments

Junio C Hamano Feb. 2, 2021, 9:56 p.m. UTC | #1
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.
diff mbox series

Patch

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