* Path with spaces in _canonical_paths @ 2022-11-18 17:41 thomas 2022-11-21 3:57 ` Bart Schaefer 0 siblings, 1 reply; 17+ messages in thread From: thomas @ 2022-11-18 17:41 UTC (permalink / raw) To: zsh-workers Hi all, I'm trying to use `_canonical_paths` (on zsh 5.9) to let the user autocomplete a fixed list of files. I hope I'm not using it incorrectly. Otherwise: It seems to not escape spaces in paths appropriately. Short example: compdef '_canonical_paths -N files files /tmp/My\ File' cmd cmd <Tab> Shows the following completions: /tmp/My File (appears twice) /tmp/My\ File i.e. I get in total 3 entries for the same path, and notably, the first option is without escaping the space, i.e. it will result in a command line: cmd /tmp/My File which is obviously not as intended. I tried scanning the code in /usr/share/zsh/functions/Completion/Unix/ _canonical_paths for the problem, but it is quite confusing to me. I was able to gather that by removing the `-Q` from the line _wanted "$tag" expl "$desc" compadd $__gopts -Q -U -a matches && ret=0 it adds escapes to all the entries, i.e. the new completions are: /tmp/My\ File (twice) /tmp/My\\\ File which is still not as desired. Much appreciated if anyone can help. Best, Thomas ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Path with spaces in _canonical_paths 2022-11-18 17:41 Path with spaces in _canonical_paths thomas @ 2022-11-21 3:57 ` Bart Schaefer 2022-11-21 10:47 ` thomas 0 siblings, 1 reply; 17+ messages in thread From: Bart Schaefer @ 2022-11-21 3:57 UTC (permalink / raw) To: thomas; +Cc: zsh-workers On Fri, Nov 18, 2022 at 9:42 AM <thomas@coldfix.de> wrote: > > I'm trying to use `_canonical_paths` (on zsh 5.9) to let the user autocomplete > a fixed list of files. I hope I'm not using it incorrectly. Tangentially to -workers: Is _canonical_paths documented in the wrong manual section? It's lumped in with functions that are expected to appear in the "completer" zstyle, which clearly it is not. > compdef '_canonical_paths -N files files /tmp/My\ File' cmd > cmd <Tab> > > Shows the following completions: > > /tmp/My File (appears twice) > /tmp/My\ File > > i.e. I get in total 3 entries for the same path I'm not able to reproduce this using nothing but compinit plus the compdef above. I get just one completion, "/tmp/My\ File", regardless of whether that file exists or not. Try this: debug_canonical_paths () { _canonical_paths -N files files /tmp/My\ File } compdef '_complete_debug debug_canonical_paths' cmd Pressing TAB then drops a file in /tmp with a debug trace of the execution of _canonical_paths. In particular look at which "zstyle" lookups are being done and which of those might be causing the extra entries. You can also look at the context around elements being added to the "matches" array. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Path with spaces in _canonical_paths 2022-11-21 3:57 ` Bart Schaefer @ 2022-11-21 10:47 ` thomas 2022-11-21 16:30 ` Bart Schaefer 0 siblings, 1 reply; 17+ messages in thread From: thomas @ 2022-11-21 10:47 UTC (permalink / raw) To: zsh-workers > I'm not able to reproduce this using nothing but compinit plus the > compdef above. I get just one completion, "/tmp/My\ File", regardless > of whether that file exists or not. > > Try this: > > debug_canonical_paths () { > _canonical_paths -N files files /tmp/My\ File > } > compdef '_complete_debug debug_canonical_paths' cmd Thanks for your answer! I can't reproduce with the minimal example anymore either. However, I still get similar behaviour by first doing: sudo ln -s /tmp /foo And then, same as before (with or without debug, with or without extra function, with or without -N): debug_canonical_paths () { _canonical_paths -N files files /tmp/My\ File } compdef '_complete_debug debug_canonical_paths' cmd cmd <Tab><Tab> Results in: /foo/My File /tmp/My\ File This is with blank zsh configuration `zsh -f` plus: autoload -U compinit; compinit > Pressing TAB then drops a file in /tmp with a debug trace of the > execution of _canonical_paths. In particular look at which "zstyle" > lookups are being done and which of those might be causing the extra > entries. You can also look at the context around elements being added > to the "matches" array. The trace shows: +_canonical_paths_add_paths:19> '(anon)' [...] +(anon):3> matches+=( '/tmp/My\ File' ) but later: +_canonical_paths_add_paths:28> matches+=( '/foo/My File' ) which explains why `compadd -Q` handles those differently. So the different unintended behaviour seems to appear only in the "alternative" paths. I originally encountered this behaviour with a file in /var/run/media/thomas/ which is symlinked to /media on my system. I get the same problem with files in /usr/bin/ which has several symlinks /usr/sbin/, /bin, /sbin on archlinux. I suspect when I previously also got a similar behaviour with my original minimal example without the symlink, there might have been tmpfs remounts or something(?), not sure what has changed, I did reboot in between. Best, Thomas ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Path with spaces in _canonical_paths 2022-11-21 10:47 ` thomas @ 2022-11-21 16:30 ` Bart Schaefer 2022-11-21 17:41 ` Thomas Gläßle 0 siblings, 1 reply; 17+ messages in thread From: Bart Schaefer @ 2022-11-21 16:30 UTC (permalink / raw) To: thomas; +Cc: zsh-workers On Mon, Nov 21, 2022 at 2:49 AM <thomas@coldfix.de> wrote: > > The trace shows: > > +_canonical_paths_add_paths:19> '(anon)' > [...] > +(anon):3> matches+=( '/tmp/My\ File' ) > > but later: > > +_canonical_paths_add_paths:28> matches+=( '/foo/My File' ) I still can't reproduce this, but given your trace output, try diff --git a/Completion/Unix/Type/_canonical_paths b/Completion/Unix/Type/_canonical_paths index a8fbbb524..b2eff84df 100644 --- a/Completion/Unix/Type/_canonical_paths +++ b/Completion/Unix/Type/_canonical_paths @@ -42,7 +42,7 @@ _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}) + matches+=( "${(@)${(@M)files:#$canpref*}/$canpref/$origpref}" ) fi for subdir in $expref?*(@); do ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Path with spaces in _canonical_paths 2022-11-21 16:30 ` Bart Schaefer @ 2022-11-21 17:41 ` Thomas Gläßle 2022-11-21 21:32 ` Bart Schaefer 0 siblings, 1 reply; 17+ messages in thread From: Thomas Gläßle @ 2022-11-21 17:41 UTC (permalink / raw) To: Bart Schaefer; +Cc: zsh-workers [-- Attachment #1.1.1: Type: text/plain, Size: 2465 bytes --] On 11/21/22 17:30, Bart Schaefer wrote: > I still can't reproduce this Weird.. With the symlink I can now on all my machines (arch, debian 11, mint 21), and even the official(?) docker: % docker run --rm -it zshusers/zsh:5.9 # ln -s /tmp /foo # autoload -U compinit; compinit # compdef '_canonical_paths -N files files /tmp/My\ File' cmd # cmd <Tab> /foo/My File /tmp/My\ File foo/My File tmp/My File (output split to several lines, to make it more readable) > diff --git a/Completion/Unix/Type/_canonical_paths > b/Completion/Unix/Type/_canonical_paths > index a8fbbb524..b2eff84df 100644 > --- a/Completion/Unix/Type/_canonical_paths > +++ b/Completion/Unix/Type/_canonical_paths > @@ -42,7 +42,7 @@ _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}) > + matches+=( "${(@)${(@M)files:#$canpref*}/$canpref/$origpref}" ) > fi > > for subdir in $expref?*(@); do This didn't fix the problem. But applying the same escaping "hack"(?) as in the if-case above, did: --- a/Completion/Unix/Type/_canonical_paths +++ b/Completion/Unix/Type/_canonical_paths @@ -30,19 +30,17 @@ canpref+=$rltrim [[ $expref == *[^/] && $canpref == */ ]] && origpref+=/ + local -a tmp_buffer + compadd -A tmp_buffer "$__gopts[@]" -a files # Append to $matches the subset of $files that matches $canpref. if [[ $canpref == $origpref ]]; then # This codepath honours any -M matchspec parameters. - () { - local -a tmp_buffer - compadd -A tmp_buffer "$__gopts[@]" -a files - matches+=( "${(@)tmp_buffer/$canpref/$origpref}" ) - } + 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 for subdir in $expref?*(@); do [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 19183 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 840 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Path with spaces in _canonical_paths 2022-11-21 17:41 ` Thomas Gläßle @ 2022-11-21 21:32 ` Bart Schaefer 2022-11-23 14:13 ` Daniel Shahaf 0 siblings, 1 reply; 17+ messages in thread From: Bart Schaefer @ 2022-11-21 21:32 UTC (permalink / raw) To: Thomas Gläßle; +Cc: zsh-workers On Mon, Nov 21, 2022 at 9:41 AM Thomas Gläßle <thomas@coldfix.de> 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 I'm not confident that's the right fix for other examples, given the "Ideally" comment there. Daniel, you were the last editor of that section of _canonical_paths but the change appears to have been related to its use in _mount (where it is no longer used now). Any comment? ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Path with spaces in _canonical_paths 2022-11-21 21:32 ` Bart Schaefer @ 2022-11-23 14:13 ` Daniel Shahaf 2022-11-23 21:36 ` Bart Schaefer 0 siblings, 1 reply; 17+ messages in thread From: Daniel Shahaf @ 2022-11-23 14:13 UTC (permalink / raw) To: zsh-workers; +Cc: Thomas Gläßle 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@coldfix.de> 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 > > I'm not confident that's the right fix for other examples, given the > "Ideally" comment there. Daniel, you were the last editor of that > section of _canonical_paths but the change appears to have been > related to its use in _mount (where it is no longer used now). The change in question is workers/39070 (= zsh-5.2-406-gaa041f7a5). > Any comment? Not really. The log message and thread of that change describe what use-case that change was fixing: namely, «umount /f/b<TAB>» → «umount /foo/bar». 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 — and implies that that's not exactly equivalent to the former, and that the former would be preferred over the latter. Adding or removing -Q or {(@)} (or ${(b)}, cf. workers/39080) might well be independent of that issue, though. HTH. Daniel P.S. Feel free to Cc me directly when there's a question to me specifically. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Path with spaces in _canonical_paths 2022-11-23 14:13 ` Daniel Shahaf @ 2022-11-23 21:36 ` Bart Schaefer 2022-11-23 22:24 ` Daniel Shahaf 0 siblings, 1 reply; 17+ messages in thread From: Bart Schaefer @ 2022-11-23 21:36 UTC (permalink / raw) To: Daniel Shahaf; +Cc: zsh-workers, Thomas Gläßle On Wed, Nov 23, 2022 at 6:14 AM Daniel Shahaf <d.s@daniel.shahaf.name> 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@coldfix.de> 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/. 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? ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Path with spaces in _canonical_paths 2022-11-23 21:36 ` Bart Schaefer @ 2022-11-23 22:24 ` Daniel Shahaf 2022-11-23 22:42 ` Bart Schaefer 0 siblings, 1 reply; 17+ messages in thread From: Daniel Shahaf @ 2022-11-23 22:24 UTC (permalink / raw) To: Bart Schaefer; +Cc: zsh-workers, Thomas Gläßle 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@daniel.shahaf.name> 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@coldfix.de> 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Path with spaces in _canonical_paths 2022-11-23 22:24 ` Daniel Shahaf @ 2022-11-23 22:42 ` Bart Schaefer 2022-11-23 23:06 ` Daniel Shahaf 2022-11-23 23:36 ` Thomas Gläßle 0 siblings, 2 replies; 17+ messages in thread From: Bart Schaefer @ 2022-11-23 22:42 UTC (permalink / raw) To: Daniel Shahaf; +Cc: zsh-workers, Thomas Gläßle On Wed, Nov 23, 2022 at 2:24 PM Daniel Shahaf <d.s@daniel.shahaf.name> 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. That was not clear. > 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Path with spaces in _canonical_paths 2022-11-23 22:42 ` Bart Schaefer @ 2022-11-23 23:06 ` Daniel Shahaf 2022-11-23 23:12 ` Bart Schaefer 2022-11-23 23:36 ` Thomas Gläßle 1 sibling, 1 reply; 17+ messages in thread From: Daniel Shahaf @ 2022-11-23 23:06 UTC (permalink / raw) To: Bart Schaefer; +Cc: zsh-workers, Thomas Gläßle 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@daniel.shahaf.name> 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Path with spaces in _canonical_paths 2022-11-23 23:06 ` Daniel Shahaf @ 2022-11-23 23:12 ` Bart Schaefer 2022-11-24 0:12 ` Daniel Shahaf 0 siblings, 1 reply; 17+ messages in thread From: Bart Schaefer @ 2022-11-23 23:12 UTC (permalink / raw) To: Daniel Shahaf; +Cc: zsh-workers, Thomas Gläßle On Wed, Nov 23, 2022 at 3:06 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > That comment is using "###" comments (a conventional "TODO" marker) Also something I was not familiar with. > Anyway, I see you've extended it to clarify. Thanks for this. Remains to be seen if it works ... I still can't figure out how to cause that code path ("else") to be followed when I try to reproduce Thomas's error. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Path with spaces in _canonical_paths 2022-11-23 23:12 ` Bart Schaefer @ 2022-11-24 0:12 ` Daniel Shahaf 2022-11-24 18:42 ` Bart Schaefer 0 siblings, 1 reply; 17+ messages in thread From: Daniel Shahaf @ 2022-11-24 0:12 UTC (permalink / raw) To: zsh-workers; +Cc: Thomas Gläßle Bart Schaefer wrote on Wed, Nov 23, 2022 at 15:12:40 -0800: > On Wed, Nov 23, 2022 at 3:06 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > Anyway, I see you've extended it to clarify. Thanks for this. > > Remains to be seen if it works ... I still can't figure out how to > cause that code path ("else") to be followed when I try to reproduce > Thomas's error. In the zsh tree, . _f() { _canonical_paths tag desc $PWD/README } compdef _f f f <TAB> . is enough for me to get the else path to be taken. For the variant with -N and quoting, . touch /tmp/My\ File ln -s /tmp slashtmp compdef '_canonical_paths -N files files /tmp/My\ File' cmd cmd <TAB> . gives . 1 >>> files 2 /tmp/My\ File slashtmp/My File That's with current HEAD and my nearly-minimal test setup (sets little besides 'format', 'group-name', $PS1, $PS4, and $fpath). Cheers, Daniel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Path with spaces in _canonical_paths 2022-11-24 0:12 ` Daniel Shahaf @ 2022-11-24 18:42 ` Bart Schaefer 0 siblings, 0 replies; 17+ messages in thread From: Bart Schaefer @ 2022-11-24 18:42 UTC (permalink / raw) To: Daniel Shahaf; +Cc: zsh-workers, Thomas Gläßle On Wed, Nov 23, 2022 at 4:13 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > . > _f() { _canonical_paths tag desc $PWD/README } > compdef _f f > f <TAB> > . > is enough for me to get the else path to be taken. For that one I do get % f <TAB> /Users/schaefer/Public/zsh/README README > . > touch /tmp/My\ File > ln -s /tmp slashtmp > compdef '_canonical_paths -N files files /tmp/My\ File' cmd > cmd <TAB> > . But here I just get % cmd <TAB> % cmd /tmp/My\ File I haven't actually tried the docker image. Does the above have something to do with being on a Mac, where /tmp itself is already a symlink to /private/tmp? ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Path with spaces in _canonical_paths 2022-11-23 22:42 ` Bart Schaefer 2022-11-23 23:06 ` Daniel Shahaf @ 2022-11-23 23:36 ` Thomas Gläßle 2022-11-23 23:40 ` Thomas Gläßle 2022-11-24 18:51 ` Bart Schaefer 1 sibling, 2 replies; 17+ messages in thread From: Thomas Gläßle @ 2022-11-23 23:36 UTC (permalink / raw) To: Bart Schaefer, Daniel Shahaf; +Cc: zsh-workers [-- Attachment #1.1.1: Type: text/plain, Size: 1104 bytes --] On 11/23/22 23:42, Bart Schaefer wrote: > + # ### The following approximates that. > + matches+=(${(q)${(M)files:#$canpref*}/$canpref/$origpref}) Seems to resolve the issue! However, there is another weird behaviour that I just now noticed (but it's unrelated to this patch). When resolving relative paths from within a symlinked directory, it seems to assume the resolved path of the cwd as basepath. Maybe best explained by another example: # ln -s /usr/local/bin /mnt # ln -s /tmp /foo # cd /mnt # compdef '_canonical_paths -N files files /tmp/My\ File' cmd # cmd <Tab> ../../../foo/My\ File ../../../tmp/My\ File Notice the amount of ../ > Remains to be seen if it works ... I still can't figure out how to > cause that code path ("else") to be followed when I try to reproduce > Thomas's error. Are you still not able to reproduce, even on the docker? How is that possible? Can it be different due to terminal? Doesn't really make sense to me.. Are you able to reproduce @Daniel? Best, Thomas [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 19183 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 840 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Path with spaces in _canonical_paths 2022-11-23 23:36 ` Thomas Gläßle @ 2022-11-23 23:40 ` Thomas Gläßle 2022-11-24 18:51 ` Bart Schaefer 1 sibling, 0 replies; 17+ messages in thread From: Thomas Gläßle @ 2022-11-23 23:40 UTC (permalink / raw) To: Bart Schaefer, Daniel Shahaf; +Cc: zsh-workers [-- Attachment #1.1.1: Type: text/plain, Size: 190 bytes --] On 11/24/22 00:36, Thomas Gläßle wrote: > > # ln -s /usr/local/bin /mnt > # cd /mnt Of course, /mnt was a bad choice because it already exists. Take /bar instead. [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 19183 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 840 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Path with spaces in _canonical_paths 2022-11-23 23:36 ` Thomas Gläßle 2022-11-23 23:40 ` Thomas Gläßle @ 2022-11-24 18:51 ` Bart Schaefer 1 sibling, 0 replies; 17+ messages in thread From: Bart Schaefer @ 2022-11-24 18:51 UTC (permalink / raw) To: Thomas Gläßle; +Cc: Daniel Shahaf, zsh-workers On Wed, Nov 23, 2022 at 3:36 PM Thomas Gläßle <thomas@coldfix.de> wrote: > > On 11/23/22 23:42, Bart Schaefer wrote: > > + # ### The following approximates that. > > + matches+=(${(q)${(M)files:#$canpref*}/$canpref/$origpref}) > > Seems to resolve the issue! Good! > However, there is another weird behaviour that I just now noticed> > > # cmd <Tab> > ../../../foo/My\ File > ../../../tmp/My\ File > When resolving relative paths from within > a symlinked directory, it seems to assume the resolved path of the cwd > as basepath. Yes, there's a loop that walks up the tree if there's a path prefix on the file being completed. How far up the tree it will look is controlled by the canonical-paths-back-limit zstyle (default 8 levels). This is being exacerbated by the -N option because without -N the :P modifier on the input paths would already have resolved the symlinks. Independently, I wonder if we should switch from :P to :A in the not-dash-N case. ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2022-11-24 18:52 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-11-18 17:41 Path with spaces in _canonical_paths thomas 2022-11-21 3:57 ` Bart Schaefer 2022-11-21 10:47 ` thomas 2022-11-21 16:30 ` Bart Schaefer 2022-11-21 17:41 ` Thomas Gläßle 2022-11-21 21:32 ` Bart Schaefer 2022-11-23 14:13 ` Daniel Shahaf 2022-11-23 21:36 ` Bart Schaefer 2022-11-23 22:24 ` Daniel Shahaf 2022-11-23 22:42 ` Bart Schaefer 2022-11-23 23:06 ` Daniel Shahaf 2022-11-23 23:12 ` Bart Schaefer 2022-11-24 0:12 ` Daniel Shahaf 2022-11-24 18:42 ` Bart Schaefer 2022-11-23 23:36 ` Thomas Gläßle 2022-11-23 23:40 ` Thomas Gläßle 2022-11-24 18:51 ` Bart Schaefer
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ 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 NNTP newsgroup(s).