[lldb][NFC] Remove WordComplete mode, make result array indexed from 0 and remove any undocumented/redundant return values

Summary:
We still have some leftovers of the old completion API in the internals of
LLDB that haven't been replaced by the new CompletionRequest. These leftovers
are:

* The return values (int/size_t) in all completion functions.
* Our result array that starts indexing at 1.
* `WordComplete` mode.

I didn't replace them back then because it's tricky to figure out what exactly they
are used for and the completion code is relatively untested. I finally got around
to writing more tests for the API and understanding the semantics, so I think it's
a good time to get rid of them.

A few words why those things should be removed/replaced:

* The return values are really cryptic, partly redundant and rarely documented.
  They are also completely ignored by Xcode, so whatever information they contain will end up
  breaking Xcode's completion mechanism. They are also partly impossible to even implement
  as we assign negative values special meaning and our completion API sometimes returns size_t.

  Completion functions are supposed to return -2 to rewrite the current line. We seem to use this
  in some untested code path to expand the history repeat character to the full command, but
  I haven't figured out why that doesn't work at the moment.
  Completion functions return -1 to 'insert the completion character', but that isn't implemented
  (even though we seem to activate this feature in LLDB sometimes).
  All positive values have to match the number of results. This is obviously just redundant information
  as the user can just look at the result list to get that information (which is what Xcode does).

* The result array that starts indexing at 1 is obviously unexpected. The first element of the array is
  reserved for the common prefix of all completions (e.g. "foobar" and "footar" -> "foo"). The idea is
  that we calculate this to make the life of the API caller easier, but obviously forcing people to have
  1-based indices is not helpful (or even worse, forces them to manually copy the results to make it
  0-based like Xcode has to do).

* The `WordComplete` mode indicates that LLDB should enter a space behind the completion. The
  idea is that we let the top-level API know that we just provided a full completion. Interestingly we
  `WordComplete` is just a single bool that somehow represents all N completions. And we always
  provide full completions in LLDB, so in theory it should always be true.
  The only use it currently serves is providing redundant information about whether we have a single
  definitive completion or not (which we already know from the number of results we get).

This patch essentially removes `WordComplete` mode and makes the result array indexed from 0.
It also removes all return values from all internal completion functions. The only non-redundant information
they contain is about rewriting the current line (which is broken), so that functionality was moved
to the CompletionRequest API. So you can now do `addCompletion("blub", "description", CompletionMode::RewriteLine)`
to do the same.

For the SB API we emulate the old behaviour by making the array indexed from 1 again with the common
prefix at index 0. I didn't keep the special negative return codes as we either never sent them before (e.g. -2) or we
didn't even implement them in the Editline handler (e.g. -1).

I tried to keep this patch minimal and I'm aware we can probably now even further simplify a bunch of related code,
but I would prefer doing this in follow-up NFC commits

Reviewers: JDevlieghere

Reviewed By: JDevlieghere

Subscribers: arphaman, abidh, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D66536

git-svn-id: https://llvm.org/svn/llvm-project/lldb/trunk@369624 91177308-0d34-0410-b5e6-96231b3b80d8

1 files changed, 83 insertions, 72 deletions

