* [PATCH 00/11] refactor git switch completion
@ 2020-04-25 2:20 Jacob Keller
2020-04-25 2:20 ` [PATCH 01/11] completion: add some simple test cases for " Jacob Keller
` (13 more replies)
0 siblings, 14 replies; 26+ messages in thread
From: Jacob Keller @ 2020-04-25 2:20 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, Jacob Keller
From: Jacob Keller <jacob.keller@gmail.com>
completion support for git switch is subpar for a number of cases. Most
notable is difference between these two completions:
$git switch <TAB>
Display all 784 possibilities? (y or n)
<list of all references and DWIM remotes>
$git switch --track<TAB>
jk-refactor-git-switch-completion master`
If --track is provided, tab completion becomes almost useless, because we
would expect to complete remote references, but instead can only complete
local branches!
This series was motivated by a desire to fix the completion for the above
two cases, but I noticed several other issues on the way, including some
issues understanding what the current logic did.
This series aims to improve the completion support, and comes with many
additional test cases that should help highlight the buggy behavior and
hopefully prevent future regressions.
The first few commits just add new test cases, most of which currently fail.
Following this is a commit to change __git_complete_refs so that it uses
"--dwim" instead of "--track", since this made reading _git_checkout() and
_git_switch() difficult to read. "--track" was both used as the "enable DWIM
remote branch names" and also the option name for --track.
Following this are some patches to extract displaying DWIM remote branch
names from __git_refs() and refactoring __git_complete_refs to take a mode
argument that switches between calling __git_heads, __git_refs, and a new
__git_remotes.
By doing this, it becomes easier to do things like complete DWIM remote
branches in addition to just regular branches, rather than all references.
With this series applied, completion for git switch behaves more like the
following examples:
$git switch <TAB>
HEAD master todo
jk-refactor-git-switch-completion next
maint pu
$git switch --track <TAB>
origin/HEAD origin/maint origin/master origin/next origin/pu
origin/todo
Jacob Keller (11):
completion: add some simple test cases for git switch completion
completion: add test showing subpar git switch completion
completion: add test highlighting subpar git switch --track completion
completion: add tests showing lack of support for git switch -c/-C
completion: remove completion for git switch --orphan
completion: rename --track option of __git_complete_refs
completion: extract function __git_dwim_remote_heads
completion: perform DWIM logic directly in __git_complete_refs
completion: fix completion for git switch with no options
completion: recognize -c/-C when completing for git switch
completion: complete remote branches for git switch --track
contrib/completion/git-completion.bash | 146 +++++++++++++++++++------
t/t9902-completion.sh | 103 +++++++++++++++++
2 files changed, 215 insertions(+), 34 deletions(-)
--
2.25.2
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 01/11] completion: add some simple test cases for git switch completion
2020-04-25 2:20 [PATCH 00/11] refactor git switch completion Jacob Keller
@ 2020-04-25 2:20 ` Jacob Keller
2020-04-25 2:20 ` [PATCH 02/11] completion: add test showing subpar " Jacob Keller
` (12 subsequent siblings)
13 siblings, 0 replies; 26+ messages in thread
From: Jacob Keller @ 2020-04-25 2:20 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, Jacob Keller
From: Jacob Keller <jacob.keller@gmail.com>
Future work is going to refactor the completion support for git switch,
so add some new test cases in order to help prevent regressions.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
t/t9902-completion.sh | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 5505e5aa249e..03e8188f023d 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1240,6 +1240,31 @@ test_expect_success '__git_complete_fetch_refspecs - fully qualified & prefix' '
test_cmp expected out
'
+test_expect_success 'git switch - with --no-guess, complete only local branches' '
+ test_completion "git switch --no-guess " <<-\EOF
+ master Z
+ matching-branch Z
+ EOF
+'
+
+test_expect_success 'git switch - with --detach, complete all references' '
+ test_completion "git switch --detach " <<-\EOF
+ HEAD Z
+ master Z
+ matching-branch Z
+ matching-tag Z
+ other/branch-in-other Z
+ other/master-in-other Z
+ EOF
+'
+
+test_expect_success 'git switch - with --no-track, complete only local branch names' '
+ test_completion "git switch --no-track " <<-\EOF
+ master Z
+ matching-branch Z
+ EOF
+'
+
test_expect_success 'teardown after ref completion' '
git branch -d matching-branch &&
git tag -d matching-tag &&
--
2.25.2
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 02/11] completion: add test showing subpar git switch completion
2020-04-25 2:20 [PATCH 00/11] refactor git switch completion Jacob Keller
2020-04-25 2:20 ` [PATCH 01/11] completion: add some simple test cases for " Jacob Keller
@ 2020-04-25 2:20 ` Jacob Keller
2020-04-25 2:20 ` [PATCH 03/11] completion: add test highlighting subpar git switch --track completion Jacob Keller
` (11 subsequent siblings)
13 siblings, 0 replies; 26+ messages in thread
From: Jacob Keller @ 2020-04-25 2:20 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, Jacob Keller
From: Jacob Keller <jacob.keller@gmail.com>
With no options, git switch only allows switching branches or DWIM to
create a local branch tracking a remote branch of the same name.
However, tab completion will expand "git switch <TAB>" to any local
reference, including pseudorefs like HEAD, or tags.
Add a test case which highlights this failure, which will be fixed in
a future refactoring of git switch completion support.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
t/t9902-completion.sh | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 03e8188f023d..af4661cbcc73 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1240,6 +1240,19 @@ test_expect_success '__git_complete_fetch_refspecs - fully qualified & prefix' '
test_cmp expected out
'
+# TODO: git switch by default should only include local branches and anything which
+# would be understood by the DWIM logic. Currently it will complete most
+# references including pseudorefs like HEAD and FETCH_HEAD, as well as tags.
+# These should not be completed unless certain options have been enabled.
+test_expect_failure 'git switch - with no options, complete local branches and unique remote branch names for DWIM logic' '
+ test_completion "git switch " <<-\EOF
+ branch-in-other Z
+ master Z
+ master-in-other Z
+ matching-branch Z
+ EOF
+'
+
test_expect_success 'git switch - with --no-guess, complete only local branches' '
test_completion "git switch --no-guess " <<-\EOF
master Z
--
2.25.2
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 03/11] completion: add test highlighting subpar git switch --track completion
2020-04-25 2:20 [PATCH 00/11] refactor git switch completion Jacob Keller
2020-04-25 2:20 ` [PATCH 01/11] completion: add some simple test cases for " Jacob Keller
2020-04-25 2:20 ` [PATCH 02/11] completion: add test showing subpar " Jacob Keller
@ 2020-04-25 2:20 ` Jacob Keller
2020-04-25 2:20 ` [PATCH 04/11] completion: add tests showing lack of support for git switch -c/-C Jacob Keller
` (10 subsequent siblings)
13 siblings, 0 replies; 26+ messages in thread
From: Jacob Keller @ 2020-04-25 2:20 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, Jacob Keller
From: Jacob Keller <jacob.keller@gmail.com>
If git switch is called with --track, it will attempts to DWIM into
creating a local tracking branch that tracks the provided remote branch.
It seems reasonable that, to support this behavior, we should only
complete remote branches in the form "<remote>/<branch>".
Indeed, current completion is not just sub-par, but could almost be
described as entirely useless.
$git switch --track <TAB>
will only report *local* branch names. Indeed a new test case highlights
this quite well:
--- expected 2020-04-25 00:25:34.424965326 +0000
+++ out_sorted 2020-04-25 00:25:34.441965370 +0000
@@ -1,2 +1,2 @@
-other/branch-in-other
-other/master-in-other
+master
+matching-branch
not ok 100 - git switch - with --track, complete only remote branches # TODO known breakage
Understanding exactly what causes this is not that simple.
First we enable DWIM output by default. Then, if "--track",
"--no-track", or "--no-guess" is enabled on the command line, we disable
DWIM. This makes sense, because --track should not include the default
"DWIM" remote branch names.
Following this, there is a check for "--detach". If "--detach" is *not*
present, then we set only_local_ref=y. this is done because we would
like to avoid printing remote references. This immediately seems wrong
because --track should allow completing remote references.
Finally, if only_local_ref is 'y', and the track_opt for DWIM logic is
disabled, we complete only local branches. This occurs because --track
disabled track_opt, and not providing --detach sets only_local_ref to
'y'.
Fixing this correctly is not trivial, so it is left to a follow up
change.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
t/t9902-completion.sh | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index af4661cbcc73..002223160058 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1271,6 +1271,16 @@ test_expect_success 'git switch - with --detach, complete all references' '
EOF
'
+# TODO: Since --track on its own will perform a DWIM to extract the local
+# branch name, we should complete only the remote branches with their remote
+# name.
+test_expect_failure 'git switch - with --track, complete only remote branches' '
+ test_completion "git switch --track " <<-\EOF
+ other/branch-in-other Z
+ other/master-in-other Z
+ EOF
+'
+
test_expect_success 'git switch - with --no-track, complete only local branch names' '
test_completion "git switch --no-track " <<-\EOF
master Z
--
2.25.2
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 04/11] completion: add tests showing lack of support for git switch -c/-C
2020-04-25 2:20 [PATCH 00/11] refactor git switch completion Jacob Keller
` (2 preceding siblings ...)
2020-04-25 2:20 ` [PATCH 03/11] completion: add test highlighting subpar git switch --track completion Jacob Keller
@ 2020-04-25 2:20 ` Jacob Keller
2020-04-25 2:20 ` [PATCH 05/11] completion: add test showing subpar completion for git switch --orphan Jacob Keller
` (9 subsequent siblings)
13 siblings, 0 replies; 26+ messages in thread
From: Jacob Keller @ 2020-04-25 2:20 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, Jacob Keller
From: Jacob Keller <jacob.keller@gmail.com>
Add several tests for git switch completion which highlight the expected
behavior if -c or -C have been provided.
Show that properly recognizing and supporting -c should override the
behavior for --track.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
t/t9902-completion.sh | 63 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 63 insertions(+)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 002223160058..a134a8791076 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1288,6 +1288,69 @@ test_expect_success 'git switch - with --no-track, complete only local branch na
EOF
'
+# TODO: git switch completion does not yet support checking for -c, but it
+# should be able to complete all possible references. Based on a quick
+# examination of the switch/checkout code, -c will disable DWIM logic and thus
+# we should not complete unique remote branch names with -c or -C either.
+test_expect_failure 'git switch - with -c, complete all references' '
+ test_completion "git switch -c new-branch " <<-\EOF
+ HEAD Z
+ master Z
+ matching-branch Z
+ matching-tag Z
+ other/branch-in-other Z
+ other/master-in-other Z
+ EOF
+'
+
+test_expect_failure 'git switch - with -c, complete all references' '
+ test_completion "git switch -C new-branch " <<-\EOF
+ HEAD Z
+ master Z
+ matching-branch Z
+ matching-tag Z
+ other/branch-in-other Z
+ other/master-in-other Z
+ EOF
+'
+
+# TODO: ensure that the completion rules for -c override --track
+test_expect_failure 'git switch - with -c and --track, complete all references' '
+ test_completion "git switch -c new-branch --track " <<-EOF
+ HEAD Z
+ master Z
+ matching-branch Z
+ matching-tag Z
+ other/branch-in-other Z
+ other/master-in-other Z
+ EOF
+'
+
+# TODO: git switch with -c and --no-track should allow creating a branch using
+# any reference as a starting point. Because completion support does not
+# recognize -c or -C, this doesn't work yet.
+test_expect_failure 'git switch - with -c and --no-track, complete all references' '
+ test_completion "git switch -c new-branch --no-track " <<-\EOF
+ HEAD Z
+ master Z
+ matching-branch Z
+ matching-tag Z
+ other/branch-in-other Z
+ other/master-in-other Z
+ EOF
+'
+
+test_expect_failure 'git switch - with -C and --no-track, complete all references' '
+ test_completion "git switch -C new-branch --no-track " <<-\EOF
+ HEAD Z
+ master Z
+ matching-branch Z
+ matching-tag Z
+ other/branch-in-other Z
+ other/master-in-other Z
+ EOF
+'
+
test_expect_success 'teardown after ref completion' '
git branch -d matching-branch &&
git tag -d matching-tag &&
--
2.25.2
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 05/11] completion: add test showing subpar completion for git switch --orphan
2020-04-25 2:20 [PATCH 00/11] refactor git switch completion Jacob Keller
` (3 preceding siblings ...)
2020-04-25 2:20 ` [PATCH 04/11] completion: add tests showing lack of support for git switch -c/-C Jacob Keller
@ 2020-04-25 2:20 ` Jacob Keller
2020-04-27 23:34 ` Junio C Hamano
2020-04-25 2:20 ` [PATCH 05/11] completion: remove " Jacob Keller
` (8 subsequent siblings)
13 siblings, 1 reply; 26+ messages in thread
From: Jacob Keller @ 2020-04-25 2:20 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, Jacob Keller
From: Jacob Keller <jacob.keller@gmail.com>
git switch with the --orphan option is used to create a new branch that
is not connected to any history and is instead based on the empty tree.
It does not make sense for completion to return anything in this case,
because there is nothing to complete. Check for --orphan, and if it's
found, immediately return from _git_switch() without completing
anything.
Add a test case which documents this expected behavior.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
contrib/completion/git-completion.bash | 11 ++++++++++-
t/t9902-completion.sh | 7 +++++++
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c21786f2fd00..08d3406cf3e4 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2223,9 +2223,18 @@ _git_switch ()
__gitcomp_builtin switch
;;
*)
+ local track_opt="--track" only_local_ref=n
+
+ # --orphan is used to create a branch disconnected from the
+ # current history, based on the empty tree. Since the only
+ # option required is the branch name, it doesn't make sense to
+ # complete anything here.
+ if [ -n "$(__git_find_on_cmdline "--orphan")" ]; then
+ return
+ fi
+
# check if --track, --no-track, or --no-guess was specified
# if so, disable DWIM mode
- local track_opt="--track" only_local_ref=n
if [ "$GIT_COMPLETION_CHECKOUT_NO_GUESS" = "1" ] ||
[ -n "$(__git_find_on_cmdline "--track --no-track --no-guess")" ]; then
track_opt=''
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index a134a8791076..9d02de167219 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1351,6 +1351,13 @@ test_expect_failure 'git switch - with -C and --no-track, complete all reference
EOF
'
+# TODO: completion does not yet recognize --orphan
+test_expect_failure 'git switch - with --orphan, do not complete anything' '
+ test_completion "git switch --orphan " <<-\EOF
+
+ EOF
+'
+
test_expect_success 'teardown after ref completion' '
git branch -d matching-branch &&
git tag -d matching-tag &&
--
2.25.2
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 05/11] completion: remove completion for git switch --orphan
2020-04-25 2:20 [PATCH 00/11] refactor git switch completion Jacob Keller
` (4 preceding siblings ...)
2020-04-25 2:20 ` [PATCH 05/11] completion: add test showing subpar completion for git switch --orphan Jacob Keller
@ 2020-04-25 2:20 ` Jacob Keller
2020-04-25 2:20 ` [PATCH 06/11] completion: rename --track option of __git_complete_refs Jacob Keller
` (7 subsequent siblings)
13 siblings, 0 replies; 26+ messages in thread
From: Jacob Keller @ 2020-04-25 2:20 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, Jacob Keller
From: Jacob Keller <jacob.keller@gmail.com>
git switch with the --orphan option is used to create a new branch that
is not connected to any history and is instead based on the empty tree.
It does not make sense for completion to return anything in this case,
because there is nothing to complete. Check for --orphan, and if it's
found, immediately return from _git_switch() without completing
anything.
Add a test case which documents this expected behavior.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
contrib/completion/git-completion.bash | 11 ++++++++++-
t/t9902-completion.sh | 7 +++++++
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c21786f2fd00..08d3406cf3e4 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2223,9 +2223,18 @@ _git_switch ()
__gitcomp_builtin switch
;;
*)
+ local track_opt="--track" only_local_ref=n
+
+ # --orphan is used to create a branch disconnected from the
+ # current history, based on the empty tree. Since the only
+ # option required is the branch name, it doesn't make sense to
+ # complete anything here.
+ if [ -n "$(__git_find_on_cmdline "--orphan")" ]; then
+ return
+ fi
+
# check if --track, --no-track, or --no-guess was specified
# if so, disable DWIM mode
- local track_opt="--track" only_local_ref=n
if [ "$GIT_COMPLETION_CHECKOUT_NO_GUESS" = "1" ] ||
[ -n "$(__git_find_on_cmdline "--track --no-track --no-guess")" ]; then
track_opt=''
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index a134a8791076..9d02de167219 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1351,6 +1351,13 @@ test_expect_failure 'git switch - with -C and --no-track, complete all reference
EOF
'
+# TODO: completion does not yet recognize --orphan
+test_expect_failure 'git switch - with --orphan, do not complete anything' '
+ test_completion "git switch --orphan " <<-\EOF
+
+ EOF
+'
+
test_expect_success 'teardown after ref completion' '
git branch -d matching-branch &&
git tag -d matching-tag &&
--
2.25.2
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 06/11] completion: rename --track option of __git_complete_refs
2020-04-25 2:20 [PATCH 00/11] refactor git switch completion Jacob Keller
` (5 preceding siblings ...)
2020-04-25 2:20 ` [PATCH 05/11] completion: remove " Jacob Keller
@ 2020-04-25 2:20 ` Jacob Keller
2020-04-25 2:20 ` [PATCH 07/11] completion: extract function __git_dwim_remote_heads Jacob Keller
` (6 subsequent siblings)
13 siblings, 0 replies; 26+ messages in thread
From: Jacob Keller @ 2020-04-25 2:20 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, Jacob Keller
From: Jacob Keller <jacob.keller@gmail.com>
The __git_complete_refs uses the "--track" option to specify when to
enable listing of unique remote branches which could be used by the
DWIMery of git checkout and git switch.
This is confusing, because both git checkout and git switch have
a --track option. This makes looking at the completion code for these
two functions confusing.
Rename this parameter to --dwim. Because it is plausible for users to
have developed their own completions which rely on __git_complete_ref,
keep --track as a synonym for --dwim, but do not use it in our own
completion code.
This reduces the confusion when seeing --track in the _git_switch() and
_git_checkout() completion functions.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
contrib/completion/git-completion.bash | 28 ++++++++++++++------------
1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 08d3406cf3e4..02dc1203559c 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -749,7 +749,7 @@ __git_refs ()
# Usage: __git_complete_refs [<option>]...
# --remote=<remote>: The remote to list refs from, can be the name of a
# configured remote, a path, or a URL.
-# --track: List unique remote branches for 'git checkout's tracking DWIMery.
+# --dwim: List unique remote branches for 'git switch's tracking DWIMery.
# --pfx=<prefix>: A prefix to be added to each ref.
# --cur=<word>: The current ref to be completed. Defaults to the current
# word to be completed.
@@ -757,12 +757,14 @@ __git_refs ()
# space.
__git_complete_refs ()
{
- local remote track pfx cur_="$cur" sfx=" "
+ local remote dwim pfx cur_="$cur" sfx=" "
while test $# != 0; do
case "$1" in
--remote=*) remote="${1##--remote=}" ;;
- --track) track="yes" ;;
+ --dwim) dwim="yes" ;;
+ # --track is an old spelling of --dwim
+ --track) dwim="yes" ;;
--pfx=*) pfx="${1##--pfx=}" ;;
--cur=*) cur_="${1##--cur=}" ;;
--sfx=*) sfx="${1##--sfx=}" ;;
@@ -771,7 +773,7 @@ __git_complete_refs ()
shift
done
- __gitcomp_direct "$(__git_refs "$remote" "$track" "$pfx" "$cur_" "$sfx")"
+ __gitcomp_direct "$(__git_refs "$remote" "$dwim" "$pfx" "$cur_" "$sfx")"
}
# __git_refs2 requires 1 argument (to pass to __git_refs)
@@ -1370,12 +1372,12 @@ _git_checkout ()
*)
# check if --track, --no-track, or --no-guess was specified
# if so, disable DWIM mode
- local flags="--track --no-track --no-guess" track_opt="--track"
+ local flags="--track --no-track --no-guess" dwim_opt="--dwim"
if [ "$GIT_COMPLETION_CHECKOUT_NO_GUESS" = "1" ] ||
[ -n "$(__git_find_on_cmdline "$flags")" ]; then
- track_opt=''
+ dwim_opt=''
fi
- __git_complete_refs $track_opt
+ __git_complete_refs $dwim_opt
;;
esac
}
@@ -2223,7 +2225,7 @@ _git_switch ()
__gitcomp_builtin switch
;;
*)
- local track_opt="--track" only_local_ref=n
+ local dwim_opt="--dwim" only_local_ref=n
# --orphan is used to create a branch disconnected from the
# current history, based on the empty tree. Since the only
@@ -2237,24 +2239,24 @@ _git_switch ()
# if so, disable DWIM mode
if [ "$GIT_COMPLETION_CHECKOUT_NO_GUESS" = "1" ] ||
[ -n "$(__git_find_on_cmdline "--track --no-track --no-guess")" ]; then
- track_opt=''
+ dwim_opt=''
fi
# explicit --guess enables DWIM mode regardless of
# $GIT_COMPLETION_CHECKOUT_NO_GUESS
if [ -n "$(__git_find_on_cmdline "--guess")" ]; then
- track_opt='--track'
+ dwim_opt='--dwim'
fi
if [ -z "$(__git_find_on_cmdline "-d --detach")" ]; then
only_local_ref=y
else
# --guess --detach is invalid combination, no
# dwim will be done when --detach is specified
- track_opt=
+ dwim_opt=
fi
- if [ $only_local_ref = y -a -z "$track_opt" ]; then
+ if [ $only_local_ref = y -a -z "$dwim_opt" ]; then
__gitcomp_direct "$(__git_heads "" "$cur" " ")"
else
- __git_complete_refs $track_opt
+ __git_complete_refs $dwim_opt
fi
;;
esac
--
2.25.2
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 07/11] completion: extract function __git_dwim_remote_heads
2020-04-25 2:20 [PATCH 00/11] refactor git switch completion Jacob Keller
` (6 preceding siblings ...)
2020-04-25 2:20 ` [PATCH 06/11] completion: rename --track option of __git_complete_refs Jacob Keller
@ 2020-04-25 2:20 ` Jacob Keller
2020-04-25 2:20 ` [PATCH 08/11] completion: perform DWIM logic directly in __git_complete_refs Jacob Keller
` (5 subsequent siblings)
13 siblings, 0 replies; 26+ messages in thread
From: Jacob Keller @ 2020-04-25 2:20 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, Jacob Keller
From: Jacob Keller <jacob.keller@gmail.com>
__git_refs() has the ability to report unique remote names for
supporting completion of remote branch names for the DWIMery of git
checkout and git switch.
For git checkout, this is fine, because it always supports completing
all local references.
However, git switch by default only supports either switching branches
or using this DWIMery to create a local branch tracking the remote
branch.
Future work to cleanup and improve completion support for git switch
will be aided if the remote branch names can be completed separately
from __git_refs.
Extract this logic to a function __git_dwim_remote_heads(), and use it
in __git_refs.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
contrib/completion/git-completion.bash | 28 +++++++++++++++++++-------
1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 02dc1203559c..f1ee25f05690 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -621,6 +621,26 @@ __git_tags ()
"refs/tags/$cur_*" "refs/tags/$cur_*/**"
}
+# List unique branches from refs/remotes used for 'git checkout' and 'git
+# switch' tracking DWIMery.
+# 1: A prefix to be added to each listed branch (optional)
+# 2: List only branches matching this word (optional; list all branches if
+# unset or empty).
+# 3: A suffix to be appended to each listed branch (optional).
+__git_dwim_remote_heads ()
+{
+ local pfx="${1-}" cur_="${2-}" sfx="${3-}"
+ local fer_pfx="${pfx//\%/%%}" # "escape" for-each-ref format specifiers
+
+ # employ the heuristic used by git checkout and git switch
+ # Try to find a remote branch that cur_es the completion word
+ # but only output if the branch name is unique
+ __git for-each-ref --format="$fer_pfx%(refname:strip=3)$sfx" \
+ --sort="refname:strip=3" \
+ "refs/remotes/*/$cur_*" "refs/remotes/*/$cur_*/**" | \
+ uniq -u
+}
+
# Lists refs from the local (by default) or from a remote repository.
# It accepts 0, 1 or 2 arguments:
# 1: The remote to list refs from (optional; ignored, if set but empty).
@@ -696,13 +716,7 @@ __git_refs ()
__git_dir="$dir" __git for-each-ref --format="$fer_pfx%($format)$sfx" \
"${refs[@]}"
if [ -n "$track" ]; then
- # employ the heuristic used by git checkout
- # Try to find a remote branch that matches the completion word
- # but only output if the branch name is unique
- __git for-each-ref --format="$fer_pfx%(refname:strip=3)$sfx" \
- --sort="refname:strip=3" \
- "refs/remotes/*/$match*" "refs/remotes/*/$match*/**" | \
- uniq -u
+ __git_dwim_remote_heads "$pfx" "$match" "$sfx"
fi
return
fi
--
2.25.2
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 08/11] completion: perform DWIM logic directly in __git_complete_refs
2020-04-25 2:20 [PATCH 00/11] refactor git switch completion Jacob Keller
` (7 preceding siblings ...)
2020-04-25 2:20 ` [PATCH 07/11] completion: extract function __git_dwim_remote_heads Jacob Keller
@ 2020-04-25 2:20 ` Jacob Keller
2020-04-25 2:20 ` [PATCH 09/11] completion: fix completion for git switch with no options Jacob Keller
` (4 subsequent siblings)
13 siblings, 0 replies; 26+ messages in thread
From: Jacob Keller @ 2020-04-25 2:20 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, Jacob Keller
From: Jacob Keller <jacob.keller@gmail.com>
__git_complete_refs is the main function used for completing references.
It is primarily used as a wrapper around __git_refs, and is easier to
extend since its arguments are option-like.
One major downside of __git_complete_refs and __git_refs currently, is
the lack of ability to complete only a subset of refs such as branches
(refs/heads) or tags (refs/tags).
Normally, a caller might just decide to use __git_heads() or
__git_tags(). However, in the case of git-switch, it is useful to
complete both branches *and* DWIM remote branch names.
Due to the complexity and implementation of __git_refs, it is not easy
to extend it to support listing only a subset of references.
Instead, we can extend __git_complete_refs to do this. For this to be
done, we must first ensure that "--dwim" support is not tied to calling
__git_refs.
Instead of passing $dwim into __git_refs, we can implement
a __gitcomp_direct_append function which can append to COMPREPLY after
a call to __gitcomp_direct.
If --dwim is passed to __git_complete_refs, use __gitcomp_direct_append
to add the output of __git_dwim_remote_heads to the completion list.
In this way, --dwim support is now independent of calling __git_refs.
A future change will add an additional option to control what set of
references __git_complete_refs will output.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
contrib/completion/git-completion.bash | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index f1ee25f05690..c582e070711f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -301,6 +301,19 @@ __gitcomp_direct ()
COMPREPLY=($1)
}
+# Similar to __gitcomp_direct, but appends to COMPREPLY instead.
+# Callers must take care of providing only words that match the current word
+# to be completed and adding any prefix and/or suffix (trailing space!), if
+# necessary.
+# 1: List of newline-separated matching completion words, complete with
+# prefix and suffix.
+__gitcomp_direct_append ()
+{
+ local IFS=$'\n'
+
+ COMPREPLY+=($1)
+}
+
__gitcompappend ()
{
local x i=${#COMPREPLY[@]}
@@ -787,7 +800,11 @@ __git_complete_refs ()
shift
done
- __gitcomp_direct "$(__git_refs "$remote" "$dwim" "$pfx" "$cur_" "$sfx")"
+ __gitcomp_direct "$(__git_refs "$remote" "" "$pfx" "$cur_" "$sfx")"
+
+ if [ "$dwim" = "yes" ]; then
+ __gitcomp_direct_append "$(__git_dwim_remote_heads "$pfx" "$cur_" "$sfx")"
+ fi
}
# __git_refs2 requires 1 argument (to pass to __git_refs)
--
2.25.2
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 09/11] completion: fix completion for git switch with no options
2020-04-25 2:20 [PATCH 00/11] refactor git switch completion Jacob Keller
` (8 preceding siblings ...)
2020-04-25 2:20 ` [PATCH 08/11] completion: perform DWIM logic directly in __git_complete_refs Jacob Keller
@ 2020-04-25 2:20 ` Jacob Keller
2020-04-25 2:20 ` [PATCH 10/11] completion: recognize -c/-C when completing for git switch Jacob Keller
` (3 subsequent siblings)
13 siblings, 0 replies; 26+ messages in thread
From: Jacob Keller @ 2020-04-25 2:20 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, Jacob Keller
From: Jacob Keller <jacob.keller@gmail.com>
Add a new --mode option to __git_complete_refs, which allows changing
the behavior to call __git_heads instead of __git_refs.
By passing --mode=heads, __git_complete_refs will only output local
branches. This enables using "--mode=heads --dwim" to enable listing
local branches or the remote unique branch names for DWIM.
Refactor completion support in _git_switch() to decide whether to use
this mode instead of calling __git_heads directly.
While doing this, cleanup the function so that it is a little less
confusing.
We can simplify this a little by moving the check for
GIT_COMPLETION_CHECKOUT_NO_GUESS first, so that we do not get weird
interactions by re-enabling --dwim after checking for other options that
might disable it.
If --track, --no-track, --no-guess, -d, or --detach are provided, we
will disable --dwim, as these DWIM branch names won't work in these
modes.
Choose the --mode to specify to __git_complete_refs by also checking
command line parameters. Default to using --mode=heads, to list only
local branches.
If -d or --detach are provided, switch to --mode=refs, so that we
display all references.
In this way, the basic support for completing just "git switch <TAB>"
will result in only local branches and remote unique names for DWIM, so
switch this test to test_expect_success.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
contrib/completion/git-completion.bash | 59 ++++++++++++++++----------
t/t9902-completion.sh | 6 +--
2 files changed, 38 insertions(+), 27 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c582e070711f..0384b136763a 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -782,9 +782,12 @@ __git_refs ()
# word to be completed.
# --sfx=<suffix>: A suffix to be appended to each ref instead of the default
# space.
+# --mode=<mode>: What set of refs to complete, one of 'refs' (the default) to
+# complete all refs, or 'heads' to complete only branches. Note
+# that --remote is only compatible with --mode=refs.
__git_complete_refs ()
{
- local remote dwim pfx cur_="$cur" sfx=" "
+ local remote dwim pfx cur_="$cur" sfx=" " mode="refs"
while test $# != 0; do
case "$1" in
@@ -795,13 +798,23 @@ __git_complete_refs ()
--pfx=*) pfx="${1##--pfx=}" ;;
--cur=*) cur_="${1##--cur=}" ;;
--sfx=*) sfx="${1##--sfx=}" ;;
+ --mode=*) mode="${1##--mode=}" ;;
*) return 1 ;;
esac
shift
done
- __gitcomp_direct "$(__git_refs "$remote" "" "$pfx" "$cur_" "$sfx")"
+ # complete references based on the specified mode
+ case "$mode" in
+ refs)
+ __gitcomp_direct "$(__git_refs "$remote" "" "$pfx" "$cur_" "$sfx")" ;;
+ heads)
+ __gitcomp_direct "$(__git_heads "$pfx" "$cur_" "$sfx")" ;;
+ *)
+ return 1 ;;
+ esac
+ # Append DWIM remote branch names if requested
if [ "$dwim" = "yes" ]; then
__gitcomp_direct_append "$(__git_dwim_remote_heads "$pfx" "$cur_" "$sfx")"
fi
@@ -2256,7 +2269,7 @@ _git_switch ()
__gitcomp_builtin switch
;;
*)
- local dwim_opt="--dwim" only_local_ref=n
+ local dwim_opt="--dwim" mode="heads"
# --orphan is used to create a branch disconnected from the
# current history, based on the empty tree. Since the only
@@ -2266,29 +2279,31 @@ _git_switch ()
return
fi
- # check if --track, --no-track, or --no-guess was specified
- # if so, disable DWIM mode
- if [ "$GIT_COMPLETION_CHECKOUT_NO_GUESS" = "1" ] ||
- [ -n "$(__git_find_on_cmdline "--track --no-track --no-guess")" ]; then
+ # By default, git switch will DWIM with remote branch names by
+ # allowing these to expand into creating a local tracking
+ # branch of the same name. Completion for this can be disabled
+ # via GIT_COMPLETION_CHECKOUT_NO_GUESS, unless the user
+ # explicitly asked this behavior with --guess
+ if [ "$GIT_COMPLETION_CHECKOUT_NO_GUESS" = "1" ] &&
+ [ -z "$(__git_find_on_cmdline "--guess")" ]; then
dwim_opt=''
fi
- # explicit --guess enables DWIM mode regardless of
- # $GIT_COMPLETION_CHECKOUT_NO_GUESS
- if [ -n "$(__git_find_on_cmdline "--guess")" ]; then
- dwim_opt='--dwim'
+
+ # Certain combinations of options also disable this DWIM mode,
+ # so we should not complete such names in these cases.
+ if [ -n "$(__git_find_on_cmdline "--track --no-track --no-guess -d --detach")" ]; then
+ dwim_opt=''
fi
- if [ -z "$(__git_find_on_cmdline "-d --detach")" ]; then
- only_local_ref=y
- else
- # --guess --detach is invalid combination, no
- # dwim will be done when --detach is specified
- dwim_opt=
- fi
- if [ $only_local_ref = y -a -z "$dwim_opt" ]; then
- __gitcomp_direct "$(__git_heads "" "$cur" " ")"
- else
- __git_complete_refs $dwim_opt
+
+ # By default, git switch will only allow switching between
+ # local branches, or DWIM with remote branch names. However,
+ # certain options for creating branches or detaching should
+ # complete all references.
+ if [ -n "$(__git_find_on_cmdline "-d --detach")" ]; then
+ mode="refs"
fi
+
+ __git_complete_refs $dwim_opt --mode=$mode
;;
esac
}
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 9d02de167219..cfd27e4857b7 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1240,11 +1240,7 @@ test_expect_success '__git_complete_fetch_refspecs - fully qualified & prefix' '
test_cmp expected out
'
-# TODO: git switch by default should only include local branches and anything which
-# would be understood by the DWIM logic. Currently it will complete most
-# references including pseudorefs like HEAD and FETCH_HEAD, as well as tags.
-# These should not be completed unless certain options have been enabled.
-test_expect_failure 'git switch - with no options, complete local branches and unique remote branch names for DWIM logic' '
+test_expect_success 'git switch - with no options, complete local branches and unique remote branch names for DWIM logic' '
test_completion "git switch " <<-\EOF
branch-in-other Z
master Z
--
2.25.2
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 10/11] completion: recognize -c/-C when completing for git switch
2020-04-25 2:20 [PATCH 00/11] refactor git switch completion Jacob Keller
` (9 preceding siblings ...)
2020-04-25 2:20 ` [PATCH 09/11] completion: fix completion for git switch with no options Jacob Keller
@ 2020-04-25 2:20 ` Jacob Keller
2020-04-25 2:20 ` [PATCH 11/11] completion: complete remote branches for git switch --track Jacob Keller
` (2 subsequent siblings)
13 siblings, 0 replies; 26+ messages in thread
From: Jacob Keller @ 2020-04-25 2:20 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, Jacob Keller
From: Jacob Keller <jacob.keller@gmail.com>
If a user wants to create a new branch using git switch, we should be
able to complete any starting point reference. Additionally, DWIM logic
will not work.
Add -c and -C to the list of options for disabling --dwim and for
choosing the --refs mode.
This fixes several known breakages, so update these tests accordingly.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
contrib/completion/git-completion.bash | 4 ++--
t/t9902-completion.sh | 18 +++++-------------
2 files changed, 7 insertions(+), 15 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 0384b136763a..f9be0dabb03e 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2291,7 +2291,7 @@ _git_switch ()
# Certain combinations of options also disable this DWIM mode,
# so we should not complete such names in these cases.
- if [ -n "$(__git_find_on_cmdline "--track --no-track --no-guess -d --detach")" ]; then
+ if [ -n "$(__git_find_on_cmdline "--track --no-track --no-guess -d --detach -c -C")" ]; then
dwim_opt=''
fi
@@ -2299,7 +2299,7 @@ _git_switch ()
# local branches, or DWIM with remote branch names. However,
# certain options for creating branches or detaching should
# complete all references.
- if [ -n "$(__git_find_on_cmdline "-d --detach")" ]; then
+ if [ -n "$(__git_find_on_cmdline "-d --detach -c -C")" ]; then
mode="refs"
fi
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index cfd27e4857b7..68296d79a3e9 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1284,11 +1284,7 @@ test_expect_success 'git switch - with --no-track, complete only local branch na
EOF
'
-# TODO: git switch completion does not yet support checking for -c, but it
-# should be able to complete all possible references. Based on a quick
-# examination of the switch/checkout code, -c will disable DWIM logic and thus
-# we should not complete unique remote branch names with -c or -C either.
-test_expect_failure 'git switch - with -c, complete all references' '
+test_expect_success 'git switch - with -c, complete all references' '
test_completion "git switch -c new-branch " <<-\EOF
HEAD Z
master Z
@@ -1299,7 +1295,7 @@ test_expect_failure 'git switch - with -c, complete all references' '
EOF
'
-test_expect_failure 'git switch - with -c, complete all references' '
+test_expect_success 'git switch - with -c, complete all references' '
test_completion "git switch -C new-branch " <<-\EOF
HEAD Z
master Z
@@ -1310,8 +1306,7 @@ test_expect_failure 'git switch - with -c, complete all references' '
EOF
'
-# TODO: ensure that the completion rules for -c override --track
-test_expect_failure 'git switch - with -c and --track, complete all references' '
+test_expect_success 'git switch - with -c and --track, complete all references' '
test_completion "git switch -c new-branch --track " <<-EOF
HEAD Z
master Z
@@ -1322,10 +1317,7 @@ test_expect_failure 'git switch - with -c and --track, complete all references'
EOF
'
-# TODO: git switch with -c and --no-track should allow creating a branch using
-# any reference as a starting point. Because completion support does not
-# recognize -c or -C, this doesn't work yet.
-test_expect_failure 'git switch - with -c and --no-track, complete all references' '
+test_expect_success 'git switch - with -c and --no-track, complete all references' '
test_completion "git switch -c new-branch --no-track " <<-\EOF
HEAD Z
master Z
@@ -1336,7 +1328,7 @@ test_expect_failure 'git switch - with -c and --no-track, complete all reference
EOF
'
-test_expect_failure 'git switch - with -C and --no-track, complete all references' '
+test_expect_success 'git switch - with -C and --no-track, complete all references' '
test_completion "git switch -C new-branch --no-track " <<-\EOF
HEAD Z
master Z
--
2.25.2
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH 11/11] completion: complete remote branches for git switch --track
2020-04-25 2:20 [PATCH 00/11] refactor git switch completion Jacob Keller
` (10 preceding siblings ...)
2020-04-25 2:20 ` [PATCH 10/11] completion: recognize -c/-C when completing for git switch Jacob Keller
@ 2020-04-25 2:20 ` Jacob Keller
2020-04-25 22:14 ` [PATCH 00/11] refactor git switch completion Jacob Keller
2020-04-30 22:56 ` Junio C Hamano
13 siblings, 0 replies; 26+ messages in thread
From: Jacob Keller @ 2020-04-25 2:20 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, Jacob Keller
From: Jacob Keller <jacob.keller@gmail.com>
git switch --track <remote>/<branch> will be interpreted as a DWIM to
create a local tracking branch named <branch> tracking the specified
remote.
Completion support for this case is buggy, because it will instead
report only local branches.
Fix this by extending __git_complete_refs with a new mode,
"remote-heads" which will list all reference under refs/remotes/*.
By doing this, "git switch --track <TAB>" changes from the rather
useless set of local branches to only listing remote branches.
Note that if the user has specified "-c" or "-C" already on the command
line, this will still enable completing all references, as the check to
use the mode "refs" occurs first in an if-elif chain.
This finally fixes the "git switch --track" test case, so it is updated
accordingly.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
contrib/completion/git-completion.bash | 25 +++++++++++++++++++++++--
t/t9902-completion.sh | 5 +----
2 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index f9be0dabb03e..cdd141b2ba1d 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -624,6 +624,19 @@ __git_heads ()
"refs/heads/$cur_*" "refs/heads/$cur_*/**"
}
+# Lists branches from remote repositories.
+# 1: A prefix to be added to each listed branch (optional).
+# 2: List only branches matching this word (optional; list all branches if
+# unset or empty).
+# 3: A suffix to be appended to each listed branch (optional).
+__git_remote_heads ()
+{
+ local pfx="${1-}" cur_="${2-}" sfx="${3-}"
+
+ __git for-each-ref --format="${pfx//\%/%%}%(refname:strip=2)$sfx" \
+ "refs/remotes/$cur_*" "refs/remotes/$cur_*/**"
+}
+
# Lists tags from the local repository.
# Accepts the same positional parameters as __git_heads() above.
__git_tags ()
@@ -783,8 +796,9 @@ __git_refs ()
# --sfx=<suffix>: A suffix to be appended to each ref instead of the default
# space.
# --mode=<mode>: What set of refs to complete, one of 'refs' (the default) to
-# complete all refs, or 'heads' to complete only branches. Note
-# that --remote is only compatible with --mode=refs.
+# complete all refs, 'heads' to complete only branches, or
+# 'remote-heads' to complete only remote branches. Note that
+# --remote is only compatible with --mode=refs.
__git_complete_refs ()
{
local remote dwim pfx cur_="$cur" sfx=" " mode="refs"
@@ -810,6 +824,8 @@ __git_complete_refs ()
__gitcomp_direct "$(__git_refs "$remote" "" "$pfx" "$cur_" "$sfx")" ;;
heads)
__gitcomp_direct "$(__git_heads "$pfx" "$cur_" "$sfx")" ;;
+ remote-heads)
+ __gitcomp_direct "$(__git_remote_heads "$pfx" "$cur_" "$sfx")" ;;
*)
return 1 ;;
esac
@@ -2299,8 +2315,13 @@ _git_switch ()
# local branches, or DWIM with remote branch names. However,
# certain options for creating branches or detaching should
# complete all references.
+ #
+ # Additionally, if --track is provided on its own, we should
+ # complete only remote branch names.
if [ -n "$(__git_find_on_cmdline "-d --detach -c -C")" ]; then
mode="refs"
+ elif [ -n "$(__git_find_on_cmdline "--track")" ]; then
+ mode="remote-heads"
fi
__git_complete_refs $dwim_opt --mode=$mode
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 68296d79a3e9..7491d8c3b72d 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1267,10 +1267,7 @@ test_expect_success 'git switch - with --detach, complete all references' '
EOF
'
-# TODO: Since --track on its own will perform a DWIM to extract the local
-# branch name, we should complete only the remote branches with their remote
-# name.
-test_expect_failure 'git switch - with --track, complete only remote branches' '
+test_expect_success 'git switch - with --track, complete only remote branches' '
test_completion "git switch --track " <<-\EOF
other/branch-in-other Z
other/master-in-other Z
--
2.25.2
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 00/11] refactor git switch completion
2020-04-25 2:20 [PATCH 00/11] refactor git switch completion Jacob Keller
` (11 preceding siblings ...)
2020-04-25 2:20 ` [PATCH 11/11] completion: complete remote branches for git switch --track Jacob Keller
@ 2020-04-25 22:14 ` Jacob Keller
2020-04-30 22:56 ` Junio C Hamano
13 siblings, 0 replies; 26+ messages in thread
From: Jacob Keller @ 2020-04-25 22:14 UTC (permalink / raw)
To: Jacob Keller; +Cc: Git mailing list, Jonathan Nieder
On Fri, Apr 24, 2020 at 7:20 PM Jacob Keller <jacob.e.keller@intel.com> wrote:
> Following this are some patches to extract displaying DWIM remote branch
> names from __git_refs() and refactoring __git_complete_refs to take a mode
> argument that switches between calling __git_heads, __git_refs, and a new
> __git_remotes.
This should read __git_remote_heads, since I changed the name to be
more descriptive to avoid thinking it prints the remote names on their
own.
Thanks,
Jake
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 05/11] completion: add test showing subpar completion for git switch --orphan
2020-04-25 2:20 ` [PATCH 05/11] completion: add test showing subpar completion for git switch --orphan Jacob Keller
@ 2020-04-27 23:34 ` Junio C Hamano
2020-04-28 2:12 ` Jacob Keller
2020-04-28 2:20 ` Jacob Keller
0 siblings, 2 replies; 26+ messages in thread
From: Junio C Hamano @ 2020-04-27 23:34 UTC (permalink / raw)
To: Jacob Keller; +Cc: git, Jonathan Nieder, Jacob Keller
Jacob Keller <jacob.e.keller@intel.com> writes:
> From: Jacob Keller <jacob.keller@gmail.com>
>
> git switch with the --orphan option is used to create a new branch that
> is not connected to any history and is instead based on the empty tree.
>
> It does not make sense for completion to return anything in this case,
> because there is nothing to complete. Check for --orphan, and if it's
> found, immediately return from _git_switch() without completing
> anything.
>
> Add a test case which documents this expected behavior.
>
> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
> ---
> contrib/completion/git-completion.bash | 11 ++++++++++-
> t/t9902-completion.sh | 7 +++++++
> 2 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index c21786f2fd00..08d3406cf3e4 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2223,9 +2223,18 @@ _git_switch ()
> __gitcomp_builtin switch
> ;;
> *)
> + local track_opt="--track" only_local_ref=n
> +
> + # --orphan is used to create a branch disconnected from the
> + # current history, based on the empty tree. Since the only
> + # option required is the branch name, it doesn't make sense to
> + # complete anything here.
> + if [ -n "$(__git_find_on_cmdline "--orphan")" ]; then
> + return
> + fi
> +
> # check if --track, --no-track, or --no-guess was specified
> # if so, disable DWIM mode
> - local track_opt="--track" only_local_ref=n
> if [ "$GIT_COMPLETION_CHECKOUT_NO_GUESS" = "1" ] ||
> [ -n "$(__git_find_on_cmdline "--track --no-track --no-guess")" ]; then
> track_opt=''
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index a134a8791076..9d02de167219 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -1351,6 +1351,13 @@ test_expect_failure 'git switch - with -C and --no-track, complete all reference
> EOF
> '
>
> +# TODO: completion does not yet recognize --orphan
> +test_expect_failure 'git switch - with --orphan, do not complete anything' '
> + test_completion "git switch --orphan " <<-\EOF
> +
> + EOF
> +'
> +
I am getting "TODO passed" at 7276ffe0 (completion: add test showing
subpar completion for git switch --orphan, 2020-04-24), which hasn't
hit 'next' yet.
Perhaps we got some rebase gotcha here?
> test_expect_success 'teardown after ref completion' '
> git branch -d matching-branch &&
> git tag -d matching-tag &&
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 05/11] completion: add test showing subpar completion for git switch --orphan
2020-04-27 23:34 ` Junio C Hamano
@ 2020-04-28 2:12 ` Jacob Keller
2020-04-28 2:20 ` Jacob Keller
1 sibling, 0 replies; 26+ messages in thread
From: Jacob Keller @ 2020-04-28 2:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jacob Keller, Git mailing list, Jonathan Nieder
On Mon, Apr 27, 2020 at 4:34 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Jacob Keller <jacob.e.keller@intel.com> writes:
>
> > From: Jacob Keller <jacob.keller@gmail.com>
> >
> > git switch with the --orphan option is used to create a new branch that
> > is not connected to any history and is instead based on the empty tree.
> >
> > It does not make sense for completion to return anything in this case,
> > because there is nothing to complete. Check for --orphan, and if it's
> > found, immediately return from _git_switch() without completing
> > anything.
> >
> > Add a test case which documents this expected behavior.
> >
> > Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
> > ---
> > contrib/completion/git-completion.bash | 11 ++++++++++-
> > t/t9902-completion.sh | 7 +++++++
> > 2 files changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> > index c21786f2fd00..08d3406cf3e4 100644
> > --- a/contrib/completion/git-completion.bash
> > +++ b/contrib/completion/git-completion.bash
> > @@ -2223,9 +2223,18 @@ _git_switch ()
> > __gitcomp_builtin switch
> > ;;
> > *)
> > + local track_opt="--track" only_local_ref=n
> > +
> > + # --orphan is used to create a branch disconnected from the
> > + # current history, based on the empty tree. Since the only
> > + # option required is the branch name, it doesn't make sense to
> > + # complete anything here.
> > + if [ -n "$(__git_find_on_cmdline "--orphan")" ]; then
> > + return
> > + fi
> > +
> > # check if --track, --no-track, or --no-guess was specified
> > # if so, disable DWIM mode
> > - local track_opt="--track" only_local_ref=n
> > if [ "$GIT_COMPLETION_CHECKOUT_NO_GUESS" = "1" ] ||
> > [ -n "$(__git_find_on_cmdline "--track --no-track --no-guess")" ]; then
> > track_opt=''
> > diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> > index a134a8791076..9d02de167219 100755
> > --- a/t/t9902-completion.sh
> > +++ b/t/t9902-completion.sh
> > @@ -1351,6 +1351,13 @@ test_expect_failure 'git switch - with -C and --no-track, complete all reference
> > EOF
> > '
> >
> > +# TODO: completion does not yet recognize --orphan
> > +test_expect_failure 'git switch - with --orphan, do not complete anything' '
> > + test_completion "git switch --orphan " <<-\EOF
> > +
> > + EOF
> > +'
> > +
>
> I am getting "TODO passed" at 7276ffe0 (completion: add test showing
> subpar completion for git switch --orphan, 2020-04-24), which hasn't
> hit 'next' yet.
>
> Perhaps we got some rebase gotcha here?
>
Hmm.. Yea, I think there is a problem. Originally I had one commit
which added the test case for --orphan and I fixed it later, but I
rebased things so it was one commit. I probably just forgot to update
the tests.
Thanks,
Jake
> > test_expect_success 'teardown after ref completion' '
> > git branch -d matching-branch &&
> > git tag -d matching-tag &&
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 05/11] completion: add test showing subpar completion for git switch --orphan
2020-04-27 23:34 ` Junio C Hamano
2020-04-28 2:12 ` Jacob Keller
@ 2020-04-28 2:20 ` Jacob Keller
2020-04-28 16:24 ` Junio C Hamano
1 sibling, 1 reply; 26+ messages in thread
From: Jacob Keller @ 2020-04-28 2:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jacob Keller, Git mailing list, Jonathan Nieder
The proposed SQUASH you have in gitster/jk/complete-git-switch is
correct. The commit message body is correct, but the title could be
reworded to
"completion: stop completing refs for git switch --orphan"
I can send a v2 if that would be helpful, and I've got it fixed up
locally if other review increases the need for a new spin.
Thanks,
Jake
On Mon, Apr 27, 2020 at 4:34 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Jacob Keller <jacob.e.keller@intel.com> writes:
>
> > From: Jacob Keller <jacob.keller@gmail.com>
> >
> > git switch with the --orphan option is used to create a new branch that
> > is not connected to any history and is instead based on the empty tree.
> >
> > It does not make sense for completion to return anything in this case,
> > because there is nothing to complete. Check for --orphan, and if it's
> > found, immediately return from _git_switch() without completing
> > anything.
> >
> > Add a test case which documents this expected behavior.
> >
> > Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
> > ---
> > contrib/completion/git-completion.bash | 11 ++++++++++-
> > t/t9902-completion.sh | 7 +++++++
> > 2 files changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> > index c21786f2fd00..08d3406cf3e4 100644
> > --- a/contrib/completion/git-completion.bash
> > +++ b/contrib/completion/git-completion.bash
> > @@ -2223,9 +2223,18 @@ _git_switch ()
> > __gitcomp_builtin switch
> > ;;
> > *)
> > + local track_opt="--track" only_local_ref=n
> > +
> > + # --orphan is used to create a branch disconnected from the
> > + # current history, based on the empty tree. Since the only
> > + # option required is the branch name, it doesn't make sense to
> > + # complete anything here.
> > + if [ -n "$(__git_find_on_cmdline "--orphan")" ]; then
> > + return
> > + fi
> > +
> > # check if --track, --no-track, or --no-guess was specified
> > # if so, disable DWIM mode
> > - local track_opt="--track" only_local_ref=n
> > if [ "$GIT_COMPLETION_CHECKOUT_NO_GUESS" = "1" ] ||
> > [ -n "$(__git_find_on_cmdline "--track --no-track --no-guess")" ]; then
> > track_opt=''
> > diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> > index a134a8791076..9d02de167219 100755
> > --- a/t/t9902-completion.sh
> > +++ b/t/t9902-completion.sh
> > @@ -1351,6 +1351,13 @@ test_expect_failure 'git switch - with -C and --no-track, complete all reference
> > EOF
> > '
> >
> > +# TODO: completion does not yet recognize --orphan
> > +test_expect_failure 'git switch - with --orphan, do not complete anything' '
> > + test_completion "git switch --orphan " <<-\EOF
> > +
> > + EOF
> > +'
> > +
>
> I am getting "TODO passed" at 7276ffe0 (completion: add test showing
> subpar completion for git switch --orphan, 2020-04-24), which hasn't
> hit 'next' yet.
>
> Perhaps we got some rebase gotcha here?
>
> > test_expect_success 'teardown after ref completion' '
> > git branch -d matching-branch &&
> > git tag -d matching-tag &&
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 05/11] completion: add test showing subpar completion for git switch --orphan
2020-04-28 2:20 ` Jacob Keller
@ 2020-04-28 16:24 ` Junio C Hamano
2020-04-28 17:32 ` Jacob Keller
0 siblings, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2020-04-28 16:24 UTC (permalink / raw)
To: Jacob Keller; +Cc: Jacob Keller, Git mailing list, Jonathan Nieder
Jacob Keller <jacob.keller@gmail.com> writes:
> The proposed SQUASH you have in gitster/jk/complete-git-switch is
> correct. The commit message body is correct, but the title could be
> reworded to
>
> "completion: stop completing refs for git switch --orphan"
>
> I can send a v2 if that would be helpful, and I've got it fixed up
> locally if other review increases the need for a new spin.
Thnaks. In the meantime, what I have is attached (the test title,
in addition to the commit title, has been updated).
The same logic would apply to "git checkout -b <TAB>" (or "git
switch -c <TAB>") as this change: these are meant to create a new
branch so you do not want to offer an existing branch name.
I have a mixed feelings about that reasoning, though. I understand
that not offering any existing branch name would avoid offering a
branch name that would cause an error message from the command,
which at a first glance feels like a nice help to the user, but I am
not sure if it is really helping.
When you are on jk/complete-switch branch to work on this topic, you
may want to keep the current state of the branch and use a "v2"
branch to play around (running "rebase -i" etc.), for which the way
I would hope to work would be:
git checkout -b jk/comp<TAB>
that would complete to an existing branch
git checkout -b jk/complete-switch
and then I can just type "-v2<Enter>" (or "<BackSpace>-v2<Enter>" to
remove the inter-word space completion adds?) after that. After
all, "git checkout -b jk/complete-switch" in that scenario would
error out by saying that you already use that name, which is a good
enough protection.
And that same "is this really helping?" reasoning applies equally to
the "--orphan" option.
I dunno.
-- >8 --
From: Jacob Keller <jacob.keller@gmail.com>
Date: Fri, 24 Apr 2020 19:20:38 -0700
Subject: [PATCH] completion: stop completing refs for git switch --orphan
git switch with the --orphan option is used to create a new branch that
is not connected to any history and is instead based on the empty tree.
It does not make sense for completion to return anything in this case,
because there is nothing to complete. Check for --orphan, and if it's
found, immediately return from _git_switch() without completing
anything.
Add a test case which documents this expected behavior.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
contrib/completion/git-completion.bash | 11 ++++++++++-
t/t9902-completion.sh | 6 ++++++
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c21786f2fd..08d3406cf3 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2223,9 +2223,18 @@ _git_switch ()
__gitcomp_builtin switch
;;
*)
+ local track_opt="--track" only_local_ref=n
+
+ # --orphan is used to create a branch disconnected from the
+ # current history, based on the empty tree. Since the only
+ # option required is the branch name, it doesn't make sense to
+ # complete anything here.
+ if [ -n "$(__git_find_on_cmdline "--orphan")" ]; then
+ return
+ fi
+
# check if --track, --no-track, or --no-guess was specified
# if so, disable DWIM mode
- local track_opt="--track" only_local_ref=n
if [ "$GIT_COMPLETION_CHECKOUT_NO_GUESS" = "1" ] ||
[ -n "$(__git_find_on_cmdline "--track --no-track --no-guess")" ]; then
track_opt=''
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index a134a87910..7e4dd8e722 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1351,6 +1351,12 @@ test_expect_failure 'git switch - with -C and --no-track, complete all reference
EOF
'
+test_expect_success 'git switch --orphan completes no references' '
+ test_completion "git switch --orphan " <<-\EOF
+
+ EOF
+'
+
test_expect_success 'teardown after ref completion' '
git branch -d matching-branch &&
git tag -d matching-tag &&
--
2.26.2-266-ge870325ee8
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 05/11] completion: add test showing subpar completion for git switch --orphan
2020-04-28 16:24 ` Junio C Hamano
@ 2020-04-28 17:32 ` Jacob Keller
2020-04-28 18:10 ` Junio C Hamano
0 siblings, 1 reply; 26+ messages in thread
From: Jacob Keller @ 2020-04-28 17:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jacob Keller, Git mailing list, Jonathan Nieder
On Tue, Apr 28, 2020 at 9:24 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Jacob Keller <jacob.keller@gmail.com> writes:
>
> > The proposed SQUASH you have in gitster/jk/complete-git-switch is
> > correct. The commit message body is correct, but the title could be
> > reworded to
> >
> > "completion: stop completing refs for git switch --orphan"
> >
> > I can send a v2 if that would be helpful, and I've got it fixed up
> > locally if other review increases the need for a new spin.
>
> Thnaks. In the meantime, what I have is attached (the test title,
> in addition to the commit title, has been updated).
>
> The same logic would apply to "git checkout -b <TAB>" (or "git
> switch -c <TAB>") as this change: these are meant to create a new
> branch so you do not want to offer an existing branch name.
>
While true, you may optionally have a 2nd argument which provides a
start point that can be an arbitrary reference.
> I have a mixed feelings about that reasoning, though. I understand
> that not offering any existing branch name would avoid offering a
> branch name that would cause an error message from the command,
> which at a first glance feels like a nice help to the user, but I am
> not sure if it is really helping.
>
> When you are on jk/complete-switch branch to work on this topic, you
> may want to keep the current state of the branch and use a "v2"
> branch to play around (running "rebase -i" etc.), for which the way
> I would hope to work would be:
>
> git checkout -b jk/comp<TAB>
>
> that would complete to an existing branch
>
> git checkout -b jk/complete-switch
>
Hmm.. Yes this would be useful.
> and then I can just type "-v2<Enter>" (or "<BackSpace>-v2<Enter>" to
> remove the inter-word space completion adds?) after that. After
> all, "git checkout -b jk/complete-switch" in that scenario would
> error out by saying that you already use that name, which is a good
> enough protection.
>
> And that same "is this really helping?" reasoning applies equally to
> the "--orphan" option.
>
> I dunno.
>
Fair enough, new branches based on previous branches makes sense.
Thanks,
Jake
> -- >8 --
> From: Jacob Keller <jacob.keller@gmail.com>
> Date: Fri, 24 Apr 2020 19:20:38 -0700
> Subject: [PATCH] completion: stop completing refs for git switch --orphan
>
> git switch with the --orphan option is used to create a new branch that
> is not connected to any history and is instead based on the empty tree.
>
> It does not make sense for completion to return anything in this case,
> because there is nothing to complete. Check for --orphan, and if it's
> found, immediately return from _git_switch() without completing
> anything.
>
> Add a test case which documents this expected behavior.
>
> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> contrib/completion/git-completion.bash | 11 ++++++++++-
> t/t9902-completion.sh | 6 ++++++
> 2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index c21786f2fd..08d3406cf3 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2223,9 +2223,18 @@ _git_switch ()
> __gitcomp_builtin switch
> ;;
> *)
> + local track_opt="--track" only_local_ref=n
> +
> + # --orphan is used to create a branch disconnected from the
> + # current history, based on the empty tree. Since the only
> + # option required is the branch name, it doesn't make sense to
> + # complete anything here.
> + if [ -n "$(__git_find_on_cmdline "--orphan")" ]; then
> + return
> + fi
> +
> # check if --track, --no-track, or --no-guess was specified
> # if so, disable DWIM mode
> - local track_opt="--track" only_local_ref=n
> if [ "$GIT_COMPLETION_CHECKOUT_NO_GUESS" = "1" ] ||
> [ -n "$(__git_find_on_cmdline "--track --no-track --no-guess")" ]; then
> track_opt=''
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index a134a87910..7e4dd8e722 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -1351,6 +1351,12 @@ test_expect_failure 'git switch - with -C and --no-track, complete all reference
> EOF
> '
>
> +test_expect_success 'git switch --orphan completes no references' '
> + test_completion "git switch --orphan " <<-\EOF
> +
> + EOF
> +'
> +
> test_expect_success 'teardown after ref completion' '
> git branch -d matching-branch &&
> git tag -d matching-tag &&
> --
> 2.26.2-266-ge870325ee8
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 05/11] completion: add test showing subpar completion for git switch --orphan
2020-04-28 17:32 ` Jacob Keller
@ 2020-04-28 18:10 ` Junio C Hamano
2020-04-28 18:45 ` Jacob Keller
0 siblings, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2020-04-28 18:10 UTC (permalink / raw)
To: Jacob Keller; +Cc: Jacob Keller, Git mailing list, Jonathan Nieder
Jacob Keller <jacob.keller@gmail.com> writes:
>> And that same "is this really helping?" reasoning applies equally to
>> the "--orphan" option.
>>
>> I dunno.
>>
>
> Fair enough, new branches based on previous branches makes sense.
I actually do not have a strong opinion either way. I just wanted
to say that it would be good to make it consistent across "checkout
-b", "switch -c" and "checkout/switch --orphan".
It would be nice if "checkout -B" and "switch -C" pair offered
existing branches, as the intention of using the capital letter is
clear---the user wants to overwrite an existing one.
On the other hand, I am OK if "checkout -b", "switch -c" and
"--orphan" offered either:
(1) nothing, as your patch does, or
(2) all branches, except for the currently checked out one.
It would be preferrable if they did the same thing.
Thanks.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 05/11] completion: add test showing subpar completion for git switch --orphan
2020-04-28 18:10 ` Junio C Hamano
@ 2020-04-28 18:45 ` Jacob Keller
2020-04-28 19:16 ` Junio C Hamano
0 siblings, 1 reply; 26+ messages in thread
From: Jacob Keller @ 2020-04-28 18:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jacob Keller, Git mailing list, Jonathan Nieder
On Tue, Apr 28, 2020 at 11:10 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Jacob Keller <jacob.keller@gmail.com> writes:
>
> >> And that same "is this really helping?" reasoning applies equally to
> >> the "--orphan" option.
> >>
> >> I dunno.
> >>
> >
> > Fair enough, new branches based on previous branches makes sense.
>
> I actually do not have a strong opinion either way. I just wanted
> to say that it would be good to make it consistent across "checkout
> -b", "switch -c" and "checkout/switch --orphan".
>
> It would be nice if "checkout -B" and "switch -C" pair offered
> existing branches, as the intention of using the capital letter is
> clear---the user wants to overwrite an existing one.
>
> On the other hand, I am OK if "checkout -b", "switch -c" and
> "--orphan" offered either:
>
> (1) nothing, as your patch does, or
>
> (2) all branches, except for the currently checked out one.
Why reject the currently checked out one? If the premise is: "use a
current branch to build a new branch name", I don't see a reason to
restrict including the current branch here as well.
>
> It would be preferrable if they did the same thing.
>
> Thanks.
>
The problem I see, is that regardless, I would like to see the following:
git switch -c new-branch-name <SPACE><TAB>
complete all references for the starting point.
With this series, that's handled by just checking for "-c/-C" on the
command line and enabling all references. There's no positional checks
done, so every word can complete to a reference.
I don't know offhand how to add completion which depends on the
position of the word we're completing, so I'd have to investigate how
to make that happen.
Thanks,
Jake
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 05/11] completion: add test showing subpar completion for git switch --orphan
2020-04-28 18:45 ` Jacob Keller
@ 2020-04-28 19:16 ` Junio C Hamano
2020-04-28 20:41 ` Jacob Keller
0 siblings, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2020-04-28 19:16 UTC (permalink / raw)
To: Jacob Keller; +Cc: Jacob Keller, Git mailing list, Jonathan Nieder
Jacob Keller <jacob.keller@gmail.com> writes:
>> On the other hand, I am OK if "checkout -b", "switch -c" and
>> "--orphan" offered either:
>>
>> (1) nothing, as your patch does, or
>>
>> (2) all branches, except for the currently checked out one.
>
> Why reject the currently checked out one? If the premise is: "use a
> current branch to build a new branch name", I don't see a reason to
> restrict including the current branch here as well.
Yeah, "(3) all branches, without any exception" makes sense too.
>> It would be preferrable if they did the same thing.
>>
>> Thanks.
> The problem I see, is that regardless, I would like to see the following:
>
> git switch -c new-branch-name <SPACE><TAB>
>
> complete all references for the starting point.
Makes sense. The starting point could be any branch or even a tag.
> With this series, that's handled by just checking for "-c/-C" on the
> command line and enabling all references. There's no positional checks
> done, so every word can complete to a reference.
>
> I don't know offhand how to add completion which depends on the
> position of the word we're completing, so I'd have to investigate how
> to make that happen.
I see. Even though I say "(3) all branches" is a reasonable
behaviour, too, if the starting point has to be more (i.e. including
tags), that would not help the issue you have here. It may not be
too bad if we offered all refs (including tags, which may not be a
good idea) in that case. I dunno.
Thanks.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 05/11] completion: add test showing subpar completion for git switch --orphan
2020-04-28 19:16 ` Junio C Hamano
@ 2020-04-28 20:41 ` Jacob Keller
2020-04-28 20:57 ` Junio C Hamano
0 siblings, 1 reply; 26+ messages in thread
From: Jacob Keller @ 2020-04-28 20:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jacob Keller, Git mailing list, Jonathan Nieder
On Tue, Apr 28, 2020 at 12:16 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Jacob Keller <jacob.keller@gmail.com> writes:
> > With this series, that's handled by just checking for "-c/-C" on the
> > command line and enabling all references. There's no positional checks
> > done, so every word can complete to a reference.
> >
> > I don't know offhand how to add completion which depends on the
> > position of the word we're completing, so I'd have to investigate how
> > to make that happen.
>
> I see. Even though I say "(3) all branches" is a reasonable
> behaviour, too, if the starting point has to be more (i.e. including
> tags), that would not help the issue you have here. It may not be
> too bad if we offered all refs (including tags, which may not be a
> good idea) in that case. I dunno.
>
> Thanks.
>
Basically, when creating a branch, you often do "git switch -c
branch-name start-point"
branch-name would be the new branch name you want, and I think we
should complete local branches for this, but start-point can be any
reference, i.e. a tag, a local branch, or a remote reference.
I assumed completion would be most useful to complete the start-point,
and so I opted to lean towards fixing completion for -c/-C to complete
all references. However, I don't think the branch name should
necessarily complete from all references and it would make sense to
complete that portion only by local branch names.
I'm just not sure how best to implement that in our completion logic,
and I would rather ensure that start-point completes properly, if we
have to choose.
Thanks,
Jake
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 05/11] completion: add test showing subpar completion for git switch --orphan
2020-04-28 20:41 ` Jacob Keller
@ 2020-04-28 20:57 ` Junio C Hamano
0 siblings, 0 replies; 26+ messages in thread
From: Junio C Hamano @ 2020-04-28 20:57 UTC (permalink / raw)
To: Jacob Keller; +Cc: Jacob Keller, Git mailing list, Jonathan Nieder
Jacob Keller <jacob.keller@gmail.com> writes:
> I assumed completion would be most useful to complete the start-point,
> and so I opted to lean towards fixing completion for -c/-C to complete
> all references. However, I don't think the branch name should
> necessarily complete from all references and it would make sense to
> complete that portion only by local branch names.
>
> I'm just not sure how best to implement that in our completion logic,
> and I would rather ensure that start-point completes properly, if we
> have to choose.
Sure. What I was saying that if it is easier to implement by
completing both new branch name and starting point from the same
set, you could choose to include both branches and tags, and your
"excuse" for choosing that design could be that a user who wants to
fix something in 2.26 release may want the completion and typing
progression go like so:
$ git switch -c v2.2<TAB>
... offers v2.20, v2.21, ..., v2.26 as candidates
... complete to v2.26 and type "-frotz-fix"
$ git switch -c v2.26-frotz-fix
$ git switch -c v2.26-frotz-fix v2.2<TAB>
... again offers v2.20, v2.21, ..., v2.26
;-)
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 00/11] refactor git switch completion
2020-04-25 2:20 [PATCH 00/11] refactor git switch completion Jacob Keller
` (12 preceding siblings ...)
2020-04-25 22:14 ` [PATCH 00/11] refactor git switch completion Jacob Keller
@ 2020-04-30 22:56 ` Junio C Hamano
2020-05-01 21:53 ` Jacob Keller
13 siblings, 1 reply; 26+ messages in thread
From: Junio C Hamano @ 2020-04-30 22:56 UTC (permalink / raw)
To: Jacob Keller, SZEDER Gábor; +Cc: git, Jonathan Nieder, Jacob Keller
Jacob Keller <jacob.e.keller@intel.com> writes:
> From: Jacob Keller <jacob.keller@gmail.com>
>
> completion support for git switch is subpar for a number of cases. Most
> notable is difference between these two completions:
>
> $git switch <TAB>
> Display all 784 possibilities? (y or n)
> <list of all references and DWIM remotes>
>
> $git switch --track<TAB>
> jk-refactor-git-switch-completion master`
> ...
We've discussed that it may be a good idea to make sure that "switch
-c", "checkout -b" and "switch/checkout --orphan" complete the new
branch name the same way, but haven't done anything else. I'd very
much appreciate to see the patches reviewed by those involved more
in the completion script, before we decide to merge the topic to
'next'.
Thanks.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 00/11] refactor git switch completion
2020-04-30 22:56 ` Junio C Hamano
@ 2020-05-01 21:53 ` Jacob Keller
0 siblings, 0 replies; 26+ messages in thread
From: Jacob Keller @ 2020-05-01 21:53 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jacob Keller, SZEDER Gábor, Git mailing list, Jonathan Nieder
On Thu, Apr 30, 2020 at 3:57 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Jacob Keller <jacob.e.keller@intel.com> writes:
>
> > From: Jacob Keller <jacob.keller@gmail.com>
> >
> > completion support for git switch is subpar for a number of cases. Most
> > notable is difference between these two completions:
> >
> > $git switch <TAB>
> > Display all 784 possibilities? (y or n)
> > <list of all references and DWIM remotes>
> >
> > $git switch --track<TAB>
> > jk-refactor-git-switch-completion master`
> > ...
>
> We've discussed that it may be a good idea to make sure that "switch
> -c", "checkout -b" and "switch/checkout --orphan" complete the new
> branch name the same way, but haven't done anything else. I'd very
> much appreciate to see the patches reviewed by those involved more
> in the completion script, before we decide to merge the topic to
> 'next'.
It is my intention to extend this series with a notion of reading the
previous word on the command line, and using that to determine if we
are completing an argument for an option.
Thanks,
Jake
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2020-05-01 21:53 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-25 2:20 [PATCH 00/11] refactor git switch completion Jacob Keller
2020-04-25 2:20 ` [PATCH 01/11] completion: add some simple test cases for " Jacob Keller
2020-04-25 2:20 ` [PATCH 02/11] completion: add test showing subpar " Jacob Keller
2020-04-25 2:20 ` [PATCH 03/11] completion: add test highlighting subpar git switch --track completion Jacob Keller
2020-04-25 2:20 ` [PATCH 04/11] completion: add tests showing lack of support for git switch -c/-C Jacob Keller
2020-04-25 2:20 ` [PATCH 05/11] completion: add test showing subpar completion for git switch --orphan Jacob Keller
2020-04-27 23:34 ` Junio C Hamano
2020-04-28 2:12 ` Jacob Keller
2020-04-28 2:20 ` Jacob Keller
2020-04-28 16:24 ` Junio C Hamano
2020-04-28 17:32 ` Jacob Keller
2020-04-28 18:10 ` Junio C Hamano
2020-04-28 18:45 ` Jacob Keller
2020-04-28 19:16 ` Junio C Hamano
2020-04-28 20:41 ` Jacob Keller
2020-04-28 20:57 ` Junio C Hamano
2020-04-25 2:20 ` [PATCH 05/11] completion: remove " Jacob Keller
2020-04-25 2:20 ` [PATCH 06/11] completion: rename --track option of __git_complete_refs Jacob Keller
2020-04-25 2:20 ` [PATCH 07/11] completion: extract function __git_dwim_remote_heads Jacob Keller
2020-04-25 2:20 ` [PATCH 08/11] completion: perform DWIM logic directly in __git_complete_refs Jacob Keller
2020-04-25 2:20 ` [PATCH 09/11] completion: fix completion for git switch with no options Jacob Keller
2020-04-25 2:20 ` [PATCH 10/11] completion: recognize -c/-C when completing for git switch Jacob Keller
2020-04-25 2:20 ` [PATCH 11/11] completion: complete remote branches for git switch --track Jacob Keller
2020-04-25 22:14 ` [PATCH 00/11] refactor git switch completion Jacob Keller
2020-04-30 22:56 ` Junio C Hamano
2020-05-01 21:53 ` Jacob Keller
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).