git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / 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 v3 3/3] sparse-checkout: limit tab completion to a single level
Date: Wed, 12 Jan 2022 16:38:34 -0800	[thread overview]
Message-ID: <CABPp-BEvUCPzcWhuVP==Rk=2pJXxRSGZVjdEzPuzbwTcw7kMRQ@mail.gmail.com> (raw)
In-Reply-To: <0e4bb6f1-337e-38b3-75b2-fe11ff8d68b2@gmail.com>

On Wed, Jan 12, 2022 at 3:43 PM Lessley Dennington
<lessleydennington@gmail.com> wrote:
>
> > +__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.

> > +     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.

> > +         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.
> > +         __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?

> > +     fi
> > +}
>
> Added my review as mentioned in [1].
>
> [1]:
> https://lore.kernel.org/git/pull.1108.v2.git.1640892413.gitgitgadget@gmail.com/T/#md3da435452988b0366ab4c2ee4bc06df2d17cb36

  parent reply	other threads:[~2022-01-13  0:38 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
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 [this message]
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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to='CABPp-BEvUCPzcWhuVP==Rk=2pJXxRSGZVjdEzPuzbwTcw7kMRQ@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 \
    --subject='Re: [PATCH v3 3/3] sparse-checkout: limit tab completion to a single level' \
    /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

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).