Subject: Re: [PATCH v3 3/3] sparse-checkout: limit tab completion to a single level

From: Junio C Hamano <[email-masked]>
Date: Wed, 12 Jan 2022 16:00:34 -0800

Lessley Dennington <[email-masked]> writes:

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

The substitution operator "s" in "sed" can take any letter as the
match-replacement delimiter.  People use 's/match/replacement/'
usually because that is how textbooks teach them, but whatever
letter chosen as the delimiter, if it appears in either match or
replacement, it needs to be quoted, i.e. 's/match/replace\/ment/'.

Using a delimiter letter other than '/' relieves you from having to
quote a slash when slash is part of match-replacement.  Even though
it is more common to use a letter that is a more clearly delimiter
looking, e.g. "s|match|replace/ment|", it is not a crime to use
letters like '%' and '#', or even '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'
> Does c need to be declared before the loop?
>> +         for c in $_tmp_completions; do

bash completion script runs in the same namespace as the end-user's
interactive session, so we MUST be careful not to contaminate the
namespace or clobber variable the end-user is using.  "local c" before
we use $c for our own use is a way to make sure that when this function
that says "local c" is left, the value (or non-presence) of "c" is
restored to its original value before this function was entered.