Narrow the CompletionRequest API to being append-only.
Summary:
We currently allow any completion handler to read and manipulate the list of matches we
calculated so far. This leads to a few problems:
Firstly, a completion handler's logic can now depend on previously calculated results
by another handlers. No completion handler should have such an implicit dependency,
but the current API makes it likely that this could happen (or already happens). Especially
the fact that some completion handler deleted all previously calculated results can mess
things up right now.
Secondly, all completion handlers have knowledge about our internal data structures with
this API. This makes refactoring this internal data structure much harder than it should be.
Especially planned changes like the support of descriptions for completions are currently
giant patches because we have to refactor every single completion handler.
This patch narrows the contract the CompletionRequest has with the different handlers to:
1. A handler can suggest a completion.
2. A handler can ask how many suggestions we already have.
Point 2 obviously means we still have a dependency left between the different handlers, but
getting rid of this is too large to just append it to this patch.
Otherwise this patch just completely hides the internal StringList to the different handlers.
The CompletionRequest API now also ensures that the list of completions is unique and we
don't suggest the same value multiple times to the user. This property has been so far only
been ensured by the `Option` handler, but is now applied globally. This is part of this patch
as the OptionHandler is no longer able to implement this functionality itself.
Reviewers: jingham, davide, labath
Reviewed By: davide
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D49322
llvm-svn: 338151
diff --git a/lldb/source/Commands/CommandCompletions.cpp b/lldb/source/Commands/CommandCompletions.cpp
index bdfdbb8..7b351c5 100644
--- a/lldb/source/Commands/CommandCompletions.cpp
+++ b/lldb/source/Commands/CommandCompletions.cpp
@@ -90,7 +90,7 @@
} else {
completer.DoCompletion(searcher);
}
- return request.GetMatches().GetSize();
+ return request.GetNumberOfMatches();
}
static int DiskFilesOrDirectories(const llvm::Twine &partial_name,
@@ -103,7 +103,7 @@
partial_name.toVector(CompletionBuffer);
if (CompletionBuffer.size() >= PATH_MAX)
- return 0;
+ return matches.GetSize();
namespace fs = llvm::sys::fs;
namespace path = llvm::sys::path;
@@ -145,7 +145,7 @@
// Make sure it ends with a separator.
path::append(CompletionBuffer, path::get_separator());
matches.AppendString(CompletionBuffer);
- return 1;
+ return matches.GetSize();
}
// We want to keep the form the user typed, so we special case this to
@@ -224,13 +224,21 @@
return matches.GetSize();
}
+static int DiskFilesOrDirectories(CompletionRequest &request,
+ bool only_directories) {
+ request.SetWordComplete(false);
+ StandardTildeExpressionResolver resolver;
+ StringList matches;
+ DiskFilesOrDirectories(request.GetCursorArgumentPrefix(), only_directories,
+ matches, resolver);
+ request.AddCompletions(matches);
+ return request.GetNumberOfMatches();
+}
+
int CommandCompletions::DiskFiles(CommandInterpreter &interpreter,
CompletionRequest &request,
SearchFilter *searcher) {
- request.SetWordComplete(false);
- StandardTildeExpressionResolver Resolver;
- return DiskFiles(request.GetCursorArgumentPrefix(), request.GetMatches(),
- Resolver);
+ return DiskFilesOrDirectories(request, /*only_dirs*/ false);
}
int CommandCompletions::DiskFiles(const llvm::Twine &partial_file_name,
@@ -242,10 +250,7 @@
int CommandCompletions::DiskDirectories(CommandInterpreter &interpreter,
CompletionRequest &request,
SearchFilter *searcher) {
- request.SetWordComplete(false);
- StandardTildeExpressionResolver Resolver;
- return DiskDirectories(request.GetCursorArgumentPrefix(),
- request.GetMatches(), Resolver);
+ return DiskFilesOrDirectories(request, /*only_dirs*/ true);
}
int CommandCompletions::DiskDirectories(const llvm::Twine &partial_file_name,
@@ -267,7 +272,7 @@
} else {
completer.DoCompletion(searcher);
}
- return request.GetMatches().GetSize();
+ return request.GetNumberOfMatches();
}
int CommandCompletions::Symbols(CommandInterpreter &interpreter,
@@ -283,7 +288,7 @@
} else {
completer.DoCompletion(searcher);
}
- return request.GetMatches().GetSize();
+ return request.GetNumberOfMatches();
}
int CommandCompletions::SettingsNames(CommandInterpreter &interpreter,
@@ -304,20 +309,23 @@
}
size_t exact_matches_idx = SIZE_MAX;
- const size_t num_matches =
- g_property_names.AutoComplete(request.GetCursorArgumentPrefix(),
- request.GetMatches(), exact_matches_idx);
+ StringList matches;
+ g_property_names.AutoComplete(request.GetCursorArgumentPrefix(), matches,
+ exact_matches_idx);
request.SetWordComplete(exact_matches_idx != SIZE_MAX);
- return num_matches;
+ request.AddCompletions(matches);
+ return request.GetNumberOfMatches();
}
int CommandCompletions::PlatformPluginNames(CommandInterpreter &interpreter,
CompletionRequest &request,
SearchFilter *searcher) {
- const uint32_t num_matches = PluginManager::AutoCompletePlatformName(
- request.GetCursorArgumentPrefix(), request.GetMatches());
+ StringList new_matches;
+ std::size_t num_matches = PluginManager::AutoCompletePlatformName(
+ request.GetCursorArgumentPrefix(), new_matches);
request.SetWordComplete(num_matches == 1);
- return num_matches;
+ request.AddCompletions(new_matches);
+ return request.GetNumberOfMatches();
}
int CommandCompletions::ArchitectureNames(CommandInterpreter &interpreter,
@@ -409,10 +417,10 @@
filter->Search(*this);
// Now convert the filelist to completions:
for (size_t i = 0; i < m_matching_files.GetSize(); i++) {
- m_request.GetMatches().AppendString(
+ m_request.AddCompletion(
m_matching_files.GetFileSpecAtIndex(i).GetFilename().GetCString());
}
- return m_request.GetMatches().GetSize();
+ return m_request.GetNumberOfMatches();
}
//----------------------------------------------------------------------
@@ -478,9 +486,9 @@
filter->Search(*this);
collection::iterator pos = m_match_set.begin(), end = m_match_set.end();
for (pos = m_match_set.begin(); pos != end; pos++)
- m_request.GetMatches().AppendString((*pos).GetCString());
+ m_request.AddCompletion((*pos).GetCString());
- return m_request.GetMatches().GetSize();
+ return m_request.GetNumberOfMatches();
}
//----------------------------------------------------------------------
@@ -517,7 +525,7 @@
match = false;
if (match) {
- m_request.GetMatches().AppendString(cur_file_name);
+ m_request.AddCompletion(cur_file_name);
}
}
return Searcher::eCallbackReturnContinue;
@@ -525,5 +533,5 @@
size_t CommandCompletions::ModuleCompleter::DoCompletion(SearchFilter *filter) {
filter->Search(*this);
- return m_request.GetMatches().GetSize();
+ return m_request.GetNumberOfMatches();
}
diff --git a/lldb/source/Commands/CommandObjectCommands.cpp b/lldb/source/Commands/CommandObjectCommands.cpp
index 3012ee4..333f720 100644
--- a/lldb/source/Commands/CommandObjectCommands.cpp
+++ b/lldb/source/Commands/CommandObjectCommands.cpp
@@ -241,7 +241,7 @@
CommandCompletions::InvokeCommonCompletionCallbacks(
GetCommandInterpreter(), CommandCompletions::eDiskFileCompletion,
request, nullptr);
- return request.GetMatches().GetSize();
+ return request.GetNumberOfMatches();
}
Options *GetOptions() override { return &m_options; }
@@ -1429,7 +1429,7 @@
CommandCompletions::InvokeCommonCompletionCallbacks(
GetCommandInterpreter(), CommandCompletions::eDiskFileCompletion,
request, nullptr);
- return request.GetMatches().GetSize();
+ return request.GetNumberOfMatches();
}
Options *GetOptions() override { return &m_options; }
diff --git a/lldb/source/Commands/CommandObjectFrame.cpp b/lldb/source/Commands/CommandObjectFrame.cpp
index 0183c43..64de14f 100644
--- a/lldb/source/Commands/CommandObjectFrame.cpp
+++ b/lldb/source/Commands/CommandObjectFrame.cpp
@@ -470,7 +470,7 @@
CommandCompletions::InvokeCommonCompletionCallbacks(
GetCommandInterpreter(), CommandCompletions::eVariablePathCompletion,
request, nullptr);
- return request.GetMatches().GetSize();
+ return request.GetNumberOfMatches();
}
protected:
diff --git a/lldb/source/Commands/CommandObjectMultiword.cpp b/lldb/source/Commands/CommandObjectMultiword.cpp
index ade1a2d..19fcf60 100644
--- a/lldb/source/Commands/CommandObjectMultiword.cpp
+++ b/lldb/source/Commands/CommandObjectMultiword.cpp
@@ -143,7 +143,7 @@
if (num_subcmd_matches > 0) {
error_msg.append(" Possible completions:");
- for (size_t i = 0; i < num_subcmd_matches; i++) {
+ for (size_t i = 0; i < matches.GetSize(); i++) {
error_msg.append("\n\t");
error_msg.append(matches.GetStringAtIndex(i));
}
@@ -190,21 +190,22 @@
// Any of the command matches will provide a complete word, otherwise the
// individual completers will override this.
request.SetWordComplete(true);
- auto &matches = request.GetMatches();
auto arg0 = request.GetParsedLine()[0].ref;
if (request.GetCursorIndex() == 0) {
- AddNamesMatchingPartialString(m_subcommand_dict, arg0, matches);
+ StringList new_matches;
+ AddNamesMatchingPartialString(m_subcommand_dict, arg0, new_matches);
+ request.AddCompletions(new_matches);
- if (matches.GetSize() == 1 && matches.GetStringAtIndex(0) != nullptr &&
- (arg0 == matches.GetStringAtIndex(0))) {
+ if (new_matches.GetSize() == 1 &&
+ new_matches.GetStringAtIndex(0) != nullptr &&
+ (arg0 == new_matches.GetStringAtIndex(0))) {
StringList temp_matches;
CommandObject *cmd_obj = GetSubcommandObject(arg0, &temp_matches);
if (cmd_obj != nullptr) {
if (request.GetParsedLine().GetArgumentCount() == 1) {
request.SetWordComplete(true);
} else {
- matches.DeleteStringAtIndex(0);
request.GetParsedLine().Shift();
request.SetCursorCharPosition(0);
request.GetParsedLine().AppendArgument(llvm::StringRef());
@@ -212,14 +213,17 @@
}
}
}
- return matches.GetSize();
+ return new_matches.GetSize();
} else {
- CommandObject *sub_command_object = GetSubcommandObject(arg0, &matches);
+ StringList new_matches;
+ CommandObject *sub_command_object = GetSubcommandObject(arg0, &new_matches);
if (sub_command_object == nullptr) {
- return matches.GetSize();
+ request.AddCompletions(new_matches);
+ return request.GetNumberOfMatches();
} else {
// Remove the one match that we got from calling GetSubcommandObject.
- matches.DeleteStringAtIndex(0);
+ new_matches.DeleteStringAtIndex(0);
+ request.AddCompletions(new_matches);
request.GetParsedLine().Shift();
request.SetCursorIndex(request.GetCursorIndex() - 1);
return sub_command_object->HandleCompletion(request);
@@ -366,7 +370,6 @@
CommandObject *proxy_command = GetProxyCommandObject();
if (proxy_command)
return proxy_command->HandleCompletion(request);
- request.GetMatches().Clear();
return 0;
}
@@ -375,7 +378,6 @@
CommandObject *proxy_command = GetProxyCommandObject();
if (proxy_command)
return proxy_command->HandleArgumentCompletion(request, opt_element_vector);
- request.GetMatches().Clear();
return 0;
}
diff --git a/lldb/source/Commands/CommandObjectPlatform.cpp b/lldb/source/Commands/CommandObjectPlatform.cpp
index f822a8b..22a9a16 100644
--- a/lldb/source/Commands/CommandObjectPlatform.cpp
+++ b/lldb/source/Commands/CommandObjectPlatform.cpp
@@ -181,7 +181,7 @@
int HandleCompletion(CompletionRequest &request) override {
CommandCompletions::PlatformPluginNames(GetCommandInterpreter(), request,
nullptr);
- return request.GetMatches().GetSize();
+ return request.GetNumberOfMatches();
}
Options *GetOptions() override { return &m_option_group; }
@@ -1583,9 +1583,9 @@
const uint32_t num_matches = process_infos.GetSize();
if (num_matches > 0) {
for (uint32_t i = 0; i < num_matches; ++i) {
- request.GetMatches().AppendString(
+ request.AddCompletion(llvm::StringRef(
process_infos.GetProcessNameAtIndex(i),
- process_infos.GetProcessNameLengthAtIndex(i));
+ process_infos.GetProcessNameLengthAtIndex(i)));
}
}
}
diff --git a/lldb/source/Commands/CommandObjectPlugin.cpp b/lldb/source/Commands/CommandObjectPlugin.cpp
index 1f379a2..13ef6b2 100644
--- a/lldb/source/Commands/CommandObjectPlugin.cpp
+++ b/lldb/source/Commands/CommandObjectPlugin.cpp
@@ -48,7 +48,7 @@
CommandCompletions::InvokeCommonCompletionCallbacks(
GetCommandInterpreter(), CommandCompletions::eDiskFileCompletion,
request, nullptr);
- return request.GetMatches().GetSize();
+ return request.GetNumberOfMatches();
}
protected:
diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp
index 3ac2791..eb5a19a 100644
--- a/lldb/source/Commands/CommandObjectProcess.cpp
+++ b/lldb/source/Commands/CommandObjectProcess.cpp
@@ -141,7 +141,7 @@
CommandCompletions::InvokeCommonCompletionCallbacks(
GetCommandInterpreter(), CommandCompletions::eDiskFileCompletion,
request, nullptr);
- return request.GetMatches().GetSize();
+ return request.GetNumberOfMatches();
}
Options *GetOptions() override { return &m_options; }
@@ -410,9 +410,9 @@
const size_t num_matches = process_infos.GetSize();
if (num_matches > 0) {
for (size_t i = 0; i < num_matches; ++i) {
- request.GetMatches().AppendString(
+ request.AddCompletion(llvm::StringRef(
process_infos.GetProcessNameAtIndex(i),
- process_infos.GetProcessNameLengthAtIndex(i));
+ process_infos.GetProcessNameLengthAtIndex(i)));
}
}
}
diff --git a/lldb/source/Commands/CommandObjectSettings.cpp b/lldb/source/Commands/CommandObjectSettings.cpp
index f259f2f..3db1e35 100644
--- a/lldb/source/Commands/CommandObjectSettings.cpp
+++ b/lldb/source/Commands/CommandObjectSettings.cpp
@@ -172,7 +172,7 @@
}
}
}
- return request.GetMatches().GetSize();
+ return request.GetNumberOfMatches();
}
protected:
@@ -272,7 +272,7 @@
CommandCompletions::InvokeCommonCompletionCallbacks(
GetCommandInterpreter(), CommandCompletions::eSettingsNameCompletion,
request, nullptr);
- return request.GetMatches().GetSize();
+ return request.GetNumberOfMatches();
}
protected:
@@ -338,7 +338,7 @@
CommandCompletions::InvokeCommonCompletionCallbacks(
GetCommandInterpreter(), CommandCompletions::eSettingsNameCompletion,
request, nullptr);
- return request.GetMatches().GetSize();
+ return request.GetNumberOfMatches();
}
protected:
@@ -427,7 +427,7 @@
CommandCompletions::InvokeCommonCompletionCallbacks(
GetCommandInterpreter(), CommandCompletions::eSettingsNameCompletion,
request, nullptr);
- return request.GetMatches().GetSize();
+ return request.GetNumberOfMatches();
}
protected:
@@ -544,7 +544,7 @@
GetCommandInterpreter(), CommandCompletions::eSettingsNameCompletion,
request, nullptr);
- return request.GetMatches().GetSize();
+ return request.GetNumberOfMatches();
}
protected:
@@ -644,7 +644,7 @@
GetCommandInterpreter(), CommandCompletions::eSettingsNameCompletion,
request, nullptr);
- return request.GetMatches().GetSize();
+ return request.GetNumberOfMatches();
}
protected:
@@ -749,7 +749,7 @@
GetCommandInterpreter(), CommandCompletions::eSettingsNameCompletion,
request, nullptr);
- return request.GetMatches().GetSize();
+ return request.GetNumberOfMatches();
}
protected:
@@ -843,7 +843,7 @@
GetCommandInterpreter(), CommandCompletions::eSettingsNameCompletion,
request, nullptr);
- return request.GetMatches().GetSize();
+ return request.GetNumberOfMatches();
}
protected:
@@ -924,7 +924,7 @@
GetCommandInterpreter(), CommandCompletions::eSettingsNameCompletion,
request, nullptr);
- return request.GetMatches().GetSize();
+ return request.GetNumberOfMatches();
}
protected:
diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp
index a9062c1..8be43cb 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -201,7 +201,7 @@
CommandCompletions::InvokeCommonCompletionCallbacks(
GetCommandInterpreter(), CommandCompletions::eDiskFileCompletion,
request, nullptr);
- return request.GetMatches().GetSize();
+ return request.GetNumberOfMatches();
}
protected:
@@ -1810,7 +1810,7 @@
CommandCompletions::InvokeCommonCompletionCallbacks(
GetCommandInterpreter(), CommandCompletions::eModuleCompletion, request,
nullptr);
- return request.GetMatches().GetSize();
+ return request.GetNumberOfMatches();
}
};
@@ -1851,7 +1851,7 @@
CommandCompletions::InvokeCommonCompletionCallbacks(
GetCommandInterpreter(), CommandCompletions::eSourceFileCompletion,
request, nullptr);
- return request.GetMatches().GetSize();
+ return request.GetNumberOfMatches();
}
};
@@ -2393,7 +2393,7 @@
CommandCompletions::InvokeCommonCompletionCallbacks(
GetCommandInterpreter(), CommandCompletions::eDiskFileCompletion,
request, nullptr);
- return request.GetMatches().GetSize();
+ return request.GetNumberOfMatches();
}
protected:
@@ -3987,7 +3987,7 @@
CommandCompletions::InvokeCommonCompletionCallbacks(
GetCommandInterpreter(), CommandCompletions::eDiskFileCompletion,
request, nullptr);
- return request.GetMatches().GetSize();
+ return request.GetNumberOfMatches();
}
Options *GetOptions() override { return &m_option_group; }