| Message ID | pull.1108.v6.git.1643945198.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | completion: sparse-checkout updates | expand |
Hi, On Thu, Feb 3, 2022 at 7:26 PM Lessley Dennington via GitGitGadget <gitgitgadget@gmail.com> 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 <newren@gmail.com> Nice work!
Elijah Newren <newren@gmail.com> writes: >> 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 <newren@gmail.com> > > Nice work! Thanks, both. Will queue. Let's mark it to be merged down to 'next' soonish.
On Fri, Feb 4, 2022 at 9:04 AM Junio C Hamano <gitster@pobox.com> wrote: > > Elijah Newren <newren@gmail.com> writes: > > >> 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 <newren@gmail.com> > > > > Nice work! > > Thanks, both. Will queue. Let's mark it to be merged down to > 'next' soonish. Ævar had a good comment about code coverage on Windows that we might want to address first[1]. (Namely, splitting one test into two -- one that tests a path with backslashes that can be skipped on windows, and a separate test that checks paths with spaces, tabs, and non-ascii that can be run on all platforms.) But other than that, yeah, this should be ready for 'next'. [1] https://lore.kernel.org/git/220204.86h79f45nf.gmgdl@evledraar.gmail.com/
Elijah Newren <newren@gmail.com> writes: > Ævar had a good comment about code coverage on Windows that we might > want to address first[1]. (Namely, splitting one test into two -- one > that tests a path with backslashes that can be skipped on windows, and > a separate test that checks paths with spaces, tabs, and non-ascii > that can be run on all platforms.) According to the lazy-prereq definition for FUNNYNAMES, we seem to skip tab-embedded names on windows, so it may not be used on all platforms, but such a detail aside... I do not get why funny letters should matter and need to be tested specially in the first place, to be honest, but because we have with FUNNYNAMES prereq already, hiding some tests behind it would be a good idea regardless. > But other than that, yeah, this should be ready for 'next'. > > [1] https://lore.kernel.org/git/220204.86h79f45nf.gmgdl@evledraar.gmail.com/ Thanks.
On Fri, Feb 4, 2022 at 11:54 AM Junio C Hamano <gitster@pobox.com> wrote: > > Elijah Newren <newren@gmail.com> writes: > > > Ævar had a good comment about code coverage on Windows that we might > > want to address first[1]. (Namely, splitting one test into two -- one > > that tests a path with backslashes that can be skipped on windows, and > > a separate test that checks paths with spaces, tabs, and non-ascii > > that can be run on all platforms.) > > According to the lazy-prereq definition for FUNNYNAMES, we seem to > skip tab-embedded names on windows, so it may not be used on all > platforms, but such a detail aside... > > I do not get why funny letters should matter and need to be tested > specially in the first place, to be honest, but because we have with > FUNNYNAMES prereq already, hiding some tests behind it would be a > good idea regardless. Earlier versions of the patch series failed to handle paths that contained various special characters; see the second comment at https://lore.kernel.org/git/CABPp-BEq9pTqsy_R_SR1DSgUK58ubNR1Gk4G1RoL8wkadyo6zw@mail.gmail.com/. Handling them was specifically the job of the third patch in the series, and thus it made sense to add some kind of test for them.
Elijah Newren <newren@gmail.com> writes: > Handling them was specifically the job of the third patch in the > series, and thus it made sense to add some kind of test for them. Ah, yeah, reading from ls-tree via process substitution to avoid a pipe that makes the loop a subshell and using "read -d$'\0'" with IFS disabled to grok the -z output. Full of bash-isms of the good kind ;-). Nicely done, and it deserves to be showed off in tests.
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' '