[completion] Remove extra quoting on completion candidates

Specifically, for complete -F bashfunc

This makes upstream git-completion.bash work!  (one old copy is in
testdata/completion/git-completion.bash)

It uncovered some bugs in 'complete -W' though.  There are more tests we
can do.

This is issue #915.

core/completion.py

@@ -854,14 +854,13 @@ def PrintSpec(self, f):
         f.write('  prefix: %s\n' % self.prefix)
         f.write('  suffix: %s\n' % self.prefix)
 
-    def Matches(self, comp):
-        # type: (Api) -> Iterator[Tuple[str, bool]]
+    def AllMatches(self, comp):
+        # type: (Api) -> Iterator[Tuple[str, comp_action_t]]
         """yield completion candidates."""
         num_matches = 0
 
         for a in self.actions:
             action_kind = a.ActionKind()
-            is_fs_action = action_kind == comp_action_e.FileSystem
             for match in a.Matches(comp):
                 # Special case hack to match bash for compgen -F.  It doesn't filter by
                 # to_complete!
@@ -874,7 +873,7 @@ def Matches(self, comp):
                 # There are two kinds of filters: changing the string, and filtering
                 # the set of strings.  So maybe have modifiers AND filters?  A triple.
                 if show:
-                    yield self.prefix + match + self.suffix, is_fs_action
+                    yield self.prefix + match + self.suffix, action_kind
                     num_matches += 1
 
         # NOTE: extra_actions and else_actions don't respect -X, -P or -S, and we
@@ -884,13 +883,15 @@ def Matches(self, comp):
         # for -o plusdirs
         for a in self.extra_actions:
             for match in a.Matches(comp):
-                yield match, True  # We know plusdirs is a file system action
+                # We know plusdirs is a file system action
+                yield match, comp_action_e.FileSystem
 
         # for -o default and -o dirnames
         if num_matches == 0:
             for a in self.else_actions:
                 for match in a.Matches(comp):
-                    yield match, True  # both are FileSystemAction
+                    # both are FileSystemAction
+                    yield match, comp_action_e.FileSystem
 
         # What if the cursor is not at the end of line?  See readline interface.
         # That's OK -- we just truncate the line at the cursor?
@@ -1304,7 +1305,7 @@ def _PostProcess(
         # TODO: dedupe candidates?  You can get two 'echo' in bash, which is dumb.
 
         i = 0
-        for candidate, is_fs_action in user_spec.Matches(comp):
+        for candidate, action_kind in user_spec.AllMatches(comp):
             # SUBTLE: dynamic_opts is part of compopt_state, which ShellFuncAction
             # can mutate!  So we don't want to pull this out of the loop.
             #
@@ -1340,7 +1341,7 @@ def _PostProcess(
 
             # compopt -o filenames is for user-defined actions.  Or any
             # FileSystemAction needs it.
-            if is_fs_action or opt_filenames:
+            if action_kind == comp_action_e.FileSystem or opt_filenames:
                 if path_stat.isdir(candidate):
                     s = line_until_word + ShellQuoteB(candidate) + '/'
                     yield s
@@ -1351,7 +1352,10 @@ def _PostProcess(
                 opt_nospace = dynamic_opts['nospace']
 
             sp = '' if opt_nospace else ' '
-            yield line_until_word + ShellQuoteB(candidate) + sp
+            cand = (candidate if action_kind == comp_action_e.BashFunc else
+                    ShellQuoteB(candidate))
+
+            yield line_until_word + cand + sp
 
             # NOTE: Can't use %.2f in production build!
             i += 1

core/completion_test.py

@@ -13,7 +13,7 @@
 import sys
 
 from _devbuild.gen.option_asdl import option_i
-from _devbuild.gen.runtime_asdl import value_e, Proc
+from _devbuild.gen.runtime_asdl import value_e, Proc, comp_action_e
 from _devbuild.gen.syntax_asdl import proc_sig
 from core import completion  # module under test
 from core import comp_ui
@@ -212,14 +212,16 @@ def testShellFuncExecution(self):
 
     def testUserSpec(self):
         comp = self._CompApi(['f'], 0, 'f')
-        matches = list(U1.Matches(comp))
-        self.assertEqual([('foo.py', False), ('foo', False)], matches)
+        matches = list(U1.AllMatches(comp))
+        self.assertEqual(
+                [('foo.py', comp_action_e.Other), ('foo', comp_action_e.Other)],
+                matches)
 
         predicate = completion.GlobPredicate(False, '*.py')
         c2 = completion.UserSpec([A1], [], [], predicate, '', '')
         comp = self._CompApi(['f'], 0, 'f')
-        matches = list(c2.Matches(comp))
-        self.assertEqual([('foo.py', False)], matches)
+        matches = list(c2.AllMatches(comp))
+        self.assertEqual([('foo.py', comp_action_e.Other)], matches)
 
 
 class RootCompleterTest(unittest.TestCase):

osh/builtin_comp.py

@@ -379,7 +379,7 @@ def Run(self, cmd_val):
         comp = completion.Api('', 0, 0)  # empty string
         comp.Update('compgen', to_complete, '', -1, None)
         try:
-            for m, _ in user_spec.Matches(comp):
+            for m, _ in user_spec.AllMatches(comp):
                 matched = True
                 print(m)
         except error.FatalRuntime:

spec/ysh-completion.test.sh

@@ -1,4 +1,4 @@
-## oils_failures_allowed: 8
+## oils_failures_allowed: 7
 
 #### compexport
 
@@ -44,12 +44,23 @@ compexport -c 'cd ca'
 
 . $REPO_ROOT/testdata/completion/quoting.bash
 
-compexport -c 'b-argv '
+compexport -c 'pq-argv c'
 echo
 
-compexport -c 'sq-argv '
+# note: excluding "can't" because there's an intentional bug
+compexport -c 'sq-argv ch'
+
+# Quoting doesn't match bash exactly, but it definitely works interactively!
 
 ## STDOUT:
+"pq-argv $'can\\'t' "
+"pq-argv 'ch with space' "
+"pq-argv checkout "
+"pq-argv cherry "
+
+"sq-argv 'ch with space' "
+"sq-argv 'checkout' "
+"sq-argv 'cherry' "
 ## END
 
 #### complete -W quoting

testdata/completion/quoting.bash

@@ -31,7 +31,7 @@
 
 declare -a commands=(
   cherry checkout
-  'file with space'
+  'ch with space'  # you have to type 'ch\ ' to select this completion
   "can't"  # apostrophe
   $'one\ntwo'  # newline
   '$file_not_var'
@@ -42,7 +42,7 @@ declare -a commands=(
 # You might need to add quotes
 #declare -a commands=( cherry checkout 'check\ spaces' )
 
-__backslash() {
+__printf_q() {
   #argv "$@"
 
   local cur=$2
@@ -87,7 +87,7 @@ __sq() {
   for cmd in "${commands[@]}"; do
     case $cmd in
       $cur*)
-        # Wrong when there's a single quote!
+        # BUG when there's a single quote!
         local quoted="'$cmd'"
         #local quoted="$cmd"
         results+=( "$quoted" )
@@ -106,7 +106,7 @@ argv() {
   python3 -c 'import sys; print(sys.argv[1:])' "$@"
 }
 
-b-argv() {
+pq-argv() {
   argv "$@"
 }
 
@@ -142,7 +142,7 @@ v-argv() {
   argv "$@"
 }
 
-complete -F __backslash b-argv
+complete -F __printf_q pq-argv
 complete -F __sq sq-argv
 
 # Hm this doesn't work.  It comes across as one candidate.