Re: Path with spaces in _canonical_paths

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

Bart Schaefer wrote on Wed, 23 Nov 2022 22:42 +00:00:
> On Wed, Nov 23, 2022 at 2:24 PM Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx> wrote:
>>
>> Yes.  That's why the comment says s/$origpref/$canpref/ rather than
>> s/$canpref/$origpref/ as the code in HEAD says.
>
> Ah, I see.  The comment is explaining what the else-branch OUGHT to be
> accomplishing, rather than what it actually IS doing.

Right.

> That was not clear.
>

Thanks for the feedback.

That comment is using "###" comments (a conventional "TODO" marker), the
word "ideally", a conditional ("would"), and the order of pattern and
replacement is the opposite of what it is in the code.  I had thought it
was clear.

Anyway, I see you've extended it to clarify.  Thanks for this.

>> 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?
>
> Callers expect it in the form from the if-branch (what compadd
> produces), so I think the else-branch needs some variant of (q).
>
> Thomas, can you try this?
>
> diff --git a/Completion/Unix/Type/_canonical_paths
> b/Completion/Unix/Type/_canonical_paths
> index a8fbbb524..1444bc165 100644
> --- a/Completion/Unix/Type/_canonical_paths
> +++ b/Completion/Unix/Type/_canonical_paths
> @@ -42,7 +42,8 @@ _canonical_paths_add_paths () {
>      # ### 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})
> +    # ### The following approximates that.
> +    matches+=(${(q)${(M)files:#$canpref*}/$canpref/$origpref})
>    fi
>
>    for subdir in $expref?*(@); do