git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Elijah Newren <newren@gmail.com>
To: Lessley Dennington <lessleydennington@gmail.com>
Cc: Lessley Dennington via GitGitGadget <gitgitgadget@gmail.com>,
	Git Mailing List <git@vger.kernel.org>,
	Derrick Stolee <stolee@gmail.com>,
	Junio C Hamano <gitster@pobox.com>,
	johannes.schindelin@gmail.com
Subject: Re: [PATCH v2 2/2] sparse-checkout: custom tab completion
Date: Tue, 4 Jan 2022 12:42:34 -0800	[thread overview]
Message-ID: <CABPp-BFJF7hyr5onMFQNC7r_x+XfJVn9wXhxyMV6Lu+pxbfyPA@mail.gmail.com> (raw)
In-Reply-To: <676e8408-5ac3-4293-22ee-c43e9bd6916b@gmail.com>

On Tue, Jan 4, 2022 at 11:41 AM Lessley Dennington
<lessleydennington@gmail.com> wrote:
>
>
>
> On 12/31/21 4:52 PM, Elijah Newren wrote:
> > On Fri, Dec 31, 2021 at 2:33 AM Lessley Dennington via GitGitGadget
> > <gitgitgadget@gmail.com> wrote:
> >>
> >> From: Lessley Dennington <lessleydennington@gmail.com>
> >>
> >> 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 <lessleydennington@gmail.com>
> >> ---
> >>   contrib/completion/git-completion.bash | 38 ++++++++++++++++++--------
> >>   t/t9902-completion.sh                  |  8 +++---
> >>   2 files changed, 30 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> >> index c82ccaebcc7..7de997ee64e 100644
> >> --- a/contrib/completion/git-completion.bash
> >> +++ b/contrib/completion/git-completion.bash
> >> @@ -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 ()
> >>   {
> >> -       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
> >>          fi
> >>
> >> -       case "$subcommand,$cur" in
> >> -       init,--*)
> >> -               __gitcomp "--cone"
> >> -               ;;
> >> -       set,--*)
> >> -               __gitcomp "--stdin"
> >> -               ;;
> >> -       *)
> >> -               ;;
> >> +       case "$prev" in
> >
> > Shouldn't this be "$subcommand" rather than "$prev"?  I think with
> > prev, it will only correctly complete the first path after "set",
> > "add", etc.
> >
> Good catch, thank you! Fixing in v3.
> >> +               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)"
> >
> > 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.

  reply	other threads:[~2022-01-04 20:42 UTC|newest]

