In-snap bash tab completion #3150

Merged
merged 14 commits into from May 5, 2017

some more filtering / validatiion

  • Loading branch information...
commit 6dcbcded52192f57dedb980872e6ad22a84a93e8 @chipaca chipaca committed Apr 26, 2017
@@ -41,7 +41,7 @@ _complete_from_snap() {
if [ ! "$bounced" ]; then
local IFS=$'\n'
- COMPREPLY=( $(cat) )
+ COMPREPLY=( $( \grep -v '[[:cntrl:];?*{}]' ) )
@jdstrand

jdstrand Apr 26, 2017

Contributor

Thanks for this! Note that shellcheck had this to say:

$ shellcheck -s bash ./complete.sh
In /usr/lib/snapd/complete.sh line 45:
            COMPREPLY=( $( \grep -v '[[:cntrl:];?*{}]' ) )
                           ^-- SC1001: This \g will be a regular 'g' in this context.

We use shellcheck for our shell scripts elsewhere and think adding this to run-checks for this and etelpmoc.sh would be a great idea.

@jdstrand

jdstrand Apr 26, 2017

Contributor

I meant to also say to please document the arguments to this grep. Eg:

# Ignore any suspicious results that are uncommon in filenames and that
# might be used to trick the user. A whitelist approach would be better
# but is impractical with UTF-8 and common characters like quotes.

All that said, I did the following and think we can probably do better:

$ mkdir foo ; cd foo
$ touch "fooz'ball" 'party "animal"' "disc (1)"
$ complexion <tab>
  |
   -> $ complexion
      disc (1)        fooz'ball       party "animal"
      $ complexion d<tab>
        |
         -> $ complexion disc (1) # tab completion has no escapes! Pressing 'enter' doesn't work as expected
            -bash: syntax error near unexpected token `('
$ ls <tab>
  |
   -> $ ls
      disc (1)        fooz'ball       party "animal"
      $ ls d<tab>
        |
         -> $ ls disc\ \(1\) # tab completion has escapes! Pressing 'enter' works as expected
            disc (1)

This demonstrates a usability bug. I don't think we can get away with not quoting. The trick will be not quoting the listing, but quoting the actual completion, like 'ls' does.

IFS="$oldIFS"
fi
@@ -62,7 +62,7 @@ _complete_from_snap() {
# what -D would've done before).
_complete_from_snap_maybe() {
# catch /snap/bin and /var/lib/snapd/snap/bin
- if [[ "$(which "$1")" =~ /snap/bin/ ]]; then
+ if [[ "$(which "$1")" =~ /snap/bin/ && ( -e /var/lib/snapd/snap/core/current/usr/lib/snapd/etelpmoc.sh || -e /snap/core/current/usr/lib/snapd/etelpmoc.sh ) ]]; then
_complete_from_snap "$1"
return $?
fi