[PATCH 2/6] gdb: move display of completion results into completion_result class

Andrew Burgess
2024-03-29T11:43:49Z

When using the 'complete' command to complete file names we have some
problems. The suggested completion should include any required
escaping, so if there is a filename '/tmp/aa"bb' (without the single
quotes), then this should be displayed in the completion output like:

(gdb) complete file /tmp/aa
file /tmp/aa\"bb

This doesn't currently happen. We already have some code in
filename_completer (completer.c) that handles adding the trailing '/'
character if the completion result is a directory, so we might be
tempted to add any needed escaping in that function, however if we do
then we run into a problem.

If there are multiple completion results from a 'complete' command,
then in complete_command (cli/cli-cmds.c) we sort the completion
results prior to printing them.

If those results have already had the escaping added then the sort
will be done on the text including the escape characters. This
means that results from the 'complete' command will appear in a
different order than readline would present them; readline sorts the
results and then adds and required escaping.

I think that the 'complete' command should behave the same as
readline, sort the entries and then add the escaping. This means that
we need to sort before adding the escaping.

There is a second problem when using the 'complete' command with file
names, that is trailing quote characters. The addition of a trailing
quote character is a bit complex due (I think) to the structure of
readline itself.

Adding the quote character currently occurs in two places in GDB. In
completion_tracker::build_completion_result (completer.c) we add the
trailing quote in the case where we only have a single result, and in
complete_command (cli/cli-cmds.c) we add the trailing quote character
if we have more than one result.

With these two cases we ensure that the 'complete' command always adds
a trailing quote (when necessary) to the results it displays.

However, I think that for filename completion we don't always want a
trailing quote. Consider if we have a file '/tmp/xxx/foo.c' (without
the quotes), and then we do:

(gdb) complete file /tmp/xx

What should the result of this be? Well, if we use TAB completion
like this:

(gdb) file /tmp/xx<TAB>
(gdb) file /tmp/xxx/

then readline completes the directory 'xxx/', but doesn't try to
complete the filename within the xxx/ directory until we press <TAB>
again. So I think that the results of the 'complete' command should
share this behaviour:

(gdb) complete file /tmp/xx
file /tmp/xxx/

And this is what we get right now, but what if the user adds some
opening quotes? Right now we get this:

(gdb) complete file "/tmp/xx
file "/tmp/xxx/

Which I think is correct, there's no trailing quote added. This is
because in completion_tracker::build_completion_result the
completion_tracker object doesn't know that the double quote is the
quote_char(). However, this causes a problem, if we do this:

(gdb) complete file "/tmp/xxx/f
file "/tmp/xxx/foo.c

We're still missing the trailing quote, even though in this case, when
we've expanded a complete filename, adding the trailing quote would be
the correct thing to do. The cause is, of course, the same, the
completion_tracker is unaware of what the quote character is, and so
doesn't add the quote when needed.

However, the problem gets worse, if we create an additional file
'/tmp/xxa/bar.c', notice that this is in a different directory. So
now when we do this:

(gdb) complete file "/tmp/xx
file "/tmp/xxa/"
file "/tmp/xxx/"

Now the trailing quote has been added even though we haven't completed
a full filename. This is because in complete_command (cli/cli-cmds.c)
we do know what the quote character is, and the trailing quote is
always added.

There are multiple problems here, but at least part of the problem
will be solved by making the result printing in complete_command
smarter. My plan is to allow different completion functions to modify
how the 'complete' results are printed.

This commit doesn't solve any of the problems listed above. Instead
this commit is just a refactor. I've moved the result printing logic
out of complete_command and created a new function
completion_result::print_matches. And that's it. Nothing has
functionally changed yet, that will come in later commits when the
print_matches function is made smarter.

There should be no user visible changes after this commit.
---
gdb/cli/cli-cmds.c | 26 +-------------------------
gdb/completer.c | 33 +++++++++++++++++++++++++++++++++
gdb/completer.h | 13 +++++++++++++
3 files changed, 47 insertions(+), 25 deletions(-)

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index df11f956245..0b621f65917 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -424,31 +424,7 @@ complete_command (const char *arg, int from_tty)
{
std::string arg_prefix (arg, word - arg);

- if (result.number_matches == 1)
- printf_unfiltered ("%s%s\n", arg_prefix.c_str (), result.match_list[0]);
- else
- {
- result.sort_match_list ();
-
- for (size_t i = 0; i < result.number_matches; i++)
- {
- printf_unfiltered ("%s%s",
- arg_prefix.c_str (),
- result.match_list[i + 1]);
- if (quote_char)
- printf_unfiltered ("%c", quote_char);
- printf_unfiltered ("\n");
- }
- }
-
- if (result.number_matches == max_completions)
- {
- /* ARG_PREFIX and WORD are included in the output so that emacs
- will include the message in the output. */
- printf_unfiltered (_("%s%s %s\n"),
- arg_prefix.c_str (), word,
- get_max_completions_reached_message ());
- }
+ result.print_matches (arg_prefix, word, quote_char);
}
}

diff --git a/gdb/completer.c b/gdb/completer.c
index 4cda5f3a383..9b4041da01a 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -2443,6 +2443,39 @@ completion_result::reset_match_list ()
}
}

+/* See completer.h */
+
+void
+completion_result::print_matches (const std::string &prefix,
+ const char *word, int quote_char)
+{
+ if (this->number_matches == 1)
+ printf_unfiltered ("%s%s\n", prefix.c_str (), this->match_list[0]);
+ else
+ {
+ this->sort_match_list ();
+
+ for (size_t i = 0; i < this->number_matches; i++)
+ {
+ printf_unfiltered ("%s%s", prefix.c_str (),
+ this->match_list[i + 1]);
+ if (quote_char)
+ printf_unfiltered ("%c", quote_char);
+ printf_unfiltered ("\n");
+ }
+ }
+
+ if (this->number_matches == max_completions)
+ {
+ /* PREFIX and WORD are included in the output so that emacs will
+ include the message in the output. */
+ printf_unfiltered (_("%s%s %s\n"),
+ prefix.c_str (), word,
+ get_max_completions_reached_message ());
+ }
+
+}
+
/* Helper for gdb_rl_attempted_completion_function, which does most of
the work. This is called by readline to build the match list array
and to determine the lowest common denominator. The real matches
diff --git a/gdb/completer.h b/gdb/completer.h
index 98a12f3907c..4419c8f6d30 100644
--- a/gdb/completer.h
+++ b/gdb/completer.h
@@ -268,6 +268,19 @@ struct completion_result
/* Sort the match list. */
void sort_match_list ();

+ /* Called to display all matches (used by the 'complete' command).
+ PREFIX is everything before the completion word. WORD is the word
+ being completed, this is only used if we reach the maximum number of
+ completions, otherwise, each line of output consists of PREFIX
+ followed by one of the possible completion words.
+
+ The QUOTE_CHAR is appended after each possible completion word and
+ should be the quote character that appears before the completion word,
+ or the null-character if there is no quote before the completion
+ word. */
+ void print_matches (const std::string &prefix, const char *word,
+ int quote_char);
+
private:
/* Destroy the match list array and its contents. */
void reset_match_list ();
--
2.25.4