From: Lessley Dennington via GitGitGadget <hidden> Date: 2021-12-30 00:32:37
This series updates custom tab completion for the sparse-checkout command.
Specifically, it corrects the following issues with the current method:
1. git sparse-checkout <TAB> results in an incomplete list of subcommands
(it is missing reapply and add).
2. git sparse-checkout --<TAB> does not complete the help option.
3. Options for subcommands are not tab-completable.
4. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> show
both file names and directory names.
The first commit in this series is a set of failing tests that highlight
each of the above issues. The next commit updates the _git_sparse_checkout
method in git-completion.bash to enable each of these tests to pass.
Thanks, Lessley
Lessley Dennington (2):
sparse-checkout: custom tab completion tests
sparse-checkout: custom tab completion
contrib/completion/git-completion.bash | 34 +++++++-----
t/t9902-completion.sh | 74 ++++++++++++++++++++++++++
2 files changed, 96 insertions(+), 12 deletions(-)
base-commit: 2ae0a9cb8298185a94e5998086f380a355dd8907
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1108%2Fldennington%2Fsparse-checkout-bash-completion-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1108/ldennington/sparse-checkout-bash-completion-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1108
--
gitgitgadget
From: Lessley Dennington via GitGitGadget <hidden> Date: 2021-12-30 00:32:38
From: Lessley Dennington <redacted>
Fix custom tab completion for sparse-checkout command. This will ensure:
1. The full list of subcommands is provided when users enter git
sparse-checkout <TAB>.
2. The --help option is tab-completable.
3. Subcommand options are tab-completable.
4. A list of directories (but not files) is provided when users enter git
sparse-checkout add <TAB> or git sparse-checkout set <TAB>.
Failing tests that were added in the previous commit to verify these
scenarios are now passing with these updates.
Signed-off-by: Lessley Dennington <redacted>
---
contrib/completion/git-completion.bash | 34 +++++++++++++++++---------
t/t9902-completion.sh | 8 +++---
2 files changed, 26 insertions(+), 16 deletions(-)
From: Lessley Dennington via GitGitGadget <hidden> Date: 2021-12-30 00:32:38
From: Lessley Dennington <redacted>
Add tests for missing/incorrect components of custom tab completion for the
sparse-checkout command. These tests specifically highlight the following:
1. git sparse-checkout <TAB> results in an incomplete list of subcommands
(it is missing reapply and add).
2. git sparse-checkout --<TAB> does not complete the help option.
3. Options for subcommands are not tab-completable.
4. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> show
both file names and directory names.
Although these tests currently fail, they will succeed with the
sparse-checkout modifications in git-completion.bash in the next commit in
this series.
Signed-off-by: Lessley Dennington <redacted>
---
t/t9902-completion.sh | 74 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 74 insertions(+)
On 12/29/2021 7:32 PM, Lessley Dennington via GitGitGadget wrote:
From: Lessley Dennington <redacted>
Add tests for missing/incorrect components of custom tab completion for the
sparse-checkout command. These tests specifically highlight the following:
1. git sparse-checkout <TAB> results in an incomplete list of subcommands
(it is missing reapply and add).
2. git sparse-checkout --<TAB> does not complete the help option.
3. Options for subcommands are not tab-completable.
4. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> show
both file names and directory names.
Thanks for these. I've never looked at this test script before, but
I can surmise how it works from your tests.
Although these tests currently fail, they will succeed with the
sparse-checkout modifications in git-completion.bash in the next commit in
this series.
+test_expect_failure 'sparse-checkout completes subcommand options' '
+ test_completion "git sparse-checkout init --" <<-\EOF &&
+ --cone Z
+ --sparse-index Z
+ --no-sparse-index Z
+ EOF
+
+ test_completion "git sparse-checkout set --" <<-\EOF &&
+ --stdin Z
In en/sparse-checkout-set, the 'set' subcommand learns the options
for init, including --cone, --sparse-index, and --no-sparse-index.
I think technically, --no-cone is in there, too.
Further, the 'reapply' subcommand learns these options in that
series and I see you do not include that subcommand in this test.
You might want to rebase onto en/sparse-checkout-set and adjust
these tests for the new options (plus change the next patch that
implements the completion).
I'm guessing that the implementation actually requires this, but
I'll take a look in the next patch. It might just be slow to expand
the full list of directories in the sparse index case.
+ git -C sparse-checkout sparse-checkout init --cone &&
+ git -C sparse-checkout sparse-checkout set folder1/0 folder3 &&
+
+ # test tab completion
+ (
+ cd sparse-checkout &&
+ test_completion "git sparse-checkout set f" <<-\EOF
+ folder1 Z
+ folder1/0 Z
+ folder1/0/1 Z
+ folder2 Z
+ folder2/0 Z
+ folder3 Z
This tab-completion doing a full directory walk seems like it could
be expensive for a large repository, but I suppose it is the only way
to allow the following sequence:
fol<tab> -> folder
folder1/<tab> -> folder1/0
folder1/0/<tab> -> folder1/0/1
(Hopefully that makes sense.)
It would be more efficient to only go one level deep at a time, but
that might not be possible with the tab completion mechanisms.
+ EOF
+ ) &&
+
+ (
+ cd sparse-checkout/folder1 &&
+ test_completion "git sparse-checkout add " <<-\EOF
+ ./ Z
+ 0 Z
+ 0/1 Z
+ EOF
I do like seeing this test within a directory. Thanks!
-Stolee
On 12/29/2021 7:32 PM, Lessley Dennington via GitGitGadget wrote:
quoted hunk
From: Lessley Dennington <redacted>
Fix custom tab completion for sparse-checkout command. This will ensure:
1. The full list of subcommands is provided when users enter git
sparse-checkout <TAB>.
2. The --help option is tab-completable.
3. Subcommand options are tab-completable.
4. A list of directories (but not files) is provided when users enter git
sparse-checkout add <TAB> or git sparse-checkout set <TAB>.
Failing tests that were added in the previous commit to verify these
scenarios are now passing with these updates.
Signed-off-by: Lessley Dennington <redacted>
---
contrib/completion/git-completion.bash | 34 +++++++++++++++++---------
t/t9902-completion.sh | 8 +++---
2 files changed, 26 insertions(+), 16 deletions(-)
@@ -2986,24 +2986,34 @@ _git_show_branch ()__git_complete_revlist}+__git_sparse_checkout_init_opts="--cone --sparse-index --no-sparse-index"+ _git_sparse_checkout(){-localsubcommands="list init set disable"+localsubcommands="list init set disable add reapply"localsubcommand="$(__git_find_on_cmdline"$subcommands")"+if[-z"$subcommand"];then-__gitcomp"$subcommands"-return+case"$cur"in+--*)+__gitcomp"--help"+;;+*)+__gitcomp"$subcommands"+;;+esac
This part fixes the --<tab> completion. I suppose if someone
did "-<tab>" then nothing would show up?
fi
- case "$subcommand,$cur" in
- init,--*)
- __gitcomp "--cone"
- ;;
- set,--*)
- __gitcomp "--stdin"
- ;;
- *)
- ;;
+ case "$prev" in
+ init)
+ __gitcomp "$__git_sparse_checkout_init_opts"
+ ;;
+ add|set)
+ __gitcomp "--stdin"
+ __gitcomp "$(git ls-tree -d -r HEAD --name-only)"> + ;;
With the thinking of rebasing onto en/sparse-checkout-set, this
could possibly be rearranged so the add|set) cases pass-through
into the init) and reapply) cases (skip the ;; between) to save
some duplication. Or, it is possible that doesn't work, but it
might be worth a try.
Also, since you are using 'git ls-tree' and not 'git ls-files',
the sparse index will not have an effect on the performance.
There will be some corner cases where a directory exists in one
of HEAD or the index but not the other. That's probably still
the right way to go since 'git ls-files' doesn't have a way to
only list directories. It just means that you probably don't
need to explicitly disable the sparse index in your test.
Thanks,
-Stolee
+test_expect_failure 'sparse-checkout completes subcommand options' '
+ test_completion "git sparse-checkout init --" <<-\EOF &&
+ --cone Z
+ --sparse-index Z
+ --no-sparse-index Z
+ EOF
+
+ test_completion "git sparse-checkout set --" <<-\EOF &&
+ --stdin Z
In en/sparse-checkout-set, the 'set' subcommand learns the options
for init, including --cone, --sparse-index, and --no-sparse-index.
I think technically, --no-cone is in there, too.
Further, the 'reapply' subcommand learns these options in that
series and I see you do not include that subcommand in this test.
You might want to rebase onto en/sparse-checkout-set and adjust
these tests for the new options (plus change the next patch that
implements the completion).
Ah great point - I was going off of the sparse-checkout docs and
forgot about Elijah's changes. I will do the rebase and make your
suggested corrections in v2.
I'm guessing that the implementation actually requires this, but
I'll take a look in the next patch. It might just be slow to expand
the full list of directories in the sparse index case.
I'll plan to remove in v2 per your comments in [1].>> + # test tab completion
quoted
+ (
+ cd sparse-checkout &&
+ test_completion "git sparse-checkout set f" <<-\EOF
+ folder1 Z
+ folder1/0 Z
+ folder1/0/1 Z
+ folder2 Z
+ folder2/0 Z
+ folder3 Z
This tab-completion doing a full directory walk seems like it could
be expensive for a large repository, but I suppose it is the only way
to allow the following sequence:
fol<tab> -> folder
folder1/<tab> -> folder1/0
folder1/0/<tab> -> folder1/0/1
(Hopefully that makes sense.)
Yes, it does.
It would be more efficient to only go one level deep at a time, but
that might not be possible with the tab completion mechanisms.
+__git_sparse_checkout_init_opts="--cone --sparse-index --no-sparse-index"
+
_git_sparse_checkout ()
{
- local subcommands="list init set disable"
+ local subcommands="list init set disable add reapply"
local subcommand="$(__git_find_on_cmdline "$subcommands")"
+
if [ -z "$subcommand" ]; then
- __gitcomp "$subcommands"
- return
+ case "$cur" in
+ --*)
+ __gitcomp "--help"
+ ;;
+ *)
+ __gitcomp "$subcommands"
+ ;;
+ esac
This part fixes the --<tab> completion. I suppose if someone
did "-<tab>" then nothing would show up?
It actually shows a list of files that contain -- (if any exist).
quoted
fi
- case "$subcommand,$cur" in
- init,--*)
- __gitcomp "--cone"
- ;;
- set,--*)
- __gitcomp "--stdin"
- ;;
- *)
- ;;
+ case "$prev" in
+ init)
+ __gitcomp "$__git_sparse_checkout_init_opts"
+ ;;
+ add|set)
+ __gitcomp "--stdin"
+ __gitcomp "$(git ls-tree -d -r HEAD --name-only)"> + ;;
With the thinking of rebasing onto en/sparse-checkout-set, this
could possibly be rearranged so the add|set) cases pass-through
into the init) and reapply) cases (skip the ;; between) to save
some duplication. Or, it is possible that doesn't work, but it
might be worth a try.
Thanks, I'll give this a go!
Also, since you are using 'git ls-tree' and not 'git ls-files',
the sparse index will not have an effect on the performance.
There will be some corner cases where a directory exists in one
of HEAD or the index but not the other. That's probably still
the right way to go since 'git ls-files' doesn't have a way to
only list directories. It just means that you probably don't
need to explicitly disable the sparse index in your test.
+ (
+ cd sparse-checkout &&
+ test_completion "git sparse-checkout set f" <<-\EOF
+ folder1 Z
+ folder1/0 Z
+ folder1/0/1 Z
+ folder2 Z
+ folder2/0 Z
+ folder3 Z
This tab-completion doing a full directory walk seems like it could
be expensive for a large repository, but I suppose it is the only way
to allow the following sequence:
fol<tab> -> folder
folder1/<tab> -> folder1/0
folder1/0/<tab> -> folder1/0/1
(Hopefully that makes sense.)
Yes, it does.
quoted
It would be more efficient to only go one level deep at a time, but
that might not be possible with the tab completion mechanisms.
When you say one level deep, do you mean that from the sparse-checkout
directory, tab completion would only show the following?
folder1
folder2
folder3
That's what I mean by one level deep at a time, but I also am not
sure that that is the correct functionality. I would leave the full
expansion as you have now as the design.
Thanks,
-Stolee
From: Lessley Dennington via GitGitGadget <hidden> Date: 2021-12-30 19:27:00
From: Lessley Dennington <redacted>
Add tests for missing/incorrect components of custom tab completion for the
sparse-checkout command. These tests specifically highlight the following:
1. git sparse-checkout <TAB> results in an incomplete list of subcommands
(it is missing reapply and add).
2. git sparse-checkout --<TAB> does not complete the help option.
3. Options for subcommands are not tab-completable.
4. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> show
both file names and directory names.
Although these tests currently fail, they will succeed with the
sparse-checkout modifications in git-completion.bash in the next commit in
this series.
Signed-off-by: Lessley Dennington <redacted>
---
t/t9902-completion.sh | 85 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 85 insertions(+)
From: Lessley Dennington via GitGitGadget <hidden> Date: 2021-12-30 19:27:03
From: Lessley Dennington <redacted>
Fix custom tab completion for sparse-checkout command. This will ensure:
1. The full list of subcommands is provided when users enter git
sparse-checkout <TAB>.
2. The --help option is tab-completable.
3. Subcommand options are tab-completable.
4. A list of directories (but not files) is provided when users enter git
sparse-checkout add <TAB> or git sparse-checkout set <TAB>.
Failing tests that were added in the previous commit to verify these
scenarios are now passing with these updates.
Signed-off-by: Lessley Dennington <redacted>
---
contrib/completion/git-completion.bash | 38 ++++++++++++++++++--------
t/t9902-completion.sh | 8 +++---
2 files changed, 30 insertions(+), 16 deletions(-)
On Fri, Dec 31, 2021 at 2:30 AM Derrick Stolee [off-list ref] wrote:
On 12/30/2021 11:19 AM, Lessley Dennington wrote:
quoted
On 12/30/21 7:43 AM, Derrick Stolee wrote:
quoted
quoted
quoted
+ (
+ cd sparse-checkout &&
+ test_completion "git sparse-checkout set f" <<-\EOF
+ folder1 Z
+ folder1/0 Z
+ folder1/0/1 Z
+ folder2 Z
+ folder2/0 Z
+ folder3 Z
This tab-completion doing a full directory walk seems like it could
be expensive for a large repository, but I suppose it is the only way
to allow the following sequence:
fol<tab> -> folder
folder1/<tab> -> folder1/0
folder1/0/<tab> -> folder1/0/1
(Hopefully that makes sense.)
Yes, it does.
quoted
It would be more efficient to only go one level deep at a time, but
that might not be possible with the tab completion mechanisms.
When you say one level deep, do you mean that from the sparse-checkout
directory, tab completion would only show the following?
folder1
folder2
folder3
That's what I mean by one level deep at a time, but I also am not
sure that that is the correct functionality. I would leave the full
expansion as you have now as the design.
I think one level at a time is better and is the optimal design. By
way of comparison, note that if I do the following:
mkdir tmp
cd tmp
mkdir -p a/b/c/d
touch a/b/c/d/e
cd <TAB>
I do not see a/b/c/d as the completion, I only get "a/" as the
completion. If I hit tab again, then I get "a/b/". Tab again, and I
get "a/b/c/".
I don't think normal tab completion recursively checks directories, so
it'd be better for us to not do so with git either. However, if it's
hard to avoid automatically completing just a directory at a time,
then I think a fair first cut is completing to full depths, but call
it out as something we'd like to fix in the commit message.
On Fri, Dec 31, 2021 at 2:32 AM Lessley Dennington via GitGitGadget
[off-list ref] wrote:
From: Lessley Dennington <redacted>
Add tests for missing/incorrect components of custom tab completion for the
sparse-checkout command. These tests specifically highlight the following:
1. git sparse-checkout <TAB> results in an incomplete list of subcommands
(it is missing reapply and add).
2. git sparse-checkout --<TAB> does not complete the help option.
3. Options for subcommands are not tab-completable.
4. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> show
both file names and directory names.
Two thoughts on this last item:
I completely agree that only directories should be completed in cone
mode, but completing on both might technically be considered correct
behavior for non-cone mode. However, even in non-cone mode, I kind of
like the idea of encouraging people to sparsify only on directories so
I'm totally fine with us only tab-completing directories. (I have a
bit of a disdain for non-cone mode, though, so my desire to deprecate
it might be showing through. At the very least, I'm still thinking we
should make cone mode the default in each of `sparse-checkout init`,
`sparse-checkout set`, and `clone --sparse`[1])
[1] https://lore.kernel.org/git/6e09ab19-7ffb-e58e-7b08-6e560b421c06@gmail.com/
Second, and this item is unrelated to your series but your comment
made me realize it....sparse-checkout unfortunately ignores prefix and
creates a bad .git/info/sparse-checkout file. For example:
$ git init -b main tmp
$ cd tmp
$ mkdir -p a/b/c
$ touch a/b/c/d a/b/c/e
$ git add a/
$ git commit -m "initial"
$ cd a/ # Not at the toplevel anymore
$ git sparse-checkout set --cone b/c # So we expect that a/b/c will
be the specified sparsity path
$ git -C .. sparse-checkout list
b/c
$ cat ../.git/info/sparse-checkout
/*
!/*/
/b/
!/b/*/
/b/c/
$ pwd -P
pwd: error retrieving current directory: getcwd: cannot access parent
directories: No such file or directory
I think the loss of the current working directory will be fixed by the
en/keep-cwd directory (currently in next and marked for merging to
master), but the fact that the wrong paths end up in the
sparse-checkout file is unfortunate. It basically means that the
`set` and `add` subcommands of `sparse-checkout` can only be safely
run from the toplevel directory.
quoted hunk
Although these tests currently fail, they will succeed with the
sparse-checkout modifications in git-completion.bash in the next commit in
this series.
Signed-off-by: Lessley Dennington <redacted>
---
t/t9902-completion.sh | 85 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 85 insertions(+)
@@ -1447,6 +1447,91 @@ test_expect_success 'git checkout - with --detach, complete only references' 'EOF'+test_expect_failure'sparse-checkout completes subcommands''+test_completion"git sparse-checkout "<<-\EOF+listZ+initZ+setZ+addZ+reapplyZ+disableZ+EOF+'++test_expect_failure'sparse-checkout completes options''+test_completion"git sparse-checkout --"<<-\EOF+--helpZ+EOF+'++test_expect_failure'sparse-checkout completes subcommand options''+test_completion"git sparse-checkout init --"<<-\EOF&&+--coneZ+--no-coneZ+--sparse-indexZ+--no-sparse-indexZ+EOF++test_completion"git sparse-checkout set --"<<-\EOF&&+--coneZ+--no-coneZ+--sparse-indexZ+--no-sparse-indexZ+--stdinZ+EOF++test_completion"git sparse-checkout reapply --"<<-\EOF&&+--coneZ+--no-coneZ+--sparse-indexZ+--no-sparse-indexZ+EOF++test_completion"git sparse-checkout add --"<<-\EOF+--stdinZ+EOF+'++test_expect_failure'sparse-checkout completes directory names''+# set up sparse-checkout repo+gitinitsparse-checkout&&+(+cdsparse-checkout&&+mkdir-pfolder1/0/1folder2/0folder3&&+touchfolder1/0/1/t.txt&&+touchfolder2/0/t.txt&&+touchfolder3/t.txt&&+gitadd.&&+gitcommit-am"Initial commit"+)&&++# initialize sparse-checkout definitions+git-Csparse-checkoutsparse-checkoutinit--cone&&+git-Csparse-checkoutsparse-checkoutsetfolder1/0folder3&&++# test tab completion+(+cdsparse-checkout&&+test_completion"git sparse-checkout set f"<<-\EOF+folder1Z+folder1/0Z+folder1/0/1Z+folder2Z+folder2/0Z+folder3Z+EOF+)&&++(+cdsparse-checkout/folder1&&+test_completion"git sparse-checkout add "<<-\EOF+./Z+0Z+0/1Z+EOF+)+'+ test_expect_success'git switch - with -d, complete all references''test_completion"git switch -d "<<-\EOFHEADZ--
gitgitgadget
Patch looks okay to me, but we might want to add some kind of wording
around the directories-only decision and cone vs. non-cone mode to the
commit message.
On Fri, Dec 31, 2021 at 2:33 AM Lessley Dennington via GitGitGadget
[off-list ref] wrote:
quoted hunk
From: Lessley Dennington <redacted>
Fix custom tab completion for sparse-checkout command. This will ensure:
1. The full list of subcommands is provided when users enter git
sparse-checkout <TAB>.
2. The --help option is tab-completable.
3. Subcommand options are tab-completable.
4. A list of directories (but not files) is provided when users enter git
sparse-checkout add <TAB> or git sparse-checkout set <TAB>.
Failing tests that were added in the previous commit to verify these
scenarios are now passing with these updates.
Signed-off-by: Lessley Dennington <redacted>
---
contrib/completion/git-completion.bash | 38 ++++++++++++++++++--------
t/t9902-completion.sh | 8 +++---
2 files changed, 30 insertions(+), 16 deletions(-)
I was going to make a simple suggestion for making it just complete
one additional level at a time and leaving out the -r, and then tried
it out and found out it wasn't simple. I got something working,
eventually, but it's slightly ugly...so it probably belongs in a
separate patch anyway. If you're curious, it's basically replacing
the second __gitcomp... call for each of set and add with
`__gitcomp_directories`, after first defining:
__gitcomp_directories ()
{
local _tmp_dir _tmp_completions
# Get the directory of the current token; this differs from dirname
# in that it keeps up to the final trailing slash. If no slash found
# that's fine too.
[[ "$cur" =~ .*/ ]]
_tmp_dir=$BASH_REMATCH
# Find possible directory completions, adding trailing '/' characters
_tmp_completions="$(git ls-tree -d --name-only HEAD $_tmp_dir |
sed -e s%$%/%)"
if [[ -n "$_tmp_completions" ]]; then
# There were some directory completions, so find ones that
# start with "$cur", the current token, and put those in COMPREPLY
local i=0 c IFS=$' \t\n'
for c in $_tmp_completions; do
if [[ $c == "$cur"* ]]; then
COMPREPLY+=("$c")
fi
done
elif [[ "$cur" =~ /$ ]]; then
# No possible further completions any deeper, so assume we're at
# a leaf directory and just consider it complete
__gitcomp_direct_append "$cur "
fi
}
But I don't think that needs to be part of this series; I can submit
it later and hopefully get a completion expert to point out
better/cleaner ways of what I've done above.
On Fri, Dec 31, 2021 at 2:30 AM Derrick Stolee [off-list ref] wrote:
quoted
On 12/30/2021 11:19 AM, Lessley Dennington wrote:
quoted
On 12/30/21 7:43 AM, Derrick Stolee wrote:
quoted
quoted
quoted
+ (
+ cd sparse-checkout &&
+ test_completion "git sparse-checkout set f" <<-\EOF
+ folder1 Z
+ folder1/0 Z
+ folder1/0/1 Z
+ folder2 Z
+ folder2/0 Z
+ folder3 Z
This tab-completion doing a full directory walk seems like it could
be expensive for a large repository, but I suppose it is the only way
to allow the following sequence:
fol<tab> -> folder
folder1/<tab> -> folder1/0
folder1/0/<tab> -> folder1/0/1
(Hopefully that makes sense.)
Yes, it does.
quoted
It would be more efficient to only go one level deep at a time, but
that might not be possible with the tab completion mechanisms.
When you say one level deep, do you mean that from the sparse-checkout
directory, tab completion would only show the following?
folder1
folder2
folder3
That's what I mean by one level deep at a time, but I also am not
sure that that is the correct functionality. I would leave the full
expansion as you have now as the design.
I think one level at a time is better and is the optimal design. By
way of comparison, note that if I do the following:
mkdir tmp
cd tmp
mkdir -p a/b/c/d
touch a/b/c/d/e
cd <TAB>
I do not see a/b/c/d as the completion, I only get "a/" as the
completion. If I hit tab again, then I get "a/b/". Tab again, and I
get "a/b/c/".
I don't think normal tab completion recursively checks directories, so
it'd be better for us to not do so with git either. However, if it's
hard to avoid automatically completing just a directory at a time,
then I think a fair first cut is completing to full depths, but call
it out as something we'd like to fix in the commit message.
On Fri, Dec 31, 2021 at 2:32 AM Lessley Dennington via GitGitGadget
[off-list ref] wrote:
quoted
From: Lessley Dennington <redacted>
Add tests for missing/incorrect components of custom tab completion for the
sparse-checkout command. These tests specifically highlight the following:
1. git sparse-checkout <TAB> results in an incomplete list of subcommands
(it is missing reapply and add).
2. git sparse-checkout --<TAB> does not complete the help option.
3. Options for subcommands are not tab-completable.
4. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> show
both file names and directory names.
Two thoughts on this last item:
I completely agree that only directories should be completed in cone
mode, but completing on both might technically be considered correct
behavior for non-cone mode. However, even in non-cone mode, I kind of
like the idea of encouraging people to sparsify only on directories so
I'm totally fine with us only tab-completing directories. (I have a
bit of a disdain for non-cone mode, though, so my desire to deprecate
it might be showing through. At the very least, I'm still thinking we
should make cone mode the default in each of `sparse-checkout init`,
`sparse-checkout set`, and `clone --sparse`[1])
[1] https://lore.kernel.org/git/6e09ab19-7ffb-e58e-7b08-6e560b421c06@gmail.com/
I'm still supportive of this 😊. I'll chat with others who are heavily
involved with sparse-checkout to make sure I have buy-in. If so, I can
pick it up as my next work item.
Second, and this item is unrelated to your series but your comment
made me realize it....sparse-checkout unfortunately ignores prefix and
creates a bad .git/info/sparse-checkout file. For example:
$ git init -b main tmp
$ cd tmp
$ mkdir -p a/b/c
$ touch a/b/c/d a/b/c/e
$ git add a/
$ git commit -m "initial"
$ cd a/ # Not at the toplevel anymore
$ git sparse-checkout set --cone b/c # So we expect that a/b/c will
be the specified sparsity path
$ git -C .. sparse-checkout list
b/c
$ cat ../.git/info/sparse-checkout
/*
!/*/
/b/
!/b/*/
/b/c/
$ pwd -P
pwd: error retrieving current directory: getcwd: cannot access parent
directories: No such file or directory
I think the loss of the current working directory will be fixed by the
en/keep-cwd directory (currently in next and marked for merging to
master), but the fact that the wrong paths end up in the
sparse-checkout file is unfortunate. It basically means that the
`set` and `add` subcommands of `sparse-checkout` can only be safely
run from the toplevel directory.
quoted
Although these tests currently fail, they will succeed with the
sparse-checkout modifications in git-completion.bash in the next commit in
this series.
Signed-off-by: Lessley Dennington <redacted>
---
t/t9902-completion.sh | 85 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 85 insertions(+)
@@ -1447,6 +1447,91 @@ test_expect_success 'git checkout - with --detach, complete only references' 'EOF'+test_expect_failure'sparse-checkout completes subcommands''+test_completion"git sparse-checkout "<<-\EOF+listZ+initZ+setZ+addZ+reapplyZ+disableZ+EOF+'++test_expect_failure'sparse-checkout completes options''+test_completion"git sparse-checkout --"<<-\EOF+--helpZ+EOF+'++test_expect_failure'sparse-checkout completes subcommand options''+test_completion"git sparse-checkout init --"<<-\EOF&&+--coneZ+--no-coneZ+--sparse-indexZ+--no-sparse-indexZ+EOF++test_completion"git sparse-checkout set --"<<-\EOF&&+--coneZ+--no-coneZ+--sparse-indexZ+--no-sparse-indexZ+--stdinZ+EOF++test_completion"git sparse-checkout reapply --"<<-\EOF&&+--coneZ+--no-coneZ+--sparse-indexZ+--no-sparse-indexZ+EOF++test_completion"git sparse-checkout add --"<<-\EOF+--stdinZ+EOF+'++test_expect_failure'sparse-checkout completes directory names''+# set up sparse-checkout repo+gitinitsparse-checkout&&+(+cdsparse-checkout&&+mkdir-pfolder1/0/1folder2/0folder3&&+touchfolder1/0/1/t.txt&&+touchfolder2/0/t.txt&&+touchfolder3/t.txt&&+gitadd.&&+gitcommit-am"Initial commit"+)&&++# initialize sparse-checkout definitions+git-Csparse-checkoutsparse-checkoutinit--cone&&+git-Csparse-checkoutsparse-checkoutsetfolder1/0folder3&&++# test tab completion+(+cdsparse-checkout&&+test_completion"git sparse-checkout set f"<<-\EOF+folder1Z+folder1/0Z+folder1/0/1Z+folder2Z+folder2/0Z+folder3Z+EOF+)&&++(+cdsparse-checkout/folder1&&+test_completion"git sparse-checkout add "<<-\EOF+./Z+0Z+0/1Z+EOF+)+'+test_expect_success'git switch - with -d, complete all references''test_completion"git switch -d "<<-\EOFHEADZ--
gitgitgadget
Patch looks okay to me, but we might want to add some kind of wording
around the directories-only decision and cone vs. non-cone mode to the
commit message.
On Fri, Dec 31, 2021 at 2:33 AM Lessley Dennington via GitGitGadget
[off-list ref] wrote:
quoted
From: Lessley Dennington <redacted>
Fix custom tab completion for sparse-checkout command. This will ensure:
1. The full list of subcommands is provided when users enter git
sparse-checkout <TAB>.
2. The --help option is tab-completable.
3. Subcommand options are tab-completable.
4. A list of directories (but not files) is provided when users enter git
sparse-checkout add <TAB> or git sparse-checkout set <TAB>.
Failing tests that were added in the previous commit to verify these
scenarios are now passing with these updates.
Signed-off-by: Lessley Dennington <redacted>
---
contrib/completion/git-completion.bash | 38 ++++++++++++++++++--------
t/t9902-completion.sh | 8 +++---
2 files changed, 30 insertions(+), 16 deletions(-)
I was going to make a simple suggestion for making it just complete
one additional level at a time and leaving out the -r, and then tried
it out and found out it wasn't simple. I got something working,
eventually, but it's slightly ugly...so it probably belongs in a
separate patch anyway. If you're curious, it's basically replacing
the second __gitcomp... call for each of set and add with
`__gitcomp_directories`, after first defining:
__gitcomp_directories ()
{
local _tmp_dir _tmp_completions
# Get the directory of the current token; this differs from dirname
# in that it keeps up to the final trailing slash. If no slash found
# that's fine too.
[[ "$cur" =~ .*/ ]]
_tmp_dir=$BASH_REMATCH
# Find possible directory completions, adding trailing '/' characters
_tmp_completions="$(git ls-tree -d --name-only HEAD $_tmp_dir |
sed -e s%$%/%)"
if [[ -n "$_tmp_completions" ]]; then
# There were some directory completions, so find ones that
# start with "$cur", the current token, and put those in COMPREPLY
local i=0 c IFS=$' \t\n'
for c in $_tmp_completions; do
if [[ $c == "$cur"* ]]; then
COMPREPLY+=("$c")
fi
done
elif [[ "$cur" =~ /$ ]]; then
# No possible further completions any deeper, so assume we're at
# a leaf directory and just consider it complete
__gitcomp_direct_append "$cur "
fi
}
But I don't think that needs to be part of this series; I can submit
it later and hopefully get a completion expert to point out
better/cleaner ways of what I've done above.
I'm admittedly curious about what made this so difficult. I removed the
'-r' and updated my tests to expect only directories at one level, and
they passed. But I imagine I'm being too simplistic.
On Tue, Jan 4, 2022 at 11:41 AM Lessley Dennington
[off-list ref] wrote:
On 12/31/21 4:52 PM, Elijah Newren wrote:
quoted
On Fri, Dec 31, 2021 at 2:33 AM Lessley Dennington via GitGitGadget
[off-list ref] wrote:
quoted
From: Lessley Dennington <redacted>
Fix custom tab completion for sparse-checkout command. This will ensure:
1. The full list of subcommands is provided when users enter git
sparse-checkout <TAB>.
2. The --help option is tab-completable.
3. Subcommand options are tab-completable.
4. A list of directories (but not files) is provided when users enter git
sparse-checkout add <TAB> or git sparse-checkout set <TAB>.
Failing tests that were added in the previous commit to verify these
scenarios are now passing with these updates.
Signed-off-by: Lessley Dennington <redacted>
---
contrib/completion/git-completion.bash | 38 ++++++++++++++++++--------
t/t9902-completion.sh | 8 +++---
2 files changed, 30 insertions(+), 16 deletions(-)
I was going to make a simple suggestion for making it just complete
one additional level at a time and leaving out the -r, and then tried
it out and found out it wasn't simple. I got something working,
eventually, but it's slightly ugly...so it probably belongs in a
separate patch anyway. If you're curious, it's basically replacing
the second __gitcomp... call for each of set and add with
`__gitcomp_directories`, after first defining:
__gitcomp_directories ()
{
local _tmp_dir _tmp_completions
# Get the directory of the current token; this differs from dirname
# in that it keeps up to the final trailing slash. If no slash found
# that's fine too.
[[ "$cur" =~ .*/ ]]
_tmp_dir=$BASH_REMATCH
# Find possible directory completions, adding trailing '/' characters
_tmp_completions="$(git ls-tree -d --name-only HEAD $_tmp_dir |
sed -e s%$%/%)"
if [[ -n "$_tmp_completions" ]]; then
# There were some directory completions, so find ones that
# start with "$cur", the current token, and put those in COMPREPLY
local i=0 c IFS=$' \t\n'
for c in $_tmp_completions; do
if [[ $c == "$cur"* ]]; then
COMPREPLY+=("$c")
fi
done
elif [[ "$cur" =~ /$ ]]; then
# No possible further completions any deeper, so assume we're at
# a leaf directory and just consider it complete
__gitcomp_direct_append "$cur "
fi
}
But I don't think that needs to be part of this series; I can submit
it later and hopefully get a completion expert to point out
better/cleaner ways of what I've done above.
I'm admittedly curious about what made this so difficult. I removed the
'-r' and updated my tests to expect only directories at one level, and
they passed. But I imagine I'm being too simplistic.
I've forgotten some details since last Saturday, but I think the
problem was that doing that would only ever complete toplevel
directories; after completing those you could keep tabbing to get a
deeper directory. First, let's get a comparison point; ignoring
sparse-checkout, I can do:
cd $GIT_CLONE
cd cont<TAB>b<TAB><TAB>
and the ls line will replace those <TAB>s so that I see
ls contrib/buildsystems/Generators
Now, if we just removed the '-r' from your git-completion.bash
changes, and then typed
git sparse-checkout set cont<TAB>b<TAB><TAB>
Then you'd see
git sparse-checkout set contrib
and see 'bin-wrappers', 'block-sha1', and 'builtin' as potential
completions, but not as subdirs of contrib but instead sibling dirs to
contrib. That completion rule wouldn't let you look within contrib/.
My more complicated rule had to avoid calling __gitcomp to avoid
adding spaces so that completions could continue (but should add them
if we have tabbed all the way down and there are no more
subdirectories), had to add trailing '/' characters so that we know
when we have the full directory name to pass along to ls-tree, and
then had to manually do the work that __gitcomp would manually do with
making sure to only provide completions relevant to what has been
typed so far.
On Tue, Jan 4, 2022 at 11:41 AM Lessley Dennington
[off-list ref] wrote:
quoted
On 12/31/21 4:52 PM, Elijah Newren wrote:
quoted
On Fri, Dec 31, 2021 at 2:33 AM Lessley Dennington via GitGitGadget
[off-list ref] wrote:
quoted
From: Lessley Dennington <redacted>
Fix custom tab completion for sparse-checkout command. This will ensure:
1. The full list of subcommands is provided when users enter git
sparse-checkout <TAB>.
2. The --help option is tab-completable.
3. Subcommand options are tab-completable.
4. A list of directories (but not files) is provided when users enter git
sparse-checkout add <TAB> or git sparse-checkout set <TAB>.
Failing tests that were added in the previous commit to verify these
scenarios are now passing with these updates.
Signed-off-by: Lessley Dennington <redacted>
---
contrib/completion/git-completion.bash | 38 ++++++++++++++++++--------
t/t9902-completion.sh | 8 +++---
2 files changed, 30 insertions(+), 16 deletions(-)
I was going to make a simple suggestion for making it just complete
one additional level at a time and leaving out the -r, and then tried
it out and found out it wasn't simple. I got something working,
eventually, but it's slightly ugly...so it probably belongs in a
separate patch anyway. If you're curious, it's basically replacing
the second __gitcomp... call for each of set and add with
`__gitcomp_directories`, after first defining:
__gitcomp_directories ()
{
local _tmp_dir _tmp_completions
# Get the directory of the current token; this differs from dirname
# in that it keeps up to the final trailing slash. If no slash found
# that's fine too.
[[ "$cur" =~ .*/ ]]
_tmp_dir=$BASH_REMATCH
# Find possible directory completions, adding trailing '/' characters
_tmp_completions="$(git ls-tree -d --name-only HEAD $_tmp_dir |
sed -e s%$%/%)"
if [[ -n "$_tmp_completions" ]]; then
# There were some directory completions, so find ones that
# start with "$cur", the current token, and put those in COMPREPLY
local i=0 c IFS=$' \t\n'
for c in $_tmp_completions; do
if [[ $c == "$cur"* ]]; then
COMPREPLY+=("$c")
fi
done
elif [[ "$cur" =~ /$ ]]; then
# No possible further completions any deeper, so assume we're at
# a leaf directory and just consider it complete
__gitcomp_direct_append "$cur "
fi
}
But I don't think that needs to be part of this series; I can submit
it later and hopefully get a completion expert to point out
better/cleaner ways of what I've done above.
I'm admittedly curious about what made this so difficult. I removed the
'-r' and updated my tests to expect only directories at one level, and
they passed. But I imagine I'm being too simplistic.
I've forgotten some details since last Saturday, but I think the
problem was that doing that would only ever complete toplevel
directories; after completing those you could keep tabbing to get a
deeper directory. First, let's get a comparison point; ignoring
sparse-checkout, I can do:
cd $GIT_CLONE
cd cont<TAB>b<TAB><TAB>
and the ls line will replace those <TAB>s so that I see
ls contrib/buildsystems/Generators
Now, if we just removed the '-r' from your git-completion.bash
changes, and then typed
git sparse-checkout set cont<TAB>b<TAB><TAB>
Then you'd see
git sparse-checkout set contrib
and see 'bin-wrappers', 'block-sha1', and 'builtin' as potential
completions, but not as subdirs of contrib but instead sibling dirs to
contrib. That completion rule wouldn't let you look within contrib/.
My more complicated rule had to avoid calling __gitcomp to avoid
adding spaces so that completions could continue (but should add them
if we have tabbed all the way down and there are no more
subdirectories), had to add trailing '/' characters so that we know
when we have the full directory name to pass along to ls-tree, and
then had to manually do the work that __gitcomp would manually do with
making sure to only provide completions relevant to what has been
typed so far.
Ah, I see. Thank you so much for the thorough explanation. I know you said
this series could go through without that update, but I feel like it
should probably be added. Don't want to start off with the wrong behavior.
On Wed, Jan 5, 2022 at 12:20 PM Lessley Dennington
[off-list ref] wrote:
On 1/4/22 2:42 PM, Elijah Newren wrote:
quoted
On Tue, Jan 4, 2022 at 11:41 AM Lessley Dennington
[off-list ref] wrote:
quoted
On 12/31/21 4:52 PM, Elijah Newren wrote:
quoted
On Fri, Dec 31, 2021 at 2:33 AM Lessley Dennington via GitGitGadget
[off-list ref] wrote:
quoted
From: Lessley Dennington <redacted>
Fix custom tab completion for sparse-checkout command. This will ensure:
1. The full list of subcommands is provided when users enter git
sparse-checkout <TAB>.
2. The --help option is tab-completable.
3. Subcommand options are tab-completable.
4. A list of directories (but not files) is provided when users enter git
sparse-checkout add <TAB> or git sparse-checkout set <TAB>.
Failing tests that were added in the previous commit to verify these
scenarios are now passing with these updates.
Signed-off-by: Lessley Dennington <redacted>
---
contrib/completion/git-completion.bash | 38 ++++++++++++++++++--------
t/t9902-completion.sh | 8 +++---
2 files changed, 30 insertions(+), 16 deletions(-)
I was going to make a simple suggestion for making it just complete
one additional level at a time and leaving out the -r, and then tried
it out and found out it wasn't simple. I got something working,
eventually, but it's slightly ugly...so it probably belongs in a
separate patch anyway. If you're curious, it's basically replacing
the second __gitcomp... call for each of set and add with
`__gitcomp_directories`, after first defining:
__gitcomp_directories ()
{
local _tmp_dir _tmp_completions
# Get the directory of the current token; this differs from dirname
# in that it keeps up to the final trailing slash. If no slash found
# that's fine too.
[[ "$cur" =~ .*/ ]]
_tmp_dir=$BASH_REMATCH
# Find possible directory completions, adding trailing '/' characters
_tmp_completions="$(git ls-tree -d --name-only HEAD $_tmp_dir |
sed -e s%$%/%)"
if [[ -n "$_tmp_completions" ]]; then
# There were some directory completions, so find ones that
# start with "$cur", the current token, and put those in COMPREPLY
local i=0 c IFS=$' \t\n'
for c in $_tmp_completions; do
if [[ $c == "$cur"* ]]; then
COMPREPLY+=("$c")
fi
done
elif [[ "$cur" =~ /$ ]]; then
# No possible further completions any deeper, so assume we're at
# a leaf directory and just consider it complete
__gitcomp_direct_append "$cur "
fi
}
But I don't think that needs to be part of this series; I can submit
it later and hopefully get a completion expert to point out
better/cleaner ways of what I've done above.
I'm admittedly curious about what made this so difficult. I removed the
'-r' and updated my tests to expect only directories at one level, and
they passed. But I imagine I'm being too simplistic.
I've forgotten some details since last Saturday, but I think the
problem was that doing that would only ever complete toplevel
directories; after completing those you could keep tabbing to get a
deeper directory. First, let's get a comparison point; ignoring
sparse-checkout, I can do:
cd $GIT_CLONE
cd cont<TAB>b<TAB><TAB>
and the ls line will replace those <TAB>s so that I see
ls contrib/buildsystems/Generators
Now, if we just removed the '-r' from your git-completion.bash
changes, and then typed
git sparse-checkout set cont<TAB>b<TAB><TAB>
Then you'd see
git sparse-checkout set contrib
and see 'bin-wrappers', 'block-sha1', and 'builtin' as potential
completions, but not as subdirs of contrib but instead sibling dirs to
contrib. That completion rule wouldn't let you look within contrib/.
My more complicated rule had to avoid calling __gitcomp to avoid
adding spaces so that completions could continue (but should add them
if we have tabbed all the way down and there are no more
subdirectories), had to add trailing '/' characters so that we know
when we have the full directory name to pass along to ls-tree, and
then had to manually do the work that __gitcomp would manually do with
making sure to only provide completions relevant to what has been
typed so far.
Ah, I see. Thank you so much for the thorough explanation. I know you said
this series could go through without that update, but I feel like it
should probably be added. Don't want to start off with the wrong behavior.
The wrong behavior only occurs if you drop the `-r` from your patch.
If you keep the `-r`, as in your patch submission, you get the right
behavior, it just might be a bit slow. The only reason I investigated
dropping the `-r` and then following up with all the extra workarounds
needed when the `-r` was dropped was because some repositories may be
big enough that immediately recursing trees down to the lowest depths
may be expensive.
For example, in linux.git (very small compared to the Microsoft
repos): `time git ls-tree -rd HEAD >/dev/null` was 0.785s (cold cache;
a mere 0.084s on second run). If repositories get much bigger than
that, folks might not like the slowness of using `-r`. But I think
what you have is fine as a first cut.
That said, if you want to add a patch to this series that switches
your straightforward implementation to my much more complex but faster
alternative, that's fine too.
From: Lessley Dennington via GitGitGadget <hidden> Date: 2022-01-10 18:59:59
This series is based on en/sparse-checkout-set. It updates custom tab
completion for the sparse-checkout command. Specifically, it corrects the
following issues with the current method:
1. git sparse-checkout <TAB> results in an incomplete list of subcommands
(it is missing reapply and add).
2. git sparse-checkout --<TAB> does not complete the help option.
3. Options for subcommands are not tab-completable.
4. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> show
both file names and directory names.
The first commit in this series is a set of failing tests that highlight
each of the above issues. The next commit updates the _git_sparse_checkout
method in git-completion.bash to enable each of these tests to pass. The
final commit modifies the original implementation (which recursively printed
prospective directories for completion) to only print directories at the
current level for improved performance.
Changes since V2
================
* Change use of $prev to $subcommand in _git_sparse_checkout() method in
git-completion.bash.
* State explicitly that directory completion applies in both cone and
non-cone mode in 'sparse-checkout: custom tab completion' commit
* Add new patch with __gitcomp_directories method to improve performance by
only outputting directories at the current level.
Changes since V1
================
* Rebase onto en/sparse-checkout-set.
* Add subcommand options (including --no-cone) for set and reapply.
* Extend 'sparse-checkout completes subcommand options' test to validate
new set/reapply subcommand options.
* No longer set index.sparse to false in 'sparse-checkout completes
directory names' test.
Thanks, Lessley
Lessley Dennington (3):
sparse-checkout: custom tab completion tests
sparse-checkout: custom tab completion
sparse-checkout: limit tab completion to a single level
contrib/completion/git-completion.bash | 68 +++++++++++++++----
t/t9902-completion.sh | 94 ++++++++++++++++++++++++++
2 files changed, 150 insertions(+), 12 deletions(-)
base-commit: dfac9b609f86cd4f6ce896df9e1172d2a02cde48
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1108%2Fldennington%2Fsparse-checkout-bash-completion-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1108/ldennington/sparse-checkout-bash-completion-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1108
Range-diff vs v2:
1: 955fcab0052 = 1: bbc2d21e1d1 sparse-checkout: custom tab completion tests
2: cecf501e076 ! 2: 256e5f034c6 sparse-checkout: custom tab completion
@@ Commit message
2. The --help option is tab-completable.
3. Subcommand options are tab-completable.
4. A list of directories (but not files) is provided when users enter git
- sparse-checkout add <TAB> or git sparse-checkout set <TAB>.
+ sparse-checkout add <TAB> or git sparse-checkout set <TAB>. It is
+ important to note that this will apply for both cone mode and non-cone
+ mode (even though non-cone mode matches on patterns rather than
+ directories).
Failing tests that were added in the previous commit to verify these
scenarios are now passing with these updates.
@@ contrib/completion/git-completion.bash: _git_show_branch ()
- ;;
- *)
- ;;
-+ case "$prev" in
++ case "$subcommand" in
+ set)
+ __gitcomp "$__git_sparse_checkout_subcommand_opts --stdin"
+ __gitcomp "$(git ls-tree -d -r HEAD --name-only)"
-: ----------- > 3: aa9ea67180d sparse-checkout: limit tab completion to a single level
--
gitgitgadget
From: Lessley Dennington via GitGitGadget <hidden> Date: 2022-01-10 19:00:00
From: Lessley Dennington <redacted>
Fix custom tab completion for sparse-checkout command. This will ensure:
1. The full list of subcommands is provided when users enter git
sparse-checkout <TAB>.
2. The --help option is tab-completable.
3. Subcommand options are tab-completable.
4. A list of directories (but not files) is provided when users enter git
sparse-checkout add <TAB> or git sparse-checkout set <TAB>. It is
important to note that this will apply for both cone mode and non-cone
mode (even though non-cone mode matches on patterns rather than
directories).
Failing tests that were added in the previous commit to verify these
scenarios are now passing with these updates.
Signed-off-by: Lessley Dennington <redacted>
---
contrib/completion/git-completion.bash | 38 ++++++++++++++++++--------
t/t9902-completion.sh | 8 +++---
2 files changed, 30 insertions(+), 16 deletions(-)
From: Lessley Dennington via GitGitGadget <hidden> Date: 2022-01-10 19:00:01
From: Lessley Dennington <redacted>
Add tests for missing/incorrect components of custom tab completion for the
sparse-checkout command. These tests specifically highlight the following:
1. git sparse-checkout <TAB> results in an incomplete list of subcommands
(it is missing reapply and add).
2. git sparse-checkout --<TAB> does not complete the help option.
3. Options for subcommands are not tab-completable.
4. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> show
both file names and directory names.
Although these tests currently fail, they will succeed with the
sparse-checkout modifications in git-completion.bash in the next commit in
this series.
Signed-off-by: Lessley Dennington <redacted>
---
t/t9902-completion.sh | 85 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 85 insertions(+)
From: Lessley Dennington via GitGitGadget <hidden> Date: 2022-01-10 19:00:03
From: Lessley Dennington <redacted>
Ensure only directories at the current level will be tab-completed with
the sparse-checkout command. For example, if paths a/b/c/ and a/d/ exist
in the current directory, running a/<TAB> will result in:
a/b/
a/d/
The 'sparse-checkout completes directory names' test has also been
updated/extended according to these changes.
Co-authored-by: Elijah Newren [off-list ref]
Co-authored-by: Lessley Dennington [off-list ref]
Signed-off-by: Lessley Dennington <redacted>
---
contrib/completion/git-completion.bash | 34 ++++++++++++++++++++++++--
t/t9902-completion.sh | 29 ++++++++++++++--------
2 files changed, 51 insertions(+), 12 deletions(-)
@@ -2986,6 +2986,36 @@ _git_show_branch ()__git_complete_revlist}+__gitcomp_directories()+{+local_tmp_dir_tmp_completions++# Get the directory of the current token; this differs from dirname+# in that it keeps up to the final trailing slash. If no slash found+# that's fine too.+[["$cur"=~.*/]]+_tmp_dir=$BASH_REMATCH++# Find possible directory completions, adding trailing '/' characters+_tmp_completions="$(gitls-tree-d--name-onlyHEAD$_tmp_dir|+sed-es%$%/%)"++if[[-n"$_tmp_completions"]];then+# There were some directory completions, so find ones that+# start with "$cur", the current token, and put those in COMPREPLY+locali=0cIFS=$' \t\n'+forcin$_tmp_completions;do+if[[$c=="$cur"*]];then+COMPREPLY+=("$c")+fi+done+elif[["$cur"=~/$]];then+# No possible further completions any deeper, so assume we're at+# a leaf directory and just consider it complete+__gitcomp_direct_append"$cur "+fi+}+__git_sparse_checkout_subcommand_opts="--cone --no-cone --sparse-index --no-sparse-index" _git_sparse_checkout()
On Mon, Jan 10, 2022 at 10:59 AM Lessley Dennington via GitGitGadget
[off-list ref] wrote:
This series is based on en/sparse-checkout-set. It updates custom tab
completion for the sparse-checkout command. Specifically, it corrects the
following issues with the current method:
1. git sparse-checkout <TAB> results in an incomplete list of subcommands
(it is missing reapply and add).
2. git sparse-checkout --<TAB> does not complete the help option.
3. Options for subcommands are not tab-completable.
4. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> show
both file names and directory names.
The first commit in this series is a set of failing tests that highlight
each of the above issues. The next commit updates the _git_sparse_checkout
method in git-completion.bash to enable each of these tests to pass. The
final commit modifies the original implementation (which recursively printed
prospective directories for completion) to only print directories at the
current level for improved performance.
Changes since V2
================
* Change use of $prev to $subcommand in _git_sparse_checkout() method in
git-completion.bash.
* State explicitly that directory completion applies in both cone and
non-cone mode in 'sparse-checkout: custom tab completion' commit
* Add new patch with __gitcomp_directories method to improve performance by
only outputting directories at the current level.
Changes since V1
================
* Rebase onto en/sparse-checkout-set.
* Add subcommand options (including --no-cone) for set and reapply.
* Extend 'sparse-checkout completes subcommand options' test to validate
new set/reapply subcommand options.
* No longer set index.sparse to false in 'sparse-checkout completes
directory names' test.
Thanks, Lessley
Lessley Dennington (3):
sparse-checkout: custom tab completion tests
sparse-checkout: custom tab completion
sparse-checkout: limit tab completion to a single level
contrib/completion/git-completion.bash | 68 +++++++++++++++----
t/t9902-completion.sh | 94 ++++++++++++++++++++++++++
2 files changed, 150 insertions(+), 12 deletions(-)
base-commit: dfac9b609f86cd4f6ce896df9e1172d2a02cde48
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1108%2Fldennington%2Fsparse-checkout-bash-completion-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1108/ldennington/sparse-checkout-bash-completion-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1108
Range-diff vs v2:
1: 955fcab0052 = 1: bbc2d21e1d1 sparse-checkout: custom tab completion tests
2: cecf501e076 ! 2: 256e5f034c6 sparse-checkout: custom tab completion
@@ Commit message
2. The --help option is tab-completable.
3. Subcommand options are tab-completable.
4. A list of directories (but not files) is provided when users enter git
- sparse-checkout add <TAB> or git sparse-checkout set <TAB>.
+ sparse-checkout add <TAB> or git sparse-checkout set <TAB>. It is
+ important to note that this will apply for both cone mode and non-cone
+ mode (even though non-cone mode matches on patterns rather than
+ directories).
I would instead phrase this as "(even though non-cone mode can match
general gitignore patterns rather than just directories)".
The basic idea behind the rewording is that I want to highlight that
the completions we provide are still valid in non-cone mode, they just
aren't comprehensive. Since there's no way to provide a comprehensive
list of possible patterns for non-cone mode, I think what we are
choosing to provide is a pretty reasonable choice.
Failing tests that were added in the previous commit to verify these
scenarios are now passing with these updates.
@@ contrib/completion/git-completion.bash: _git_show_branch ()
- ;;
- *)
- ;;
-+ case "$prev" in
++ case "$subcommand" in
+ set)
+ __gitcomp "$__git_sparse_checkout_subcommand_opts --stdin"
+ __gitcomp "$(git ls-tree -d -r HEAD --name-only)"
-: ----------- > 3: aa9ea67180d sparse-checkout: limit tab completion to a single level
Other than that one nit, patches 1-2 (and the testcases in patch 3) are:
Reviewed-by: Elijah Newren <redacted>
Since I wrote the new __gitcomp() function in patch 3, it might be
nice if we can find another reviewer for it. Especially since I've
only lightly touched the completion code and there might be better
ways of achieving what I wrote there.
On Mon, Jan 10, 2022 at 10:59 AM Lessley Dennington via GitGitGadget
[off-list ref] wrote:
quoted
This series is based on en/sparse-checkout-set. It updates custom tab
completion for the sparse-checkout command. Specifically, it corrects the
following issues with the current method:
1. git sparse-checkout <TAB> results in an incomplete list of subcommands
(it is missing reapply and add).
2. git sparse-checkout --<TAB> does not complete the help option.
3. Options for subcommands are not tab-completable.
4. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> show
both file names and directory names.
The first commit in this series is a set of failing tests that highlight
each of the above issues. The next commit updates the _git_sparse_checkout
method in git-completion.bash to enable each of these tests to pass. The
final commit modifies the original implementation (which recursively printed
prospective directories for completion) to only print directories at the
current level for improved performance.
Changes since V2
================
* Change use of $prev to $subcommand in _git_sparse_checkout() method in
git-completion.bash.
* State explicitly that directory completion applies in both cone and
non-cone mode in 'sparse-checkout: custom tab completion' commit
* Add new patch with __gitcomp_directories method to improve performance by
only outputting directories at the current level.
Changes since V1
================
* Rebase onto en/sparse-checkout-set.
* Add subcommand options (including --no-cone) for set and reapply.
* Extend 'sparse-checkout completes subcommand options' test to validate
new set/reapply subcommand options.
* No longer set index.sparse to false in 'sparse-checkout completes
directory names' test.
Thanks, Lessley
Lessley Dennington (3):
sparse-checkout: custom tab completion tests
sparse-checkout: custom tab completion
sparse-checkout: limit tab completion to a single level
contrib/completion/git-completion.bash | 68 +++++++++++++++----
t/t9902-completion.sh | 94 ++++++++++++++++++++++++++
2 files changed, 150 insertions(+), 12 deletions(-)
base-commit: dfac9b609f86cd4f6ce896df9e1172d2a02cde48
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1108%2Fldennington%2Fsparse-checkout-bash-completion-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1108/ldennington/sparse-checkout-bash-completion-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1108
Range-diff vs v2:
1: 955fcab0052 = 1: bbc2d21e1d1 sparse-checkout: custom tab completion tests
2: cecf501e076 ! 2: 256e5f034c6 sparse-checkout: custom tab completion
@@ Commit message
2. The --help option is tab-completable.
3. Subcommand options are tab-completable.
4. A list of directories (but not files) is provided when users enter git
- sparse-checkout add <TAB> or git sparse-checkout set <TAB>.
+ sparse-checkout add <TAB> or git sparse-checkout set <TAB>. It is
+ important to note that this will apply for both cone mode and non-cone
+ mode (even though non-cone mode matches on patterns rather than
+ directories).
I would instead phrase this as "(even though non-cone mode can match
general gitignore patterns rather than just directories)".
The basic idea behind the rewording is that I want to highlight that
the completions we provide are still valid in non-cone mode, they just
aren't comprehensive. Since there's no way to provide a comprehensive
list of possible patterns for non-cone mode, I think what we are
choosing to provide is a pretty reasonable choice.
Thank you! That is much better wording. Will update in v4 along with any
feedback I get on your function (see below).
quoted
Failing tests that were added in the previous commit to verify these
scenarios are now passing with these updates.
@@ contrib/completion/git-completion.bash: _git_show_branch ()
- ;;
- *)
- ;;
-+ case "$prev" in
++ case "$subcommand" in
+ set)
+ __gitcomp "$__git_sparse_checkout_subcommand_opts --stdin"
+ __gitcomp "$(git ls-tree -d -r HEAD --name-only)"
-: ----------- > 3: aa9ea67180d sparse-checkout: limit tab completion to a single level
Other than that one nit, patches 1-2 (and the testcases in patch 3) are:
Reviewed-by: Elijah Newren <redacted>
Since I wrote the new __gitcomp() function in patch 3, it might be
nice if we can find another reviewer for it. Especially since I've
only lightly touched the completion code and there might be better
ways of achieving what I wrote there.
I agree - I'll chat with some folks today to try to find the best person
to take a look.
From: Taylor Blau <hidden> Date: 2022-01-11 19:45:35
On Tue, Jan 11, 2022 at 09:17:38AM -0800, Lessley Dennington wrote:
On 1/10/22 2:38 PM, Elijah Newren wrote:
quoted
Other than that one nit, patches 1-2 (and the testcases in patch 3) are:
Reviewed-by: Elijah Newren <redacted>
Since I wrote the new __gitcomp() function in patch 3, it might be
nice if we can find another reviewer for it. Especially since I've
only lightly touched the completion code and there might be better
ways of achieving what I wrote there.
I agree - I'll chat with some folks today to try to find the best person
to take a look.
Having another set of eyes on this code never hurts, but looking through
this file's shortlog (especially with `--since=1.year.ago`), I'm not
sure how many active reviewers in this area we still have.
Perhaps the best thing to do would be to grow some new area experts here
instead. If I were in Lessley's shoes, I would "think aloud" to make
sure I agreed with your implementation for the new `__gitcomp()`
function.
If other reviewers happen to be around (perhaps Denton, who I added to
the CC) that doesn't hurt either, but short of that...
Thanks,
Taylor
On Tue, Jan 11, 2022 at 09:17:38AM -0800, Lessley Dennington wrote:
quoted
On 1/10/22 2:38 PM, Elijah Newren wrote:
quoted
Other than that one nit, patches 1-2 (and the testcases in patch 3) are:
Reviewed-by: Elijah Newren <redacted>
Since I wrote the new __gitcomp() function in patch 3, it might be
nice if we can find another reviewer for it. Especially since I've
only lightly touched the completion code and there might be better
ways of achieving what I wrote there.
I agree - I'll chat with some folks today to try to find the best person
to take a look.
Having another set of eyes on this code never hurts, but looking through
this file's shortlog (especially with `--since=1.year.ago`), I'm not
sure how many active reviewers in this area we still have.
Perhaps the best thing to do would be to grow some new area experts here
instead. If I were in Lessley's shoes, I would "think aloud" to make
sure I agreed with your implementation for the new `__gitcomp()`
function.
If other reviewers happen to be around (perhaps Denton, who I added to
the CC) that doesn't hurt either, but short of that...
Thanks,
Taylor
Thanks for the help here, Taylor! I like this suggestion and will add my
thoughts to Elijah's __gitcomp_directories function.
+__gitcomp_directories ()
+{
+ local _tmp_dir _tmp_completions
+
+ # Get the directory of the current token; this differs from dirname
+ # in that it keeps up to the final trailing slash. If no slash found
+ # that's fine too.
+ [[ "$cur" =~ .*/ ]]
+ _tmp_dir=$BASH_REMATCH
+
+ # Find possible directory completions, adding trailing '/' characters
+ _tmp_completions="$(git ls-tree -d --name-only HEAD $_tmp_dir |
+ sed -e s%$%/%)"
+
I am admittedly unfamiliar with the use of this format in sed expressions
(I'm generally more accustomed to '/' instead of '%'). It's definitely
working as it should, I'm just not quite sure of how.
+ if [[ -n "$_tmp_completions" ]]; then
+ # There were some directory completions, so find ones that
+ # start with "$cur", the current token, and put those in COMPREPLY
+ local i=0 c IFS=$' \t\n'
Does c need to be declared before the loop?
+ for c in $_tmp_completions; do
+ if [[ $c == "$cur"* ]]; then
+ COMPREPLY+=("$c")
+ fi
+ done
+ elif [[ "$cur" =~ /$ ]]; then
+ # No possible further completions any deeper, so assume we're at
+ # a leaf directory and just consider it complete
Thank you so much for the detailed comments on this change - it made it
really easy to parse.
On Wed, Jan 12, 2022 at 3:43 PM Lessley Dennington
[off-list ref] wrote:
quoted
+__gitcomp_directories ()
+{
+ local _tmp_dir _tmp_completions
+
+ # Get the directory of the current token; this differs from dirname
+ # in that it keeps up to the final trailing slash. If no slash found
+ # that's fine too.
+ [[ "$cur" =~ .*/ ]]
+ _tmp_dir=$BASH_REMATCH
+
+ # Find possible directory completions, adding trailing '/' characters
+ _tmp_completions="$(git ls-tree -d --name-only HEAD $_tmp_dir |
+ sed -e s%$%/%)"
+
I am admittedly unfamiliar with the use of this format in sed expressions
(I'm generally more accustomed to '/' instead of '%'). It's definitely
working as it should, I'm just not quite sure of how.
These are the same in sed:
sed -e s/apple/banana/
sed -e s@apple@banana@
sed -e s%apple%banana%
You can pick any character you like, but '/' is what people most often
use. My expression involved a '/', though, so I changed the delimiter
to avoid ugly backslash escapes.
quoted
+ if [[ -n "$_tmp_completions" ]]; then
+ # There were some directory completions, so find ones that
+ # start with "$cur", the current token, and put those in COMPREPLY
+ local i=0 c IFS=$' \t\n'
Does c need to be declared before the loop?
It was, but it's super easy to miss. Look at the "local" line just
before your comment; it almost reads like line noise, but basically
there are three variables declared with two of them given initial
values. c is in the middle, without an initial value.
quoted
+ for c in $_tmp_completions; do
+ if [[ $c == "$cur"* ]]; then
+ COMPREPLY+=("$c")
+ fi
+ done
+ elif [[ "$cur" =~ /$ ]]; then
+ # No possible further completions any deeper, so assume we're at
+ # a leaf directory and just consider it complete
Thank you so much for the detailed comments on this change - it made it
really easy to parse.
quoted
+ __gitcomp_direct_append "$cur "
What's the reason for the trailing space here?
The space was related to the "just consider it complete" aspect of the
comment above. Anyway, to understand why the space character signals
the completion being final for this token, let's use some comparisons
with bash-completion of just a plain 'ls' command...
In git.git, at the toplevel, if I type
ls README.md <TAB>
(note the space after README.md before pressing <TAB>), then
completion assumes I'm trying to get another term besides just
README.md, and can complete on anything in the directory. In
contrast, if I type
ls README.md<TAB>
(with no space between README.md and <TAB>), then completion figures
I'm trying to find filenames that start with "README.md", finds only
one, and returns "README.md " (note the trailing space). That
trailing space makes my command line become "ls README.md " (again,
with a trailing space), so that if I try to do any more tab
completions, it's for adding another argument to the ls command, not
extending the README.md one.
You can see similar things with ls and directories. For example, if you type
ls Doc<TAB>tec<TAB>m<TAB>
then you'll note after the first <TAB> you'll see
ls Documentation/
with no trailing space; after the second <TAB> you'll see
ls Documentation/technical/
with no trailing space; and after the third <TAB> you'll see
ls Documentation/technical/multi-pack-index.txt
WITH a trailing space. In the first two cases, further completions
were possible so they didn't add a trailing space to signify the
completion was final, but in the third case it is final and needed the
trailing space to denote that.
Does that help?
On Wed, Jan 12, 2022 at 3:43 PM Lessley Dennington
[off-list ref] wrote:
quoted
quoted
+__gitcomp_directories ()
+{
+ local _tmp_dir _tmp_completions
+
+ # Get the directory of the current token; this differs from dirname
+ # in that it keeps up to the final trailing slash. If no slash found
+ # that's fine too.
+ [[ "$cur" =~ .*/ ]]
+ _tmp_dir=$BASH_REMATCH
+
+ # Find possible directory completions, adding trailing '/' characters
+ _tmp_completions="$(git ls-tree -d --name-only HEAD $_tmp_dir |
+ sed -e s%$%/%)"
+
I am admittedly unfamiliar with the use of this format in sed expressions
(I'm generally more accustomed to '/' instead of '%'). It's definitely
working as it should, I'm just not quite sure of how.
These are the same in sed:
sed -e s/apple/banana/
sed -e s@apple@banana@
sed -e s%apple%banana%
You can pick any character you like, but '/' is what people most often
use. My expression involved a '/', though, so I changed the delimiter
to avoid ugly backslash escapes.
Got it, thanks!
quoted
quoted
+ if [[ -n "$_tmp_completions" ]]; then
+ # There were some directory completions, so find ones that
+ # start with "$cur", the current token, and put those in COMPREPLY
+ local i=0 c IFS=$' \t\n'
Does c need to be declared before the loop?
It was, but it's super easy to miss. Look at the "local" line just
before your comment; it almost reads like line noise, but basically
there are three variables declared with two of them given initial
values. c is in the middle, without an initial value.
Sorry, I should have made my question clearer - I saw that c is declared
before the loop but was wondering if that was actually necessary. I
removed it locally and ran the tests, which seemed to work without it.
quoted
quoted
+ for c in $_tmp_completions; do
+ if [[ $c == "$cur"* ]]; then
+ COMPREPLY+=("$c")
+ fi
+ done
+ elif [[ "$cur" =~ /$ ]]; then
+ # No possible further completions any deeper, so assume we're at
+ # a leaf directory and just consider it complete
Thank you so much for the detailed comments on this change - it made it
really easy to parse.
quoted
+ __gitcomp_direct_append "$cur "
What's the reason for the trailing space here?
The space was related to the "just consider it complete" aspect of the
comment above. Anyway, to understand why the space character signals
the completion being final for this token, let's use some comparisons
with bash-completion of just a plain 'ls' command...
In git.git, at the toplevel, if I type
ls README.md <TAB>
(note the space after README.md before pressing <TAB>), then
completion assumes I'm trying to get another term besides just
README.md, and can complete on anything in the directory. In
contrast, if I type
ls README.md<TAB>
(with no space between README.md and <TAB>), then completion figures
I'm trying to find filenames that start with "README.md", finds only
one, and returns "README.md " (note the trailing space). That
trailing space makes my command line become "ls README.md " (again,
with a trailing space), so that if I try to do any more tab
completions, it's for adding another argument to the ls command, not
extending the README.md one.
You can see similar things with ls and directories. For example, if you type
ls Doc<TAB>tec<TAB>m<TAB>
then you'll note after the first <TAB> you'll see
ls Documentation/
with no trailing space; after the second <TAB> you'll see
ls Documentation/technical/
with no trailing space; and after the third <TAB> you'll see
ls Documentation/technical/multi-pack-index.txt
WITH a trailing space. In the first two cases, further completions
were possible so they didn't add a trailing space to signify the
completion was final, but in the third case it is final and needed the
trailing space to denote that.
Does that help?
None of these patches touch sparse-checkout, but only the completion
script and its tests. Therefore "completion:" would be a better
matching area prefix.
Fix custom tab completion for sparse-checkout command. This will ensure:
1. The full list of subcommands is provided when users enter git
sparse-checkout <TAB>.
2. The --help option is tab-completable.
This is inconsistent with the rest of the completion script, because
it doesn't list '--help' for any commands or subcommand (with the sole
exception of 'git --<TAB>').
3. Subcommand options are tab-completable.
4. A list of directories (but not files) is provided when users enter git
sparse-checkout add <TAB> or git sparse-checkout set <TAB>.
Why limit completion only to directories? Both of those subcommands
accept files, and I think 'git sparse-checkout set README.md' is a
perfectly reasonable command.
quoted hunk
It is
important to note that this will apply for both cone mode and non-cone
mode (even though non-cone mode matches on patterns rather than
directories).
Failing tests that were added in the previous commit to verify these
scenarios are now passing with these updates.
Signed-off-by: Lessley Dennington <redacted>
---
contrib/completion/git-completion.bash | 38 ++++++++++++++++++--------
t/t9902-completion.sh | 8 +++---
2 files changed, 30 insertions(+), 16 deletions(-)
@@ -2986,24 +2986,38 @@ _git_show_branch ()__git_complete_revlist}+__git_sparse_checkout_subcommand_opts="--cone --no-cone --sparse-index --no-sparse-index"+ _git_sparse_checkout(){-localsubcommands="list init set disable"+localsubcommands="list init set disable add reapply"localsubcommand="$(__git_find_on_cmdline"$subcommands")"+if[-z"$subcommand"];then-__gitcomp"$subcommands"-return+case"$cur"in+--*)+__gitcomp"--help"+;;+*)+__gitcomp"$subcommands"+;;+esacfi-case"$subcommand,$cur"in-init,--*)-__gitcomp"--cone"-;;-set,--*)-__gitcomp"--stdin"-;;-*)-;;+case"$subcommand"in+set)+__gitcomp"$__git_sparse_checkout_subcommand_opts --stdin"
Oh, a hard-coded list of --options is so old school :)
All subcommands of 'git sparse-checkout' use parse-options (even those
that don't have any --options at the moment), so their options can be
completed programmatically, and then we wouldn't have to worry about
updating the list of options in the completion script ever again. It
would also make several tests added in the previous patch unnecessary.
You could use the completion function of 'git notes' for inspiration
on how to do that.
+ __gitcomp "$(git ls-tree -d -r HEAD --name-only)"
This will have troubles with unusual characters in pathnames:
- If a pathname contains a space (or any other IFS character), then
the shell will split that into multiple words.
- If a pathname contains a special character, e.g. double quote or
backslash, then 'git ls-tree' will quote that path. Furthermore,
by default it will quote pathnames containing e.g. UTF-8
characters as well.
- The shell has its own special characteers that have to be
quoted/escaped on the command line to strip them from their
special meaning, that quoting/escaping will interfere with listing
only paths matching the current word to be completed.
You should use the __git_complete_index_file() helper function
instead, which takes care of most of this.
Furthermore, these two subsequent __gitcomp() calls will offer both
options and paths for 'git sparse-checkout set <TAB>', which is
inconsistent with the rest of the completion script. Usually it lists
--options only after e.g. 'git rm --<TAB>' or 'git log --<TAB>', but
after 'git rm <TAB>' and 'git log <TAB>' it lists only files and refs,
respectively.
None of these patches touch sparse-checkout, but only the completion
script and its tests. Therefore "completion:" would be a better
matching area prefix.
Thanks for the detailed feedback and guidance in your review. Very
helpful. I'll omit quoting most of it here, but I do want to comment
on the point about directories.
...
quoted
4. A list of directories (but not files) is provided when users enter git
sparse-checkout add <TAB> or git sparse-checkout set <TAB>.
Why limit completion only to directories? Both of those subcommands
accept files,
Discussed in part at [1], but let me give a more detailed answer.
Both of these commands accept not only directories and files, but also
nearly arbitrary input as far as I can tell. (In cone-mode, it'll
accept anything so long as it doesn't look like a relative path that
tries to reach above the toplevel directory with '../' sequences. In
non-cone mode, I think it accepts completely arbitrary input). If our
guide is merely what the command swallows, then we should forgo
completion for these subcommands, because it's not possible to
enumerate all possible completions. I don't think that's a useful
guide or starting point, so we instead need to discuss what are
reasonable completions.
cone-mode works exclusively on directories. So, in that mode,
directories are what we want to complete on. (And if a file is
specified, cone-mode will treat it as a directory and add expressions
for including all the files under that "directory", which might be
confusing. sparse-checkout doesn't verify it is a diretory, because
it *might* name a directory in a different branch, including one not
yet downloaded. But "might name a directory on another branch" is no
reason to suggest picking that pathname with completion.)
In non-cone mode, arbitrary expressions are valid and will be treated
as gitignore-style expressions. That again leaves us with either not
providing completions, or choosing a subset of possible inputs that
are reasonable suggestions for users. I prefer the latter, and in
particular I feel that directories are reasonable suggestions. In
contrast, I don't think providing files is helpful, because it
reinforces the design flaw of non-cone mode. Non-cone mode has
quadratic performance baked into its design, and since
sparse-checkouts are about performance, non-cone mode kind of defeats
the purpose of the command. (In addition to other problems[2].) So,
I think non-cone mode should be deprecated and excised. Patches
elsewhere are moving in the direction of deprecation already[3], and
we've already discussed multiple steps we'll likely take soon
continuing in that direction. In the meantime, providing just
directories for completion seems like a good direction to me.
[1] https://lore.kernel.org/git/CABPp-BG=wr81CPtW1M12xFN_0dyS8mAZjM6o=77LA20Zge8Xng@mail.gmail.com/
[2] https://lore.kernel.org/git/CABPp-BF=-1aZd=nFHF6spo7Ksa7f7Wb7ervCt0QvtNitMY=ZBA@mail.gmail.com/
[3] https://lore.kernel.org/git/0af00779128e594aff0ee4ec5378addeac8e88a2.1642175983.git.gitgitgadget@gmail.com/
("This mode is harder to use and less performant, and is thus not
recommended.")
and I think 'git sparse-checkout set README.md' is a
perfectly reasonable command.
Reasonable in what sense? That it makes it (vastly) easier to
implement the completion and sparse-checkout set|add will swallow it,
or that it's something that should actually be recommended for users
doing sparse-checkouts? While the former certainly holds, I don't
think the latter does.
None of these patches touch sparse-checkout, but only the completion
script and its tests. Therefore "completion:" would be a better
matching area prefix.
Thanks for the detailed feedback and guidance in your review. Very
helpful. I'll omit quoting most of it here, but I do want to comment
on the point about directories.
I second this - am planning to apply all feedback (with the exception of
completing
on both files and directories) in v4 of this series.
None of these patches touch sparse-checkout, but only the completion
script and its tests. Therefore "completion:" would be a better
matching area prefix.
Thanks for the detailed feedback and guidance in your review. Very
helpful. I'll omit quoting most of it here, but I do want to comment
on the point about directories.
...
quoted
quoted
4. A list of directories (but not files) is provided when users enter git
sparse-checkout add <TAB> or git sparse-checkout set <TAB>.
Why limit completion only to directories? Both of those subcommands
accept files,
Discussed in part at [1], but let me give a more detailed answer.
It was a semi-rhetorical question. Whether the reasons for expluding
files are sound or not, it should be convincingly justified in the
commit message.
Both of these commands accept not only directories and files, but also
nearly arbitrary input as far as I can tell. (In cone-mode, it'll
accept anything so long as it doesn't look like a relative path that
tries to reach above the toplevel directory with '../' sequences. In
non-cone mode, I think it accepts completely arbitrary input). If our
guide is merely what the command swallows, then we should forgo
completion for these subcommands, because it's not possible to
enumerate all possible completions. I don't think that's a useful
guide or starting point, so we instead need to discuss what are
reasonable completions.
cone-mode works exclusively on directories. So, in that mode,
directories are what we want to complete on. (And if a file is
specified, cone-mode will treat it as a directory and add expressions
for including all the files under that "directory", which might be
confusing. sparse-checkout doesn't verify it is a diretory, because
it *might* name a directory in a different branch, including one not
yet downloaded. But "might name a directory on another branch" is no
reason to suggest picking that pathname with completion.)
In non-cone mode, arbitrary expressions are valid and will be treated
as gitignore-style expressions. That again leaves us with either not
providing completions, or choosing a subset of possible inputs that
are reasonable suggestions for users. I prefer the latter, and in
particular I feel that directories are reasonable suggestions. In
contrast, I don't think providing files is helpful, because it
reinforces the design flaw of non-cone mode. Non-cone mode has
quadratic performance baked into its design, and since
sparse-checkouts are about performance, non-cone mode kind of defeats
the purpose of the command.
Or about disk space. Which, because of the potentially significantly
reduced number of files in the work tree can bring along significant
performance benefits, even with quadratic behavior.
and I think 'git sparse-checkout set README.md' is a
perfectly reasonable command.
Reasonable in what sense? That it makes it (vastly) easier to
implement the completion and sparse-checkout set|add will swallow it,
or that it's something that should actually be recommended for users
doing sparse-checkouts? While the former certainly holds, I don't
think the latter does.
I used the following command to create a sparse-checkout from
linux.git to build and play with 'perf':
git sparse-checkout set tools/perf/ tools/scripts/ tools/build/ tools/include/ tools/arch/x86/ tools/lib/ /.gitignore /.gitattributes
Including the top-level '.gitignore' and '.gitattributes' was
important, becase those ignore object files and specify userdiff.
Now, I wouldn't mind having other files present in the top-level
directory, because there are only a handful of files there. However,
being able to specify just those two files to 'git sparse-checkout'
was great, because I didn't even have to think about what wildcard
pattern to use, and what negative pattern to use to exclude anything
that might have been included recursively.
I don't remember having any performance issues with it, on the
contrary, I do remember that Git suddenly became much faster that in
the full worktree.
So I'm fairly convinced that specifying files to sparse-checkout is a
feature that can make users' life easier. It certainly made my life
easier.
On a related note: I just noticed the leading slashes in '/.gitignore'
and '/.gitattributes'. __git_complete_index_file() is not ready for
that, I'm afraid; but I don't think the proposed patches could deal
with that, either (but I didn't actually try).
It would be great if completion could cope with patterns starting with
'/' and/or '!'.
On Tue, Jan 18, 2022 at 2:22 PM SZEDER Gábor [off-list ref] wrote:
On Sat, Jan 15, 2022 at 05:03:24PM -0800, Elijah Newren wrote:
quoted
On Sat, Jan 15, 2022 at 1:57 AM SZEDER Gábor [off-list ref] wrote:
quoted
On Mon, Jan 10, 2022 at 06:59:51PM +0000, Lessley Dennington via GitGitGadget wrote:
...
quoted
quoted
quoted
4. A list of directories (but not files) is provided when users enter git
sparse-checkout add <TAB> or git sparse-checkout set <TAB>.
Why limit completion only to directories? Both of those subcommands
accept files,
Discussed in part at [1], but let me give a more detailed answer.
It was a semi-rhetorical question. Whether the reasons for expluding
files are sound or not, it should be convincingly justified in the
commit message.
Fair enough.
...
I used the following command to create a sparse-checkout from
linux.git to build and play with 'perf':
git sparse-checkout set tools/perf/ tools/scripts/ tools/build/ tools/include/ tools/arch/x86/ tools/lib/ /.gitignore /.gitattributes
Including the top-level '.gitignore' and '.gitattributes' was
important, becase those ignore object files and specify userdiff.
Now, I wouldn't mind having other files present in the top-level
directory, because there are only a handful of files there. However,
being able to specify just those two files to 'git sparse-checkout'
was great, because I didn't even have to think about what wildcard
pattern to use, and what negative pattern to use to exclude anything
that might have been included recursively.
Yeah, we should really make cone mode the default, because then this
simplifies to:
git sparse-checkout set tools/perf/ tools/scripts/ tools/build/
tools/include/ tools/arch/x86/ tools/lib/
In cone-mode you automatically get the directories specified PLUS:
* all files in the toplevel directory (you always get these in cone mode)
* all files directly within a parent (or ancestor) of one of the
directories you specified (thus all files directly under tools/ and
tools/arch/).
I don't remember having any performance issues with it, on the
contrary, I do remember that Git suddenly became much faster that in
the full worktree.
Cool, thanks for testing it out and reporting.
If I can be permitted to share a bit of my experience...
In this specific case, you did keep it to 8 patterns and the linux
kernel "only" has ~60k files. So O(N*M) isn't too bad here...but
experience at $DAYJOB is that even with only 60k files, the pattern
list tends to grow and it's not clear to users why there are so many
ugly pauses when "Git used to be fast". Users tend to not make the
connection either between the sparsity patterns and the slowness for
whatever reason, and when I'm asked questions about the slowness, I
may have to check a few other possible causes before finally realizing
that it's just "too many sparsity patterns".
At least, that's the way it was at $DAYJOB before I switched our
internal "sparsify" tool to just use cone mode. Then that type of
problem went away.
So I'm fairly convinced that specifying files to sparse-checkout is a
feature that can make users' life easier. It certainly made my life
easier.
Or we can make it even easier and faster by using cone mode. That
really, really should be the default (as we've been talking about for
a couple cycles now), and I think your email reinforces my belief in
it. I've got a series ready after some other things merge down to
next.
On a related note: I just noticed the leading slashes in '/.gitignore'
and '/.gitattributes'. __git_complete_index_file() is not ready for
that, I'm afraid; but I don't think the proposed patches could deal
with that, either (but I didn't actually try).
It would be great if completion could cope with patterns starting with
'/' and/or '!'.
Ah, very good point. The leading '/' is kind of critical here,
otherwise you get all .gitignore files in every directory at every
depth, so completing on files (as currently done) wouldn't really help
at all in your case.
Since you brought up a new good point here, can I also mention two
others not previously covered in this thread (i.e. besides scaling)?
* If we complete file and directory names in non-cone mode, and those
file and directory names contain a leading '#' or '!' or contain
anywhere in their names a '*', '?', '\', '[', or ']', then the users
may be in for a very nasty surprise. (Which perhaps suggests that in
non-cone mode, we shouldn't provide file OR directory completions for
set/add?)
* sparse-checkout currently ignores prefix[1], causing file and
directory completions to be wrong if the user is running from a
subdirectory. This is just a bug in cone mode, though I'm not sure if
it's bug or feature in non-cone mode[2]. A decision about what it is
might have a bearing on what kinds of completions make sense in
non-cone mode (and might reinforce files/directories, or suggest
something else.).
[1] https://lore.kernel.org/git/29f0410e-6dfa-2e86-394d-b1fb735e7608@gmail.com/
[2] https://lore.kernel.org/git/CABPp-BH5woi6KY7OBpnsS-M2EmgLHii9zs8rSwrgcPFkOAvn_A@mail.gmail.com/
From: Lessley Dennington via GitGitGadget <hidden> Date: 2022-01-27 21:21:59
This series is based on en/sparse-checkout-set. It updates custom tab
completion for the sparse-checkout command. Specifically, it corrects the
following issues with the current method:
1. git sparse-checkout <TAB> results in an incomplete list of subcommands
(it is missing reapply and add).
2. Options for subcommands are not tab-completable.
3. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> show
both file names and directory names. While this is the desired behavior
for non-cone mode, we want to only show directories in cone mode.
The first commit in this series is a set of tests that highlight each of the
above issues. The next commit updates the _git_sparse_checkout method in
git-completion.bash to enable previously-failing tests to pass. The final
commit modifies the original implementation (which recursively printed
prospective directories for completion) to only print directories at the
current level for improved performance.
Changes since V3
================
* Update commit descriptions to begin with completion instead of
sparse-checkout.
* Remove explicit completion of --help, as this is inconsistent with the
rest of the completion script.
* Use __gitcomp_builtin to auto-complete subcommand options (and remove
option completion test from previous iterations, as it is no longer
necessary).
* Use __git_complete_index_file helper function in place of __gitcomp
"$(git ls-tree -d -r HEAD --name-only).
* If in cone-mode sparse checkout, only show directory completions. If in
non-cone mode sparse checkout show both files and directories.
Changes since V2
================
* Change use of $prev to $subcommand in _git_sparse_checkout() method in
git-completion.bash.
* State explicitly that directory completion applies in both cone and
non-cone mode in 'sparse-checkout: custom tab completion' commit
* Add new patch with __gitcomp_directories method to improve performance by
only outputting directories at the current level.
Changes since V1
================
* Rebase onto en/sparse-checkout-set.
* Add subcommand options (including --no-cone) for set and reapply.
* Extend 'sparse-checkout completes subcommand options' test to validate
new set/reapply subcommand options.
* No longer set index.sparse to false in 'sparse-checkout completes
directory names' test.
Thanks, Lessley
Lessley Dennington (3):
completion: add sparse-checkout tests
completion: sparse-checkout updates
completion: ensure cone mode completion with multiple <TAB>s
contrib/completion/git-completion.bash | 44 ++++++++++++---
t/t9902-completion.sh | 75 ++++++++++++++++++++++++++
2 files changed, 112 insertions(+), 7 deletions(-)
base-commit: dfac9b609f86cd4f6ce896df9e1172d2a02cde48
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1108%2Fldennington%2Fsparse-checkout-bash-completion-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1108/ldennington/sparse-checkout-bash-completion-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1108
Range-diff vs v3:
1: bbc2d21e1d1 ! 1: 5bb598a055d sparse-checkout: custom tab completion tests
@@ Metadata
Author: Lessley Dennington [off-list ref]
## Commit message ##
- sparse-checkout: custom tab completion tests
+ completion: add sparse-checkout tests
- Add tests for missing/incorrect components of custom tab completion for the
- sparse-checkout command. These tests specifically highlight the following:
+ Add tests for missing/incorrect components of custom tab completion for
+ the sparse-checkout command. These tests specifically highlight the
+ following:
1. git sparse-checkout <TAB> results in an incomplete list of subcommands
(it is missing reapply and add).
- 2. git sparse-checkout --<TAB> does not complete the help option.
- 3. Options for subcommands are not tab-completable.
- 4. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> show
- both file names and directory names.
+ 2. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> show
+ both file names and directory names. While this is the correct behavior
+ for non-cone mode, cone mode sparse checkouts should complete only
+ directory names.
- Although these tests currently fail, they will succeed with the
- sparse-checkout modifications in git-completion.bash in the next commit in
- this series.
+ Although the first two of these tests currently fail, they will succeed
+ with the sparse-checkout modifications in git-completion.bash in the next
+ commit in this series.
Signed-off-by: Lessley Dennington [off-list ref]
@@ t/t9902-completion.sh: test_expect_success 'git checkout - with --detach, comple
+ EOF
+'
+
-+test_expect_failure 'sparse-checkout completes options' '
-+ test_completion "git sparse-checkout --" <<-\EOF
-+ --help Z
-+ EOF
-+'
-+
-+test_expect_failure 'sparse-checkout completes subcommand options' '
-+ test_completion "git sparse-checkout init --" <<-\EOF &&
-+ --cone Z
-+ --no-cone Z
-+ --sparse-index Z
-+ --no-sparse-index Z
-+ EOF
-+
-+ test_completion "git sparse-checkout set --" <<-\EOF &&
-+ --cone Z
-+ --no-cone Z
-+ --sparse-index Z
-+ --no-sparse-index Z
-+ --stdin Z
-+ EOF
-+
-+ test_completion "git sparse-checkout reapply --" <<-\EOF &&
-+ --cone Z
-+ --no-cone Z
-+ --sparse-index Z
-+ --no-sparse-index Z
-+ EOF
-+
-+ test_completion "git sparse-checkout add --" <<-\EOF
-+ --stdin Z
-+ EOF
-+'
-+
-+test_expect_failure 'sparse-checkout completes directory names' '
++test_expect_failure 'cone mode sparse-checkout completes directory names' '
+ # set up sparse-checkout repo
+ git init sparse-checkout &&
+ (
@@ t/t9902-completion.sh: test_expect_success 'git checkout - with --detach, comple
+ ) &&
+
+ # initialize sparse-checkout definitions
-+ git -C sparse-checkout sparse-checkout init --cone &&
-+ git -C sparse-checkout sparse-checkout set folder1/0 folder3 &&
++ git -C sparse-checkout sparse-checkout set --cone folder1/0 folder3 &&
+
+ # test tab completion
+ (
+ cd sparse-checkout &&
+ test_completion "git sparse-checkout set f" <<-\EOF
-+ folder1 Z
-+ folder1/0 Z
-+ folder1/0/1 Z
-+ folder2 Z
-+ folder2/0 Z
-+ folder3 Z
++ folder1
++ folder2
++ folder3
++ EOF
++ ) &&
++
++ (
++ cd sparse-checkout &&
++ test_completion "git sparse-checkout set folder1/" <<-\EOF
++ folder1/0
++ EOF
++ ) &&
++
++ (
++ cd sparse-checkout &&
++ test_completion "git sparse-checkout set folder1/0/" <<-\EOF
++ folder1/0/1
+ EOF
+ ) &&
+
+ (
+ cd sparse-checkout/folder1 &&
-+ test_completion "git sparse-checkout add " <<-\EOF
-+ ./ Z
-+ 0 Z
-+ 0/1 Z
++ test_completion "git sparse-checkout add 0" <<-\EOF
++ 0
++ EOF
++ )
++'
++
++test_expect_success 'non-cone mode sparse-checkout uses bash completion' '
++ # reset sparse-checkout repo to non-cone mode
++ git -C sparse-checkout sparse-checkout disable &&
++ git -C sparse-checkout sparse-checkout set &&
++
++ # test tab completion
++ (
++ cd sparse-checkout &&
++ # expected to be empty since we have not configured
++ # custom completion for non-cone mode
++ test_completion "git sparse-checkout set f" <<-\EOF
++
+ EOF
+ )
+'
2: 256e5f034c6 ! 2: 7d70beb2a6b sparse-checkout: custom tab completion
@@ Metadata
Author: Lessley Dennington [off-list ref]
## Commit message ##
- sparse-checkout: custom tab completion
+ completion: sparse-checkout updates
- Fix custom tab completion for sparse-checkout command. This will ensure:
+ Fix custom tab completion for the sparse-checkout command. This will
+ ensure:
1. The full list of subcommands is provided when users enter git
sparse-checkout <TAB>.
- 2. The --help option is tab-completable.
- 3. Subcommand options are tab-completable.
- 4. A list of directories (but not files) is provided when users enter git
- sparse-checkout add <TAB> or git sparse-checkout set <TAB>. It is
- important to note that this will apply for both cone mode and non-cone
- mode (even though non-cone mode matches on patterns rather than
- directories).
+ 2. Subcommand options are tab-completable.
+ 3. A list of directories (but not files) is provided when users enter git
+ sparse-checkout add <TAB> or git sparse-checkout set <TAB> in cone mode
+ only. In non-cone mode, these commands will continue to complete both
+ directory and file names.
Failing tests that were added in the previous commit to verify these
scenarios are now passing with these updates.
@@ Commit message
## contrib/completion/git-completion.bash ##
@@ contrib/completion/git-completion.bash: _git_show_branch ()
- __git_complete_revlist
- }
-+__git_sparse_checkout_subcommand_opts="--cone --no-cone --sparse-index --no-sparse-index"
-+
_git_sparse_checkout ()
{
- local subcommands="list init set disable"
@@ contrib/completion/git-completion.bash: _git_show_branch ()
local subcommand="$(__git_find_on_cmdline "$subcommands")"
+
if [ -z "$subcommand" ]; then
-- __gitcomp "$subcommands"
-- return
-+ case "$cur" in
-+ --*)
-+ __gitcomp "--help"
-+ ;;
-+ *)
-+ __gitcomp "$subcommands"
-+ ;;
-+ esac
+ __gitcomp "$subcommands"
+ return
fi
-- case "$subcommand,$cur" in
+ case "$subcommand,$cur" in
- init,--*)
- __gitcomp "--cone"
- ;;
- set,--*)
- __gitcomp "--stdin"
-- ;;
++ *,--*)
++ __gitcomp_builtin sparse-checkout_$subcommand "" "--"
+ ;;
- *)
-- ;;
-+ case "$subcommand" in
-+ set)
-+ __gitcomp "$__git_sparse_checkout_subcommand_opts --stdin"
-+ __gitcomp "$(git ls-tree -d -r HEAD --name-only)"
-+ ;;
-+ add)
-+ __gitcomp "--stdin"
-+ __gitcomp "$(git ls-tree -d -r HEAD --name-only)"
-+ ;;
-+ init|reapply)
-+ __gitcomp "$__git_sparse_checkout_subcommand_opts"
-+ ;;
-+ *)
-+ ;;
++ set,*|add,*)
++ if [ $(__git config core.sparseCheckoutCone) ]; then
++ __git_complete_index_file "--directory"
++ fi
+ ;;
esac
}
-
## t/t9902-completion.sh ##
@@ t/t9902-completion.sh: test_expect_success 'git checkout - with --detach, complete only references' '
@@ t/t9902-completion.sh: test_expect_failure 'sparse-checkout completes subcommand
EOF
'
--test_expect_failure 'sparse-checkout completes options' '
-+test_expect_success 'sparse-checkout completes options' '
- test_completion "git sparse-checkout --" <<-\EOF
- --help Z
- EOF
- '
-
--test_expect_failure 'sparse-checkout completes subcommand options' '
-+test_expect_success 'sparse-checkout completes subcommand options' '
- test_completion "git sparse-checkout init --" <<-\EOF &&
- --cone Z
- --no-cone Z
-@@ t/t9902-completion.sh: test_expect_failure 'sparse-checkout completes subcommand options' '
- EOF
- '
-
--test_expect_failure 'sparse-checkout completes directory names' '
-+test_expect_success 'sparse-checkout completes directory names' '
+-test_expect_failure 'cone mode sparse-checkout completes directory names' '
++test_expect_success 'cone mode sparse-checkout completes directory names' '
# set up sparse-checkout repo
git init sparse-checkout &&
(
3: aa9ea67180d ! 3: 760cae85bd4 sparse-checkout: limit tab completion to a single level
@@ Metadata
Author: Lessley Dennington [off-list ref]
## Commit message ##
- sparse-checkout: limit tab completion to a single level
+ completion: ensure cone mode completion with multiple <TAB>s
- Ensure only directories at the current level will be tab-completed with
- the sparse-checkout command. For example, if paths a/b/c/ and a/d/ exist
- in the current directory, running a/<TAB> will result in:
+ Ensure users in a cone mode sparse checkout can tab-complete to
+ directories at multiple levels with multiple <TAB>s. For example, if path
+ a/b/c/ exists in the current directory, running a<TAB>b<TAB> will result
+ in:
- a/b/
- a/d/
+ a/b/c/
The 'sparse-checkout completes directory names' test has also been
- updated/extended according to these changes.
+ updated with trailing slashes according to these changes.
Co-authored-by: Elijah Newren [off-list ref]
Co-authored-by: Lessley Dennington [off-list ref]
@@ contrib/completion/git-completion.bash: _git_show_branch ()
+ fi
+}
+
- __git_sparse_checkout_subcommand_opts="--cone --no-cone --sparse-index --no-sparse-index"
-
_git_sparse_checkout ()
+ {
+ local subcommands="list init set disable add reapply"
+ local subcommand="$(__git_find_on_cmdline "$subcommands")"
+-
+ if [ -z "$subcommand" ]; then
+ __gitcomp "$subcommands"
+ return
@@ contrib/completion/git-completion.bash: _git_sparse_checkout ()
- case "$subcommand" in
- set)
- __gitcomp "$__git_sparse_checkout_subcommand_opts --stdin"
-- __gitcomp "$(git ls-tree -d -r HEAD --name-only)"
-+ __gitcomp_directories
- ;;
- add)
- __gitcomp "--stdin"
-- __gitcomp "$(git ls-tree -d -r HEAD --name-only)"
-+ __gitcomp_directories
- ;;
- init|reapply)
- __gitcomp "$__git_sparse_checkout_subcommand_opts"
+ ;;
+ set,*|add,*)
+ if [ $(__git config core.sparseCheckoutCone) ]; then
+- __git_complete_index_file "--directory"
++ __gitcomp_directories
+ fi
+ ;;
+ esac
## t/t9902-completion.sh ##
-@@ t/t9902-completion.sh: test_expect_success 'sparse-checkout completes directory names' '
+@@ t/t9902-completion.sh: test_expect_success 'cone mode sparse-checkout completes directory names' '
(
cd sparse-checkout &&
test_completion "git sparse-checkout set f" <<-\EOF
-- folder1 Z
-- folder1/0 Z
-- folder1/0/1 Z
-- folder2 Z
-- folder2/0 Z
-- folder3 Z
+- folder1
+- folder2
+- folder3
+ folder1/
+ folder2/
+ folder3/
-+ EOF
-+ ) &&
-+
-+ (
-+ cd sparse-checkout &&
-+ test_completion "git sparse-checkout set folder1/" <<-\EOF
+ EOF
+ ) &&
+
+ (
+ cd sparse-checkout &&
+ test_completion "git sparse-checkout set folder1/" <<-\EOF
+- folder1/0
+ folder1/0/
-+ EOF
-+ ) &&
-+
-+ (
-+ cd sparse-checkout &&
-+ test_completion "git sparse-checkout set folder1/0/" <<-\EOF
+ EOF
+ ) &&
+
+ (
+ cd sparse-checkout &&
+ test_completion "git sparse-checkout set folder1/0/" <<-\EOF
+- folder1/0/1
+ folder1/0/1/
EOF
) &&
(
cd sparse-checkout/folder1 &&
-- test_completion "git sparse-checkout add " <<-\EOF
-- ./ Z
-- 0 Z
-- 0/1 Z
-+ test_completion "git sparse-checkout add 0" <<-\EOF
+ test_completion "git sparse-checkout add 0" <<-\EOF
+- 0
+ 0/
EOF
)
--
gitgitgadget
From: Lessley Dennington via GitGitGadget <hidden> Date: 2022-01-27 21:22:00
From: Lessley Dennington <redacted>
Add tests for missing/incorrect components of custom tab completion for
the sparse-checkout command. These tests specifically highlight the
following:
1. git sparse-checkout <TAB> results in an incomplete list of subcommands
(it is missing reapply and add).
2. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> show
both file names and directory names. While this is the correct behavior
for non-cone mode, cone mode sparse checkouts should complete only
directory names.
Although the first two of these tests currently fail, they will succeed
with the sparse-checkout modifications in git-completion.bash in the next
commit in this series.
Signed-off-by: Lessley Dennington <redacted>
---
t/t9902-completion.sh | 75 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 75 insertions(+)
@@ -1447,6 +1447,81 @@ test_expect_success 'git checkout - with --detach, complete only references' 'EOF'+test_expect_failure'sparse-checkout completes subcommands''+test_completion"git sparse-checkout "<<-\EOF+listZ+initZ+setZ+addZ+reapplyZ+disableZ+EOF+'++test_expect_failure'cone mode sparse-checkout completes directory names''+# set up sparse-checkout repo+gitinitsparse-checkout&&+(+cdsparse-checkout&&+mkdir-pfolder1/0/1folder2/0folder3&&+touchfolder1/0/1/t.txt&&+touchfolder2/0/t.txt&&+touchfolder3/t.txt&&+gitadd.&&+gitcommit-am"Initial commit"+)&&++# initialize sparse-checkout definitions+git-Csparse-checkoutsparse-checkoutset--conefolder1/0folder3&&++# test tab completion+(+cdsparse-checkout&&+test_completion"git sparse-checkout set f"<<-\EOF+folder1+folder2+folder3+EOF+)&&++(+cdsparse-checkout&&+test_completion"git sparse-checkout set folder1/"<<-\EOF+folder1/0+EOF+)&&++(+cdsparse-checkout&&+test_completion"git sparse-checkout set folder1/0/"<<-\EOF+folder1/0/1+EOF+)&&++(+cdsparse-checkout/folder1&&+test_completion"git sparse-checkout add 0"<<-\EOF+0+EOF+)+'++test_expect_success'non-cone mode sparse-checkout uses bash completion''+# reset sparse-checkout repo to non-cone mode+git-Csparse-checkoutsparse-checkoutdisable&&+git-Csparse-checkoutsparse-checkoutset&&++# test tab completion+(+cdsparse-checkout&&+# expected to be empty since we have not configured+# custom completion for non-cone mode+test_completion"git sparse-checkout set f"<<-\EOF++EOF+)+'+ test_expect_success'git switch - with -d, complete all references''test_completion"git switch -d "<<-\EOFHEADZ
From: Lessley Dennington via GitGitGadget <hidden> Date: 2022-01-27 21:22:01
From: Lessley Dennington <redacted>
Fix custom tab completion for the sparse-checkout command. This will
ensure:
1. The full list of subcommands is provided when users enter git
sparse-checkout <TAB>.
2. Subcommand options are tab-completable.
3. A list of directories (but not files) is provided when users enter git
sparse-checkout add <TAB> or git sparse-checkout set <TAB> in cone mode
only. In non-cone mode, these commands will continue to complete both
directory and file names.
Failing tests that were added in the previous commit to verify these
scenarios are now passing with these updates.
Signed-off-by: Lessley Dennington <redacted>
---
contrib/completion/git-completion.bash | 15 ++++++++-------
t/t9902-completion.sh | 4 ++--
2 files changed, 10 insertions(+), 9 deletions(-)
From: Lessley Dennington via GitGitGadget <hidden> Date: 2022-01-27 21:22:02
From: Lessley Dennington <redacted>
Ensure users in a cone mode sparse checkout can tab-complete to
directories at multiple levels with multiple <TAB>s. For example, if path
a/b/c/ exists in the current directory, running a<TAB>b<TAB> will result
in:
a/b/c/
The 'sparse-checkout completes directory names' test has also been
updated with trailing slashes according to these changes.
Co-authored-by: Elijah Newren [off-list ref]
Co-authored-by: Lessley Dennington [off-list ref]
Signed-off-by: Lessley Dennington <redacted>
---
contrib/completion/git-completion.bash | 33 ++++++++++++++++++++++++--
t/t9902-completion.sh | 12 +++++-----
2 files changed, 37 insertions(+), 8 deletions(-)
@@ -2986,11 +2986,40 @@ _git_show_branch ()__git_complete_revlist}+__gitcomp_directories()+{+local_tmp_dir_tmp_completions++# Get the directory of the current token; this differs from dirname+# in that it keeps up to the final trailing slash. If no slash found+# that's fine too.+[["$cur"=~.*/]]+_tmp_dir=$BASH_REMATCH++# Find possible directory completions, adding trailing '/' characters+_tmp_completions="$(gitls-tree-d--name-onlyHEAD$_tmp_dir|+sed-es%$%/%)"++if[[-n"$_tmp_completions"]];then+# There were some directory completions, so find ones that+# start with "$cur", the current token, and put those in COMPREPLY+locali=0cIFS=$' \t\n'+forcin$_tmp_completions;do+if[[$c=="$cur"*]];then+COMPREPLY+=("$c")+fi+done+elif[["$cur"=~/$]];then+# No possible further completions any deeper, so assume we're at+# a leaf directory and just consider it complete+__gitcomp_direct_append"$cur "+fi+}+ _git_sparse_checkout(){localsubcommands="list init set disable add reapply"localsubcommand="$(__git_find_on_cmdline"$subcommands")"-if[-z"$subcommand"];then__gitcomp"$subcommands"return
On Thu, Jan 27, 2022 at 1:21 PM Lessley Dennington via GitGitGadget
[off-list ref] wrote:
From: Lessley Dennington <redacted>
Add tests for missing/incorrect components of custom tab completion for
the sparse-checkout command. These tests specifically highlight the
following:
1. git sparse-checkout <TAB> results in an incomplete list of subcommands
(it is missing reapply and add).
2. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> show
both file names and directory names. While this is the correct behavior
s/is/may be/
(Yes, Junio declared it to be the behavior he wanted to see, but
there's a difference between "this is clearly desired behavior" and
"here's the official edict declaring which bad option is the lesser
evil" situation. As Junio pointed out, completing on files and
directories avoids one type of surprise for users -- but as I've
pointed out elsewhere, it introduces multiple other types of negative
surprises.)
quoted hunk
for non-cone mode, cone mode sparse checkouts should complete only
directory names.
Although the first two of these tests currently fail, they will succeed
with the sparse-checkout modifications in git-completion.bash in the next
commit in this series.
Signed-off-by: Lessley Dennington <redacted>
---
t/t9902-completion.sh | 75 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 75 insertions(+)
@@ -1447,6 +1447,81 @@ test_expect_success 'git checkout - with --detach, complete only references' 'EOF'+test_expect_failure'sparse-checkout completes subcommands''+test_completion"git sparse-checkout "<<-\EOF+listZ+initZ+setZ+addZ+reapplyZ+disableZ+EOF+'++test_expect_failure'cone mode sparse-checkout completes directory names''+# set up sparse-checkout repo+gitinitsparse-checkout&&+(+cdsparse-checkout&&+mkdir-pfolder1/0/1folder2/0folder3&&+touchfolder1/0/1/t.txt&&+touchfolder2/0/t.txt&&+touchfolder3/t.txt&&+gitadd.&&+gitcommit-am"Initial commit"+)&&++# initialize sparse-checkout definitions+git-Csparse-checkoutsparse-checkoutset--conefolder1/0folder3&&++# test tab completion+(+cdsparse-checkout&&+test_completion"git sparse-checkout set f"<<-\EOF+folder1+folder2+folder3+EOF+)&&++(+cdsparse-checkout&&+test_completion"git sparse-checkout set folder1/"<<-\EOF+folder1/0+EOF+)&&++(+cdsparse-checkout&&+test_completion"git sparse-checkout set folder1/0/"<<-\EOF+folder1/0/1+EOF+)&&++(+cdsparse-checkout/folder1&&+test_completion"git sparse-checkout add 0"<<-\EOF+0+EOF+)+'++test_expect_success'non-cone mode sparse-checkout uses bash completion''+# reset sparse-checkout repo to non-cone mode+git-Csparse-checkoutsparse-checkoutdisable&&+git-Csparse-checkoutsparse-checkoutset&&
Can we add a --no-cone here in preparation for the default to switch?
+
+ # test tab completion
+ (
+ cd sparse-checkout &&
+ # expected to be empty since we have not configured
+ # custom completion for non-cone mode
+ test_completion "git sparse-checkout set f" <<-\EOF
+
+ EOF
+ )
+'
+
test_expect_success 'git switch - with -d, complete all references' '
test_completion "git switch -d " <<-\EOF
HEAD Z
--
gitgitgadget
On Thu, Jan 27, 2022 at 1:21 PM Lessley Dennington via GitGitGadget
[off-list ref] wrote:
quoted hunk
From: Lessley Dennington <redacted>
Fix custom tab completion for the sparse-checkout command. This will
ensure:
1. The full list of subcommands is provided when users enter git
sparse-checkout <TAB>.
2. Subcommand options are tab-completable.
3. A list of directories (but not files) is provided when users enter git
sparse-checkout add <TAB> or git sparse-checkout set <TAB> in cone mode
only. In non-cone mode, these commands will continue to complete both
directory and file names.
Failing tests that were added in the previous commit to verify these
scenarios are now passing with these updates.
Signed-off-by: Lessley Dennington <redacted>
---
contrib/completion/git-completion.bash | 15 ++++++++-------
t/t9902-completion.sh | 4 ++--
2 files changed, 10 insertions(+), 9 deletions(-)
@@ -2988,21 +2988,22 @@ _git_show_branch () _git_sparse_checkout(){-localsubcommands="list init set disable"+localsubcommands="list init set disable add reapply"localsubcommand="$(__git_find_on_cmdline"$subcommands")"+if[-z"$subcommand"];then__gitcomp"$subcommands"returnficase"$subcommand,$cur"in-init,--*)-__gitcomp"--cone"-;;-set,--*)-__gitcomp"--stdin"+*,--*)+__gitcomp_builtinsparse-checkout_$subcommand"""--";;-*)+set,*|add,*)+if[$(__gitconfigcore.sparseCheckoutCone)];then+__git_complete_index_file"--directory"+fi
--directory only affects how untracked files are presented in `git
ls-files` (which _git_complete_index_file uses), and you're not asking
for untracked paths at all here. So you've got a useless flag.
Testing this commit out, I can verify that it completes on all tracked
paths in the index. Perhaps that was an intended intermediate step,
but if so the commit message needs to be changed since it claims this
commit is doing something else.
There's also a problem in that `git sparse-checkout --cone path<TAB>`
is not using this codepath. If the user specifies --cone, then we
should give them cone-mode-style completion the same as if they
already have the config setting set. Or, we at least need to mention
in the commit message that we don't yet correctly check for the
`--cone` option to get the right style of completion and are leaving
it for future work.
On Thu, Jan 27, 2022 at 1:21 PM Lessley Dennington via GitGitGadget
[off-list ref] wrote:
From: Lessley Dennington <redacted>
Ensure users in a cone mode sparse checkout can tab-complete to
directories at multiple levels with multiple <TAB>s. For example, if path
a/b/c/ exists in the current directory, running a<TAB>b<TAB> will result
in:
a/b/c/
Um, didn't that already happen before this commit (other than the
trailing slash)?
In my mind, the reasons to switch from __git_complete_index_file to
__gitcomp_directories are:
* __git_complete_index_file() completes on files, and we want only directories
* __git_complete_index_file() can only complete on paths stored in
the index. With --sparse-index mode, the directory (and all files
under it) we want to complete on may not exist in the index.
Both of these things make __git_complete_index_file() unsuitable.
quoted hunk
The 'sparse-checkout completes directory names' test has also been
updated with trailing slashes according to these changes.
Co-authored-by: Elijah Newren [off-list ref]
Co-authored-by: Lessley Dennington [off-list ref]
Signed-off-by: Lessley Dennington <redacted>
---
contrib/completion/git-completion.bash | 33 ++++++++++++++++++++++++--
t/t9902-completion.sh | 12 +++++-----
2 files changed, 37 insertions(+), 8 deletions(-)
@@ -2986,11 +2986,40 @@ _git_show_branch ()__git_complete_revlist}+__gitcomp_directories()+{+local_tmp_dir_tmp_completions++# Get the directory of the current token; this differs from dirname+# in that it keeps up to the final trailing slash. If no slash found+# that's fine too.+[["$cur"=~.*/]]+_tmp_dir=$BASH_REMATCH++# Find possible directory completions, adding trailing '/' characters+_tmp_completions="$(gitls-tree-d--name-onlyHEAD$_tmp_dir|+sed-es%$%/%)"++if[[-n"$_tmp_completions"]];then+# There were some directory completions, so find ones that+# start with "$cur", the current token, and put those in COMPREPLY+locali=0cIFS=$' \t\n'+forcin$_tmp_completions;do+if[[$c=="$cur"*]];then+COMPREPLY+=("$c")+fi+done+elif[["$cur"=~/$]];then+# No possible further completions any deeper, so assume we're at+# a leaf directory and just consider it complete+__gitcomp_direct_append"$cur "+fi+}
As noted at [1], this is going to have problems with special
characters. In the testcase setup in t9902, modify the repository as
follows:
mkdir "spaces are evil"
mkdir "$(printf "tabs\tare\tworse")"
>"spaces are evil/randomfile"
>"$(printf "tabs\tare\tworse")/randomfile"
git add .
git commit -m "Add directories containing unusual characters"
and then try to complete any of these:
git sparse-checkout set --cone spa<TAB>
git sparse-checkout set --cone tab<TAB>
git sparse-checkout set --cone ev<TAB>
git sparse-checkout set --cone "tab<TAB>
You'll see it either gets the wrong thing, or doesn't get nothing (and
when it should get nothing, it gets something instead).
We do definitely want to use some form of __gitcomp_directories as I
noted above, but we'll have to lift some of the intelligent quoting
logic out of __git_complete_index_file. It may be a little easier
since we don't have to worry about "full file path needs to be quoted
even though leading directory does not need to be".
(Also, if it makes it easier to deal with the testcases, you can just
use non-ascii characters instead of the tab character -- just run `git
shortlog -sn origin/master` in git.git, find someone's name with such
a character and copy and paste it into the directory name. For
example, you could use the á from Gábor)
[1] https://lore.kernel.org/git/20220115095725.GA1738@szeder.dev/
quoted hunk
+
_git_sparse_checkout ()
{
local subcommands="list init set disable add reapply"
local subcommand="$(__git_find_on_cmdline "$subcommands")"
-
if [ -z "$subcommand" ]; then
__gitcomp "$subcommands"
return
On Thu, Jan 27, 2022 at 1:21 PM Lessley Dennington via GitGitGadget
[off-list ref] wrote:
quoted
From: Lessley Dennington <redacted>
Fix custom tab completion for the sparse-checkout command. This will
ensure:
1. The full list of subcommands is provided when users enter git
sparse-checkout <TAB>.
2. Subcommand options are tab-completable.
3. A list of directories (but not files) is provided when users enter git
sparse-checkout add <TAB> or git sparse-checkout set <TAB> in cone mode
only. In non-cone mode, these commands will continue to complete both
directory and file names.
Failing tests that were added in the previous commit to verify these
scenarios are now passing with these updates.
Signed-off-by: Lessley Dennington <redacted>
---
contrib/completion/git-completion.bash | 15 ++++++++-------
t/t9902-completion.sh | 4 ++--
2 files changed, 10 insertions(+), 9 deletions(-)
@@ -2988,21 +2988,22 @@ _git_show_branch ()_git_sparse_checkout(){-localsubcommands="list init set disable"+localsubcommands="list init set disable add reapply"localsubcommand="$(__git_find_on_cmdline"$subcommands")"+if[-z"$subcommand"];then__gitcomp"$subcommands"returnficase"$subcommand,$cur"in-init,--*)-__gitcomp"--cone"-;;-set,--*)-__gitcomp"--stdin"+*,--*)+__gitcomp_builtinsparse-checkout_$subcommand"""--";;-*)+set,*|add,*)+if[$(__gitconfigcore.sparseCheckoutCone)];then+__git_complete_index_file"--directory"+fi
--directory only affects how untracked files are presented in `git
ls-files` (which _git_complete_index_file uses), and you're not asking
for untracked paths at all here. So you've got a useless flag.
Testing this commit out, I can verify that it completes on all tracked
paths in the index. Perhaps that was an intended intermediate step,
but if so the commit message needs to be changed since it claims this
commit is doing something else.
There's also a problem in that `git sparse-checkout --cone path<TAB>`
is not using this codepath. If the user specifies --cone, then we
should give them cone-mode-style completion the same as if they
already have the config setting set. Or, we at least need to mention
in the commit message that we don't yet correctly check for the
`--cone` option to get the right style of completion and are leaving
it for future work.
Thank you for your feedback, it is very helpful. As I've been thinking
about this, it feels like this commit should perhaps be dropped in favor
of the __gitcomp_directories commit (with some modifications per [1]). I
know you initially said to add the __gitcomp_directories commit in
addition to what I had, though, so I wanted to confirm whether removing
this is the right course of action.
[1]:
https://lore.kernel.org/git/CABPp-BEq9pTqsy_R_SR1DSgUK58ubNR1Gk4G1RoL8wkadyo6zw@mail.gmail.com/
On Mon, Jan 31, 2022 at 12:03 PM Lessley Dennington
[off-list ref] wrote:
On 1/27/22 5:21 PM, Elijah Newren wrote:
quoted
On Thu, Jan 27, 2022 at 1:21 PM Lessley Dennington via GitGitGadget
[off-list ref] wrote:
quoted
From: Lessley Dennington <redacted>
Fix custom tab completion for the sparse-checkout command. This will
ensure:
1. The full list of subcommands is provided when users enter git
sparse-checkout <TAB>.
2. Subcommand options are tab-completable.
3. A list of directories (but not files) is provided when users enter git
sparse-checkout add <TAB> or git sparse-checkout set <TAB> in cone mode
only. In non-cone mode, these commands will continue to complete both
directory and file names.
Failing tests that were added in the previous commit to verify these
scenarios are now passing with these updates.
Signed-off-by: Lessley Dennington <redacted>
---
contrib/completion/git-completion.bash | 15 ++++++++-------
t/t9902-completion.sh | 4 ++--
2 files changed, 10 insertions(+), 9 deletions(-)
@@ -2988,21 +2988,22 @@ _git_show_branch ()_git_sparse_checkout(){-localsubcommands="list init set disable"+localsubcommands="list init set disable add reapply"localsubcommand="$(__git_find_on_cmdline"$subcommands")"+if[-z"$subcommand"];then__gitcomp"$subcommands"returnficase"$subcommand,$cur"in-init,--*)-__gitcomp"--cone"-;;-set,--*)-__gitcomp"--stdin"+*,--*)+__gitcomp_builtinsparse-checkout_$subcommand"""--";;-*)+set,*|add,*)+if[$(__gitconfigcore.sparseCheckoutCone)];then+__git_complete_index_file"--directory"+fi
--directory only affects how untracked files are presented in `git
ls-files` (which _git_complete_index_file uses), and you're not asking
for untracked paths at all here. So you've got a useless flag.
Testing this commit out, I can verify that it completes on all tracked
paths in the index. Perhaps that was an intended intermediate step,
but if so the commit message needs to be changed since it claims this
commit is doing something else.
There's also a problem in that `git sparse-checkout --cone path<TAB>`
is not using this codepath. If the user specifies --cone, then we
should give them cone-mode-style completion the same as if they
already have the config setting set. Or, we at least need to mention
in the commit message that we don't yet correctly check for the
`--cone` option to get the right style of completion and are leaving
it for future work.
quoted
Thank you for your feedback, it is very helpful. As I've been thinking
about this, it feels like this commit should perhaps be dropped in favor
of the __gitcomp_directories commit (with some modifications per [1]). I
know you initially said to add the __gitcomp_directories commit in
addition to what I had, though, so I wanted to confirm whether removing
this is the right course of action.
[1]:
https://lore.kernel.org/git/CABPp-BEq9pTqsy_R_SR1DSgUK58ubNR1Gk4G1RoL8wkadyo6zw@mail.gmail.com/
I think the link you meant was
https://lore.kernel.org/git/CABPp-BG_Jgyr89z_D355Ytzz31J40nBGX=2cr+aXtcf3U1y6Dg@mail.gmail.com/.
Anyway...
I thought the "git ls-tree -d -r --name-only HEAD" step you did
previously was a useful intermediate step because it was really simple
and it highlighted the direction we wanted to go for cone mode -- it
just had some caveats (it was potentially slow, and it has the same
quoting issues that my patch had). I feel that sometimes an
intermediate step helps reviewers -- both current ones and folks
digging through the code years from now.
I think __git_complete_index_file() is a step in the wrong direction,
and as such adding a patch that uses it does not help people
understand the path to the end goal. So, if the choice is about
__git_complete_index_file(), then yeah, we shouldn't include that
step. But your old step 2 might have value.
I think there's a few possible choices you could make to replace your
current patches 2 & 3; including:
* 1 patch: Just use the __gitcomp_directories commit, modified to
handle the special quoting needed
* 2 patches: Insert your old "git ls-tree -d -r --name-only HEAD"
patch, made specific to cone mode, mentioning the performance and
quoting shortcomings in the commit message and stating that a
subsequent commit will address those issues. Then add the
__gitcomp_directories commit, modified to handle special quoting
* 3 patches: First commit same as the case above. Then add the
__gitcomp_directories commit without changing it to handle special
path quoting (i.e. the second patch only addresses the performance
shortcoming). Then add a 3rd commit handling the special path quoting
(addressing the remaining issue).
* some other split that occurs to you while you do the work.
Of the first three, I have a small preference for replacing patches 2
& 3 from this round with the 3-patch replacement above (making your
series be 4 patches total), because it provides a "10,000 ft overview"
before jumping into the weeds, it naturally documents why a simpler
solution that might occur to folks isn't good enough, it allows
reviewers to focus on just performance or on just special quoting when
reviewing so they don't have to try to guess which end goal each line
of code is working to achieve as they attempt to review. But I'm
partially guessing at how much work is involved in the special path
quoting, and at how localized each part of the solution might be. If
either of the performance or special quoting concerns are nicely
localized in the patch anyway, then the separation of commits becomes
less important. And perhaps a comment in the commit message is good
enough to provide the 10,000 ft overview so you don't need the
explicit "git ls-tree -d -r --name-only HEAD" step.
Anyway, that's how I think about it. Other than the fact that
__git_complete_index_file() should not be included as a step, any of
the choices could be fine so I'll leave it up to you.
From: Lessley Dennington via GitGitGadget <hidden> Date: 2022-02-03 20:47:04
From: Lessley Dennington <redacted>
Correct multiple issues with tab completion of the git sparse-checkout
command. These issues were:
1. git sparse-checkout <TAB> previously resulted in an incomplete list of
subcommands (it was missing reapply and add).
2. Subcommand options were not tab-completable.
3. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> showed
both file names and directory names. While this may be a less surprising
behavior for non-cone mode, cone mode sparse checkouts should complete
only directory names.
Note that while the new strategy of just using git ls-tree to complete on
directory names is simple and a step in the right direction, it does have
some caveats. These are:
1. Likelihood of poor performance in large monorepos (as a result of
recursively completing directory names).
2. Inability to handle paths containing unusual characters.
These caveats will be fixed by subsequent commits in this series.
Signed-off-by: Lessley Dennington <redacted>
---
contrib/completion/git-completion.bash | 16 ++---
t/t9902-completion.sh | 83 ++++++++++++++++++++++++++
2 files changed, 91 insertions(+), 8 deletions(-)
@@ -1447,6 +1447,89 @@ test_expect_success 'git checkout - with --detach, complete only references' 'EOF'+test_expect_success'setup sparse-checkout tests''+# set up sparse-checkout repo+gitinitsparse-checkout&&+(+cdsparse-checkout&&+mkdir-pfolder1/0/1folder2/0folder3&&+touchfolder1/0/1/t.txt&&+touchfolder2/0/t.txt&&+touchfolder3/t.txt&&+gitadd.&&+gitcommit-am"Initial commit"+)+'++test_expect_success'sparse-checkout completes subcommands''+test_completion"git sparse-checkout "<<-\EOF+listZ+initZ+setZ+addZ+reapplyZ+disableZ+EOF+'++test_expect_success'cone mode sparse-checkout completes directory names''+# initialize sparse-checkout definitions+git-Csparse-checkoutsparse-checkoutset--conefolder1/0folder3&&++# test tab completion+(+cdsparse-checkout&&+test_completion"git sparse-checkout set f"<<-\EOF+folder1Z+folder1/0Z+folder1/0/1Z+folder2Z+folder2/0Z+folder3Z+EOF+)&&++(+cdsparse-checkout/folder1&&+test_completion"git sparse-checkout add "<<-\EOF+./Z+0Z+0/1Z+EOF+)+'++test_expect_success'non-cone mode sparse-checkout uses bash completion''+# reset sparse-checkout repo to non-cone mode+git-Csparse-checkoutsparse-checkoutdisable&&+git-Csparse-checkoutsparse-checkoutset--no-cone&&++(+cdsparse-checkout&&+# expected to be empty since we have not configured+# custom completion for non-cone mode+test_completion"git sparse-checkout set f"<<-\EOF++EOF+)+'++test_expect_success'git sparse-checkout set --cone completes directory names''+git-Csparse-checkoutsparse-checkoutdisable&&++(+cdsparse-checkout&&+test_completion"git sparse-checkout set --cone f"<<-\EOF+folder1Z+folder1/0Z+folder1/0/1Z+folder2Z+folder2/0Z+folder3Z+EOF+)+'+ test_expect_success'git switch - with -d, complete all references''test_completion"git switch -d "<<-\EOFHEADZ
From: Lessley Dennington via GitGitGadget <hidden> Date: 2022-02-03 20:47:05
This series is based on en/sparse-checkout-set. It updates custom tab
completion for the sparse-checkout command. Specifically, it corrects the
following issues with the current method:
1. git sparse-checkout <TAB> results in an incomplete list of subcommands
(it is missing reapply and add).
2. Options for subcommands are not tab-completable.
3. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> show
both file names and directory names. While this may be a less surprising
behavior for non-cone mode, we want to only show directories in cone
mode.
The first commit in this series is an intermediate step that fixes issues 1
and 2 above and introduces a simple fix for issue 3 with some performance
and unusual character-related caveats. The next commit adds a new
__gitcomp_directories method that fixes the performance-related caveat from
the first commit by completing just a single level of directories. The final
commit modifies __gitcomp_directories to handle unusual characters in
directory names.
Changes since V4
================
* Clarify that completing on files and directories is a less-surprising
behavior in non-cone mode (rather than definitely the correct behavior)
in commit message and cover letter.
* Add explicit --no-cone to non-cone mode sparse-checkout completion test.
* Replace use of __git_complete_index_file with a new version of
__gitcomp_directories that is able to handle unusual characters.
* Ensure that users running git sparse-checkout set --cone get cone
mode-style completion whether or not they already have the cone mode
config set.
* Refactor creation of sparse-checkout test repo into a sparse-checkout
setup test.
Changes since V3
================
* Update commit descriptions to begin with completion instead of
sparse-checkout.
* Remove explicit completion of --help, as this is inconsistent with the
rest of the completion script.
* Use __gitcomp_builtin to auto-complete subcommand options (and remove
option completion test from previous iterations, as it is no longer
necessary).
* Use __git_complete_index_file helper function in place of __gitcomp
"$(git ls-tree -d -r HEAD --name-only).
* If in cone-mode sparse checkout, only show directory completions. If in
non-cone mode sparse checkout show both files and directories.
Changes since V2
================
* Change use of $prev to $subcommand in _git_sparse_checkout() method in
git-completion.bash.
* State explicitly that directory completion applies in both cone and
non-cone mode in 'sparse-checkout: custom tab completion' commit
* Add new patch with __gitcomp_directories method to improve performance by
only outputting directories at the current level.
Changes since V1
================
* Rebase onto en/sparse-checkout-set.
* Add subcommand options (including --no-cone) for set and reapply.
* Extend 'sparse-checkout completes subcommand options' test to validate
new set/reapply subcommand options.
* No longer set index.sparse to false in 'sparse-checkout completes
directory names' test.
* Refactor creation of sparse-checkout repo into a setup test.
Thanks, Lessley
Lessley Dennington (3):
completion: address sparse-checkout issues
completion: improve sparse-checkout cone mode directory completion
completion: handle unusual characters for sparse-checkout
contrib/completion/git-completion.bash | 44 ++++++++--
t/t9902-completion.sh | 117 +++++++++++++++++++++++++
2 files changed, 153 insertions(+), 8 deletions(-)
base-commit: dfac9b609f86cd4f6ce896df9e1172d2a02cde48
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1108%2Fldennington%2Fsparse-checkout-bash-completion-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1108/ldennington/sparse-checkout-bash-completion-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/1108
Range-diff vs v4:
1: 5bb598a055d ! 1: a1c46c763fd completion: add sparse-checkout tests
@@ Metadata
Author: Lessley Dennington [off-list ref]
## Commit message ##
- completion: add sparse-checkout tests
+ completion: address sparse-checkout issues
- Add tests for missing/incorrect components of custom tab completion for
- the sparse-checkout command. These tests specifically highlight the
- following:
+ Correct multiple issues with tab completion of the git sparse-checkout
+ command. These issues were:
- 1. git sparse-checkout <TAB> results in an incomplete list of subcommands
- (it is missing reapply and add).
- 2. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> show
- both file names and directory names. While this is the correct behavior
- for non-cone mode, cone mode sparse checkouts should complete only
- directory names.
+ 1. git sparse-checkout <TAB> previously resulted in an incomplete list of
+ subcommands (it was missing reapply and add).
+ 2. Subcommand options were not tab-completable.
+ 3. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> showed
+ both file names and directory names. While this may be a less surprising
+ behavior for non-cone mode, cone mode sparse checkouts should complete
+ only directory names.
- Although the first two of these tests currently fail, they will succeed
- with the sparse-checkout modifications in git-completion.bash in the next
- commit in this series.
+ Note that while the new strategy of just using git ls-tree to complete on
+ directory names is simple and a step in the right direction, it does have
+ some caveats. These are:
+
+ 1. Likelihood of poor performance in large monorepos (as a result of
+ recursively completing directory names).
+ 2. Inability to handle paths containing unusual characters.
+
+ These caveats will be fixed by subsequent commits in this series.
Signed-off-by: Lessley Dennington [off-list ref]
+ ## contrib/completion/git-completion.bash ##
+@@ contrib/completion/git-completion.bash: _git_show_branch ()
+
+ _git_sparse_checkout ()
+ {
+- local subcommands="list init set disable"
++ local subcommands="list init set disable add reapply"
+ local subcommand="$(__git_find_on_cmdline "$subcommands")"
+ if [ -z "$subcommand" ]; then
+ __gitcomp "$subcommands"
+@@ contrib/completion/git-completion.bash: _git_sparse_checkout ()
+ fi
+
+ case "$subcommand,$cur" in
+- init,--*)
+- __gitcomp "--cone"
+- ;;
+- set,--*)
+- __gitcomp "--stdin"
+- ;;
+- *)
++ *,--*)
++ __gitcomp_builtin sparse-checkout_$subcommand "" "--"
+ ;;
++ set,*|add,*)
++ if [ $(__git config core.sparseCheckoutCone) ] ||
++ [ "$(__git_find_on_cmdline --cone)" ]; then
++ __gitcomp "$(git ls-tree -d -r HEAD --name-only)"
++ fi
+ esac
+ }
+
+
## t/t9902-completion.sh ##
@@ t/t9902-completion.sh: test_expect_success 'git checkout - with --detach, complete only references' '
EOF
'
-+test_expect_failure 'sparse-checkout completes subcommands' '
-+ test_completion "git sparse-checkout " <<-\EOF
-+ list Z
-+ init Z
-+ set Z
-+ add Z
-+ reapply Z
-+ disable Z
-+ EOF
-+'
-+
-+test_expect_failure 'cone mode sparse-checkout completes directory names' '
++test_expect_success 'setup sparse-checkout tests' '
+ # set up sparse-checkout repo
+ git init sparse-checkout &&
+ (
@@ t/t9902-completion.sh: test_expect_success 'git checkout - with --detach, comple
+ touch folder3/t.txt &&
+ git add . &&
+ git commit -am "Initial commit"
-+ ) &&
++ )
++'
++
++test_expect_success 'sparse-checkout completes subcommands' '
++ test_completion "git sparse-checkout " <<-\EOF
++ list Z
++ init Z
++ set Z
++ add Z
++ reapply Z
++ disable Z
++ EOF
++'
+
++test_expect_success 'cone mode sparse-checkout completes directory names' '
+ # initialize sparse-checkout definitions
+ git -C sparse-checkout sparse-checkout set --cone folder1/0 folder3 &&
+
@@ t/t9902-completion.sh: test_expect_success 'git checkout - with --detach, comple
+ (
+ cd sparse-checkout &&
+ test_completion "git sparse-checkout set f" <<-\EOF
-+ folder1
-+ folder2
-+ folder3
-+ EOF
-+ ) &&
-+
-+ (
-+ cd sparse-checkout &&
-+ test_completion "git sparse-checkout set folder1/" <<-\EOF
-+ folder1/0
-+ EOF
-+ ) &&
-+
-+ (
-+ cd sparse-checkout &&
-+ test_completion "git sparse-checkout set folder1/0/" <<-\EOF
-+ folder1/0/1
++ folder1 Z
++ folder1/0 Z
++ folder1/0/1 Z
++ folder2 Z
++ folder2/0 Z
++ folder3 Z
+ EOF
+ ) &&
+
+ (
+ cd sparse-checkout/folder1 &&
-+ test_completion "git sparse-checkout add 0" <<-\EOF
-+ 0
++ test_completion "git sparse-checkout add " <<-\EOF
++ ./ Z
++ 0 Z
++ 0/1 Z
+ EOF
+ )
+'
@@ t/t9902-completion.sh: test_expect_success 'git checkout - with --detach, comple
+test_expect_success 'non-cone mode sparse-checkout uses bash completion' '
+ # reset sparse-checkout repo to non-cone mode
+ git -C sparse-checkout sparse-checkout disable &&
-+ git -C sparse-checkout sparse-checkout set &&
++ git -C sparse-checkout sparse-checkout set --no-cone &&
+
-+ # test tab completion
+ (
+ cd sparse-checkout &&
+ # expected to be empty since we have not configured
@@ t/t9902-completion.sh: test_expect_success 'git checkout - with --detach, comple
+ EOF
+ )
+'
++
++test_expect_success 'git sparse-checkout set --cone completes directory names' '
++ git -C sparse-checkout sparse-checkout disable &&
++
++ (
++ cd sparse-checkout &&
++ test_completion "git sparse-checkout set --cone f" <<-\EOF
++ folder1 Z
++ folder1/0 Z
++ folder1/0/1 Z
++ folder2 Z
++ folder2/0 Z
++ folder3 Z
++ EOF
++ )
++'
+
test_expect_success 'git switch - with -d, complete all references' '
test_completion "git switch -d " <<-\EOF
2: 7d70beb2a6b < -: ----------- completion: sparse-checkout updates
3: 760cae85bd4 ! 2: 5b8d53402ae completion: ensure cone mode completion with multiple <TAB>s
@@ Metadata
Author: Lessley Dennington [off-list ref]
## Commit message ##
- completion: ensure cone mode completion with multiple <TAB>s
+ completion: improve sparse-checkout cone mode directory completion
- Ensure users in a cone mode sparse checkout can tab-complete to
- directories at multiple levels with multiple <TAB>s. For example, if path
- a/b/c/ exists in the current directory, running a<TAB>b<TAB> will result
- in:
+ Use new __gitcomp_directories method to complete directory names in cone
+ mode sparse-checkouts. This method addresses the caveat of poor
+ performance in monorepos from the previous commit (by completing only one
+ level of directories).
- a/b/c/
-
- The 'sparse-checkout completes directory names' test has also been
- updated with trailing slashes according to these changes.
+ The unusual character caveat from the previous commit will be fixed by the
+ final commit in this series.
Co-authored-by: Elijah Newren [off-list ref]
Co-authored-by: Lessley Dennington [off-list ref]
@@ contrib/completion/git-completion.bash: _git_show_branch ()
_git_sparse_checkout ()
{
local subcommands="list init set disable add reapply"
- local subcommand="$(__git_find_on_cmdline "$subcommands")"
--
- if [ -z "$subcommand" ]; then
- __gitcomp "$subcommands"
- return
@@ contrib/completion/git-completion.bash: _git_sparse_checkout ()
- ;;
- set,*|add,*)
- if [ $(__git config core.sparseCheckoutCone) ]; then
-- __git_complete_index_file "--directory"
-+ __gitcomp_directories
- fi
- ;;
+ set,*|add,*)
+ if [ $(__git config core.sparseCheckoutCone) ] ||
+ [ "$(__git_find_on_cmdline --cone)" ]; then
+- __gitcomp "$(git ls-tree -d -r HEAD --name-only)"
++ __gitcomp_directories
+ fi
esac
+ }
## t/t9902-completion.sh ##
@@ t/t9902-completion.sh: test_expect_success 'cone mode sparse-checkout completes directory names' '
(
cd sparse-checkout &&
test_completion "git sparse-checkout set f" <<-\EOF
-- folder1
-- folder2
-- folder3
+- folder1 Z
+- folder1/0 Z
+- folder1/0/1 Z
+- folder2 Z
+- folder2/0 Z
+- folder3 Z
+ folder1/
+ folder2/
+ folder3/
- EOF
- ) &&
-
- (
- cd sparse-checkout &&
- test_completion "git sparse-checkout set folder1/" <<-\EOF
-- folder1/0
++ EOF
++ ) &&
++
++ (
++ cd sparse-checkout &&
++ test_completion "git sparse-checkout set folder1/" <<-\EOF
+ folder1/0/
- EOF
- ) &&
-
- (
- cd sparse-checkout &&
- test_completion "git sparse-checkout set folder1/0/" <<-\EOF
-- folder1/0/1
++ EOF
++ ) &&
++
++ (
++ cd sparse-checkout &&
++ test_completion "git sparse-checkout set folder1/0/" <<-\EOF
+ folder1/0/1/
EOF
) &&
(
cd sparse-checkout/folder1 &&
- test_completion "git sparse-checkout add 0" <<-\EOF
-- 0
+- test_completion "git sparse-checkout add " <<-\EOF
+- ./ Z
+- 0 Z
+- 0/1 Z
++ test_completion "git sparse-checkout add 0" <<-\EOF
+ 0/
EOF
)
'
+@@ t/t9902-completion.sh: test_expect_success 'git sparse-checkout set --cone completes directory names' '
+ (
+ cd sparse-checkout &&
+ test_completion "git sparse-checkout set --cone f" <<-\EOF
+- folder1 Z
+- folder1/0 Z
+- folder1/0/1 Z
+- folder2 Z
+- folder2/0 Z
+- folder3 Z
++ folder1/
++ folder2/
++ folder3/
+ EOF
+ )
+ '
-: ----------- > 3: ddf5e583dd7 completion: handle unusual characters for sparse-checkout
--
gitgitgadget
From: Lessley Dennington via GitGitGadget <hidden> Date: 2022-02-03 20:47:16
From: Lessley Dennington <redacted>
Use new __gitcomp_directories method to complete directory names in cone
mode sparse-checkouts. This method addresses the caveat of poor
performance in monorepos from the previous commit (by completing only one
level of directories).
The unusual character caveat from the previous commit will be fixed by the
final commit in this series.
Co-authored-by: Elijah Newren [off-list ref]
Co-authored-by: Lessley Dennington [off-list ref]
Signed-off-by: Lessley Dennington <redacted>
---
contrib/completion/git-completion.bash | 32 +++++++++++++++++++++-
t/t9902-completion.sh | 38 +++++++++++++++-----------
2 files changed, 53 insertions(+), 17 deletions(-)
@@ -2986,6 +2986,36 @@ _git_show_branch ()__git_complete_revlist}+__gitcomp_directories()+{+local_tmp_dir_tmp_completions++# Get the directory of the current token; this differs from dirname+# in that it keeps up to the final trailing slash. If no slash found+# that's fine too.+[["$cur"=~.*/]]+_tmp_dir=$BASH_REMATCH++# Find possible directory completions, adding trailing '/' characters+_tmp_completions="$(gitls-tree-d--name-onlyHEAD$_tmp_dir|+sed-es%$%/%)"++if[[-n"$_tmp_completions"]];then+# There were some directory completions, so find ones that+# start with "$cur", the current token, and put those in COMPREPLY+locali=0cIFS=$' \t\n'+forcin$_tmp_completions;do+if[[$c=="$cur"*]];then+COMPREPLY+=("$c")+fi+done+elif[["$cur"=~/$]];then+# No possible further completions any deeper, so assume we're at+# a leaf directory and just consider it complete+__gitcomp_direct_append"$cur "+fi+}+ _git_sparse_checkout(){localsubcommands="list init set disable add reapply"
From: Lessley Dennington via GitGitGadget <hidden> Date: 2022-02-03 20:47:18
From: Lessley Dennington <redacted>
Update the __gitcomp_directories method to de-quote and handle unusual
characters in directory names. Although this initially involved an attempt
to re-use the logic in __git_index_files, this method removed
subdirectories (e.g. folder1/0/ became folder1/), so instead new custom
logic was placed directly in the __gitcomp_directories method.
Co-authored-by: Johannes Schindelin [off-list ref]
Co-authored-by: Lessley Dennington [off-list ref]
Signed-off-by: Lessley Dennington <redacted>
---
contrib/completion/git-completion.bash | 50 +++++++++++++-------------
t/t9902-completion.sh | 28 +++++++++++++++
2 files changed, 52 insertions(+), 26 deletions(-)
@@ -2988,32 +2988,30 @@ _git_show_branch () __gitcomp_directories(){-local_tmp_dir_tmp_completions--# Get the directory of the current token; this differs from dirname-# in that it keeps up to the final trailing slash. If no slash found-# that's fine too.-[["$cur"=~.*/]]-_tmp_dir=$BASH_REMATCH--# Find possible directory completions, adding trailing '/' characters-_tmp_completions="$(gitls-tree-d--name-onlyHEAD$_tmp_dir|-sed-es%$%/%)"--if[[-n"$_tmp_completions"]];then-# There were some directory completions, so find ones that-# start with "$cur", the current token, and put those in COMPREPLY-locali=0cIFS=$' \t\n'-forcin$_tmp_completions;do-if[[$c=="$cur"*]];then-COMPREPLY+=("$c")-fi-done-elif[["$cur"=~/$]];then-# No possible further completions any deeper, so assume we're at-# a leaf directory and just consider it complete-__gitcomp_direct_append"$cur "-fi+local_tmp_dir_tmp_completions_found=0++# Get the directory of the current token; this differs from dirname+# in that it keeps up to the final trailing slash. If no slash found+# that's fine too.+[["$cur"=~.*/]]+_tmp_dir=$BASH_REMATCH++# Find possible directory completions, adding trailing '/' characters,+# de-quoting, and handling unusual characters.+whileIFS=read-r-d$'\0'c;do+# If there are directory completions, find ones that start+# with "$cur", the current token, and put those in COMPREPLY+if[[$c=="$cur"*]];then+COMPREPLY+=("$c/")+_found=1+fi+done<<(gitls-tree-z-d--name-onlyHEAD$_tmp_dir)++if[[$_found==0]]&&[["$cur"=~/$]];then+# No possible further completions any deeper, so assume we're at+# a leaf directory and just consider it complete+__gitcomp_direct_append"$cur "+fi} _git_sparse_checkout()
On Thu, Feb 3, 2022 at 12:44 PM Lessley Dennington via GitGitGadget
[off-list ref] wrote:
quoted hunk
From: Lessley Dennington <redacted>
Correct multiple issues with tab completion of the git sparse-checkout
command. These issues were:
1. git sparse-checkout <TAB> previously resulted in an incomplete list of
subcommands (it was missing reapply and add).
2. Subcommand options were not tab-completable.
3. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> showed
both file names and directory names. While this may be a less surprising
behavior for non-cone mode, cone mode sparse checkouts should complete
only directory names.
Note that while the new strategy of just using git ls-tree to complete on
directory names is simple and a step in the right direction, it does have
some caveats. These are:
1. Likelihood of poor performance in large monorepos (as a result of
recursively completing directory names).
2. Inability to handle paths containing unusual characters.
These caveats will be fixed by subsequent commits in this series.
Signed-off-by: Lessley Dennington <redacted>
---
contrib/completion/git-completion.bash | 16 ++---
t/t9902-completion.sh | 83 ++++++++++++++++++++++++++
2 files changed, 91 insertions(+), 8 deletions(-)
Shouldn't this be
if [ "$(__git config core.sparseCheckoutCone)" == "true" ] ||
otherwise I think you'd trigger when it's manually set to false.
+ [ "$(__git_find_on_cmdline --cone)" ]; then
This might read better as
[ -n "$(__git_find_on_cmdline --cone)" ]; then
because otherwise you are relying on
[ "--cone" ]
to evaluate as true which just seems slightly weird.
quoted hunk
+ __gitcomp "$(git ls-tree -d -r HEAD --name-only)"
+ fi
esac
}
@@ -1447,6 +1447,89 @@ test_expect_success 'git checkout - with --detach, complete only references' 'EOF'+test_expect_success'setup sparse-checkout tests''+# set up sparse-checkout repo+gitinitsparse-checkout&&+(+cdsparse-checkout&&+mkdir-pfolder1/0/1folder2/0folder3&&+touchfolder1/0/1/t.txt&&+touchfolder2/0/t.txt&&+touchfolder3/t.txt&&+gitadd.&&+gitcommit-am"Initial commit"+)+'++test_expect_success'sparse-checkout completes subcommands''+test_completion"git sparse-checkout "<<-\EOF+listZ+initZ+setZ+addZ+reapplyZ+disableZ+EOF+'++test_expect_success'cone mode sparse-checkout completes directory names''+# initialize sparse-checkout definitions+git-Csparse-checkoutsparse-checkoutset--conefolder1/0folder3&&++# test tab completion+(+cdsparse-checkout&&+test_completion"git sparse-checkout set f"<<-\EOF+folder1Z+folder1/0Z+folder1/0/1Z+folder2Z+folder2/0Z+folder3Z+EOF+)&&++(+cdsparse-checkout/folder1&&+test_completion"git sparse-checkout add "<<-\EOF+./Z+0Z+0/1Z+EOF+)+'++test_expect_success'non-cone mode sparse-checkout uses bash completion''+# reset sparse-checkout repo to non-cone mode+git-Csparse-checkoutsparse-checkoutdisable&&+git-Csparse-checkoutsparse-checkoutset--no-cone&&++(+cdsparse-checkout&&+# expected to be empty since we have not configured+# custom completion for non-cone mode+test_completion"git sparse-checkout set f"<<-\EOF++EOF+)+'++test_expect_success'git sparse-checkout set --cone completes directory names''+git-Csparse-checkoutsparse-checkoutdisable&&++(+cdsparse-checkout&&+test_completion"git sparse-checkout set --cone f"<<-\EOF+folder1Z+folder1/0Z+folder1/0/1Z+folder2Z+folder2/0Z+folder3Z+EOF+)+'+ test_expect_success'git switch - with -d, complete all references''test_completion"git switch -d "<<-\EOFHEADZ
On Thu, Feb 3, 2022 at 12:44 PM Lessley Dennington via GitGitGadget
[off-list ref] wrote:
quoted hunk
From: Lessley Dennington <redacted>
Update the __gitcomp_directories method to de-quote and handle unusual
characters in directory names. Although this initially involved an attempt
to re-use the logic in __git_index_files, this method removed
subdirectories (e.g. folder1/0/ became folder1/), so instead new custom
logic was placed directly in the __gitcomp_directories method.
Co-authored-by: Johannes Schindelin [off-list ref]
Co-authored-by: Lessley Dennington [off-list ref]
Signed-off-by: Lessley Dennington <redacted>
---
contrib/completion/git-completion.bash | 50 +++++++++++++-------------
t/t9902-completion.sh | 28 +++++++++++++++
2 files changed, 52 insertions(+), 26 deletions(-)
@@ -2988,32 +2988,30 @@ _git_show_branch () __gitcomp_directories(){-local_tmp_dir_tmp_completions--# Get the directory of the current token; this differs from dirname-# in that it keeps up to the final trailing slash. If no slash found-# that's fine too.-[["$cur"=~.*/]]-_tmp_dir=$BASH_REMATCH--# Find possible directory completions, adding trailing '/' characters-_tmp_completions="$(gitls-tree-d--name-onlyHEAD$_tmp_dir|-sed-es%$%/%)"--if[[-n"$_tmp_completions"]];then-# There were some directory completions, so find ones that-# start with "$cur", the current token, and put those in COMPREPLY-locali=0cIFS=$' \t\n'-forcin$_tmp_completions;do-if[[$c=="$cur"*]];then-COMPREPLY+=("$c")-fi-done-elif[["$cur"=~/$]];then-# No possible further completions any deeper, so assume we're at-# a leaf directory and just consider it complete-__gitcomp_direct_append"$cur "-fi+local_tmp_dir_tmp_completions_found=0++# Get the directory of the current token; this differs from dirname+# in that it keeps up to the final trailing slash. If no slash found+# that's fine too.+[["$cur"=~.*/]]+_tmp_dir=$BASH_REMATCH++# Find possible directory completions, adding trailing '/' characters,+# de-quoting, and handling unusual characters.+whileIFS=read-r-d$'\0'c;do+# If there are directory completions, find ones that start+# with "$cur", the current token, and put those in COMPREPLY+if[[$c=="$cur"*]];then+COMPREPLY+=("$c/")+_found=1+fi+done<<(gitls-tree-z-d--name-onlyHEAD$_tmp_dir)++if[[$_found==0]]&&[["$cur"=~/$]];then+# No possible further completions any deeper, so assume we're at+# a leaf directory and just consider it complete+__gitcomp_direct_append"$cur "+fi
The indentation changes are distracting and make the patch harder to
review. Could you either remove those, or apply the indentation
changes to patch 2 so that it starts with the right indentation?
I'm slightly surprised that __gitcomp_direct_append handles the
quoting for us, but the testcases below seem to cover it, so that's
cool.
Anyway, looks pretty clever to me; I was worried this was going to
require a much bigger change.
@@ -1508,6 +1508,34 @@ test_expect_success 'cone mode sparse-checkout completes directory names' ')'+# use FUNNYNAMES to avoid running on Windows, which doesn't permit backslashes in paths+test_expect_successFUNNYNAMES'cone mode sparse-checkout completes directory names with special characters''+# reset sparse-checkout+git-Csparse-checkoutsparse-checkoutdisable&&+(+cdsparse-checkout&&+mkdir"directory with spaces"&&+mkdir"$(printf"directory\twith\ttabs")"&&+mkdir"directory\with\backslashes"&&+mkdir"directory-with-áccent"&&+>"directory with spaces/randomfile"&&+>"$(printf"directory\twith\ttabs")/randomfile"&&+>"directory\with\backslashes/randomfile"&&+>"directory-with-áccent/randomfile"&&+gitadd.&&+gitcommit-m"Add directories containing unusual characters"&&+gitsparse-checkoutset--cone"directory with spaces"\+"$(printf"directory\twith\ttabs")""directory\with\backslashes"\+"directory-with-áccent"&&+test_completion"git sparse-checkout add dir"<<-\EOF+directorywithspaces/+directorywithtabs/+directory\with\backslashes/+directory-with-áccent/+EOF+)+'
I'm glad you tested with lots of special characters -- spaces, tabs,
backslashes, and accents. Newlines might also be nice, but probably
makes the test hard. Anyway, looks good to me, other than the
indentation change.
On Thu, Feb 3, 2022 at 12:44 PM Lessley Dennington via GitGitGadget
[off-list ref] wrote:
quoted
From: Lessley Dennington <redacted>
Correct multiple issues with tab completion of the git sparse-checkout
command. These issues were:
1. git sparse-checkout <TAB> previously resulted in an incomplete list of
subcommands (it was missing reapply and add).
2. Subcommand options were not tab-completable.
3. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> showed
both file names and directory names. While this may be a less surprising
behavior for non-cone mode, cone mode sparse checkouts should complete
only directory names.
Note that while the new strategy of just using git ls-tree to complete on
directory names is simple and a step in the right direction, it does have
some caveats. These are:
1. Likelihood of poor performance in large monorepos (as a result of
recursively completing directory names).
2. Inability to handle paths containing unusual characters.
These caveats will be fixed by subsequent commits in this series.
Signed-off-by: Lessley Dennington <redacted>
---
contrib/completion/git-completion.bash | 16 ++---
t/t9902-completion.sh | 83 ++++++++++++++++++++++++++
2 files changed, 91 insertions(+), 8 deletions(-)
Shouldn't this be
if [ "$(__git config core.sparseCheckoutCone)" == "true" ] ||
otherwise I think you'd trigger when it's manually set to false.
quoted
+ [ "$(__git_find_on_cmdline --cone)" ]; then
This might read better as
[ -n "$(__git_find_on_cmdline --cone)" ]; then
because otherwise you are relying on
[ "--cone" ]
to evaluate as true which just seems slightly weird.
Yup, this was causing some problems. Odd that the failure only cropped up
when merged with 'seen' but I have a fix prepared and will submit as soon
as GGG CI completes.
quoted
+ __gitcomp "$(git ls-tree -d -r HEAD --name-only)"
+ fi
esac
}
@@ -1447,6 +1447,89 @@ test_expect_success 'git checkout - with --detach, complete only references' 'EOF'+test_expect_success'setup sparse-checkout tests''+# set up sparse-checkout repo+gitinitsparse-checkout&&+(+cdsparse-checkout&&+mkdir-pfolder1/0/1folder2/0folder3&&+touchfolder1/0/1/t.txt&&+touchfolder2/0/t.txt&&+touchfolder3/t.txt&&+gitadd.&&+gitcommit-am"Initial commit"+)+'++test_expect_success'sparse-checkout completes subcommands''+test_completion"git sparse-checkout "<<-\EOF+listZ+initZ+setZ+addZ+reapplyZ+disableZ+EOF+'++test_expect_success'cone mode sparse-checkout completes directory names''+# initialize sparse-checkout definitions+git-Csparse-checkoutsparse-checkoutset--conefolder1/0folder3&&++# test tab completion+(+cdsparse-checkout&&+test_completion"git sparse-checkout set f"<<-\EOF+folder1Z+folder1/0Z+folder1/0/1Z+folder2Z+folder2/0Z+folder3Z+EOF+)&&++(+cdsparse-checkout/folder1&&+test_completion"git sparse-checkout add "<<-\EOF+./Z+0Z+0/1Z+EOF+)+'++test_expect_success'non-cone mode sparse-checkout uses bash completion''+# reset sparse-checkout repo to non-cone mode+git-Csparse-checkoutsparse-checkoutdisable&&+git-Csparse-checkoutsparse-checkoutset--no-cone&&++(+cdsparse-checkout&&+# expected to be empty since we have not configured+# custom completion for non-cone mode+test_completion"git sparse-checkout set f"<<-\EOF++EOF+)+'++test_expect_success'git sparse-checkout set --cone completes directory names''+git-Csparse-checkoutsparse-checkoutdisable&&++(+cdsparse-checkout&&+test_completion"git sparse-checkout set --cone f"<<-\EOF+folder1Z+folder1/0Z+folder1/0/1Z+folder2Z+folder2/0Z+folder3Z+EOF+)+'+test_expect_success'git switch - with -d, complete all references''test_completion"git switch -d "<<-\EOFHEADZ
On Thu, Feb 3, 2022 at 12:44 PM Lessley Dennington via GitGitGadget
[off-list ref] wrote:
quoted
From: Lessley Dennington <redacted>
Update the __gitcomp_directories method to de-quote and handle unusual
characters in directory names. Although this initially involved an attempt
to re-use the logic in __git_index_files, this method removed
subdirectories (e.g. folder1/0/ became folder1/), so instead new custom
logic was placed directly in the __gitcomp_directories method.
Co-authored-by: Johannes Schindelin [off-list ref]
Co-authored-by: Lessley Dennington [off-list ref]
Signed-off-by: Lessley Dennington <redacted>
---
contrib/completion/git-completion.bash | 50 +++++++++++++-------------
t/t9902-completion.sh | 28 +++++++++++++++
2 files changed, 52 insertions(+), 26 deletions(-)
@@ -2988,32 +2988,30 @@ _git_show_branch ()__gitcomp_directories(){-local_tmp_dir_tmp_completions--# Get the directory of the current token; this differs from dirname-# in that it keeps up to the final trailing slash. If no slash found-# that's fine too.-[["$cur"=~.*/]]-_tmp_dir=$BASH_REMATCH--# Find possible directory completions, adding trailing '/' characters-_tmp_completions="$(gitls-tree-d--name-onlyHEAD$_tmp_dir|-sed-es%$%/%)"--if[[-n"$_tmp_completions"]];then-# There were some directory completions, so find ones that-# start with "$cur", the current token, and put those in COMPREPLY-locali=0cIFS=$' \t\n'-forcin$_tmp_completions;do-if[[$c=="$cur"*]];then-COMPREPLY+=("$c")-fi-done-elif[["$cur"=~/$]];then-# No possible further completions any deeper, so assume we're at-# a leaf directory and just consider it complete-__gitcomp_direct_append"$cur "-fi+local_tmp_dir_tmp_completions_found=0++# Get the directory of the current token; this differs from dirname+# in that it keeps up to the final trailing slash. If no slash found+# that's fine too.+[["$cur"=~.*/]]+_tmp_dir=$BASH_REMATCH++# Find possible directory completions, adding trailing '/' characters,+# de-quoting, and handling unusual characters.+whileIFS=read-r-d$'\0'c;do+# If there are directory completions, find ones that start+# with "$cur", the current token, and put those in COMPREPLY+if[[$c=="$cur"*]];then+COMPREPLY+=("$c/")+_found=1+fi+done<<(gitls-tree-z-d--name-onlyHEAD$_tmp_dir)++if[[$_found==0]]&&[["$cur"=~/$]];then+# No possible further completions any deeper, so assume we're at+# a leaf directory and just consider it complete+__gitcomp_direct_append"$cur "+fi
The indentation changes are distracting and make the patch harder to
review. Could you either remove those, or apply the indentation
changes to patch 2 so that it starts with the right indentation?
I've also corrected this as part of the fix I'm about to submit.
I'm slightly surprised that __gitcomp_direct_append handles the
quoting for us, but the testcases below seem to cover it, so that's
cool.
Anyway, looks pretty clever to me; I was worried this was going to
require a much bigger change.
@@ -1508,6 +1508,34 @@ test_expect_success 'cone mode sparse-checkout completes directory names' ')'+# use FUNNYNAMES to avoid running on Windows, which doesn't permit backslashes in paths+test_expect_successFUNNYNAMES'cone mode sparse-checkout completes directory names with special characters''+# reset sparse-checkout+git-Csparse-checkoutsparse-checkoutdisable&&+(+cdsparse-checkout&&+mkdir"directory with spaces"&&+mkdir"$(printf"directory\twith\ttabs")"&&+mkdir"directory\with\backslashes"&&+mkdir"directory-with-áccent"&&+>"directory with spaces/randomfile"&&+>"$(printf"directory\twith\ttabs")/randomfile"&&+>"directory\with\backslashes/randomfile"&&+>"directory-with-áccent/randomfile"&&+gitadd.&&+gitcommit-m"Add directories containing unusual characters"&&+gitsparse-checkoutset--cone"directory with spaces"\+"$(printf"directory\twith\ttabs")""directory\with\backslashes"\+"directory-with-áccent"&&+test_completion"git sparse-checkout add dir"<<-\EOF+directorywithspaces/+directorywithtabs/+directory\with\backslashes/+directory-with-áccent/+EOF+)+'
I'm glad you tested with lots of special characters -- spaces, tabs,
backslashes, and accents. Newlines might also be nice, but probably
makes the test hard. Anyway, looks good to me, other than the
indentation change.
From: Lessley Dennington via GitGitGadget <hidden> Date: 2022-02-04 03:26:43
This change updates custom tab completion for the sparse-checkout command.
Specifically, it corrects the following issues with the current method:
1. git sparse-checkout <TAB> results in an incomplete list of subcommands
(it is missing reapply and add).
2. Options for subcommands are not tab-completable.
3. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> show
both file names and directory names. While this may be a less surprising
behavior for non-cone mode, we want to only show directories in cone
mode.
The first commit in this series is an intermediate step that fixes issues 1
and 2 above and introduces a simple fix for issue 3 with some performance
and unusual character-related caveats. The next commit adds a new
__gitcomp_directories method that fixes the performance-related caveat from
the first commit by completing just a single level of directories. The final
commit modifies __gitcomp_directories to handle unusual characters in
directory names.
Changes since V5
================
* Fix incorrect conditional that was causing failure of non-cone mode test
(and causing 'seen' CI to fail).
* Remove __git_comp_directories indentation changes between the second and
third commits.
Changes since V4
================
* Clarify that completing on files and directories is a less-surprising
behavior in non-cone mode (rather than definitely the correct behavior)
in commit message and cover letter.
* Add explicit --no-cone to non-cone mode sparse-checkout completion test.
* Replace use of __git_complete_index_file with a new version of
__gitcomp_directories that is able to handle unusual characters.
* Ensure that users running git sparse-checkout set --cone get cone
mode-style completion whether or not they already have the cone mode
config set.
* Refactor creation of sparse-checkout test repo into a sparse-checkout
setup test.
Changes since V3
================
* Update commit descriptions to begin with completion instead of
sparse-checkout.
* Remove explicit completion of --help, as this is inconsistent with the
rest of the completion script.
* Use __gitcomp_builtin to auto-complete subcommand options (and remove
option completion test from previous iterations, as it is no longer
necessary).
* Use __git_complete_index_file helper function in place of __gitcomp
"$(git ls-tree -d -r HEAD --name-only).
* If in cone-mode sparse checkout, only show directory completions. If in
non-cone mode sparse checkout show both files and directories.
Changes since V2
================
* Change use of $prev to $subcommand in _git_sparse_checkout() method in
git-completion.bash.
* State explicitly that directory completion applies in both cone and
non-cone mode in 'sparse-checkout: custom tab completion' commit
* Add new patch with __gitcomp_directories method to improve performance by
only outputting directories at the current level.
Changes since V1
================
* Rebase onto en/sparse-checkout-set.
* Add subcommand options (including --no-cone) for set and reapply.
* Extend 'sparse-checkout completes subcommand options' test to validate
new set/reapply subcommand options.
* No longer set index.sparse to false in 'sparse-checkout completes
directory names' test.
* Refactor creation of sparse-checkout repo into a setup test.
Thanks, Lessley
Lessley Dennington (3):
completion: address sparse-checkout issues
completion: improve sparse-checkout cone mode directory completion
completion: handle unusual characters for sparse-checkout
contrib/completion/git-completion.bash | 44 ++++++++--
t/t9902-completion.sh | 117 +++++++++++++++++++++++++
2 files changed, 153 insertions(+), 8 deletions(-)
base-commit: dfac9b609f86cd4f6ce896df9e1172d2a02cde48
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1108%2Fldennington%2Fsparse-checkout-bash-completion-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1108/ldennington/sparse-checkout-bash-completion-v6
Pull-Request: https://github.com/gitgitgadget/git/pull/1108
Range-diff vs v5:
1: a1c46c763fd ! 1: 23390000dd3 completion: address sparse-checkout issues
@@ contrib/completion/git-completion.bash: _git_sparse_checkout ()
+ __gitcomp_builtin sparse-checkout_$subcommand "" "--"
;;
+ set,*|add,*)
-+ if [ $(__git config core.sparseCheckoutCone) ] ||
-+ [ "$(__git_find_on_cmdline --cone)" ]; then
++ if [ "$(__git config core.sparseCheckoutCone)" == "true" ] ||
++ [ -n "$(__git_find_on_cmdline --cone)" ]; then
+ __gitcomp "$(git ls-tree -d -r HEAD --name-only)"
+ fi
esac
2: 5b8d53402ae ! 2: 10a190ce6a1 completion: improve sparse-checkout cone mode directory completion
@@ contrib/completion/git-completion.bash: _git_show_branch ()
+__gitcomp_directories ()
+{
-+ local _tmp_dir _tmp_completions
++ local _tmp_dir _tmp_completions
+
-+ # Get the directory of the current token; this differs from dirname
-+ # in that it keeps up to the final trailing slash. If no slash found
-+ # that's fine too.
-+ [[ "$cur" =~ .*/ ]]
-+ _tmp_dir=$BASH_REMATCH
++ # Get the directory of the current token; this differs from dirname
++ # in that it keeps up to the final trailing slash. If no slash found
++ # that's fine too.
++ [[ "$cur" =~ .*/ ]]
++ _tmp_dir=$BASH_REMATCH
+
-+ # Find possible directory completions, adding trailing '/' characters
-+ _tmp_completions="$(git ls-tree -d --name-only HEAD $_tmp_dir |
-+ sed -e s%$%/%)"
++ # Find possible directory completions, adding trailing '/' characters
++ _tmp_completions="$(git ls-tree -d --name-only HEAD $_tmp_dir |
++ sed -e s%$%/%)"
+
-+ if [[ -n "$_tmp_completions" ]]; then
-+ # There were some directory completions, so find ones that
-+ # start with "$cur", the current token, and put those in COMPREPLY
-+ local i=0 c IFS=$' \t\n'
-+ for c in $_tmp_completions; do
-+ if [[ $c == "$cur"* ]]; then
-+ COMPREPLY+=("$c")
-+ fi
-+ done
-+ elif [[ "$cur" =~ /$ ]]; then
-+ # No possible further completions any deeper, so assume we're at
-+ # a leaf directory and just consider it complete
-+ __gitcomp_direct_append "$cur "
-+ fi
++ if [[ -n "$_tmp_completions" ]]; then
++ # There were some directory completions, so find ones that
++ # start with "$cur", the current token, and put those in COMPREPLY
++ local i=0 c IFS=$' \t\n'
++ for c in $_tmp_completions; do
++ if [[ $c == "$cur"* ]]; then
++ COMPREPLY+=("$c")
++ fi
++ done
++ elif [[ "$cur" =~ /$ ]]; then
++ # No possible further completions any deeper, so assume we're at
++ # a leaf directory and just consider it complete
++ __gitcomp_direct_append "$cur "
++ fi
+}
+
_git_sparse_checkout ()
@@ contrib/completion/git-completion.bash: _git_show_branch ()
local subcommands="list init set disable add reapply"
@@ contrib/completion/git-completion.bash: _git_sparse_checkout ()
set,*|add,*)
- if [ $(__git config core.sparseCheckoutCone) ] ||
- [ "$(__git_find_on_cmdline --cone)" ]; then
+ if [ "$(__git config core.sparseCheckoutCone)" == "true" ] ||
+ [ -n "$(__git_find_on_cmdline --cone)" ]; then
- __gitcomp "$(git ls-tree -d -r HEAD --name-only)"
+ __gitcomp_directories
fi
3: ddf5e583dd7 ! 3: e805dbb58cc completion: handle unusual characters for sparse-checkout
@@ contrib/completion/git-completion.bash: _git_show_branch ()
__gitcomp_directories ()
{
-- local _tmp_dir _tmp_completions
--
-- # Get the directory of the current token; this differs from dirname
-- # in that it keeps up to the final trailing slash. If no slash found
-- # that's fine too.
-- [[ "$cur" =~ .*/ ]]
-- _tmp_dir=$BASH_REMATCH
--
-- # Find possible directory completions, adding trailing '/' characters
-- _tmp_completions="$(git ls-tree -d --name-only HEAD $_tmp_dir |
-- sed -e s%$%/%)"
--
-- if [[ -n "$_tmp_completions" ]]; then
-- # There were some directory completions, so find ones that
-- # start with "$cur", the current token, and put those in COMPREPLY
-- local i=0 c IFS=$' \t\n'
-- for c in $_tmp_completions; do
-- if [[ $c == "$cur"* ]]; then
-- COMPREPLY+=("$c")
-- fi
-- done
-- elif [[ "$cur" =~ /$ ]]; then
-- # No possible further completions any deeper, so assume we're at
-- # a leaf directory and just consider it complete
-- __gitcomp_direct_append "$cur "
-- fi
+- local _tmp_dir _tmp_completions
+ local _tmp_dir _tmp_completions _found=0
-+
-+ # Get the directory of the current token; this differs from dirname
-+ # in that it keeps up to the final trailing slash. If no slash found
-+ # that's fine too.
-+ [[ "$cur" =~ .*/ ]]
-+ _tmp_dir=$BASH_REMATCH
-+
+
+ # Get the directory of the current token; this differs from dirname
+ # in that it keeps up to the final trailing slash. If no slash found
+@@ contrib/completion/git-completion.bash: __gitcomp_directories ()
+ [[ "$cur" =~ .*/ ]]
+ _tmp_dir=$BASH_REMATCH
+
+- # Find possible directory completions, adding trailing '/' characters
+- _tmp_completions="$(git ls-tree -d --name-only HEAD $_tmp_dir |
+- sed -e s%$%/%)"
+-
+- if [[ -n "$_tmp_completions" ]]; then
+- # There were some directory completions, so find ones that
+- # start with "$cur", the current token, and put those in COMPREPLY
+- local i=0 c IFS=$' \t\n'
+- for c in $_tmp_completions; do
+ # Find possible directory completions, adding trailing '/' characters,
+ # de-quoting, and handling unusual characters.
+ while IFS= read -r -d $'\0' c ; do
+ # If there are directory completions, find ones that start
+ # with "$cur", the current token, and put those in COMPREPLY
-+ if [[ $c == "$cur"* ]]; then
+ if [[ $c == "$cur"* ]]; then
+- COMPREPLY+=("$c")
+ COMPREPLY+=("$c/")
+ _found=1
-+ fi
+ fi
+- done
+- elif [[ "$cur" =~ /$ ]]; then
+ done < <(git ls-tree -z -d --name-only HEAD $_tmp_dir)
+
+ if [[ $_found == 0 ]] && [[ "$cur" =~ /$ ]]; then
-+ # No possible further completions any deeper, so assume we're at
-+ # a leaf directory and just consider it complete
-+ __gitcomp_direct_append "$cur "
-+ fi
- }
-
- _git_sparse_checkout ()
+ # No possible further completions any deeper, so assume we're at
+ # a leaf directory and just consider it complete
+ __gitcomp_direct_append "$cur "
## t/t9902-completion.sh ##
@@ t/t9902-completion.sh: test_expect_success 'cone mode sparse-checkout completes directory names' '
--
gitgitgadget
From: Lessley Dennington via GitGitGadget <hidden> Date: 2022-02-04 03:26:47
From: Lessley Dennington <redacted>
Correct multiple issues with tab completion of the git sparse-checkout
command. These issues were:
1. git sparse-checkout <TAB> previously resulted in an incomplete list of
subcommands (it was missing reapply and add).
2. Subcommand options were not tab-completable.
3. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> showed
both file names and directory names. While this may be a less surprising
behavior for non-cone mode, cone mode sparse checkouts should complete
only directory names.
Note that while the new strategy of just using git ls-tree to complete on
directory names is simple and a step in the right direction, it does have
some caveats. These are:
1. Likelihood of poor performance in large monorepos (as a result of
recursively completing directory names).
2. Inability to handle paths containing unusual characters.
These caveats will be fixed by subsequent commits in this series.
Signed-off-by: Lessley Dennington <redacted>
---
contrib/completion/git-completion.bash | 16 ++---
t/t9902-completion.sh | 83 ++++++++++++++++++++++++++
2 files changed, 91 insertions(+), 8 deletions(-)
@@ -1447,6 +1447,89 @@ test_expect_success 'git checkout - with --detach, complete only references' 'EOF'+test_expect_success'setup sparse-checkout tests''+# set up sparse-checkout repo+gitinitsparse-checkout&&+(+cdsparse-checkout&&+mkdir-pfolder1/0/1folder2/0folder3&&+touchfolder1/0/1/t.txt&&+touchfolder2/0/t.txt&&+touchfolder3/t.txt&&+gitadd.&&+gitcommit-am"Initial commit"+)+'++test_expect_success'sparse-checkout completes subcommands''+test_completion"git sparse-checkout "<<-\EOF+listZ+initZ+setZ+addZ+reapplyZ+disableZ+EOF+'++test_expect_success'cone mode sparse-checkout completes directory names''+# initialize sparse-checkout definitions+git-Csparse-checkoutsparse-checkoutset--conefolder1/0folder3&&++# test tab completion+(+cdsparse-checkout&&+test_completion"git sparse-checkout set f"<<-\EOF+folder1Z+folder1/0Z+folder1/0/1Z+folder2Z+folder2/0Z+folder3Z+EOF+)&&++(+cdsparse-checkout/folder1&&+test_completion"git sparse-checkout add "<<-\EOF+./Z+0Z+0/1Z+EOF+)+'++test_expect_success'non-cone mode sparse-checkout uses bash completion''+# reset sparse-checkout repo to non-cone mode+git-Csparse-checkoutsparse-checkoutdisable&&+git-Csparse-checkoutsparse-checkoutset--no-cone&&++(+cdsparse-checkout&&+# expected to be empty since we have not configured+# custom completion for non-cone mode+test_completion"git sparse-checkout set f"<<-\EOF++EOF+)+'++test_expect_success'git sparse-checkout set --cone completes directory names''+git-Csparse-checkoutsparse-checkoutdisable&&++(+cdsparse-checkout&&+test_completion"git sparse-checkout set --cone f"<<-\EOF+folder1Z+folder1/0Z+folder1/0/1Z+folder2Z+folder2/0Z+folder3Z+EOF+)+'+ test_expect_success'git switch - with -d, complete all references''test_completion"git switch -d "<<-\EOFHEADZ
From: Lessley Dennington via GitGitGadget <hidden> Date: 2022-02-04 03:26:47
From: Lessley Dennington <redacted>
Use new __gitcomp_directories method to complete directory names in cone
mode sparse-checkouts. This method addresses the caveat of poor
performance in monorepos from the previous commit (by completing only one
level of directories).
The unusual character caveat from the previous commit will be fixed by the
final commit in this series.
Co-authored-by: Elijah Newren [off-list ref]
Co-authored-by: Lessley Dennington [off-list ref]
Signed-off-by: Lessley Dennington <redacted>
---
contrib/completion/git-completion.bash | 32 +++++++++++++++++++++-
t/t9902-completion.sh | 38 +++++++++++++++-----------
2 files changed, 53 insertions(+), 17 deletions(-)
@@ -2986,6 +2986,36 @@ _git_show_branch ()__git_complete_revlist}+__gitcomp_directories()+{+local_tmp_dir_tmp_completions++# Get the directory of the current token; this differs from dirname+# in that it keeps up to the final trailing slash. If no slash found+# that's fine too.+[["$cur"=~.*/]]+_tmp_dir=$BASH_REMATCH++# Find possible directory completions, adding trailing '/' characters+_tmp_completions="$(gitls-tree-d--name-onlyHEAD$_tmp_dir|+sed-es%$%/%)"++if[[-n"$_tmp_completions"]];then+# There were some directory completions, so find ones that+# start with "$cur", the current token, and put those in COMPREPLY+locali=0cIFS=$' \t\n'+forcin$_tmp_completions;do+if[[$c=="$cur"*]];then+COMPREPLY+=("$c")+fi+done+elif[["$cur"=~/$]];then+# No possible further completions any deeper, so assume we're at+# a leaf directory and just consider it complete+__gitcomp_direct_append"$cur "+fi+}+ _git_sparse_checkout(){localsubcommands="list init set disable add reapply"
From: Lessley Dennington via GitGitGadget <hidden> Date: 2022-02-04 03:26:51
From: Lessley Dennington <redacted>
Update the __gitcomp_directories method to de-quote and handle unusual
characters in directory names. Although this initially involved an attempt
to re-use the logic in __git_index_files, this method removed
subdirectories (e.g. folder1/0/ became folder1/), so instead new custom
logic was placed directly in the __gitcomp_directories method.
Co-authored-by: Johannes Schindelin [off-list ref]
Co-authored-by: Lessley Dennington [off-list ref]
Signed-off-by: Lessley Dennington <redacted>
---
contrib/completion/git-completion.bash | 24 ++++++++++------------
t/t9902-completion.sh | 28 ++++++++++++++++++++++++++
2 files changed, 39 insertions(+), 13 deletions(-)
@@ -2988,7 +2988,7 @@ _git_show_branch () __gitcomp_directories(){-local_tmp_dir_tmp_completions+local_tmp_dir_tmp_completions_found=0# Get the directory of the current token; this differs from dirname# in that it keeps up to the final trailing slash. If no slash found
@@ -2996,20 +2996,18 @@ __gitcomp_directories ()[["$cur"=~.*/]]_tmp_dir=$BASH_REMATCH-# Find possible directory completions, adding trailing '/' characters-_tmp_completions="$(gitls-tree-d--name-onlyHEAD$_tmp_dir|-sed-es%$%/%)"--if[[-n"$_tmp_completions"]];then-# There were some directory completions, so find ones that-# start with "$cur", the current token, and put those in COMPREPLY-locali=0cIFS=$' \t\n'-forcin$_tmp_completions;do+# Find possible directory completions, adding trailing '/' characters,+# de-quoting, and handling unusual characters.+whileIFS=read-r-d$'\0'c;do+# If there are directory completions, find ones that start+# with "$cur", the current token, and put those in COMPREPLYif[[$c=="$cur"*]];then-COMPREPLY+=("$c")+COMPREPLY+=("$c/")+_found=1fi-done-elif[["$cur"=~/$]];then+done<<(gitls-tree-z-d--name-onlyHEAD$_tmp_dir)++if[[$_found==0]]&&[["$cur"=~/$]];then# No possible further completions any deeper, so assume we're at# a leaf directory and just consider it complete__gitcomp_direct_append"$cur "
On Thu, Feb 3, 2022 at 12:44 PM Lessley Dennington via GitGitGadget
[off-list ref] wrote:
quoted
From: Lessley Dennington <redacted>
+# use FUNNYNAMES to avoid running on Windows, which doesn't permit backslashes in paths
+test_expect_success FUNNYNAMES 'cone mode sparse-checkout completes directory names with special characters' '
+ # reset sparse-checkout
+ git -C sparse-checkout sparse-checkout disable &&
+ (
+ cd sparse-checkout &&
+ mkdir "directory with spaces" &&
+ mkdir "$(printf "directory\twith\ttabs")" &&
+ mkdir "directory\with\backslashes" &&
+ mkdir "directory-with-áccent" &&
+ >"directory with spaces/randomfile" &&
+ >"$(printf "directory\twith\ttabs")/randomfile" &&
+ >"directory\with\backslashes/randomfile" &&
+ >"directory-with-áccent/randomfile" &&
+ git add . &&
+ git commit -m "Add directories containing unusual characters" &&
+ git sparse-checkout set --cone "directory with spaces" \
+ "$(printf "directory\twith\ttabs")" "directory\with\backslashes" \
+ "directory-with-áccent" &&
+ test_completion "git sparse-checkout add dir" <<-\EOF
+ directory with spaces/
+ directory with tabs/
+ directory\with\backslashes/
+ directory-with-áccent/
+ EOF
+ )
+'
I'm glad you tested with lots of special characters -- spaces, tabs,
backslashes, and accents. Newlines might also be nice, but probably
makes the test hard. Anyway, looks good to me, other than the
indentation change.
This looks like its over-skipping tests, the comment says Windows
doesn't like \ in paths, but there's a lo more being skipped here than
that.
Shouldn't e.g. "directory-with-áccent/" be pulled out into another test,
leaving the directory\with\backslashes/ etc. as non-Windows?
Another useful method is to just try to create those paths
unconditionally, but with "mkdir" etc., and if that works run the test
with git. After all if the OS can create a given path, but we're not
handling it that's our bug.
But if the FS just doesn't support the path we'd catch that in the
"mkdir" (or whatever), which we can assume behaves sanely on that
platform (or we're horribly broken anyway...).
Hi,
On Thu, Feb 3, 2022 at 7:26 PM Lessley Dennington via GitGitGadget
[off-list ref] wrote:
This change updates custom tab completion for the sparse-checkout command.
Specifically, it corrects the following issues with the current method:
1. git sparse-checkout <TAB> results in an incomplete list of subcommands
(it is missing reapply and add).
2. Options for subcommands are not tab-completable.
3. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> show
both file names and directory names. While this may be a less surprising
behavior for non-cone mode, we want to only show directories in cone
mode.
The first commit in this series is an intermediate step that fixes issues 1
and 2 above and introduces a simple fix for issue 3 with some performance
and unusual character-related caveats. The next commit adds a new
__gitcomp_directories method that fixes the performance-related caveat from
the first commit by completing just a single level of directories. The final
commit modifies __gitcomp_directories to handle unusual characters in
directory names.
Changes since V5
================
* Fix incorrect conditional that was causing failure of non-cone mode test
(and causing 'seen' CI to fail).
* Remove __git_comp_directories indentation changes between the second and
third commits.
This round looks good to me:
Reviewed-by: Elijah Newren <redacted>
Nice work!
From: Lessley Dennington via GitGitGadget <hidden> Date: 2022-02-07 17:45:47
This change updates custom tab completion for the sparse-checkout command.
Specifically, it corrects the following issues with the current method:
1. git sparse-checkout <TAB> results in an incomplete list of subcommands
(it is missing reapply and add).
2. Options for subcommands are not tab-completable.
3. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> show
both file names and directory names. While this may be a less surprising
behavior for non-cone mode, we want to only show directories in cone
mode.
The first commit in this series is an intermediate step that fixes issues 1
and 2 above and introduces a simple fix for issue 3 with some performance
and unusual character-related caveats. The next commit adds a new
__gitcomp_directories method that fixes the performance-related caveat from
the first commit by completing just a single level of directories. The final
commit modifies __gitcomp_directories to handle unusual characters in
directory names.
Changes since V6
================
* Split tests for unusual characters so that spaces and accents are
verified on Windows (in addition to macOS and Linux). Tabs and
backslashes are still excluded on Windows (details on why are included in
the third commit message).
Changes since V5
================
* Fix incorrect conditional that was causing failure of non-cone mode test
(and causing 'seen' CI to fail).
* Remove __git_comp_directories indentation changes between the second and
third commits.
Changes since V4
================
* Clarify that completing on files and directories is a less-surprising
behavior in non-cone mode (rather than definitely the correct behavior)
in commit message and cover letter.
* Add explicit --no-cone to non-cone mode sparse-checkout completion test.
* Replace use of __git_complete_index_file with a new version of
__gitcomp_directories that is able to handle unusual characters.
* Ensure that users running git sparse-checkout set --cone get cone
mode-style completion whether or not they already have the cone mode
config set.
* Refactor creation of sparse-checkout test repo into a sparse-checkout
setup test.
Changes since V3
================
* Update commit descriptions to begin with completion instead of
sparse-checkout.
* Remove explicit completion of --help, as this is inconsistent with the
rest of the completion script.
* Use __gitcomp_builtin to auto-complete subcommand options (and remove
option completion test from previous iterations, as it is no longer
necessary).
* Use __git_complete_index_file helper function in place of __gitcomp
"$(git ls-tree -d -r HEAD --name-only).
* If in cone-mode sparse checkout, only show directory completions. If in
non-cone mode sparse checkout show both files and directories.
Changes since V2
================
* Change use of $prev to $subcommand in _git_sparse_checkout() method in
git-completion.bash.
* State explicitly that directory completion applies in both cone and
non-cone mode in 'sparse-checkout: custom tab completion' commit
* Add new patch with __gitcomp_directories method to improve performance by
only outputting directories at the current level.
Changes since V1
================
* Rebase onto en/sparse-checkout-set.
* Add subcommand options (including --no-cone) for set and reapply.
* Extend 'sparse-checkout completes subcommand options' test to validate
new set/reapply subcommand options.
* No longer set index.sparse to false in 'sparse-checkout completes
directory names' test.
* Refactor creation of sparse-checkout repo into a setup test.
Thanks, Lessley
Lessley Dennington (3):
completion: address sparse-checkout issues
completion: improve sparse-checkout cone mode directory completion
completion: handle unusual characters for sparse-checkout
contrib/completion/git-completion.bash | 44 ++++++--
t/t9902-completion.sh | 138 +++++++++++++++++++++++++
2 files changed, 174 insertions(+), 8 deletions(-)
base-commit: dfac9b609f86cd4f6ce896df9e1172d2a02cde48
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1108%2Fldennington%2Fsparse-checkout-bash-completion-v7
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1108/ldennington/sparse-checkout-bash-completion-v7
Pull-Request: https://github.com/gitgitgadget/git/pull/1108
Range-diff vs v6:
1: 23390000dd3 ! 1: 26422c759a0 completion: address sparse-checkout issues
@@ Commit message
These caveats will be fixed by subsequent commits in this series.
Signed-off-by: Lessley Dennington [off-list ref]
+ Reviewed-by: Elijah Newren [off-list ref]
## contrib/completion/git-completion.bash ##
@@ contrib/completion/git-completion.bash: _git_show_branch ()
2: 10a190ce6a1 ! 2: 2c20830b4ce completion: improve sparse-checkout cone mode directory completion
@@ Commit message
Co-authored-by: Elijah Newren [off-list ref]
Co-authored-by: Lessley Dennington [off-list ref]
Signed-off-by: Lessley Dennington [off-list ref]
+ Reviewed-by: Elijah Newren [off-list ref]
## contrib/completion/git-completion.bash ##
@@ contrib/completion/git-completion.bash: _git_show_branch ()
3: e805dbb58cc ! 3: 996cbe7dfb7 completion: handle unusual characters for sparse-checkout
@@ Commit message
subdirectories (e.g. folder1/0/ became folder1/), so instead new custom
logic was placed directly in the __gitcomp_directories method.
+ Note there are two tests for this new functionality - one for spaces and
+ accents and one for backslashes and tabs. The backslashes and tabs test
+ uses FUNNYNAMES to avoid running on Windows. This is because:
+
+ 1. Backslashes are explicitly not allowed in Windows file paths.
+ 2. Although tabs appear to be allowed when creating a file in a Windows
+ bash shell, they actually are not renderable (and appear as empty boxes
+ in the shell).
+
Co-authored-by: Johannes Schindelin [off-list ref]
Co-authored-by: Lessley Dennington [off-list ref]
+ Helped-by: Ævar Arnfjörð Bjarmason [off-list ref]
Signed-off-by: Lessley Dennington [off-list ref]
+ Reviewed-by: Elijah Newren [off-list ref]
## contrib/completion/git-completion.bash ##
@@ contrib/completion/git-completion.bash: _git_show_branch ()
@@ t/t9902-completion.sh: test_expect_success 'cone mode sparse-checkout completes
)
'
-+# use FUNNYNAMES to avoid running on Windows, which doesn't permit backslashes in paths
-+test_expect_success FUNNYNAMES 'cone mode sparse-checkout completes directory names with special characters' '
++test_expect_success 'cone mode sparse-checkout completes directory names with spaces and accents' '
+ # reset sparse-checkout
+ git -C sparse-checkout sparse-checkout disable &&
+ (
+ cd sparse-checkout &&
+ mkdir "directory with spaces" &&
-+ mkdir "$(printf "directory\twith\ttabs")" &&
-+ mkdir "directory\with\backslashes" &&
+ mkdir "directory-with-áccent" &&
+ >"directory with spaces/randomfile" &&
-+ >"$(printf "directory\twith\ttabs")/randomfile" &&
-+ >"directory\with\backslashes/randomfile" &&
+ >"directory-with-áccent/randomfile" &&
+ git add . &&
-+ git commit -m "Add directories containing unusual characters" &&
++ git commit -m "Add directory with spaces and directory with accent" &&
+ git sparse-checkout set --cone "directory with spaces" \
-+ "$(printf "directory\twith\ttabs")" "directory\with\backslashes" \
+ "directory-with-áccent" &&
-+ test_completion "git sparse-checkout add dir" <<-\EOF
++ test_completion "git sparse-checkout add dir" <<-\EOF &&
+ directory with spaces/
-+ directory with tabs/
-+ directory\with\backslashes/
+ directory-with-áccent/
+ EOF
++ rm -rf "directory with spaces" &&
++ rm -rf "directory-with-áccent" &&
++ git add . &&
++ git commit -m "Remove directory with spaces and directory with accent"
++ )
++'
++
++# use FUNNYNAMES to avoid running on Windows, which doesn't permit backslashes or tabs in paths
++test_expect_success FUNNYNAMES 'cone mode sparse-checkout completes directory names with backslashes and tabs' '
++ # reset sparse-checkout
++ git -C sparse-checkout sparse-checkout disable &&
++ (
++ cd sparse-checkout &&
++ mkdir "directory\with\backslashes" &&
++ mkdir "$(printf "directory\twith\ttabs")" &&
++ >"directory\with\backslashes/randomfile" &&
++ >"$(printf "directory\twith\ttabs")/randomfile" &&
++ git add . &&
++ git commit -m "Add directory with backslashes and directory with tabs" &&
++ git sparse-checkout set --cone "directory\with\backslashes" \
++ "$(printf "directory\twith\ttabs")" &&
++ test_completion "git sparse-checkout add dir" <<-\EOF &&
++ directory\with\backslashes/
++ directory with tabs/
++ EOF
++ rm -rf "directory\with\backslashes" &&
++ rm -rf "$(printf "directory\twith\ttabs")" &&
++ git add . &&
++ git commit -m "Remove directory with backslashes and directory with tabs"
+ )
+'
+
--
gitgitgadget
From: Lessley Dennington via GitGitGadget <hidden> Date: 2022-02-07 17:46:27
From: Lessley Dennington <redacted>
Use new __gitcomp_directories method to complete directory names in cone
mode sparse-checkouts. This method addresses the caveat of poor
performance in monorepos from the previous commit (by completing only one
level of directories).
The unusual character caveat from the previous commit will be fixed by the
final commit in this series.
Co-authored-by: Elijah Newren [off-list ref]
Co-authored-by: Lessley Dennington [off-list ref]
Signed-off-by: Lessley Dennington <redacted>
Reviewed-by: Elijah Newren <redacted>
---
contrib/completion/git-completion.bash | 32 +++++++++++++++++++++-
t/t9902-completion.sh | 38 +++++++++++++++-----------
2 files changed, 53 insertions(+), 17 deletions(-)
@@ -2986,6 +2986,36 @@ _git_show_branch ()__git_complete_revlist}+__gitcomp_directories()+{+local_tmp_dir_tmp_completions++# Get the directory of the current token; this differs from dirname+# in that it keeps up to the final trailing slash. If no slash found+# that's fine too.+[["$cur"=~.*/]]+_tmp_dir=$BASH_REMATCH++# Find possible directory completions, adding trailing '/' characters+_tmp_completions="$(gitls-tree-d--name-onlyHEAD$_tmp_dir|+sed-es%$%/%)"++if[[-n"$_tmp_completions"]];then+# There were some directory completions, so find ones that+# start with "$cur", the current token, and put those in COMPREPLY+locali=0cIFS=$' \t\n'+forcin$_tmp_completions;do+if[[$c=="$cur"*]];then+COMPREPLY+=("$c")+fi+done+elif[["$cur"=~/$]];then+# No possible further completions any deeper, so assume we're at+# a leaf directory and just consider it complete+__gitcomp_direct_append"$cur "+fi+}+ _git_sparse_checkout(){localsubcommands="list init set disable add reapply"
From: Lessley Dennington via GitGitGadget <hidden> Date: 2022-02-07 17:47:31
From: Lessley Dennington <redacted>
Update the __gitcomp_directories method to de-quote and handle unusual
characters in directory names. Although this initially involved an attempt
to re-use the logic in __git_index_files, this method removed
subdirectories (e.g. folder1/0/ became folder1/), so instead new custom
logic was placed directly in the __gitcomp_directories method.
Note there are two tests for this new functionality - one for spaces and
accents and one for backslashes and tabs. The backslashes and tabs test
uses FUNNYNAMES to avoid running on Windows. This is because:
1. Backslashes are explicitly not allowed in Windows file paths.
2. Although tabs appear to be allowed when creating a file in a Windows
bash shell, they actually are not renderable (and appear as empty boxes
in the shell).
Co-authored-by: Johannes Schindelin [off-list ref]
Co-authored-by: Lessley Dennington [off-list ref]
Helped-by: Ævar Arnfjörð Bjarmason [off-list ref]
Signed-off-by: Lessley Dennington <redacted>
Reviewed-by: Elijah Newren <redacted>
---
contrib/completion/git-completion.bash | 24 ++++++-------
t/t9902-completion.sh | 49 ++++++++++++++++++++++++++
2 files changed, 60 insertions(+), 13 deletions(-)
@@ -2988,7 +2988,7 @@ _git_show_branch () __gitcomp_directories(){-local_tmp_dir_tmp_completions+local_tmp_dir_tmp_completions_found=0# Get the directory of the current token; this differs from dirname# in that it keeps up to the final trailing slash. If no slash found
@@ -2996,20 +2996,18 @@ __gitcomp_directories ()[["$cur"=~.*/]]_tmp_dir=$BASH_REMATCH-# Find possible directory completions, adding trailing '/' characters-_tmp_completions="$(gitls-tree-d--name-onlyHEAD$_tmp_dir|-sed-es%$%/%)"--if[[-n"$_tmp_completions"]];then-# There were some directory completions, so find ones that-# start with "$cur", the current token, and put those in COMPREPLY-locali=0cIFS=$' \t\n'-forcin$_tmp_completions;do+# Find possible directory completions, adding trailing '/' characters,+# de-quoting, and handling unusual characters.+whileIFS=read-r-d$'\0'c;do+# If there are directory completions, find ones that start+# with "$cur", the current token, and put those in COMPREPLYif[[$c=="$cur"*]];then-COMPREPLY+=("$c")+COMPREPLY+=("$c/")+_found=1fi-done-elif[["$cur"=~/$]];then+done<<(gitls-tree-z-d--name-onlyHEAD$_tmp_dir)++if[[$_found==0]]&&[["$cur"=~/$]];then# No possible further completions any deeper, so assume we're at# a leaf directory and just consider it complete__gitcomp_direct_append"$cur "
@@ -1508,6 +1508,55 @@ test_expect_success 'cone mode sparse-checkout completes directory names' ')'+test_expect_success'cone mode sparse-checkout completes directory names with spaces and accents''+# reset sparse-checkout+git-Csparse-checkoutsparse-checkoutdisable&&+(+cdsparse-checkout&&+mkdir"directory with spaces"&&+mkdir"directory-with-áccent"&&+>"directory with spaces/randomfile"&&+>"directory-with-áccent/randomfile"&&+gitadd.&&+gitcommit-m"Add directory with spaces and directory with accent"&&+gitsparse-checkoutset--cone"directory with spaces"\+"directory-with-áccent"&&+test_completion"git sparse-checkout add dir"<<-\EOF&&+directorywithspaces/+directory-with-áccent/+EOF+rm-rf"directory with spaces"&&+rm-rf"directory-with-áccent"&&+gitadd.&&+gitcommit-m"Remove directory with spaces and directory with accent"+)+'++# use FUNNYNAMES to avoid running on Windows, which doesn't permit backslashes or tabs in paths+test_expect_successFUNNYNAMES'cone mode sparse-checkout completes directory names with backslashes and tabs''+# reset sparse-checkout+git-Csparse-checkoutsparse-checkoutdisable&&+(+cdsparse-checkout&&+mkdir"directory\with\backslashes"&&+mkdir"$(printf"directory\twith\ttabs")"&&+>"directory\with\backslashes/randomfile"&&+>"$(printf"directory\twith\ttabs")/randomfile"&&+gitadd.&&+gitcommit-m"Add directory with backslashes and directory with tabs"&&+gitsparse-checkoutset--cone"directory\with\backslashes"\+"$(printf"directory\twith\ttabs")"&&+test_completion"git sparse-checkout add dir"<<-\EOF&&+directory\with\backslashes/+directorywithtabs/+EOF+rm-rf"directory\with\backslashes"&&+rm-rf"$(printf"directory\twith\ttabs")"&&+gitadd.&&+gitcommit-m"Remove directory with backslashes and directory with tabs"+)+'+ test_expect_success'non-cone mode sparse-checkout uses bash completion''# reset sparse-checkout repo to non-cone modegit-Csparse-checkoutsparse-checkoutdisable&&
From: Lessley Dennington via GitGitGadget <hidden> Date: 2022-02-07 17:47:31
From: Lessley Dennington <redacted>
Correct multiple issues with tab completion of the git sparse-checkout
command. These issues were:
1. git sparse-checkout <TAB> previously resulted in an incomplete list of
subcommands (it was missing reapply and add).
2. Subcommand options were not tab-completable.
3. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> showed
both file names and directory names. While this may be a less surprising
behavior for non-cone mode, cone mode sparse checkouts should complete
only directory names.
Note that while the new strategy of just using git ls-tree to complete on
directory names is simple and a step in the right direction, it does have
some caveats. These are:
1. Likelihood of poor performance in large monorepos (as a result of
recursively completing directory names).
2. Inability to handle paths containing unusual characters.
These caveats will be fixed by subsequent commits in this series.
Signed-off-by: Lessley Dennington <redacted>
Reviewed-by: Elijah Newren <redacted>
---
contrib/completion/git-completion.bash | 16 ++---
t/t9902-completion.sh | 83 ++++++++++++++++++++++++++
2 files changed, 91 insertions(+), 8 deletions(-)
@@ -1447,6 +1447,89 @@ test_expect_success 'git checkout - with --detach, complete only references' 'EOF'+test_expect_success'setup sparse-checkout tests''+# set up sparse-checkout repo+gitinitsparse-checkout&&+(+cdsparse-checkout&&+mkdir-pfolder1/0/1folder2/0folder3&&+touchfolder1/0/1/t.txt&&+touchfolder2/0/t.txt&&+touchfolder3/t.txt&&+gitadd.&&+gitcommit-am"Initial commit"+)+'++test_expect_success'sparse-checkout completes subcommands''+test_completion"git sparse-checkout "<<-\EOF+listZ+initZ+setZ+addZ+reapplyZ+disableZ+EOF+'++test_expect_success'cone mode sparse-checkout completes directory names''+# initialize sparse-checkout definitions+git-Csparse-checkoutsparse-checkoutset--conefolder1/0folder3&&++# test tab completion+(+cdsparse-checkout&&+test_completion"git sparse-checkout set f"<<-\EOF+folder1Z+folder1/0Z+folder1/0/1Z+folder2Z+folder2/0Z+folder3Z+EOF+)&&++(+cdsparse-checkout/folder1&&+test_completion"git sparse-checkout add "<<-\EOF+./Z+0Z+0/1Z+EOF+)+'++test_expect_success'non-cone mode sparse-checkout uses bash completion''+# reset sparse-checkout repo to non-cone mode+git-Csparse-checkoutsparse-checkoutdisable&&+git-Csparse-checkoutsparse-checkoutset--no-cone&&++(+cdsparse-checkout&&+# expected to be empty since we have not configured+# custom completion for non-cone mode+test_completion"git sparse-checkout set f"<<-\EOF++EOF+)+'++test_expect_success'git sparse-checkout set --cone completes directory names''+git-Csparse-checkoutsparse-checkoutdisable&&++(+cdsparse-checkout&&+test_completion"git sparse-checkout set --cone f"<<-\EOF+folder1Z+folder1/0Z+folder1/0/1Z+folder2Z+folder2/0Z+folder3Z+EOF+)+'+ test_expect_success'git switch - with -d, complete all references''test_completion"git switch -d "<<-\EOFHEADZ
On Mon, Feb 7, 2022 at 9:31 AM Lessley Dennington via GitGitGadget
[off-list ref] wrote:
This change updates custom tab completion for the sparse-checkout command.
Specifically, it corrects the following issues with the current method:
1. git sparse-checkout <TAB> results in an incomplete list of subcommands
(it is missing reapply and add).
2. Options for subcommands are not tab-completable.
3. git sparse-checkout set <TAB> and git sparse-checkout add <TAB> show
both file names and directory names. While this may be a less surprising
behavior for non-cone mode, we want to only show directories in cone
mode.
The first commit in this series is an intermediate step that fixes issues 1
and 2 above and introduces a simple fix for issue 3 with some performance
and unusual character-related caveats. The next commit adds a new
__gitcomp_directories method that fixes the performance-related caveat from
the first commit by completing just a single level of directories. The final
commit modifies __gitcomp_directories to handle unusual characters in
directory names.
Changes since V6
================
* Split tests for unusual characters so that spaces and accents are
verified on Windows (in addition to macOS and Linux). Tabs and
backslashes are still excluded on Windows (details on why are included in
the third commit message).
Thanks for doing that. I think this series is ready for next.
(It already has my Reviewed-by, so no need for me to re-give it.)
From: Adam Dinwoodie <hidden> Date: 2022-04-06 14:04:42
On Mon, Feb 07, 2022 at 05:31:45PM +0000, Lessley Dennington via GitGitGadget wrote:
quoted hunk
<snip>
Note there are two tests for this new functionality - one for spaces and
accents and one for backslashes and tabs. The backslashes and tabs test
uses FUNNYNAMES to avoid running on Windows. This is because:
1. Backslashes are explicitly not allowed in Windows file paths.
2. Although tabs appear to be allowed when creating a file in a Windows
bash shell, they actually are not renderable (and appear as empty boxes
in the shell).
<snip>
<snip>
+# use FUNNYNAMES to avoid running on Windows, which doesn't permit backslashes or tabs in paths
+test_expect_success FUNNYNAMES 'cone mode sparse-checkout completes directory names with backslashes and tabs' '
+ # reset sparse-checkout
+ git -C sparse-checkout sparse-checkout disable &&
+ (
+ cd sparse-checkout &&
+ mkdir "directory\with\backslashes" &&
+ mkdir "$(printf "directory\twith\ttabs")" &&
+ >"directory\with\backslashes/randomfile" &&
+ >"$(printf "directory\twith\ttabs")/randomfile" &&
+ git add . &&
+ git commit -m "Add directory with backslashes and directory with tabs" &&
+ git sparse-checkout set --cone "directory\with\backslashes" \
+ "$(printf "directory\twith\ttabs")" &&
+ test_completion "git sparse-checkout add dir" <<-\EOF &&
+ directory\with\backslashes/
+ directory with tabs/
+ EOF
+ rm -rf "directory\with\backslashes" &&
+ rm -rf "$(printf "directory\twith\ttabs")" &&
+ git add . &&
+ git commit -m "Remove directory with backslashes and directory with tabs"
+ )
+'
+
test_expect_success 'non-cone mode sparse-checkout uses bash completion' '
# reset sparse-checkout repo to non-cone mode
git -C sparse-checkout sparse-checkout disable &&
On Cygwin this test is failing: `FUNNYNAMES` checks (a) the system isn't
MinGW, and (b) the filesystem supports tabs, quotes and newlines.
Cygwin isn't MinGW, and its interface to the file system supports all
those characters, but doesn't support backslashes.
I think the ideal solution here would be to split this test into two:
one part to cover the tab part, which will work on Cygwin, and a
separate part that has another prereq (which might be a new variant of
FUN*NAMES, or might be something more explicit like
SLASHALLOWEDINPATHCOMPONENTS, or might just be FUNNYNAMES,!CYGWIN). But
it might be easier to just gate this test on FUNNYNAMES,!CYGWIN and not
worry about not being able to test the tab handling on Cygwin systems,
as the chances of a Cygwin-specific regression that'd only be caught by
this test seems pretty low to me.