diff --git a/source/Host/common/Editline.cpp b/source/Host/common/Editline.cpp
index de6560c7a..a2e8773f8 100644
--- a/source/Host/common/Editline.cpp
+++ b/source/Host/common/Editline.cpp
@@ -864,26 +864,59 @@ unsigned char Editline::BufferEndCommand(int ch) {
 
 /// Prints completions and their descriptions to the given file. Only the
 /// completions in the interval [start, end) are printed.
-static void PrintCompletion(FILE *output_file, size_t start, size_t end,
-                            StringList &completions, StringList &descriptions) {
-  // This is an 'int' because of printf.
-  int max_len = 0;
-
-  for (size_t i = start; i < end; i++) {
-    const char *completion_str = completions.GetStringAtIndex(i);
-    max_len = std::max((int)strlen(completion_str), max_len);
+static void
+PrintCompletion(FILE *output_file,
+                llvm::ArrayRef<CompletionResult::Completion> results,
+                size_t max_len) {
+  for (const CompletionResult::Completion &c : results) {
+    fprintf(output_file, "\t%-*s", (int)max_len, c.GetCompletion().c_str());
+    if (!c.GetDescription().empty())
+      fprintf(output_file, " -- %s", c.GetDescription().c_str());
+    fprintf(output_file, "\n");
   }
+}
+
+static void
+DisplayCompletions(::EditLine *editline, FILE *output_file,
+                   llvm::ArrayRef<CompletionResult::Completion> results) {
+  assert(!results.empty());
 
-  for (size_t i = start; i < end; i++) {
-    const char *completion_str = completions.GetStringAtIndex(i);
-    const char *description_str = descriptions.GetStringAtIndex(i);
+  fprintf(output_file, "\n" ANSI_CLEAR_BELOW "Available completions:\n");
+  const size_t page_size = 40;
+  bool all = false;
+
+  auto longest =
+      std::max_element(results.begin(), results.end(), [](auto &c1, auto &c2) {
+        return c1.GetCompletion().size() < c2.GetCompletion().size();
+      });
 
-    if (completion_str)
-      fprintf(output_file, "\n\t%-*s", max_len, completion_str);
+  const size_t max_len = longest->GetCompletion().size();
 
-    // Print the description if we got one.
-    if (description_str && strlen(description_str))
-      fprintf(output_file, " -- %s", description_str);
+  if (results.size() < page_size) {
+    PrintCompletion(output_file, results, max_len);
+    return;
+  }
+
+  size_t cur_pos = 0;
+  while (cur_pos < results.size()) {
+    size_t remaining = results.size() - cur_pos;
+    size_t next_size = all ? remaining : std::min(page_size, remaining);
+
+    PrintCompletion(output_file, results.slice(cur_pos, next_size), max_len);
+
+    cur_pos += next_size;
+
+    if (cur_pos >= results.size())
+      break;
+
+    fprintf(output_file, "More (Y/n/a): ");
+    char reply = 'n';
+    int got_char = el_getc(editline, &reply);
+    fprintf(output_file, "\n");
+    if (got_char == -1 || reply == 'n')
+      break;
+    if (reply == 'a')
+      all = true;
   }
 }
 
@@ -892,8 +925,6 @@ unsigned char Editline::TabCommand(int ch) {
     return CC_ERROR;
 
   const LineInfo *line_info = el_line(m_editline);
-  StringList completions, descriptions;
-  int page_size = 40;
 
   llvm::StringRef line(line_info->buffer,
                        line_info->lastchar - line_info->buffer);
@@ -901,71 +932,51 @@ unsigned char Editline::TabCommand(int ch) {
   CompletionResult result;
   CompletionRequest request(line, cursor_index, result);
 
-  const int num_completions =
-      m_completion_callback(request, m_completion_callback_baton);
+  m_completion_callback(request, m_completion_callback_baton);
+
+  llvm::ArrayRef<CompletionResult::Completion> results = result.GetResults();
 
+  StringList completions;
   result.GetMatches(completions);
-  result.GetDescriptions(descriptions);
 
-  if (num_completions == 0)
+  if (results.size() == 0)
     return CC_ERROR;
-  //    if (num_completions == -1)
-  //    {
-  //        el_insertstr (m_editline, m_completion_key);
-  //        return CC_REDISPLAY;
-  //    }
-  //    else
-  if (num_completions == -2) {
-    // Replace the entire line with the first string...
-    el_deletestr(m_editline, line_info->cursor - line_info->buffer);
-    el_insertstr(m_editline, completions.GetStringAtIndex(0));
+
+  if (results.size() == 1) {
+    CompletionResult::Completion completion = results.front();
+    switch (completion.GetMode()) {
+    case CompletionMode::Normal: {
+      std::string to_add = completion.GetCompletion();
+      to_add = to_add.substr(request.GetCursorArgumentPrefix().size());
+      if (request.GetParsedArg().IsQuoted())
+        to_add.push_back(request.GetParsedArg().quote);
+      to_add.push_back(' ');
+      el_insertstr(m_editline, to_add.c_str());
+      break;
+    }
+    case CompletionMode::RewriteLine: {
+      el_deletestr(m_editline, line_info->cursor - line_info->buffer);
+      el_insertstr(m_editline, completion.GetCompletion().c_str());
+      break;
+    }
+    }
     return CC_REDISPLAY;
   }
 
   // If we get a longer match display that first.
-  const char *completion_str = completions.GetStringAtIndex(0);
-  if (completion_str != nullptr && *completion_str != '\0') {
-    el_insertstr(m_editline, completion_str);
+  std::string longest_prefix = completions.LongestCommonPrefix();
+  if (!longest_prefix.empty())
+    longest_prefix =
+        longest_prefix.substr(request.GetCursorArgumentPrefix().size());
+  if (!longest_prefix.empty()) {
+    el_insertstr(m_editline, longest_prefix.c_str());
     return CC_REDISPLAY;
   }
 
-  if (num_completions > 1) {
-    int num_elements = num_completions + 1;
-    fprintf(m_output_file, "\n" ANSI_CLEAR_BELOW "Available completions:");
-    if (num_completions < page_size) {
-      PrintCompletion(m_output_file, 1, num_elements, completions,
-                      descriptions);
-      fprintf(m_output_file, "\n");
-    } else {
-      int cur_pos = 1;
-      char reply;
-      int got_char;
-      while (cur_pos < num_elements) {
-        int endpoint = cur_pos + page_size;
-        if (endpoint > num_elements)
-          endpoint = num_elements;
-
-        PrintCompletion(m_output_file, cur_pos, endpoint, completions,
-                        descriptions);
-        cur_pos = endpoint;
-
-        if (cur_pos >= num_elements) {
-          fprintf(m_output_file, "\n");
-          break;
-        }
-
-        fprintf(m_output_file, "\nMore (Y/n/a): ");
-        reply = 'n';
-        got_char = el_getc(m_editline, &reply);
-        if (got_char == -1 || reply == 'n')
-          break;
-        if (reply == 'a')
-          page_size = num_elements - cur_pos;
-      }
-    }
-    DisplayInput();
-    MoveCursor(CursorLocation::BlockEnd, CursorLocation::EditingCursor);
-  }
+  DisplayCompletions(m_editline, m_output_file, results);
+
+  DisplayInput();
+  MoveCursor(CursorLocation::BlockEnd, CursorLocation::EditingCursor);
   return CC_REDISPLAY;
 }