* [PATCH 0/5] git-completion.bash: fixes on top of 'dl/complete-stash'
@ 2021-04-20 9:19 Denton Liu
2021-04-20 9:19 ` [PATCH 1/5] git-completion.bash: separate some commands onto their own line Denton Liu
` (5 more replies)
0 siblings, 6 replies; 18+ messages in thread
From: Denton Liu @ 2021-04-20 9:19 UTC (permalink / raw)
To: Git Mailing List; +Cc: SZEDER Gábor, Junio C Hamano
Gábor pointed out some potential fixes what's currently queued in
'dl/complete-stash'[0][1][2][3]. This series addresses those concerns.
Sorry for the long wait, I've been quite busy over the past month.
[0]: https://lore.kernel.org/git/20210327183554.GD2271@szeder.dev/
[1]: https://lore.kernel.org/git/20210328103134.GF2271@szeder.dev/
[2]: https://lore.kernel.org/git/20210328103057.GE2271@szeder.dev/
[3]: https://lore.kernel.org/git/20210328110427.GG2271@szeder.dev/
Denton Liu (5):
git-completion.bash: separate some commands onto their own line
git-completion.bash: rename to $__git_cmd_idx
git-completion.bash: use $__git_cmd_idx in more places
git-completion.bash: consolidate cases in _git_stash()
git-completion.bash: consolidate no-subcommand case for _git_stash()
contrib/completion/git-completion.bash | 118 +++++++++++++------------
t/t9902-completion.sh | 19 ++++
2 files changed, 80 insertions(+), 57 deletions(-)
--
2.31.1.424.g95a8dafae5
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/5] git-completion.bash: separate some commands onto their own line
2021-04-20 9:19 [PATCH 0/5] git-completion.bash: fixes on top of 'dl/complete-stash' Denton Liu
@ 2021-04-20 9:19 ` Denton Liu
2021-04-20 9:19 ` [PATCH 2/5] git-completion.bash: rename to $__git_cmd_idx Denton Liu
` (4 subsequent siblings)
5 siblings, 0 replies; 18+ messages in thread
From: Denton Liu @ 2021-04-20 9:19 UTC (permalink / raw)
To: Git Mailing List; +Cc: SZEDER Gábor, Junio C Hamano
In e94fb44042 (git-completion.bash: pass $__git_subcommand_idx from
__git_main(), 2021-03-24), a line was introduced which contained
multiple statements. This is difficult to read so break it into multiple
lines.
While we're at it, follow this convention for the rest of the
__git_main() and break up lines that contain multiple statements.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
contrib/completion/git-completion.bash | 37 +++++++++++++++++++-------
1 file changed, 28 insertions(+), 9 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c926ca26c6..1dedb14b47 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3400,17 +3400,35 @@ __git_main ()
while [ $c -lt $cword ]; do
i="${words[c]}"
case "$i" in
- --git-dir=*) __git_dir="${i#--git-dir=}" ;;
- --git-dir) ((c++)) ; __git_dir="${words[c]}" ;;
- --bare) __git_dir="." ;;
- --help) command="help"; break ;;
- -c|--work-tree|--namespace) ((c++)) ;;
- -C) __git_C_args[C_args_count++]=-C
+ --git-dir=*)
+ __git_dir="${i#--git-dir=}"
+ ;;
+ --git-dir)
+ ((c++))
+ __git_dir="${words[c]}"
+ ;;
+ --bare)
+ __git_dir="."
+ ;;
+ --help)
+ command="help"
+ break
+ ;;
+ -c|--work-tree|--namespace)
+ ((c++))
+ ;;
+ -C)
+ __git_C_args[C_args_count++]=-C
((c++))
__git_C_args[C_args_count++]="${words[c]}"
;;
- -*) ;;
- *) command="$i"; __git_subcommand_idx="$c"; break ;;
+ -*)
+ ;;
+ *)
+ command="$i"
+ __git_subcommand_idx="$c"
+ break
+ ;;
esac
((c++))
done
@@ -3432,7 +3450,8 @@ __git_main ()
;;
esac
case "$cur" in
- --*) __gitcomp "
+ --*)
+ __gitcomp "
--paginate
--no-pager
--git-dir=
--
2.31.1.424.g95a8dafae5
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/5] git-completion.bash: rename to $__git_cmd_idx
2021-04-20 9:19 [PATCH 0/5] git-completion.bash: fixes on top of 'dl/complete-stash' Denton Liu
2021-04-20 9:19 ` [PATCH 1/5] git-completion.bash: separate some commands onto their own line Denton Liu
@ 2021-04-20 9:19 ` Denton Liu
2021-04-20 20:50 ` Junio C Hamano
2021-04-20 9:19 ` [PATCH 3/5] git-completion.bash: use $__git_cmd_idx in more places Denton Liu
` (3 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Denton Liu @ 2021-04-20 9:19 UTC (permalink / raw)
To: Git Mailing List; +Cc: SZEDER Gábor, Junio C Hamano
In e94fb44042 (git-completion.bash: pass $__git_subcommand_idx from
__git_main(), 2021-03-24), the $__git_subcommand_idx variable was
introduced. Naming it after the index of the subcommand is flat-out
wrong as this variable really holds the index of the git comand (e.g.
"stash").
Rename this variable so that it's obvious it's about git commands. While
we're at it, shorten up its name so that it's still readable without
being a handful to type.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
contrib/completion/git-completion.bash | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 1dedb14b47..c29c129f87 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1474,12 +1474,12 @@ _git_branch ()
_git_bundle ()
{
- local cmd="${words[__git_subcommand_idx+1]}"
+ local cmd="${words[__git_cmd_idx+1]}"
case "$cword" in
- $((__git_subcommand_idx+1)))
+ $((__git_cmd_idx+1)))
__gitcomp "create list-heads verify unbundle"
;;
- $((__git_subcommand_idx+2)))
+ $((__git_cmd_idx+2)))
# looking for a file
;;
*)
@@ -1894,7 +1894,7 @@ _git_grep ()
esac
case "$cword,$prev" in
- $((__git_subcommand_idx+1)),*|*,-*)
+ $((__git_cmd_idx+1)),*|*,-*)
__git_complete_symbol && return
;;
esac
@@ -3017,7 +3017,7 @@ _git_stash ()
local subcommand="$(__git_find_on_cmdline "$subcommands save")"
if [ -z "$subcommand" ]; then
- case "$((cword - __git_subcommand_idx)),$cur" in
+ case "$((cword - __git_cmd_idx)),$cur" in
*,--*)
__gitcomp_builtin stash_push
;;
@@ -3058,7 +3058,7 @@ _git_stash ()
__gitcomp_builtin stash_branch
;;
branch,*)
- if [ $cword -eq $((__git_subcommand_idx+2)) ]; then
+ if [ $cword -eq $((__git_cmd_idx+2)) ]; then
__git_complete_refs
else
__gitcomp_nl "$(__git stash list \
@@ -3303,7 +3303,7 @@ _git_worktree ()
# be either the 'add' subcommand, the unstuck
# argument of an option (e.g. branch for -b|-B), or
# the path for the new worktree.
- if [ $cword -eq $((__git_subcommand_idx+2)) ]; then
+ if [ $cword -eq $((__git_cmd_idx+2)) ]; then
# Right after the 'add' subcommand: have to
# complete the path, so fall back to Bash
# filename completion.
@@ -3327,7 +3327,7 @@ _git_worktree ()
__git_complete_worktree_paths
;;
move,*)
- if [ $cword -eq $((__git_subcommand_idx+2)) ]; then
+ if [ $cword -eq $((__git_cmd_idx+2)) ]; then
# The first parameter must be an existing working
# tree to be moved.
__git_complete_worktree_paths
@@ -3395,7 +3395,7 @@ __git_main ()
{
local i c=1 command __git_dir __git_repo_path
local __git_C_args C_args_count=0
- local __git_subcommand_idx
+ local __git_cmd_idx
while [ $c -lt $cword ]; do
i="${words[c]}"
@@ -3426,7 +3426,7 @@ __git_main ()
;;
*)
command="$i"
- __git_subcommand_idx="$c"
+ __git_cmd_idx="$c"
break
;;
esac
--
2.31.1.424.g95a8dafae5
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/5] git-completion.bash: use $__git_cmd_idx in more places
2021-04-20 9:19 [PATCH 0/5] git-completion.bash: fixes on top of 'dl/complete-stash' Denton Liu
2021-04-20 9:19 ` [PATCH 1/5] git-completion.bash: separate some commands onto their own line Denton Liu
2021-04-20 9:19 ` [PATCH 2/5] git-completion.bash: rename to $__git_cmd_idx Denton Liu
@ 2021-04-20 9:19 ` Denton Liu
2021-04-20 9:19 ` [PATCH 4/5] git-completion.bash: consolidate cases in _git_stash() Denton Liu
` (2 subsequent siblings)
5 siblings, 0 replies; 18+ messages in thread
From: Denton Liu @ 2021-04-20 9:19 UTC (permalink / raw)
To: Git Mailing List; +Cc: SZEDER Gábor, Junio C Hamano
With the introduction of the $__git_cmd_idx variable in e94fb44042
(git-completion.bash: pass $__git_subcommand_idx from __git_main(),
2021-03-24), completion functions were able to know the index at which
the git command is listed, allowing them to skip options that are given
to the underlying git itself, not the corresponding command (e.g.
`-C asdf` in `git -C asdf branch`).
While most of the changes here are self-explanatory, some bear further
explanation.
For the __git_find_on_cmdline() and __git_find_last_on_cmdline() pair of
functions, these functions are only ever called in the context of a git
command completion function. These functions will only care about words
after the command so we can safely ignore the words before this.
For _git_worktree(), this change is technically a no-op (once the
__git_find_last_on_cmdline change is also applied). It was in poor style
to have hard-coded on the index right after `worktree`. In case
`git worktree` were to ever learn to accept options, the current
situation would be inflexible.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
contrib/completion/git-completion.bash | 26 ++++++++++++++------------
t/t9902-completion.sh | 19 +++++++++++++++++++
2 files changed, 33 insertions(+), 12 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c29c129f87..30c9a97616 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1006,8 +1006,8 @@ __git_complete_revlist ()
__git_complete_remote_or_refspec ()
{
- local cur_="$cur" cmd="${words[1]}"
- local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0
+ local cur_="$cur" cmd="${words[__git_cmd_idx]}"
+ local i c=$((__git_cmd_idx+1)) remote="" pfx="" lhs=1 no_complete_refspec=0
if [ "$cmd" = "remote" ]; then
((c++))
fi
@@ -1176,7 +1176,7 @@ __git_aliased_command ()
# --show-idx: Optionally show the index of the found word in the $words array.
__git_find_on_cmdline ()
{
- local word c=1 show_idx
+ local word c="$__git_cmd_idx" show_idx
while test $# -gt 1; do
case "$1" in
@@ -1221,7 +1221,7 @@ __git_find_last_on_cmdline ()
done
local wordlist="$1"
- while [ $c -gt 1 ]; do
+ while [ $c -gt "$__git_cmd_idx" ]; do
((c--))
for word in $wordlist; do
if [ "$word" = "${words[c]}" ]; then
@@ -1306,7 +1306,7 @@ __git_count_arguments ()
local word i c=0
# Skip "git" (first argument)
- for ((i=1; i < ${#words[@]}; i++)); do
+ for ((i="$__git_cmd_idx"; i < ${#words[@]}; i++)); do
word="${words[i]}"
case "$word" in
@@ -1442,7 +1442,7 @@ __git_ref_fieldlist="refname objecttype objectsize objectname upstream push HEAD
_git_branch ()
{
- local i c=1 only_local_ref="n" has_r="n"
+ local i c="$__git_cmd_idx" only_local_ref="n" has_r="n"
while [ $c -lt $cword ]; do
i="${words[c]}"
@@ -2474,7 +2474,7 @@ _git_switch ()
__git_config_get_set_variables ()
{
local prevword word config_file= c=$cword
- while [ $c -gt 1 ]; do
+ while [ $c -gt "$__git_cmd_idx" ]; do
word="${words[c]}"
case "$word" in
--system|--global|--local|--file=*)
@@ -3224,7 +3224,7 @@ _git_svn ()
_git_tag ()
{
- local i c=1 f=0
+ local i c="$__git_cmd_idx" f=0
while [ $c -lt $cword ]; do
i="${words[c]}"
case "$i" in
@@ -3276,9 +3276,11 @@ __git_complete_worktree_paths ()
_git_worktree ()
{
local subcommands="add list lock move prune remove unlock"
- local subcommand
+ local subcommand subcommand_idx
- subcommand="$(__git_find_on_cmdline "$subcommands")"
+ subcommand="$(__git_find_on_cmdline --show-idx "$subcommands")"
+ subcommand_idx="${subcommand% *}"
+ subcommand="${subcommand#* }"
case "$subcommand,$cur" in
,*)
@@ -3303,7 +3305,7 @@ _git_worktree ()
# be either the 'add' subcommand, the unstuck
# argument of an option (e.g. branch for -b|-B), or
# the path for the new worktree.
- if [ $cword -eq $((__git_cmd_idx+2)) ]; then
+ if [ $cword -eq $((subcommand_idx+1)) ]; then
# Right after the 'add' subcommand: have to
# complete the path, so fall back to Bash
# filename completion.
@@ -3327,7 +3329,7 @@ _git_worktree ()
__git_complete_worktree_paths
;;
move,*)
- if [ $cword -eq $((__git_cmd_idx+2)) ]; then
+ if [ $cword -eq $((subcommand_idx+1)) ]; then
# The first parameter must be an existing working
# tree to be moved.
__git_complete_worktree_paths
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 04ce884ef5..9439fec8f0 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1879,6 +1879,7 @@ test_expect_success '__git_find_on_cmdline - single match' '
(
words=(git command --opt list) &&
cword=${#words[@]} &&
+ __git_cmd_idx=1 &&
__git_find_on_cmdline "add list remove" >actual
) &&
test_cmp expect actual
@@ -1889,6 +1890,7 @@ test_expect_success '__git_find_on_cmdline - multiple matches' '
(
words=(git command -o --opt remove list add) &&
cword=${#words[@]} &&
+ __git_cmd_idx=1 &&
__git_find_on_cmdline "add list remove" >actual
) &&
test_cmp expect actual
@@ -1898,6 +1900,7 @@ test_expect_success '__git_find_on_cmdline - no match' '
(
words=(git command --opt branch) &&
cword=${#words[@]} &&
+ __git_cmd_idx=1 &&
__git_find_on_cmdline "add list remove" >actual
) &&
test_must_be_empty actual
@@ -1908,6 +1911,7 @@ test_expect_success '__git_find_on_cmdline - single match with index' '
(
words=(git command --opt list) &&
cword=${#words[@]} &&
+ __git_cmd_idx=1 &&
__git_find_on_cmdline --show-idx "add list remove" >actual
) &&
test_cmp expect actual
@@ -1918,6 +1922,7 @@ test_expect_success '__git_find_on_cmdline - multiple matches with index' '
(
words=(git command -o --opt remove list add) &&
cword=${#words[@]} &&
+ __git_cmd_idx=1 &&
__git_find_on_cmdline --show-idx "add list remove" >actual
) &&
test_cmp expect actual
@@ -1927,11 +1932,23 @@ test_expect_success '__git_find_on_cmdline - no match with index' '
(
words=(git command --opt branch) &&
cword=${#words[@]} &&
+ __git_cmd_idx=1 &&
__git_find_on_cmdline --show-idx "add list remove" >actual
) &&
test_must_be_empty actual
'
+test_expect_success '__git_find_on_cmdline - ignores matches before command with index' '
+ echo "6 remove" >expect &&
+ (
+ words=(git -C remove command -o --opt remove list add) &&
+ cword=${#words[@]} &&
+ __git_cmd_idx=3 &&
+ __git_find_on_cmdline --show-idx "add list remove" >actual
+ ) &&
+ test_cmp expect actual
+'
+
test_expect_success '__git_get_config_variables' '
cat >expect <<-EOF &&
name-1
@@ -2275,6 +2292,7 @@ do
(
words=(git push '$flag' other ma) &&
cword=${#words[@]} cur=${words[cword-1]} &&
+ __git_cmd_idx=1 &&
__git_complete_remote_or_refspec &&
print_comp
) &&
@@ -2288,6 +2306,7 @@ do
(
words=(git push other '$flag' ma) &&
cword=${#words[@]} cur=${words[cword-1]} &&
+ __git_cmd_idx=1 &&
__git_complete_remote_or_refspec &&
print_comp
) &&
--
2.31.1.424.g95a8dafae5
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/5] git-completion.bash: consolidate cases in _git_stash()
2021-04-20 9:19 [PATCH 0/5] git-completion.bash: fixes on top of 'dl/complete-stash' Denton Liu
` (2 preceding siblings ...)
2021-04-20 9:19 ` [PATCH 3/5] git-completion.bash: use $__git_cmd_idx in more places Denton Liu
@ 2021-04-20 9:19 ` Denton Liu
2021-04-20 10:44 ` Ævar Arnfjörð Bjarmason
2021-04-20 9:19 ` [PATCH 5/5] git-completion.bash: consolidate no-subcommand case for _git_stash() Denton Liu
2021-04-22 10:00 ` [PATCH v2 0/4] git-completion.bash: fixes on top of 'dl/complete-stash' Denton Liu
5 siblings, 1 reply; 18+ messages in thread
From: Denton Liu @ 2021-04-20 9:19 UTC (permalink / raw)
To: Git Mailing List; +Cc: SZEDER Gábor, Junio C Hamano
The $subcommand case statement in _git_stash() is quite repetitive.
Consolidate the cases together into one catch-all case to reduce the
repetition.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
contrib/completion/git-completion.bash | 21 ++-------------------
1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 30c9a97616..7bce9a0112 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3032,21 +3032,6 @@ _git_stash ()
fi
case "$subcommand,$cur" in
- push,--*)
- __gitcomp_builtin stash_push
- ;;
- save,--*)
- __gitcomp_builtin stash_save
- ;;
- pop,--*)
- __gitcomp_builtin stash_pop
- ;;
- apply,--*)
- __gitcomp_builtin stash_apply
- ;;
- drop,--*)
- __gitcomp_builtin stash_drop
- ;;
list,--*)
# NEEDSWORK: can we somehow unify this with the options in _git_log() and _git_show()
__gitcomp_builtin stash_list "$__git_log_common_options $__git_diff_common_options"
@@ -3054,8 +3039,8 @@ _git_stash ()
show,--*)
__gitcomp_builtin stash_show "$__git_diff_common_options"
;;
- branch,--*)
- __gitcomp_builtin stash_branch
+ *,--*)
+ __gitcomp_builtin "stash_$subcommand"
;;
branch,*)
if [ $cword -eq $((__git_cmd_idx+2)) ]; then
@@ -3069,8 +3054,6 @@ _git_stash ()
__gitcomp_nl "$(__git stash list \
| sed -n -e 's/:.*//p')"
;;
- *)
- ;;
esac
}
--
2.31.1.424.g95a8dafae5
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 5/5] git-completion.bash: consolidate no-subcommand case for _git_stash()
2021-04-20 9:19 [PATCH 0/5] git-completion.bash: fixes on top of 'dl/complete-stash' Denton Liu
` (3 preceding siblings ...)
2021-04-20 9:19 ` [PATCH 4/5] git-completion.bash: consolidate cases in _git_stash() Denton Liu
@ 2021-04-20 9:19 ` Denton Liu
2021-04-20 21:10 ` Junio C Hamano
2021-04-22 10:00 ` [PATCH v2 0/4] git-completion.bash: fixes on top of 'dl/complete-stash' Denton Liu
5 siblings, 1 reply; 18+ messages in thread
From: Denton Liu @ 2021-04-20 9:19 UTC (permalink / raw)
To: Git Mailing List; +Cc: SZEDER Gábor, Junio C Hamano
We have a separate if case for when no subcommand is given. It is
simpler to just consolidate this logic into the case statement below.
It would be nice to complete remove the magic that deals with indices
and replace it with what was originally there,
if [ -z "$subcommand" -a -n "$(__git_find_on_cmdline "-p")" ]; then
subcommand="push"
fi
but this gives a slightly incorrect completion. In the case where we're
attempting to complete `git stash -a <TAB>` we will get the subcommands
back as a respose instead of the completions for `git stash push`, which
is what we'd expect. We could potentially hardcode all of the short
options but that would be too much work to maintain so we stick with the
index solution.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
contrib/completion/git-completion.bash | 30 +++++++++++++-------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 7bce9a0112..060adc0ed7 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3016,22 +3016,22 @@ _git_stash ()
local subcommands='push list show apply clear drop pop create branch'
local subcommand="$(__git_find_on_cmdline "$subcommands save")"
- if [ -z "$subcommand" ]; then
- case "$((cword - __git_cmd_idx)),$cur" in
- *,--*)
- __gitcomp_builtin stash_push
- ;;
- 1,sa*)
- __gitcomp "save"
- ;;
- 1,*)
- __gitcomp "$subcommands"
- ;;
- esac
- return
- fi
-
case "$subcommand,$cur" in
+ ,--*)
+ __gitcomp_builtin stash_save
+ ;;
+ ,sa*)
+ __git_init_builtin_opts stash_save
+ if ((cword - __git_cmd_idx == 1)); then
+ __gitcomp "save"
+ fi
+ ;;
+ ,*)
+ __git_init_builtin_opts stash_save
+ if ((cword - __git_cmd_idx == 1)); then
+ __gitcomp "$subcommands"
+ fi
+ ;;
list,--*)
# NEEDSWORK: can we somehow unify this with the options in _git_log() and _git_show()
__gitcomp_builtin stash_list "$__git_log_common_options $__git_diff_common_options"
--
2.31.1.424.g95a8dafae5
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 4/5] git-completion.bash: consolidate cases in _git_stash()
2021-04-20 9:19 ` [PATCH 4/5] git-completion.bash: consolidate cases in _git_stash() Denton Liu
@ 2021-04-20 10:44 ` Ævar Arnfjörð Bjarmason
2021-04-21 4:03 ` Denton Liu
0 siblings, 1 reply; 18+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-04-20 10:44 UTC (permalink / raw)
To: Denton Liu; +Cc: Git Mailing List, SZEDER Gábor, Junio C Hamano
On Tue, Apr 20 2021, Denton Liu wrote:
> The $subcommand case statement in _git_stash() is quite repetitive.
> Consolidate the cases together into one catch-all case to reduce the
> repetition.
>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
> contrib/completion/git-completion.bash | 21 ++-------------------
> 1 file changed, 2 insertions(+), 19 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 30c9a97616..7bce9a0112 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -3032,21 +3032,6 @@ _git_stash ()
> fi
>
> case "$subcommand,$cur" in
> - push,--*)
> - __gitcomp_builtin stash_push
> - ;;
> - save,--*)
> - __gitcomp_builtin stash_save
> - ;;
> - pop,--*)
> - __gitcomp_builtin stash_pop
> - ;;
> - apply,--*)
> - __gitcomp_builtin stash_apply
> - ;;
> - drop,--*)
> - __gitcomp_builtin stash_drop
> - ;;
> list,--*)
> # NEEDSWORK: can we somehow unify this with the options in _git_log() and _git_show()
> __gitcomp_builtin stash_list "$__git_log_common_options $__git_diff_common_options"
> @@ -3054,8 +3039,8 @@ _git_stash ()
> show,--*)
> __gitcomp_builtin stash_show "$__git_diff_common_options"
> ;;
> - branch,--*)
> - __gitcomp_builtin stash_branch
> + *,--*)
> + __gitcomp_builtin "stash_$subcommand"
> ;;
> branch,*)
> if [ $cword -eq $((__git_cmd_idx+2)) ]; then
> @@ -3069,8 +3054,6 @@ _git_stash ()
> __gitcomp_nl "$(__git stash list \
> | sed -n -e 's/:.*//p')"
> ;;
> - *)
> - ;;
> esac
> }
One might think that this introduces a logic error in "git stash
doesnotexist" now dispatching to a non-existing "stash_doesnotexist" or
something, bu tI see that earlier (omitted from context) in the function
there's an exhaustive lit of push/save/pop etc. which guards against
this, is is that correct?
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] git-completion.bash: rename to $__git_cmd_idx
2021-04-20 9:19 ` [PATCH 2/5] git-completion.bash: rename to $__git_cmd_idx Denton Liu
@ 2021-04-20 20:50 ` Junio C Hamano
2021-04-20 21:14 ` SZEDER Gábor
0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2021-04-20 20:50 UTC (permalink / raw)
To: Denton Liu; +Cc: Git Mailing List, SZEDER Gábor
Denton Liu <liu.denton@gmail.com> writes:
> In e94fb44042 (git-completion.bash: pass $__git_subcommand_idx from
> __git_main(), 2021-03-24), the $__git_subcommand_idx variable was
> introduced. Naming it after the index of the subcommand is flat-out
> wrong as this variable really holds the index of the git comand (e.g.
comand -> command
> "stash").
>
> Rename this variable so that it's obvious it's about git commands. While
> we're at it, shorten up its name so that it's still readable without
> being a handful to type.
As the patch has already written, I won't complain too much, but to
many people "git" is a command and "add", "commit" etc. are
subcommands of "git", so I do not see git_subcommand_idx so wrong
that it needs to be renamed. I do understand that it is a bit too
long and it may be easier to type if renamed to say git_cmd_idx,
though ;-)
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
> contrib/completion/git-completion.bash | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 1dedb14b47..c29c129f87 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1474,12 +1474,12 @@ _git_branch ()
>
> _git_bundle ()
> {
> - local cmd="${words[__git_subcommand_idx+1]}"
> + local cmd="${words[__git_cmd_idx+1]}"
> case "$cword" in
> - $((__git_subcommand_idx+1)))
> + $((__git_cmd_idx+1)))
> __gitcomp "create list-heads verify unbundle"
> ;;
> - $((__git_subcommand_idx+2)))
> + $((__git_cmd_idx+2)))
> # looking for a file
> ;;
> *)
> @@ -1894,7 +1894,7 @@ _git_grep ()
> esac
>
> case "$cword,$prev" in
> - $((__git_subcommand_idx+1)),*|*,-*)
> + $((__git_cmd_idx+1)),*|*,-*)
> __git_complete_symbol && return
> ;;
> esac
> @@ -3017,7 +3017,7 @@ _git_stash ()
> local subcommand="$(__git_find_on_cmdline "$subcommands save")"
>
> if [ -z "$subcommand" ]; then
> - case "$((cword - __git_subcommand_idx)),$cur" in
> + case "$((cword - __git_cmd_idx)),$cur" in
> *,--*)
> __gitcomp_builtin stash_push
> ;;
> @@ -3058,7 +3058,7 @@ _git_stash ()
> __gitcomp_builtin stash_branch
> ;;
> branch,*)
> - if [ $cword -eq $((__git_subcommand_idx+2)) ]; then
> + if [ $cword -eq $((__git_cmd_idx+2)) ]; then
> __git_complete_refs
> else
> __gitcomp_nl "$(__git stash list \
> @@ -3303,7 +3303,7 @@ _git_worktree ()
> # be either the 'add' subcommand, the unstuck
> # argument of an option (e.g. branch for -b|-B), or
> # the path for the new worktree.
> - if [ $cword -eq $((__git_subcommand_idx+2)) ]; then
> + if [ $cword -eq $((__git_cmd_idx+2)) ]; then
> # Right after the 'add' subcommand: have to
> # complete the path, so fall back to Bash
> # filename completion.
> @@ -3327,7 +3327,7 @@ _git_worktree ()
> __git_complete_worktree_paths
> ;;
> move,*)
> - if [ $cword -eq $((__git_subcommand_idx+2)) ]; then
> + if [ $cword -eq $((__git_cmd_idx+2)) ]; then
> # The first parameter must be an existing working
> # tree to be moved.
> __git_complete_worktree_paths
> @@ -3395,7 +3395,7 @@ __git_main ()
> {
> local i c=1 command __git_dir __git_repo_path
> local __git_C_args C_args_count=0
> - local __git_subcommand_idx
> + local __git_cmd_idx
>
> while [ $c -lt $cword ]; do
> i="${words[c]}"
> @@ -3426,7 +3426,7 @@ __git_main ()
> ;;
> *)
> command="$i"
> - __git_subcommand_idx="$c"
> + __git_cmd_idx="$c"
> break
> ;;
> esac
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] git-completion.bash: consolidate no-subcommand case for _git_stash()
2021-04-20 9:19 ` [PATCH 5/5] git-completion.bash: consolidate no-subcommand case for _git_stash() Denton Liu
@ 2021-04-20 21:10 ` Junio C Hamano
2021-04-21 4:08 ` Denton Liu
0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2021-04-20 21:10 UTC (permalink / raw)
To: Denton Liu; +Cc: Git Mailing List, SZEDER Gábor
Denton Liu <liu.denton@gmail.com> writes:
> We have a separate if case for when no subcommand is given. It is
> simpler to just consolidate this logic into the case statement below.
Hmph, I am not quite sure if the removal of the first case is making
the code easier to follow. Is this supposed to be a no-op clean-up,
or is it fixing some bugs?
> It would be nice to complete remove the magic that deals with indices
> and replace it with what was originally there,
>
> if [ -z "$subcommand" -a -n "$(__git_find_on_cmdline "-p")" ]; then
> subcommand="push"
> fi
>
> but this gives a slightly incorrect completion. In the case where we're
> attempting to complete `git stash -a <TAB>` we will get the subcommands
> back as a respose instead of the completions for `git stash push`, which
> is what we'd expect. We could potentially hardcode all of the short
> options but that would be too much work to maintain so we stick with the
> index solution.
>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
> contrib/completion/git-completion.bash | 30 +++++++++++++-------------
> 1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 7bce9a0112..060adc0ed7 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -3016,22 +3016,22 @@ _git_stash ()
> local subcommands='push list show apply clear drop pop create branch'
> local subcommand="$(__git_find_on_cmdline "$subcommands save")"
>
> - if [ -z "$subcommand" ]; then
> - case "$((cword - __git_cmd_idx)),$cur" in
> - *,--*)
> - __gitcomp_builtin stash_push
> - ;;
> - 1,sa*)
> - __gitcomp "save"
> - ;;
> - 1,*)
> - __gitcomp "$subcommands"
> - ;;
> - esac
> - return
> - fi
> -
> case "$subcommand,$cur" in
> + ,--*)
> + __gitcomp_builtin stash_save
> + ;;
> + ,sa*)
> + __git_init_builtin_opts stash_save
> + if ((cword - __git_cmd_idx == 1)); then
> + __gitcomp "save"
> + fi
> + ;;
> + ,*)
> + __git_init_builtin_opts stash_save
> + if ((cword - __git_cmd_idx == 1)); then
> + __gitcomp "$subcommands"
> + fi
> + ;;
> list,--*)
> # NEEDSWORK: can we somehow unify this with the options in _git_log() and _git_show()
> __gitcomp_builtin stash_list "$__git_log_common_options $__git_diff_common_options"
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] git-completion.bash: rename to $__git_cmd_idx
2021-04-20 20:50 ` Junio C Hamano
@ 2021-04-20 21:14 ` SZEDER Gábor
2021-04-20 22:31 ` Junio C Hamano
0 siblings, 1 reply; 18+ messages in thread
From: SZEDER Gábor @ 2021-04-20 21:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Denton Liu, Git Mailing List
On Tue, Apr 20, 2021 at 01:50:19PM -0700, Junio C Hamano wrote:
> Denton Liu <liu.denton@gmail.com> writes:
>
> > In e94fb44042 (git-completion.bash: pass $__git_subcommand_idx from
> > __git_main(), 2021-03-24), the $__git_subcommand_idx variable was
> > introduced. Naming it after the index of the subcommand is flat-out
> > wrong as this variable really holds the index of the git comand (e.g.
>
> comand -> command
>
> > "stash").
> >
> > Rename this variable so that it's obvious it's about git commands. While
> > we're at it, shorten up its name so that it's still readable without
> > being a handful to type.
>
> As the patch has already written, I won't complain too much, but to
> many people "git" is a command and "add", "commit" etc. are
> subcommands of "git", so I do not see git_subcommand_idx so wrong
> that it needs to be renamed. I do understand that it is a bit too
> long and it may be easier to type if renamed to say git_cmd_idx,
> though ;-)
The completion functions for git commands having subcommands usually
start like this:
_git_remote ()
{
local subcommands="
add rename remove set-head set-branches
get-url set-url show prune update
"
local subcommand="$(__git_find_on_cmdline "$subcommands")"
if [ -z "$subcommand" ]; then
__git_subcommand_idx holds the index of the word "remote", not the
index of "add/rename/etc.", so in the context of the completion script
that name is misleading.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/5] git-completion.bash: rename to $__git_cmd_idx
2021-04-20 21:14 ` SZEDER Gábor
@ 2021-04-20 22:31 ` Junio C Hamano
0 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2021-04-20 22:31 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Denton Liu, Git Mailing List
SZEDER Gábor <szeder.dev@gmail.com> writes:
> The completion functions for git commands having subcommands usually
> start like this:
>
> _git_remote ()
> {
> local subcommands="
> add rename remove set-head set-branches
> get-url set-url show prune update
> "
> local subcommand="$(__git_find_on_cmdline "$subcommands")"
> if [ -z "$subcommand" ]; then
>
> __git_subcommand_idx holds the index of the word "remote", not the
> index of "add/rename/etc.", so in the context of the completion script
> that name is misleading.
OK. As subsubcommands would be a mouthful, calling "stash" etc. as
"cmd" would be fine.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/5] git-completion.bash: consolidate cases in _git_stash()
2021-04-20 10:44 ` Ævar Arnfjörð Bjarmason
@ 2021-04-21 4:03 ` Denton Liu
0 siblings, 0 replies; 18+ messages in thread
From: Denton Liu @ 2021-04-21 4:03 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Git Mailing List, SZEDER Gábor, Junio C Hamano
Hi Ævar,
On Tue, Apr 20, 2021 at 12:44:45PM +0200, Ævar Arnfjörð Bjarmason wrote:
>
> On Tue, Apr 20 2021, Denton Liu wrote:
>
> > The $subcommand case statement in _git_stash() is quite repetitive.
> > Consolidate the cases together into one catch-all case to reduce the
> > repetition.
> >
> > Signed-off-by: Denton Liu <liu.denton@gmail.com>
> > ---
> > contrib/completion/git-completion.bash | 21 ++-------------------
> > 1 file changed, 2 insertions(+), 19 deletions(-)
> >
> > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> > index 30c9a97616..7bce9a0112 100644
> > --- a/contrib/completion/git-completion.bash
> > +++ b/contrib/completion/git-completion.bash
> > @@ -3032,21 +3032,6 @@ _git_stash ()
> > fi
> >
> > case "$subcommand,$cur" in
> > - push,--*)
> > - __gitcomp_builtin stash_push
> > - ;;
> > - save,--*)
> > - __gitcomp_builtin stash_save
> > - ;;
> > - pop,--*)
> > - __gitcomp_builtin stash_pop
> > - ;;
> > - apply,--*)
> > - __gitcomp_builtin stash_apply
> > - ;;
> > - drop,--*)
> > - __gitcomp_builtin stash_drop
> > - ;;
> > list,--*)
> > # NEEDSWORK: can we somehow unify this with the options in _git_log() and _git_show()
> > __gitcomp_builtin stash_list "$__git_log_common_options $__git_diff_common_options"
> > @@ -3054,8 +3039,8 @@ _git_stash ()
> > show,--*)
> > __gitcomp_builtin stash_show "$__git_diff_common_options"
> > ;;
> > - branch,--*)
> > - __gitcomp_builtin stash_branch
> > + *,--*)
> > + __gitcomp_builtin "stash_$subcommand"
> > ;;
> > branch,*)
> > if [ $cword -eq $((__git_cmd_idx+2)) ]; then
> > @@ -3069,8 +3054,6 @@ _git_stash ()
> > __gitcomp_nl "$(__git stash list \
> > | sed -n -e 's/:.*//p')"
> > ;;
> > - *)
> > - ;;
> > esac
> > }
>
> One might think that this introduces a logic error in "git stash
> doesnotexist" now dispatching to a non-existing "stash_doesnotexist" or
> something, bu tI see that earlier (omitted from context) in the function
> there's an exhaustive lit of push/save/pop etc. which guards against
> this, is is that correct?
Yep, that's exactly correct.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] git-completion.bash: consolidate no-subcommand case for _git_stash()
2021-04-20 21:10 ` Junio C Hamano
@ 2021-04-21 4:08 ` Denton Liu
0 siblings, 0 replies; 18+ messages in thread
From: Denton Liu @ 2021-04-21 4:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, SZEDER Gábor
Hi Junio,
On Tue, Apr 20, 2021 at 02:10:39PM -0700, Junio C Hamano wrote:
> Denton Liu <liu.denton@gmail.com> writes:
>
> > We have a separate if case for when no subcommand is given. It is
> > simpler to just consolidate this logic into the case statement below.
>
> Hmph, I am not quite sure if the removal of the first case is making
> the code easier to follow. Is this supposed to be a no-op clean-up,
> or is it fixing some bugs?
This is simply a no-op clean-up. I am on the fence about doing this
change as well so I can drop it on the next reroll unless someone has
objections.
> > It would be nice to complete remove the magic that deals with indices
> > and replace it with what was originally there,
> >
> > if [ -z "$subcommand" -a -n "$(__git_find_on_cmdline "-p")" ]; then
> > subcommand="push"
> > fi
> >
> > but this gives a slightly incorrect completion. In the case where we're
> > attempting to complete `git stash -a <TAB>` we will get the subcommands
> > back as a respose instead of the completions for `git stash push`, which
> > is what we'd expect. We could potentially hardcode all of the short
> > options but that would be too much work to maintain so we stick with the
> > index solution.
> >
> > Signed-off-by: Denton Liu <liu.denton@gmail.com>
> > ---
> > contrib/completion/git-completion.bash | 30 +++++++++++++-------------
> > 1 file changed, 15 insertions(+), 15 deletions(-)
> >
> > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> > index 7bce9a0112..060adc0ed7 100644
> > --- a/contrib/completion/git-completion.bash
> > +++ b/contrib/completion/git-completion.bash
> > @@ -3016,22 +3016,22 @@ _git_stash ()
> > local subcommands='push list show apply clear drop pop create branch'
> > local subcommand="$(__git_find_on_cmdline "$subcommands save")"
> >
> > - if [ -z "$subcommand" ]; then
> > - case "$((cword - __git_cmd_idx)),$cur" in
> > - *,--*)
> > - __gitcomp_builtin stash_push
> > - ;;
> > - 1,sa*)
> > - __gitcomp "save"
> > - ;;
> > - 1,*)
> > - __gitcomp "$subcommands"
> > - ;;
> > - esac
> > - return
> > - fi
> > -
> > case "$subcommand,$cur" in
> > + ,--*)
> > + __gitcomp_builtin stash_save
> > + ;;
> > + ,sa*)
> > + __git_init_builtin_opts stash_save
Also, I just noticed upon re-reading this patch that this is some
leftover cruft. But moot point since I'll be dropping this patch.
> > + if ((cword - __git_cmd_idx == 1)); then
> > + __gitcomp "save"
> > + fi
> > + ;;
> > + ,*)
> > + __git_init_builtin_opts stash_save
> > + if ((cword - __git_cmd_idx == 1)); then
> > + __gitcomp "$subcommands"
> > + fi
> > + ;;
> > list,--*)
> > # NEEDSWORK: can we somehow unify this with the options in _git_log() and _git_show()
> > __gitcomp_builtin stash_list "$__git_log_common_options $__git_diff_common_options"
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 0/4] git-completion.bash: fixes on top of 'dl/complete-stash'
2021-04-20 9:19 [PATCH 0/5] git-completion.bash: fixes on top of 'dl/complete-stash' Denton Liu
` (4 preceding siblings ...)
2021-04-20 9:19 ` [PATCH 5/5] git-completion.bash: consolidate no-subcommand case for _git_stash() Denton Liu
@ 2021-04-22 10:00 ` Denton Liu
2021-04-22 10:00 ` [PATCH v2 1/4] git-completion.bash: separate some commands onto their own line Denton Liu
` (3 more replies)
5 siblings, 4 replies; 18+ messages in thread
From: Denton Liu @ 2021-04-22 10:00 UTC (permalink / raw)
To: Git Mailing List; +Cc: SZEDER Gábor, Junio C Hamano
Gábor pointed out some potential fixes what's currently queued in
'dl/complete-stash'[0][1][2][3]. This series addresses those concerns.
Changes since v1:
* Make the commit message for "git-completion.bash: rename to
$__git_cmd_idx" descriptive and accurate
* Drop "git-completion.bash: consolidate no-subcommand case for
_git_stash()"
[0]: https://lore.kernel.org/git/20210327183554.GD2271@szeder.dev/
[1]: https://lore.kernel.org/git/20210328103134.GF2271@szeder.dev/
[2]: https://lore.kernel.org/git/20210328103057.GE2271@szeder.dev/
[3]: https://lore.kernel.org/git/20210328110427.GG2271@szeder.dev/
Denton Liu (4):
git-completion.bash: separate some commands onto their own line
git-completion.bash: rename to $__git_cmd_idx
git-completion.bash: use $__git_cmd_idx in more places
git-completion.bash: consolidate cases in _git_stash()
contrib/completion/git-completion.bash | 98 ++++++++++++++------------
t/t9902-completion.sh | 19 +++++
2 files changed, 70 insertions(+), 47 deletions(-)
Range-diff against v1:
1: 65c485ea0c = 1: 65c485ea0c git-completion.bash: separate some commands onto their own line
2: 7c7d6de380 ! 2: 76328e3123 git-completion.bash: rename to $__git_cmd_idx
@@ Commit message
In e94fb44042 (git-completion.bash: pass $__git_subcommand_idx from
__git_main(), 2021-03-24), the $__git_subcommand_idx variable was
- introduced. Naming it after the index of the subcommand is flat-out
- wrong as this variable really holds the index of the git comand (e.g.
- "stash").
+ introduced. Naming it after the index of the subcommand is needlessly
+ confusing as, when this variable is used, it is in the completion
+ functions for commands (e.g. _git_remote()) where for `git remote add`,
+ the `remote` is referred to as the command and `add` is referred to as
+ the subcommand.
Rename this variable so that it's obvious it's about git commands. While
we're at it, shorten up its name so that it's still readable without
3: 63a6992585 = 3: 70fda62db1 git-completion.bash: use $__git_cmd_idx in more places
4: 4f8d015d54 = 4: 103d38e293 git-completion.bash: consolidate cases in _git_stash()
5: b4a9b0afa7 < -: ---------- git-completion.bash: consolidate no-subcommand case for _git_stash()
--
2.31.1.499.g90b4fd31cd
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 1/4] git-completion.bash: separate some commands onto their own line
2021-04-22 10:00 ` [PATCH v2 0/4] git-completion.bash: fixes on top of 'dl/complete-stash' Denton Liu
@ 2021-04-22 10:00 ` Denton Liu
2021-04-22 10:00 ` [PATCH v2 2/4] git-completion.bash: rename to $__git_cmd_idx Denton Liu
` (2 subsequent siblings)
3 siblings, 0 replies; 18+ messages in thread
From: Denton Liu @ 2021-04-22 10:00 UTC (permalink / raw)
To: Git Mailing List; +Cc: SZEDER Gábor, Junio C Hamano
In e94fb44042 (git-completion.bash: pass $__git_subcommand_idx from
__git_main(), 2021-03-24), a line was introduced which contained
multiple statements. This is difficult to read so break it into multiple
lines.
While we're at it, follow this convention for the rest of the
__git_main() and break up lines that contain multiple statements.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
contrib/completion/git-completion.bash | 37 +++++++++++++++++++-------
1 file changed, 28 insertions(+), 9 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c926ca26c6..1dedb14b47 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3400,17 +3400,35 @@ __git_main ()
while [ $c -lt $cword ]; do
i="${words[c]}"
case "$i" in
- --git-dir=*) __git_dir="${i#--git-dir=}" ;;
- --git-dir) ((c++)) ; __git_dir="${words[c]}" ;;
- --bare) __git_dir="." ;;
- --help) command="help"; break ;;
- -c|--work-tree|--namespace) ((c++)) ;;
- -C) __git_C_args[C_args_count++]=-C
+ --git-dir=*)
+ __git_dir="${i#--git-dir=}"
+ ;;
+ --git-dir)
+ ((c++))
+ __git_dir="${words[c]}"
+ ;;
+ --bare)
+ __git_dir="."
+ ;;
+ --help)
+ command="help"
+ break
+ ;;
+ -c|--work-tree|--namespace)
+ ((c++))
+ ;;
+ -C)
+ __git_C_args[C_args_count++]=-C
((c++))
__git_C_args[C_args_count++]="${words[c]}"
;;
- -*) ;;
- *) command="$i"; __git_subcommand_idx="$c"; break ;;
+ -*)
+ ;;
+ *)
+ command="$i"
+ __git_subcommand_idx="$c"
+ break
+ ;;
esac
((c++))
done
@@ -3432,7 +3450,8 @@ __git_main ()
;;
esac
case "$cur" in
- --*) __gitcomp "
+ --*)
+ __gitcomp "
--paginate
--no-pager
--git-dir=
--
2.31.1.499.g90b4fd31cd
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 2/4] git-completion.bash: rename to $__git_cmd_idx
2021-04-22 10:00 ` [PATCH v2 0/4] git-completion.bash: fixes on top of 'dl/complete-stash' Denton Liu
2021-04-22 10:00 ` [PATCH v2 1/4] git-completion.bash: separate some commands onto their own line Denton Liu
@ 2021-04-22 10:00 ` Denton Liu
2021-04-22 10:00 ` [PATCH v2 3/4] git-completion.bash: use $__git_cmd_idx in more places Denton Liu
2021-04-22 10:00 ` [PATCH v2 4/4] git-completion.bash: consolidate cases in _git_stash() Denton Liu
3 siblings, 0 replies; 18+ messages in thread
From: Denton Liu @ 2021-04-22 10:00 UTC (permalink / raw)
To: Git Mailing List; +Cc: SZEDER Gábor, Junio C Hamano
In e94fb44042 (git-completion.bash: pass $__git_subcommand_idx from
__git_main(), 2021-03-24), the $__git_subcommand_idx variable was
introduced. Naming it after the index of the subcommand is needlessly
confusing as, when this variable is used, it is in the completion
functions for commands (e.g. _git_remote()) where for `git remote add`,
the `remote` is referred to as the command and `add` is referred to as
the subcommand.
Rename this variable so that it's obvious it's about git commands. While
we're at it, shorten up its name so that it's still readable without
being a handful to type.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
contrib/completion/git-completion.bash | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 1dedb14b47..c29c129f87 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1474,12 +1474,12 @@ _git_branch ()
_git_bundle ()
{
- local cmd="${words[__git_subcommand_idx+1]}"
+ local cmd="${words[__git_cmd_idx+1]}"
case "$cword" in
- $((__git_subcommand_idx+1)))
+ $((__git_cmd_idx+1)))
__gitcomp "create list-heads verify unbundle"
;;
- $((__git_subcommand_idx+2)))
+ $((__git_cmd_idx+2)))
# looking for a file
;;
*)
@@ -1894,7 +1894,7 @@ _git_grep ()
esac
case "$cword,$prev" in
- $((__git_subcommand_idx+1)),*|*,-*)
+ $((__git_cmd_idx+1)),*|*,-*)
__git_complete_symbol && return
;;
esac
@@ -3017,7 +3017,7 @@ _git_stash ()
local subcommand="$(__git_find_on_cmdline "$subcommands save")"
if [ -z "$subcommand" ]; then
- case "$((cword - __git_subcommand_idx)),$cur" in
+ case "$((cword - __git_cmd_idx)),$cur" in
*,--*)
__gitcomp_builtin stash_push
;;
@@ -3058,7 +3058,7 @@ _git_stash ()
__gitcomp_builtin stash_branch
;;
branch,*)
- if [ $cword -eq $((__git_subcommand_idx+2)) ]; then
+ if [ $cword -eq $((__git_cmd_idx+2)) ]; then
__git_complete_refs
else
__gitcomp_nl "$(__git stash list \
@@ -3303,7 +3303,7 @@ _git_worktree ()
# be either the 'add' subcommand, the unstuck
# argument of an option (e.g. branch for -b|-B), or
# the path for the new worktree.
- if [ $cword -eq $((__git_subcommand_idx+2)) ]; then
+ if [ $cword -eq $((__git_cmd_idx+2)) ]; then
# Right after the 'add' subcommand: have to
# complete the path, so fall back to Bash
# filename completion.
@@ -3327,7 +3327,7 @@ _git_worktree ()
__git_complete_worktree_paths
;;
move,*)
- if [ $cword -eq $((__git_subcommand_idx+2)) ]; then
+ if [ $cword -eq $((__git_cmd_idx+2)) ]; then
# The first parameter must be an existing working
# tree to be moved.
__git_complete_worktree_paths
@@ -3395,7 +3395,7 @@ __git_main ()
{
local i c=1 command __git_dir __git_repo_path
local __git_C_args C_args_count=0
- local __git_subcommand_idx
+ local __git_cmd_idx
while [ $c -lt $cword ]; do
i="${words[c]}"
@@ -3426,7 +3426,7 @@ __git_main ()
;;
*)
command="$i"
- __git_subcommand_idx="$c"
+ __git_cmd_idx="$c"
break
;;
esac
--
2.31.1.499.g90b4fd31cd
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 3/4] git-completion.bash: use $__git_cmd_idx in more places
2021-04-22 10:00 ` [PATCH v2 0/4] git-completion.bash: fixes on top of 'dl/complete-stash' Denton Liu
2021-04-22 10:00 ` [PATCH v2 1/4] git-completion.bash: separate some commands onto their own line Denton Liu
2021-04-22 10:00 ` [PATCH v2 2/4] git-completion.bash: rename to $__git_cmd_idx Denton Liu
@ 2021-04-22 10:00 ` Denton Liu
2021-04-22 10:00 ` [PATCH v2 4/4] git-completion.bash: consolidate cases in _git_stash() Denton Liu
3 siblings, 0 replies; 18+ messages in thread
From: Denton Liu @ 2021-04-22 10:00 UTC (permalink / raw)
To: Git Mailing List; +Cc: SZEDER Gábor, Junio C Hamano
With the introduction of the $__git_cmd_idx variable in e94fb44042
(git-completion.bash: pass $__git_subcommand_idx from __git_main(),
2021-03-24), completion functions were able to know the index at which
the git command is listed, allowing them to skip options that are given
to the underlying git itself, not the corresponding command (e.g.
`-C asdf` in `git -C asdf branch`).
While most of the changes here are self-explanatory, some bear further
explanation.
For the __git_find_on_cmdline() and __git_find_last_on_cmdline() pair of
functions, these functions are only ever called in the context of a git
command completion function. These functions will only care about words
after the command so we can safely ignore the words before this.
For _git_worktree(), this change is technically a no-op (once the
__git_find_last_on_cmdline change is also applied). It was in poor style
to have hard-coded on the index right after `worktree`. In case
`git worktree` were to ever learn to accept options, the current
situation would be inflexible.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
contrib/completion/git-completion.bash | 26 ++++++++++++++------------
t/t9902-completion.sh | 19 +++++++++++++++++++
2 files changed, 33 insertions(+), 12 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c29c129f87..30c9a97616 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1006,8 +1006,8 @@ __git_complete_revlist ()
__git_complete_remote_or_refspec ()
{
- local cur_="$cur" cmd="${words[1]}"
- local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0
+ local cur_="$cur" cmd="${words[__git_cmd_idx]}"
+ local i c=$((__git_cmd_idx+1)) remote="" pfx="" lhs=1 no_complete_refspec=0
if [ "$cmd" = "remote" ]; then
((c++))
fi
@@ -1176,7 +1176,7 @@ __git_aliased_command ()
# --show-idx: Optionally show the index of the found word in the $words array.
__git_find_on_cmdline ()
{
- local word c=1 show_idx
+ local word c="$__git_cmd_idx" show_idx
while test $# -gt 1; do
case "$1" in
@@ -1221,7 +1221,7 @@ __git_find_last_on_cmdline ()
done
local wordlist="$1"
- while [ $c -gt 1 ]; do
+ while [ $c -gt "$__git_cmd_idx" ]; do
((c--))
for word in $wordlist; do
if [ "$word" = "${words[c]}" ]; then
@@ -1306,7 +1306,7 @@ __git_count_arguments ()
local word i c=0
# Skip "git" (first argument)
- for ((i=1; i < ${#words[@]}; i++)); do
+ for ((i="$__git_cmd_idx"; i < ${#words[@]}; i++)); do
word="${words[i]}"
case "$word" in
@@ -1442,7 +1442,7 @@ __git_ref_fieldlist="refname objecttype objectsize objectname upstream push HEAD
_git_branch ()
{
- local i c=1 only_local_ref="n" has_r="n"
+ local i c="$__git_cmd_idx" only_local_ref="n" has_r="n"
while [ $c -lt $cword ]; do
i="${words[c]}"
@@ -2474,7 +2474,7 @@ _git_switch ()
__git_config_get_set_variables ()
{
local prevword word config_file= c=$cword
- while [ $c -gt 1 ]; do
+ while [ $c -gt "$__git_cmd_idx" ]; do
word="${words[c]}"
case "$word" in
--system|--global|--local|--file=*)
@@ -3224,7 +3224,7 @@ _git_svn ()
_git_tag ()
{
- local i c=1 f=0
+ local i c="$__git_cmd_idx" f=0
while [ $c -lt $cword ]; do
i="${words[c]}"
case "$i" in
@@ -3276,9 +3276,11 @@ __git_complete_worktree_paths ()
_git_worktree ()
{
local subcommands="add list lock move prune remove unlock"
- local subcommand
+ local subcommand subcommand_idx
- subcommand="$(__git_find_on_cmdline "$subcommands")"
+ subcommand="$(__git_find_on_cmdline --show-idx "$subcommands")"
+ subcommand_idx="${subcommand% *}"
+ subcommand="${subcommand#* }"
case "$subcommand,$cur" in
,*)
@@ -3303,7 +3305,7 @@ _git_worktree ()
# be either the 'add' subcommand, the unstuck
# argument of an option (e.g. branch for -b|-B), or
# the path for the new worktree.
- if [ $cword -eq $((__git_cmd_idx+2)) ]; then
+ if [ $cword -eq $((subcommand_idx+1)) ]; then
# Right after the 'add' subcommand: have to
# complete the path, so fall back to Bash
# filename completion.
@@ -3327,7 +3329,7 @@ _git_worktree ()
__git_complete_worktree_paths
;;
move,*)
- if [ $cword -eq $((__git_cmd_idx+2)) ]; then
+ if [ $cword -eq $((subcommand_idx+1)) ]; then
# The first parameter must be an existing working
# tree to be moved.
__git_complete_worktree_paths
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 04ce884ef5..9439fec8f0 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1879,6 +1879,7 @@ test_expect_success '__git_find_on_cmdline - single match' '
(
words=(git command --opt list) &&
cword=${#words[@]} &&
+ __git_cmd_idx=1 &&
__git_find_on_cmdline "add list remove" >actual
) &&
test_cmp expect actual
@@ -1889,6 +1890,7 @@ test_expect_success '__git_find_on_cmdline - multiple matches' '
(
words=(git command -o --opt remove list add) &&
cword=${#words[@]} &&
+ __git_cmd_idx=1 &&
__git_find_on_cmdline "add list remove" >actual
) &&
test_cmp expect actual
@@ -1898,6 +1900,7 @@ test_expect_success '__git_find_on_cmdline - no match' '
(
words=(git command --opt branch) &&
cword=${#words[@]} &&
+ __git_cmd_idx=1 &&
__git_find_on_cmdline "add list remove" >actual
) &&
test_must_be_empty actual
@@ -1908,6 +1911,7 @@ test_expect_success '__git_find_on_cmdline - single match with index' '
(
words=(git command --opt list) &&
cword=${#words[@]} &&
+ __git_cmd_idx=1 &&
__git_find_on_cmdline --show-idx "add list remove" >actual
) &&
test_cmp expect actual
@@ -1918,6 +1922,7 @@ test_expect_success '__git_find_on_cmdline - multiple matches with index' '
(
words=(git command -o --opt remove list add) &&
cword=${#words[@]} &&
+ __git_cmd_idx=1 &&
__git_find_on_cmdline --show-idx "add list remove" >actual
) &&
test_cmp expect actual
@@ -1927,11 +1932,23 @@ test_expect_success '__git_find_on_cmdline - no match with index' '
(
words=(git command --opt branch) &&
cword=${#words[@]} &&
+ __git_cmd_idx=1 &&
__git_find_on_cmdline --show-idx "add list remove" >actual
) &&
test_must_be_empty actual
'
+test_expect_success '__git_find_on_cmdline - ignores matches before command with index' '
+ echo "6 remove" >expect &&
+ (
+ words=(git -C remove command -o --opt remove list add) &&
+ cword=${#words[@]} &&
+ __git_cmd_idx=3 &&
+ __git_find_on_cmdline --show-idx "add list remove" >actual
+ ) &&
+ test_cmp expect actual
+'
+
test_expect_success '__git_get_config_variables' '
cat >expect <<-EOF &&
name-1
@@ -2275,6 +2292,7 @@ do
(
words=(git push '$flag' other ma) &&
cword=${#words[@]} cur=${words[cword-1]} &&
+ __git_cmd_idx=1 &&
__git_complete_remote_or_refspec &&
print_comp
) &&
@@ -2288,6 +2306,7 @@ do
(
words=(git push other '$flag' ma) &&
cword=${#words[@]} cur=${words[cword-1]} &&
+ __git_cmd_idx=1 &&
__git_complete_remote_or_refspec &&
print_comp
) &&
--
2.31.1.499.g90b4fd31cd
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 4/4] git-completion.bash: consolidate cases in _git_stash()
2021-04-22 10:00 ` [PATCH v2 0/4] git-completion.bash: fixes on top of 'dl/complete-stash' Denton Liu
` (2 preceding siblings ...)
2021-04-22 10:00 ` [PATCH v2 3/4] git-completion.bash: use $__git_cmd_idx in more places Denton Liu
@ 2021-04-22 10:00 ` Denton Liu
3 siblings, 0 replies; 18+ messages in thread
From: Denton Liu @ 2021-04-22 10:00 UTC (permalink / raw)
To: Git Mailing List; +Cc: SZEDER Gábor, Junio C Hamano
The $subcommand case statement in _git_stash() is quite repetitive.
Consolidate the cases together into one catch-all case to reduce the
repetition.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
contrib/completion/git-completion.bash | 21 ++-------------------
1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 30c9a97616..7bce9a0112 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3032,21 +3032,6 @@ _git_stash ()
fi
case "$subcommand,$cur" in
- push,--*)
- __gitcomp_builtin stash_push
- ;;
- save,--*)
- __gitcomp_builtin stash_save
- ;;
- pop,--*)
- __gitcomp_builtin stash_pop
- ;;
- apply,--*)
- __gitcomp_builtin stash_apply
- ;;
- drop,--*)
- __gitcomp_builtin stash_drop
- ;;
list,--*)
# NEEDSWORK: can we somehow unify this with the options in _git_log() and _git_show()
__gitcomp_builtin stash_list "$__git_log_common_options $__git_diff_common_options"
@@ -3054,8 +3039,8 @@ _git_stash ()
show,--*)
__gitcomp_builtin stash_show "$__git_diff_common_options"
;;
- branch,--*)
- __gitcomp_builtin stash_branch
+ *,--*)
+ __gitcomp_builtin "stash_$subcommand"
;;
branch,*)
if [ $cword -eq $((__git_cmd_idx+2)) ]; then
@@ -3069,8 +3054,6 @@ _git_stash ()
__gitcomp_nl "$(__git stash list \
| sed -n -e 's/:.*//p')"
;;
- *)
- ;;
esac
}
--
2.31.1.499.g90b4fd31cd
^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2021-04-22 10:01 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-20 9:19 [PATCH 0/5] git-completion.bash: fixes on top of 'dl/complete-stash' Denton Liu
2021-04-20 9:19 ` [PATCH 1/5] git-completion.bash: separate some commands onto their own line Denton Liu
2021-04-20 9:19 ` [PATCH 2/5] git-completion.bash: rename to $__git_cmd_idx Denton Liu
2021-04-20 20:50 ` Junio C Hamano
2021-04-20 21:14 ` SZEDER Gábor
2021-04-20 22:31 ` Junio C Hamano
2021-04-20 9:19 ` [PATCH 3/5] git-completion.bash: use $__git_cmd_idx in more places Denton Liu
2021-04-20 9:19 ` [PATCH 4/5] git-completion.bash: consolidate cases in _git_stash() Denton Liu
2021-04-20 10:44 ` Ævar Arnfjörð Bjarmason
2021-04-21 4:03 ` Denton Liu
2021-04-20 9:19 ` [PATCH 5/5] git-completion.bash: consolidate no-subcommand case for _git_stash() Denton Liu
2021-04-20 21:10 ` Junio C Hamano
2021-04-21 4:08 ` Denton Liu
2021-04-22 10:00 ` [PATCH v2 0/4] git-completion.bash: fixes on top of 'dl/complete-stash' Denton Liu
2021-04-22 10:00 ` [PATCH v2 1/4] git-completion.bash: separate some commands onto their own line Denton Liu
2021-04-22 10:00 ` [PATCH v2 2/4] git-completion.bash: rename to $__git_cmd_idx Denton Liu
2021-04-22 10:00 ` [PATCH v2 3/4] git-completion.bash: use $__git_cmd_idx in more places Denton Liu
2021-04-22 10:00 ` [PATCH v2 4/4] git-completion.bash: consolidate cases in _git_stash() Denton Liu
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).