[bug] escaping spaces in _canonical_paths

[bug] escaping spaces in _canonical_paths

[Philipp G. Haselwarter]

Hi,

when I try to umount a directory "/mnt/name with spaces" and i type

# umount /mnt/n

i press tab and the path gets completed to

# umount /mnt/name\ with\ spaces


however, if I cd into /mnt and then try to umount the relative path

# cd /mnt
# umount nam

it gets completed to

# umount name with spaces

with no escapes for the spaces.

i don't know much about zsh completion, so unfortunately I can't
investigate further by myself.

zsh --version
zsh 5.3.1 (x86_64-unknown-linux-gnu)


Best regards,
Philipp G. Haselwarter

Re: [bug] escaping spaces in _canonical_paths

[Bart Schaefer]

On Mar 6, 10:16am, Philipp G. Haselwarter wrote:
}
} however, if I cd into /mnt and then try to umount the relative path
} 
} # cd /mnt
} # umount nam
} 
} it gets completed to
} 
} # umount name with spaces
} 
} with no escapes for the spaces.

This seems to do it, although I haven't figured out why it is necessary
to do the quote manipulation in the shell code here when it is not needed
in the other (absolute-path) branch.

diff --git a/Completion/Unix/Type/_canonical_paths b/Completion/Unix/Type/_canonical_paths
index 6eab7b6..630907b 100644
--- a/Completion/Unix/Type/_canonical_paths
+++ b/Completion/Unix/Type/_canonical_paths
@@ -41,7 +41,11 @@ _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})
+    if [[ -z $compstate[quote] ]]; then
+      matches+=(${(q)${(M)files:#$canpref*}/$canpref/$origpref})
+    else
+      matches+=(${${(M)files:#$canpref*}/$canpref/$origpref})
+    fi
   fi
 
   for subdir in $expref?*(@); do
@@ -84,7 +88,7 @@ _canonical_paths() {
     files+=($@:P)
   fi
 
-  local base=$PREFIX
+  local base=${(Q)PREFIX}
   typeset -i blimit
 
   _canonical_paths_add_paths $base

Re: [bug] escaping spaces in _canonical_paths

[Daniel Shahaf]

Bart Schaefer wrote on Thu, Mar 09, 2017 at 21:48:20 -0800:
> On Mar 6, 10:16am, Philipp G. Haselwarter wrote:
> }
> } however, if I cd into /mnt and then try to umount the relative path
> } 
> } # cd /mnt
> } # umount nam
> } 
> } it gets completed to
> } 
> } # umount name with spaces
> } 
> } with no escapes for the spaces.
> 
> This seems to do it, although I haven't figured out why it is necessary
> to do the quote manipulation in the shell code here when it is not needed
> in the other (absolute-path) branch.

Perhaps the abspath branch gets confused on mountpoint names that
contain literal ' characters?  Since the ${foo/bar/baz} replacement is
done on the compadd results, and compadd wasn't called with -Q.

(Can't test this right now)

> diff --git a/Completion/Unix/Type/_canonical_paths b/Completion/Unix/Type/_canonical_paths
> index 6eab7b6..630907b 100644
> --- a/Completion/Unix/Type/_canonical_paths
> +++ b/Completion/Unix/Type/_canonical_paths
> @@ -41,7 +41,11 @@ _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})
> +    if [[ -z $compstate[quote] ]]; then
> +      matches+=(${(q)${(M)files:#$canpref*}/$canpref/$origpref})
> +    else
> +      matches+=(${${(M)files:#$canpref*}/$canpref/$origpref})
> +    fi
>    fi
>  
>    for subdir in $expref?*(@); do
> @@ -84,7 +88,7 @@ _canonical_paths() {
>      files+=($@:P)
>    fi
>  
> -  local base=$PREFIX
> +  local base=${(Q)PREFIX}
>    typeset -i blimit
>  
>    _canonical_paths_add_paths $base

Re: [bug] escaping spaces in _canonical_paths

[Bart Schaefer]

On Mar 10,  5:53am, Daniel Shahaf wrote:
}
} Perhaps the abspath branch gets confused on mountpoint names that
} contain literal ' characters?  Since the ${foo/bar/baz} replacement is
} done on the compadd results, and compadd wasn't called with -Q.

Indeed, I created a mount point named "aa'cc 2" and with the patch
from 40801, _canonical_paths is not able to complete that as an
absolute path if given a prefix.

It will correctly add the absolute path to the menu of completions if
there is no path prefix.

On a quick test the absolute-path case needs $PREFIX as-is, whereas
the local-directory case needs ${(Q)PREFIX}.

There's yet a third case -- a relative sub-directory, e.g.

% cd /
% sudo umount <TAB>

will show mnt/aa\'cc\ 2 in the list of completions, but

% sudo umount mnt<TAB>

will not complete anything, whereas both

% sudo umount /mnt/a<TAB>

and

% cd mnt
% sudo umount a<TAB>

correctly show a result.

There may be a bug with the :P modifier -- see other thread.  With that
fixed, things are better, but still messed up when $compstate[quote] is
a substring of the mount point name (if you see what I mean).

Re: [bug] escaping spaces in _canonical_paths

[Bart Schaefer]

On Mar 10,  6:31pm, Bart Schaefer wrote:
}
} There [is] a bug with the :P modifier -- see other thread.  With that
} fixed, things are better, but still messed up when $compstate[quote] is
} a substring of the mount point name (if you see what I mean).

OK, the following is getting very close.  Ignore previous patch (40811).

Even with this, there are some oddities.  For example because "/" is
normally a mount point, if "/" is the current directory the relative
path '' (empty string) is offered as a completion.

For another, I've discovered that if you have a quote in the directory
name like my previous example:

% sudo umount mnt/aa\'<TAB>

nothing completes, because $PREFIX is actually incorrect at this point;
it has the value mnt/aa\'\' (an extra escaped quote at the end) so it
can never be correctly suffixed.  I don't know if this is a bug in
the completion internals or if _mount somehow messes it up before the
call to _canonical_paths is made.

Fascinatingly,

% sudo umount mnt/aa\<TAB>
% sudo umount mnt/aa\''cc 2'

correctly completes.

I also found one case where I managed to get a trailing space (as if
for auto-removal) to appear inside the quotes, but I can't reproduce
now.  Still, the number of cases this gets right is larger than for
the previous code, I think.  Someone will probably find a previously-
working case that this breaks.

diff --git a/Completion/Unix/Type/_canonical_paths b/Completion/Unix/Type/_canonical_paths
index 6eab7b6..9ec7376 100644
--- a/Completion/Unix/Type/_canonical_paths
+++ b/Completion/Unix/Type/_canonical_paths
@@ -41,7 +41,16 @@ _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})
+    origpref=${(Q)origpref}
+    local canpat=${(b)${(Q)canpref}}
+    case $compstate[quote] in
+    (\')
+      matches+=(${${${(M)files:#$canpref*}/$canpat/$origpref}//\'/\'\\\'\'});;
+    (\")
+      matches+=(${${${(M)files:#$canpref*}/$canpat/$origpref}//\"/\\\"});;
+    (*)
+      matches+=(${(q-)${(M)files:#$canpref*}/$canpat/$origpref});;
+    esac
   fi
 
   for subdir in $expref?*(@); do