[PATCH 0/5] stash completion miscellany

[PATCH 0/5] stash completion miscellany

[D. Ben Knoble]

Hi all,

This short patch series takes care of a few paper cuts in stash
completion. The first patch is a small code cleanup I noticed a while
back while working on bfc1b34859 (completion: add stash import, export,
2026-02-07), I think. The next 2 take care of a problem I've been having
with Zsh's completion of stashes (cf. Unix & Linux SE [1]).

[1]: https://unix.stackexchange.com/q/807580/301073

The final 2 are not only about completion; I noticed we were
special-casing save completion, did some digging, and was a bit confused
by the status of "git stash save". So, I figured while I was here I
would start a conversation about (a) restoring the git-stash(1) manual
to it's "no save" version and (b) ripping out the special case.

These last 2 may be controversial, and I'm not particularly attached to
them, so if they need to be dropped I won't really complain. OTOH,
what's the point of a deprecated subcommand we still advertise? ;)

Generated-with: git format-patch @{u}.. -o p

[1/5] completion: simplify stash subcommand completion
[2/5] completion: extract common stash listing code
[3/5] completion: fix Zsh handling of stash names
[4/5] doc: drop usage mentions of "git stash save"
[5/5] completion: drop "stash save" completion altogether

 Documentation/git-stash.adoc           |  2 --
 builtin/stash.c                        |  1 -
 contrib/completion/git-completion.bash | 22 +++++++++++++---------
 contrib/completion/git-completion.zsh  |  5 +++++
 4 files changed, 18 insertions(+), 12 deletions(-)


base-commit: 6de20f6092dcf9bdb1c8efe03db4b70c82b423dd
-- 
2.56.0.184.g5a00b4a6e9.dirty

[PATCH 1/5] completion: simplify stash subcommand completion

[D. Ben Knoble]

The stash completion helper contains a hard-coded list of commands, but
git-stash provides a mechanism to list such commands
(--git-completion-helper). The pattern for using it is wrapped by
__git_resolve_builtins, so use it, bring _git_stash closer in line with
other completion helpers.

Note that we maintain the "save" special case, since it was removed from
default completion in 0eb5a4f911 (completion: stop showing 'save' for
stash by default, 2018-04-20). We don't have to do anything special with
the exclude (3rd) argument to __git_resolve_builtins, though, because
save is already marked PARSE_OPT_NOCOMPLETE.

Signed-off-by: D. Ben Knoble <ben.knoble@gmail.com>
---
 contrib/completion/git-completion.bash | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c3a49e2e22..d60e85479c 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3553,8 +3553,12 @@ _git_sparse_checkout ()
 
 _git_stash ()
 {
-	local subcommands='push list show apply clear drop pop create branch import export'
-	local subcommand="$(__git_find_on_cmdline "$subcommands save")"
+	local subcommands subcommand
+
+	__git_resolve_builtins stash
+
+	subcommands="$___git_resolved_builtins"
+	subcommand="$(__git_find_on_cmdline "$subcommands save")"
 
 	if [ -z "$subcommand" ]; then
 		case "$((cword - __git_cmd_idx)),$cur" in
-- 
2.56.0.184.g5a00b4a6e9.dirty

[PATCH 2/5] completion: extract common stash listing code

[D. Ben Knoble]

With no change in behavior, prepare to adjust how stashes are completed
by centralizing candidate generation.

Signed-off-by: D. Ben Knoble <ben.knoble@gmail.com>
---
 contrib/completion/git-completion.bash | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index d60e85479c..697fef5462 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3593,20 +3593,23 @@ _git_stash ()
 		if [ $cword -eq $((__git_cmd_idx+2)) ]; then
 			__git_complete_refs
 		else
-			__gitcomp_nl "$(__git stash list \
-					| sed -n -e 's/:.*//p')"
+			__git_complete_stashes
 		fi
 		;;
 	import,*)
 		__git_complete_refs
 		;;
 	show,*|apply,*|drop,*|pop,*|export,*)
-		__gitcomp_nl "$(__git stash list \
-				| sed -n -e 's/:.*//p')"
+		__git_complete_stashes
 		;;
 	esac
 }
 
