From 14f6465c157b36c50ffe431463a9c94efda42b99 Mon Sep 17 00:00:00 2001 From: Raphael Isemann Date: Mon, 23 Sep 2019 09:46:17 +0000 Subject: [PATCH] [lldb] Make cursor index in CompletionRequest unsigned The fact that index==-1 means "no arguments" is not obvious and only used in one place from what I can tell. Also fixes several warnings about using the cursor index as if it was a size_t when comparing. Not fully NFC as we now also correctly update the partial argument list when injecting the fake empty argument in the CompletionRequest constructor. llvm-svn: 372566 --- lldb/include/lldb/Utility/CompletionRequest.h | 6 ++-- lldb/source/Commands/CommandObjectSettings.cpp | 5 ++- lldb/source/Interpreter/CommandInterpreter.cpp | 4 +-- lldb/source/Interpreter/Options.cpp | 4 +-- lldb/source/Utility/CompletionRequest.cpp | 40 ++++++++++-------------- lldb/unittests/Utility/CompletionRequestTest.cpp | 22 +++++++++++-- 6 files changed, 46 insertions(+), 35 deletions(-) diff --git a/lldb/include/lldb/Utility/CompletionRequest.h b/lldb/include/lldb/Utility/CompletionRequest.h index a4305195..849f0cf 100644 --- a/lldb/include/lldb/Utility/CompletionRequest.h +++ b/lldb/include/lldb/Utility/CompletionRequest.h @@ -126,8 +126,8 @@ public: m_partial_parsed_line.Shift(); } - void SetCursorIndex(int i) { m_cursor_index = i; } - int GetCursorIndex() const { return m_cursor_index; } + void SetCursorIndex(size_t i) { m_cursor_index = i; } + size_t GetCursorIndex() const { return m_cursor_index; } void SetCursorCharPosition(int pos) { m_cursor_char_position = pos; } int GetCursorCharPosition() const { return m_cursor_char_position; } @@ -208,7 +208,7 @@ private: /// The command line until the cursor position parsed as arguments. Args m_partial_parsed_line; /// The index of the argument in which the completion cursor is. - int m_cursor_index; + size_t m_cursor_index; /// The cursor position in the argument indexed by m_cursor_index. int m_cursor_char_position; diff --git a/lldb/source/Commands/CommandObjectSettings.cpp b/lldb/source/Commands/CommandObjectSettings.cpp index f1e3bf1..6ed63c1 100644 --- a/lldb/source/Commands/CommandObjectSettings.cpp +++ b/lldb/source/Commands/CommandObjectSettings.cpp @@ -130,9 +130,8 @@ insert-before or insert-after."); const size_t argc = request.GetParsedLine().GetArgumentCount(); const char *arg = nullptr; - int setting_var_idx; - for (setting_var_idx = 0; setting_var_idx < static_cast(argc); - ++setting_var_idx) { + size_t setting_var_idx; + for (setting_var_idx = 0; setting_var_idx < argc; ++setting_var_idx) { arg = request.GetParsedLine().GetArgumentAtIndex(setting_var_idx); if (arg && arg[0] != '-') break; // We found our setting variable name index diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp index c8a64b0..cb61046 100644 --- a/lldb/source/Interpreter/CommandInterpreter.cpp +++ b/lldb/source/Interpreter/CommandInterpreter.cpp @@ -1756,7 +1756,7 @@ void CommandInterpreter::HandleCompletionMatches(CompletionRequest &request) { // For any of the command completions a unique match will be a complete word. - if (request.GetCursorIndex() == -1) { + if (request.GetPartialParsedLine().GetArgumentCount() == 0) { // We got nothing on the command line, so return the list of commands bool include_aliases = true; StringList new_matches, descriptions; @@ -1780,7 +1780,7 @@ void CommandInterpreter::HandleCompletionMatches(CompletionRequest &request) { new_matches.DeleteStringAtIndex(0); new_descriptions.DeleteStringAtIndex(0); request.GetParsedLine().AppendArgument(llvm::StringRef()); - request.SetCursorIndex(request.GetCursorIndex() + 1); + request.SetCursorIndex(request.GetCursorIndex() + 1U); request.SetCursorCharPosition(0); } } diff --git a/lldb/source/Interpreter/Options.cpp b/lldb/source/Interpreter/Options.cpp index 1f63382..d2f2cfa 100644 --- a/lldb/source/Interpreter/Options.cpp +++ b/lldb/source/Interpreter/Options.cpp @@ -655,8 +655,8 @@ bool Options::HandleOptionCompletion(CompletionRequest &request, llvm::StringRef cur_opt_str = request.GetCursorArgumentPrefix(); for (size_t i = 0; i < opt_element_vector.size(); i++) { - int opt_pos = opt_element_vector[i].opt_pos; - int opt_arg_pos = opt_element_vector[i].opt_arg_pos; + size_t opt_pos = static_cast(opt_element_vector[i].opt_pos); + size_t opt_arg_pos = static_cast(opt_element_vector[i].opt_arg_pos); int opt_defs_index = opt_element_vector[i].opt_defs_index; if (opt_pos == request.GetCursorIndex()) { // We're completing the option itself. diff --git a/lldb/source/Utility/CompletionRequest.cpp b/lldb/source/Utility/CompletionRequest.cpp index 7fd5260..aaaef78 100644 --- a/lldb/source/Utility/CompletionRequest.cpp +++ b/lldb/source/Utility/CompletionRequest.cpp @@ -22,34 +22,28 @@ CompletionRequest::CompletionRequest(llvm::StringRef command_line, // parsed_line is the one containing the cursor, and the cursor is after the // last character. m_parsed_line = Args(command_line); - m_partial_parsed_line = Args(command_line.substr(0, raw_cursor_pos)); + llvm::StringRef partial_command(command_line.substr(0, raw_cursor_pos)); + m_partial_parsed_line = Args(partial_command); - m_cursor_index = m_partial_parsed_line.GetArgumentCount() - 1; - - if (m_cursor_index == -1) + if (m_partial_parsed_line.GetArgumentCount() == 0) { + m_cursor_index = 0; m_cursor_char_position = 0; - else + } else { + m_cursor_index = m_partial_parsed_line.GetArgumentCount() - 1U; m_cursor_char_position = strlen(m_partial_parsed_line.GetArgumentAtIndex(m_cursor_index)); + } - const char *cursor = command_line.data() + raw_cursor_pos; - if (raw_cursor_pos > 0 && cursor[-1] == ' ') { - // We are just after a space. If we are in an argument, then we will - // continue parsing, but if we are between arguments, then we have to - // complete whatever the next element would be. We can distinguish the two - // cases because if we are in an argument (e.g. because the space is - // protected by a quote) then the space will also be in the parsed - // argument... - - const char *current_elem = - m_partial_parsed_line.GetArgumentAtIndex(m_cursor_index); - if (m_cursor_char_position == 0 || - current_elem[m_cursor_char_position - 1] != ' ') { - m_parsed_line.InsertArgumentAtIndex(m_cursor_index + 1, llvm::StringRef(), - '\0'); - m_cursor_index++; - m_cursor_char_position = 0; - } + // The cursor is after a space but the space is not part of the argument. + // Let's add an empty fake argument to the end to make sure the completion + // code Note: The space could be part of the last argument when it's quoted. + if (partial_command.endswith(" ") && + !GetCursorArgumentPrefix().endswith(" ")) { + m_parsed_line.AppendArgument(llvm::StringRef()); + m_partial_parsed_line.AppendArgument(llvm::StringRef()); + // Set the cursor to the start of the fake argument. + m_cursor_index++; + m_cursor_char_position = 0; } } diff --git a/lldb/unittests/Utility/CompletionRequestTest.cpp b/lldb/unittests/Utility/CompletionRequestTest.cpp index ad3c560..bdd51eb 100644 --- a/lldb/unittests/Utility/CompletionRequestTest.cpp +++ b/lldb/unittests/Utility/CompletionRequestTest.cpp @@ -14,7 +14,7 @@ using namespace lldb_private; TEST(CompletionRequest, Constructor) { std::string command = "a bad c"; const unsigned cursor_pos = 3; - const int arg_index = 1; + const size_t arg_index = 1; const int arg_cursor_pos = 1; StringList matches; CompletionResult result; @@ -31,6 +31,24 @@ TEST(CompletionRequest, Constructor) { EXPECT_STREQ(request.GetPartialParsedLine().GetArgumentAtIndex(1), "b"); } +TEST(CompletionRequest, FakeLastArg) { + // We insert an empty fake argument into the argument list when the + // cursor is after a space. + std::string command = "a bad c "; + const unsigned cursor_pos = command.size(); + CompletionResult result; + + CompletionRequest request(command, cursor_pos, result); + + EXPECT_STREQ(request.GetRawLine().str().c_str(), command.c_str()); + EXPECT_EQ(request.GetRawCursorPos(), cursor_pos); + EXPECT_EQ(request.GetCursorIndex(), 3U); + EXPECT_EQ(request.GetCursorCharPosition(), 0); + + EXPECT_EQ(request.GetPartialParsedLine().GetArgumentCount(), 4U); + EXPECT_STREQ(request.GetPartialParsedLine().GetArgumentAtIndex(3), ""); +} + TEST(CompletionRequest, TryCompleteCurrentArgGood) { std::string command = "a bad c"; StringList matches, descriptions; @@ -71,7 +89,7 @@ TEST(CompletionRequest, TryCompleteCurrentArgMode) { TEST(CompletionRequest, ShiftArguments) { std::string command = "a bad c"; const unsigned cursor_pos = 3; - const int arg_index = 1; + const size_t arg_index = 1; const int arg_cursor_pos = 1; StringList matches; CompletionResult result; -- 2.7.4