Fix tab completion for command arguments containing spaces

[tberghammer]

Summary

Fix tab completion for command arguments containing spaces

If a command argument contains a space then it have to be escaped with backslash signs so the argument parsing logic can parse it properly. This CL fixes the tab completion code for the arguments to create completions with correctly escaped strings.

[labath]

source/Interpreter/Options.cpp

996 ↗

(On Diff #33688)

If there is any chance quotes will appear here (', ", `), you will need to escape those as well, as they are special to the command interpreter. It might be worth making a utility function out of that (Args::GetCommandInterpreterSafeArgument ?), as a canonical way to escape stuff.

[tberghammer]

Address review comment (the function already exists in Args)

[tberghammer]

Move escaping code to Args

[clayborg]

Looks good.

lldb/trunk/include/lldb/Interpreter/Args.h

public:
     // characters and expand them into text. So '\n' will turn into two characters
     // like "\n" which is suitable for human reading. When a character is not
     // printable and isn't one of the common in escape sequences listed in the
     // help for EncodeEscapeSequences, then it will be encoded as octal. Printable
     // characters are left alone.
     static void
     ExpandEscapedCharacters (const char *src, std::string &dst);
 
+    static std::string
+    EscapeLLDBCommandArgument (const std::string& arg, char quote_char);
+
     // This one isn't really relevant to Arguments per se, but we're using the Args as a
     // general strings container, so...
     void
     LongestCommonPrefix (std::string &common_prefix);
 
 protected:
     //------------------------------------------------------------------
     // Classes that inherit from Args can see and modify these

lldb/trunk/source/Interpreter/Args.cpp

if (src)
                         }
                         break;
                 }
             }
         }
     }
 }
 
+std::string
+Args::EscapeLLDBCommandArgument (const std::string& arg, char quote_char)
+{
+    const char* chars_to_escape = nullptr;
+    switch (quote_char)
+    {
+        case '\0':
+            chars_to_escape = " \t\\'\"`";
+            break;
+        case '\'':
+            chars_to_escape = "";
+            break;
+        case '"':
+            chars_to_escape = "$\"`\\";
+            break;
+        default:
+            assert(false && "Unhandled quote character");
+    }
+
+    std::string res;
+    res.reserve(arg.size());
+    for (char c : arg)
+    {
+        if (::strchr(chars_to_escape, c))
+            res.push_back('\\');
+        res.push_back(c);
+    }
+    return res;
+}
+

lldb/trunk/source/Interpreter/CommandInterpreter.cpp

num_command_matches = HandleCompletionMatches (parsed_line,
                                                    cursor_index, 
                                                    cursor_char_position, 
                                                    match_start_point,
                                                    max_return_elements,
                                                    word_complete,
                                                    matches);
 
     if (num_command_matches <= 0)
-            return num_command_matches;
 
     if (num_args == 0)
     {
         // If we got an empty string, insert nothing.
         matches.InsertStringAtIndex(0, "");
     }
     else
     {
         // Now figure out if there is a common substring, and if so put that in element 0, otherwise
         // put an empty string in element 0.
         std::string command_partial_str;
         if (cursor_index >= 0)
             command_partial_str.assign(parsed_line.GetArgumentAtIndex(cursor_index), 
                                        parsed_line.GetArgumentAtIndex(cursor_index) + cursor_char_position);
 
         std::string common_prefix;
         matches.LongestCommonPrefix (common_prefix);
         const size_t partial_name_len = command_partial_str.size();
+        common_prefix.erase (0, partial_name_len);
 
         // If we matched a unique single command, add a space...
         // Only do this if the completer told us this was a complete word, however...
         if (num_command_matches == 1 && word_complete)
         {
             char quote_char = parsed_line.GetArgumentQuoteCharAtIndex(cursor_index);
+            common_prefix = Args::EscapeLLDBCommandArgument(common_prefix, quote_char);
             if (quote_char != '\0')
                 common_prefix.push_back(quote_char);
-
             common_prefix.push_back(' ');
         }
-        common_prefix.erase (0, partial_name_len);
         matches.InsertStringAtIndex(0, common_prefix.c_str());
     }
     return num_command_matches;
 }
 
 
 CommandInterpreter::~CommandInterpreter ()
 {

lldb/trunk/test/functionalities/completion/Makefile

+LEVEL = ../../make
+
+CXX_SOURCES := main.cpp
+
+include $(LEVEL)/Makefile.rules

lldb/trunk/test/functionalities/completion/TestCompletion.py

def test_target_create_dash_co(self):
         self.complete_from_to('target create --co', 'target create --core ')
 
     @expectedFailureHostWindows("llvm.org/pr22274: need a pexpect replacement for windows")
     @skipIfFreeBSD # timing out on the FreeBSD buildbot
     def test_target_va(self):
         """Test that 'target va' completes to 'target variable '."""
         self.complete_from_to('target va', 'target variable ')
 
+    @skipUnlessDarwin
+    @dsym_test
+    def test_symbol_name_dsym(self):
+        self.buildDsym()
+        self.complete_from_to('''file a.out
+                                 breakpoint set -n Fo''',
+                              'breakpoint set -n Foo::Bar(int,\\ int)',
+                              turn_off_re_match=True)
+
+    @dwarf_test
+    def test_symbol_name_dwarf(self):
+        self.buildDwarf()
+        self.complete_from_to('''file a.out
+                                 breakpoint set -n Fo''',
+                              'breakpoint set -n Foo::Bar(int,\\ int)',
+                              turn_off_re_match=True)
+
     def complete_from_to(self, str_input, patterns, turn_off_re_match=False):
         """Test that the completion mechanism completes str_input to patterns,
         where patterns could be a pattern-string or a list of pattern-strings"""
         import pexpect
         # Patterns should not be None in order to proceed.
         self.assertFalse(patterns is None)
         # And should be either a string or list of strings.  Check for list type
         # below, if not, make a list out of the singleton string.  If patterns

lldb/trunk/test/functionalities/completion/main.cpp

+class Foo
+{
+public:
+    int Bar(int x, int y)
+    {
+        return x + y;
+    }
+};
+
+int main()
+{
+    Foo f;
+    f.Bar(1, 2);
+}