Re: [PATCH v3 3/3] sparse-checkout: limit tab completion to a single level
- From
Lessley Dennington <lessleydennington@gmail.com>
- Date
- Jan 13, 2022, 19:02 UTC
- Message-ID
- <92368b84-1d82-b594-9232-e67abcffe18a@gmail.com>
- In-Reply-To
- <CABPp-BEvUCPzcWhuVP==Rk=2pJXxRSGZVjdEzPuzbwTcw7kMRQ@mail.gmail.com>
On 1/12/22 6:38 PM, Elijah Newren wrote:
Show 30 quoted lines
> 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.
> Got it, thanks!
Show 11 quoted lines
>>> + 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.
Show 49 quoted lines
>>> + 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?
> Yes, very much! Thank you.
Show 7 quoted lines
>>> + fi >>> +} >> >> Added my review as mentioned in [1]. >> >> [1]: >> https://lore.kernel.org/git/pull.1108.v2.git.1640892413.gitgitgadget@gmail.com/T/#md3da435452988b0366ab4c2ee4bc06df2d17cb36