| Message ID | pull.1108.v3.git.1641841193.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | sparse checkout: custom bash completion updates | expand |
On Mon, Jan 10, 2022 at 10:59 AM Lessley Dennington via GitGitGadget <gitgitgadget@gmail.com> 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 <newren@gmail.com> 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 1/10/22 2:38 PM, Elijah Newren wrote: > On Mon, Jan 10, 2022 at 10:59 AM Lessley Dennington via GitGitGadget > <gitgitgadget@gmail.com> 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. > Thank you! That is much better wording. Will update in v4 along with any feedback I get on your function (see below). >> >> 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 <newren@gmail.com> > > 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.
On Tue, Jan 11, 2022 at 09:17:38AM -0800, Lessley Dennington wrote: > On 1/10/22 2:38 PM, Elijah Newren wrote: > > Other than that one nit, patches 1-2 (and the testcases in patch 3) are: > > > > Reviewed-by: Elijah Newren <newren@gmail.com> > > > > 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 1/11/22 1:45 PM, Taylor Blau wrote: > On Tue, Jan 11, 2022 at 09:17:38AM -0800, Lessley Dennington wrote: >> On 1/10/22 2:38 PM, Elijah Newren wrote: >>> Other than that one nit, patches 1-2 (and the testcases in patch 3) are: >>> >>> Reviewed-by: Elijah Newren <newren@gmail.com> >>> >>> 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.
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