[Completion] apply compadd replacements during prefix replacement in _canonical_paths (for umount)

[pseyfert]

NB: marked as draft because a) the comment needs to be cleaned up but also because b) this is merely parts of the file copied together, i don't see a more elegant way right now, but suspect maintainers will either know which way to go, or have reservations against merging how i copied it together. In any case this is related to an issue (see below) and I went ahead to with patching it up for my local usage.

issue

USB sticks can have white spaces in their device names (might be used on windows, they get handed around, so whoever mounts them might not be whoever named it). In my case a stick I used was just called "USB DISK". The stick gets automounted (by udiskie, but i suspect it's common to auto mount USB sticks and use the device name) to /media/pseyfert/USB DISK.

When tab completing umount ⇥ from anywhere the tab completions contain correctly /media/pseyfert/USB\ DISK (that is, the white space gets escaped. When changing to /media/pseyfert, the completions for umount still contain the mount point with absolute path, but in addition the relative path USB DISK (this time, it gets entered like that, without escaping the white space).

my change

I traced down that umount gets completed by _mount which calls _umountable which calls _canonical_paths and in there, there is the if which detects if prefix replacement should happen. in case of umount, files contains the mount points without escape symbols, the "then" applies various replacements (as specified by _umountable) and escapes the white space. The else does prefix replacment but does not do (as indicated by the comment) the -M replacement. The -M replacement is (I think) irrelevant for me here, but compadd in the "then" also takes care of escaping white spaces.

[danielshahaf]

Could you add self-contained reproduction instructions? For instance, can one just bind-mount a path with spaces in the basename? If it's reproducible even without mount(8), even better :)

[pseyfert]

I think a reproducer w/o mount will be tricky (looking at _umountable - I'm on linux btw - the completions come out of /etc/mtab).

What I tested now is:

create a mount point with whitespace

mkdir /tmp/Test\ Mount\ Point

mount something

mount /dev/sdb5 /tmp/Test\ Mount\ Point -r ro,user

or

sshfs someremote: /tmp/Test\ Mount\ Point

or

mkdir /tmp/org
mount --bind /tmp/org /tmp/Test\ Mount\ Point -o ro,user

test completion

cd /tmp
umount ⇥

correct suggestions should contain

Test\ Mount\ Point
/tmp/Test\ Mount\ Point

incorrect suggestions should contain

Test\ Mount\ Point
/tmp/Test Mount Point

EDIT: added one more mount option after re-reading your question.

[danielshahaf]

Thanks; I can reproduce the bug using the bind-mount instructions you added in your edit. 👍

[danielshahaf]

Just wanted to add that keeping the original code but changing matches+=( ${…} ) to matches+=( ${(q)${…}} ) also fixes the problem. I'm not sure which change is better (if either); I'll leave it to others to dive further.