+__git_complete_stashes ()
+{
+	__gitcomp_nl "$(__git stash list | sed -n -e 's/:.*//p')"
+}
+
 _git_submodule ()
 {
 	__git_has_doubledash && return
-- 
2.56.0.184.g5a00b4a6e9.dirty

[PATCH 3/5] completion: fix Zsh handling of stash names

[D. Ben Knoble]

When Git completes stashes for Zsh, it eventually invokes something like
this (via our redefinition of __gitcomp_nl):

    compadd -Q -S " " -p "" -- 'stash@{0}' 'stash@{1}' && _ret=0

Because of -Q, the braces are unquoted, and Zsh will leave us at (for
example)

    git stash show stash@{
                   cursor ^

From here, the list-choices widget (on Ctrl-D by default) will show
stash@{0} and stash@{1}, but the complete-word widget (Tab) will just
add another brace. Zsh things we're trying to do a brace expansion, I
think! Interestingly, using menu-complete (Ctrl-N) does cycle through
matches, since it's taking a slightly different completion path in Zsh.

We might try to fix this by passing surrounding single quotes (using the
-S/-p arguments of compadd, which we can do via __gitcomp_nl's other
parameters), and indeed Zsh will generate

    git stash show 'stash@{
                    cursor ^

but completing here won't find anything, and I'm not totally sure why.

Instead, though, we can let Zsh quote metacharacters (removing -Q) and
everything "just works." It changes the output format a bit (users see
stash@\{0\}, for example), but I find this an improvement over not being
able to complete names at all. menu-complete is unchanged aside from the
extra backslashes.

So, override stash listing completion in Zsh with a working version.

Signed-off-by: D. Ben Knoble <ben.knoble@gmail.com>
---
 contrib/completion/git-completion.zsh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh
index d5c526665b..2aea7770d2 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -135,6 +135,11 @@ __gitcomp_file_direct ()
 	__gitcomp_file "$1" ""
 }
 
+__git_complete_stashes ()
+{
+	compadd -S ' ' -- ${(f)"$(__git stash list | sed -n -e 's/:.*//p')"} && _ret=0
+}
+
 _git_zsh ()
 {
 	__gitcomp "v1.1"
-- 
2.56.0.184.g5a00b4a6e9.dirty

[PATCH 4/5] doc: drop usage mentions of "git stash save"

[D. Ben Knoble]

Even though we previously deprecated it in fd2ebf14db (stash: mark "git
stash save" deprecated in the man page, 2017-10-22), the save subcommand
clings to life. For example, 951ec747d4 (doc txt & -h consistency: make
"stash" consistent, 2022-10-13) brought back "save" to the git-stash(1)
synopsis, and 41a28eb6c1 (stash: implement '--staged' option for 'push'
and 'save', 2021-10-18) gave it new options. But we really discourage
its use in modern Git, so let's drop mention of it from -h usage and the
synopsis.

It's still described in the manual, since some descriptions talk about
the "save" command. As we haven't gone ahead with a full deprecation and
removal, we can leave these pieces for folks whose fingers still type
"save" and need a reference for its behavior.

Signed-off-by: D. Ben Knoble <ben.knoble@gmail.com>
---

Notes (benknoble/commits):
    AFAICT there was no discussion of its deprecation when re-adding save to
    the documentation, so I wonder if that was just an oversight.

 Documentation/git-stash.adoc | 2 --
 builtin/stash.c              | 1 -
 2 files changed, 3 deletions(-)

diff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc
index d6e8c4144c..6bf510702b 100644
--- a/Documentation/git-stash.adoc
+++ b/Documentation/git-stash.adoc
@@ -18,8 +18,6 @@ git stash [push] [-p | --patch] [-S | --staged] [-k | --[no-]keep-index] [-q | -
 	     [-u | --include-untracked] [-a | --all] [(-m | --message) <message>]
 	     [--pathspec-from-file=<file> [--pathspec-file-nul]]
 	     [--] [<pathspec>...]
-git stash save [-p | --patch] [-S | --staged] [-k | --[no-]keep-index] [-q | --quiet]
-           [-u | --include-untracked] [-a | --all] [<message>]
 git stash clear
 git stash create [<message>]
 git stash store [(-m | --message) <message>] [-q | --quiet] <commit>
diff --git a/builtin/stash.c b/builtin/stash.c
index 7a9843413b..94869c2429 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -75,7 +75,6 @@ static const char * const git_stash_usage[] = {
 	BUILTIN_STASH_APPLY_USAGE,
 	BUILTIN_STASH_BRANCH_USAGE,
 	BUILTIN_STASH_PUSH_USAGE,
-	BUILTIN_STASH_SAVE_USAGE,
 	BUILTIN_STASH_CLEAR_USAGE,
 	BUILTIN_STASH_CREATE_USAGE,
 	BUILTIN_STASH_STORE_USAGE,
-- 
2.56.0.184.g5a00b4a6e9.dirty

[PATCH 5/5] completion: drop "stash save" completion altogether

[D. Ben Knoble]

It's been deprecated since fd2ebf14db (stash: mark "git stash save"
deprecated in the man page, 2017-10-22) and the previous commit removed
further documentation suggesting to use it.

Since we recommend push instead, stop completing for "save" and remove a
special case in completion code.

Signed-off-by: D. Ben Knoble <ben.knoble@gmail.com>
---
 contrib/completion/git-completion.bash | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 697fef5462..9cd9c7cadb 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3558,16 +3558,13 @@ _git_stash ()
 	__git_resolve_builtins stash
 
 	subcommands="$___git_resolved_builtins"
-	subcommand="$(__git_find_on_cmdline "$subcommands save")"
+	subcommand="$(__git_find_on_cmdline "$subcommands")"
 
 	if [ -z "$subcommand" ]; then
 		case "$((cword - __git_cmd_idx)),$cur" in
 		*,--*)
 			__gitcomp_builtin stash_push
 			;;
-		1,sa*)
-			__gitcomp "save"
-			;;
 		1,*)
 			__gitcomp "$subcommands"
 			;;
-- 
2.56.0.184.g5a00b4a6e9.dirty