Thread overview: 95+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-30  0:32 [PATCH 0/2] sparse checkout: custom bash completion updates Lessley Dennington via GitGitGadget
2021-12-30  0:32 ` [PATCH 1/2] sparse-checkout: custom tab completion tests Lessley Dennington via GitGitGadget
2021-12-30 13:43   ` Derrick Stolee
2021-12-30 16:19     ` Lessley Dennington
2021-12-30 17:43       ` Derrick Stolee
2021-12-31 19:27         ` Elijah Newren
2022-01-04 19:19           ` Lessley Dennington
2021-12-30  0:32 ` [PATCH 2/2] sparse-checkout: custom tab completion Lessley Dennington via GitGitGadget
2021-12-30 13:50   ` Derrick Stolee
2021-12-30 16:24     ` Lessley Dennington
2021-12-30 19:26 ` [PATCH v2 0/2] sparse checkout: custom bash completion updates Lessley Dennington via GitGitGadget
2021-12-30 19:26   ` [PATCH v2 1/2] sparse-checkout: custom tab completion tests Lessley Dennington via GitGitGadget
2021-12-31 20:03     ` Elijah Newren
2021-12-31 22:20       ` Junio C Hamano
2021-12-31 22:25         ` Elijah Newren
2022-01-04 19:25         ` Lessley Dennington
2022-01-04 20:25           ` Elijah Newren
2022-01-05 14:05             ` Lessley Dennington
2022-01-04 19:24       ` Lessley Dennington
2021-12-30 19:26   ` [PATCH v2 2/2] sparse-checkout: custom tab completion Lessley Dennington via GitGitGadget
2021-12-31 22:52     ` Elijah Newren
2022-01-04 19:41       ` Lessley Dennington
2022-01-04 20:42         ` Elijah Newren [this message]
2022-01-05 20:20           ` Lessley Dennington
2022-01-05 22:55             ` Elijah Newren
2022-01-10 18:59   ` [PATCH v3 0/3] sparse checkout: custom bash completion updates Lessley Dennington via GitGitGadget
2022-01-10 18:59     ` [PATCH v3 1/3] sparse-checkout: custom tab completion tests Lessley Dennington via GitGitGadget
2022-01-10 18:59     ` [PATCH v3 2/3] sparse-checkout: custom tab completion Lessley Dennington via GitGitGadget
2022-01-15  9:57       ` SZEDER Gábor
2022-01-16  1:03         ` Elijah Newren
2022-01-16 22:13           ` Junio C Hamano
2022-01-17 18:14             ` Elijah Newren
2022-01-17 19:40               ` Junio C Hamano
2022-01-18 17:56                 ` Lessley Dennington
2022-01-22  1:07                   ` Lessley Dennington
2022-01-22  1:08                     ` Lessley Dennington
2022-01-22  2:11                       ` Lessley Dennington
2022-01-18 21:02               ` SZEDER Gábor
2022-01-18 21:43                 ` Elijah Newren
2022-01-18 17:59           ` Lessley Dennington
2022-01-18 22:22           ` SZEDER Gábor
2022-01-18 23:30             ` Elijah Newren
2022-01-10 18:59     ` [PATCH v3 3/3] sparse-checkout: limit tab completion to a single level Lessley Dennington via GitGitGadget
2022-01-12 23:43       ` Lessley Dennington
2022-01-13  0:00         ` Junio C Hamano
2022-01-13  0:38         ` Elijah Newren
2022-01-13 19:02           ` Lessley Dennington
2022-01-10 20:38     ` [PATCH v3 0/3] sparse checkout: custom bash completion updates Elijah Newren
2022-01-11 17:17       ` Lessley Dennington
2022-01-11 19:45         ` Taylor Blau
2022-01-12 18:35           ` Lessley Dennington
2022-01-27 21:21     ` [PATCH v4 0/3] completion: sparse-checkout updates Lessley Dennington via GitGitGadget
2022-01-27 21:21       ` [PATCH v4 1/3] completion: add sparse-checkout tests Lessley Dennington via GitGitGadget
2022-01-28  0:08         ` Elijah Newren
2022-01-28  1:56           ` Junio C Hamano
2022-01-28  2:04             ` Elijah Newren
2022-01-27 21:21       ` [PATCH v4 2/3] completion: sparse-checkout updates Lessley Dennington via GitGitGadget
2022-01-28  1:21         ` Elijah Newren
2022-01-31 20:03           ` Lessley Dennington
2022-01-31 21:37             ` Elijah Newren
2022-01-27 21:21       ` [PATCH v4 3/3] completion: ensure cone mode completion with multiple <TAB>s Lessley Dennington via GitGitGadget
2022-01-28  1:53         ` Elijah Newren
2022-02-03 20:44       ` [PATCH v5 0/3] completion: sparse-checkout updates Lessley Dennington via GitGitGadget
2022-02-03 20:44         ` [PATCH v5 1/3] completion: address sparse-checkout issues Lessley Dennington via GitGitGadget
2022-02-03 23:52           ` Elijah Newren
2022-02-04  0:34             ` Lessley Dennington
2022-02-03 20:44         ` [PATCH v5 2/3] completion: improve sparse-checkout cone mode directory completion Lessley Dennington via GitGitGadget
2022-02-03 20:44         ` [PATCH v5 3/3] completion: handle unusual characters for sparse-checkout Lessley Dennington via GitGitGadget
2022-02-03 23:58           ` Elijah Newren
2022-02-04  0:37             ` Lessley Dennington
2022-02-04  4:25             ` Ævar Arnfjörð Bjarmason
2022-02-04 21:55           ` Junio C Hamano
2022-02-03 21:48         ` [PATCH v5 0/3] completion: sparse-checkout updates Junio C Hamano
2022-02-03 22:17           ` Lessley Dennington
2022-02-03 23:28             ` Junio C Hamano
2022-02-03 23:59               ` Lessley Dennington
2022-02-04  2:43                 ` Lessley Dennington
2022-02-04  3:28                   ` Lessley Dennington
2022-02-04  4:21                   ` Ævar Arnfjörð Bjarmason
2022-02-04  3:26         ` [PATCH v6 " Lessley Dennington via GitGitGadget
2022-02-04  3:26           ` [PATCH v6 1/3] completion: address sparse-checkout issues Lessley Dennington via GitGitGadget
2022-02-04  3:26           ` [PATCH v6 2/3] completion: improve sparse-checkout cone mode directory completion Lessley Dennington via GitGitGadget
2022-02-04  3:26           ` [PATCH v6 3/3] completion: handle unusual characters for sparse-checkout Lessley Dennington via GitGitGadget
2022-02-04  6:05           ` [PATCH v6 0/3] completion: sparse-checkout updates Elijah Newren
2022-02-04 17:04             ` Junio C Hamano
2022-02-04 17:55               ` Elijah Newren
2022-02-04 19:54                 ` Junio C Hamano
2022-02-04 20:01                   ` Elijah Newren
2022-02-04 21:47                     ` Junio C Hamano
2022-02-07 17:31           ` [PATCH v7 " Lessley Dennington via GitGitGadget
2022-02-07 17:31             ` [PATCH v7 1/3] completion: address sparse-checkout issues Lessley Dennington via GitGitGadget
2022-02-07 17:31             ` [PATCH v7 2/3] completion: improve sparse-checkout cone mode directory completion Lessley Dennington via GitGitGadget
2022-02-07 17:31             ` [PATCH v7 3/3] completion: handle unusual characters for sparse-checkout Lessley Dennington via GitGitGadget
2022-04-06  9:42               ` Adam Dinwoodie
2022-02-08  4:16             ` [PATCH v7 0/3] completion: sparse-checkout updates Elijah Newren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CABPp-BFJF7hyr5onMFQNC7r_x+XfJVn9wXhxyMV6Lu+pxbfyPA@mail.gmail.com \
    --to=newren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmail.com \
    --cc=lessleydennington@gmail.com \
    --cc=stolee@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).