Re: Path with spaces in _canonical_paths

["Daniel Shahaf" <d.s@xxxxxxxxxxxxxxxxxx>]

Bart Schaefer wrote on Wed, 23 Nov 2022 21:36 +00:00:
> On Wed, Nov 23, 2022 at 6:14 AM Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx> wrote:
>>
>> Bart Schaefer wrote on Mon, Nov 21, 2022 at 13:32:41 -0800:
>> > On Mon, Nov 21, 2022 at 9:41 AM Thomas Gläßle <thomas@xxxxxxxxxx> wrote:
>> > >
>> > > +  local -a tmp_buffer
>> > > +  compadd -A tmp_buffer "$__gopts[@]" -a files
>> > >  [...]
>> > > +    matches+=( "${(@)tmp_buffer/$canpref/$origpref}" )
>> > >     else
>> > >       # ### Ideally, this codepath would do what the 'if' above does,
>> > >       # ### but telling compadd to pretend the "word on the command line"
>> > >       # ### is ${"the word on the command line"/$origpref/$canpref}.
>> > > -    matches+=(${${(M)files:#$canpref*}/$canpref/$origpref})
>> > > +    matches+=(${${(M)tmp_buffer:#$canpref*}/$canpref/$origpref})
>> > >     fi
>>
>> The comment quoted above concerns how the candidate completions are
>> compared to the word on the command line.  The comment says that,
>> instead of applying s/foo/bar/ to the word on the command line and
>> comparing the result against the raw candidate completions, the reverse
>> is done — s/bar/foo/ is applied to the candidate completions and that's
>> compared to the raw word on the command line
>
> Hrm, but in both cases (even before the diff) the substitution is
> s/$canpref/$origpref/ ?  Your explanation here would seem to imply
> that in at least the second case the substitution should look like
> s/$origpref/$canpref/.

Yes.  That's why the comment says s/$origpref/$canpref/ rather than
s/$canpref/$origpref/ as the code in HEAD says.

Perhaps an analogy.  Imagine a function that takes an unmetafied string
and an array of metafied strings and has to return 0 or 1 according to
whether or not the string is in the array, using nothing but strcmp()
for comparisons.  The function should either metafy the string or
unmetafy each string in the array in order, then.  (This analogy falls
short of explaining why one approach is better than the other.)

> The difference is that in the first case
> $tmp_buffer is the result of filtering with "compadd ... files" and in
> the second case we're altering the elements of $files directly.
>
>> Adding or removing -Q or {(@)} (or ${(b)}, cf. workers/39080) might well
>> be independent of that issue, though.
>
> I believe the difference here is also compadd -- it performs the
> quoting changes that -Q then has to account for.  So when we modify
> $files directly in the second branch, we have to also emulate the
> quote behavior of compadd.  My previous suggested patch perhaps didn't
> go far enough in that direction?

I'm not sure off the top of my head.  Perhaps that patch needs an extra
${(q)} somewhere?  Or perhaps we should add -Q to the if() codepath?
What do callers expect?

In general, I think the metafy analogy is valid here: we should define
whether $matches contains raw values or quoted values (and perhaps also
which form of quoting is used — presumably ${(q)}) and then ensure that
both branches conform to that internal API, just like for any char*
variable we define whether it's metafied or not.

Cheers,

Daniel