* [PATCHv2 1/8] gdb/doc: document how filename arguments are formatted
2024-04-20 9:10 ` [PATCHv2 0/8] Further filename completion improvements Andrew Burgess
@ 2024-04-20 9:10 ` Andrew Burgess
2024-04-20 9:44 ` Eli Zaretskii
2024-04-20 9:10 ` [PATCHv2 2/8] gdb: split apart two different types of filename completion Andrew Burgess
` (7 subsequent siblings)
8 siblings, 1 reply; 50+ messages in thread
From: Andrew Burgess @ 2024-04-20 9:10 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess, Lancelot SIX, Eli Zaretskii
In the following commits I intend to improve GDB's filename
completion. However, how filenames should be completed is a little
complex because GDB is not consistent with how it expects filename
arguments to be formatted.
This commit documents the current state of GDB when it comes to
formatting filename arguments.
Currently GDB will not correctly complete filenames inline with this
documentation; GDB will either fail to complete, or complete
incorrectly (i.e. the result of completion will not then be accepted
by GDB). However, later commits in this series will fix completion.
---
gdb/doc/gdb.texinfo | 66 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 66 insertions(+)
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 82a617e9ad3..abceb9b111b 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -1752,6 +1752,7 @@
* Command Syntax:: How to give commands to @value{GDBN}
* Command Settings:: How to change default behavior of commands
* Completion:: Command completion
+* Filename Arguments:: Filenames As Command Arguments
* Command Options:: Command options
* Help:: How to ask @value{GDBN} for help
@end menu
@@ -2123,6 +2124,56 @@
@}
@end smallexample
+@node Filename Arguments
+@section Filenames As Command Arguments
+
+When passing filenames (or directory names) as arguments to a command,
+if the filename argument does not include any whitespace, double
+quotes, or single quotes, then for all commands the filename can be
+written as a simple string, for example:
+
+@smallexample
+(@value{GDBP}) file /path/to/some/file
+@end smallexample
+
+If the filename does include whitespace, double quotes, or single
+quotes then @value{GDBN} has two approaches for how these filenames
+should be formatted, which format to use depends on which command is
+being used.
+
+Most @value{GDBN} commands don't require, or support, quoting and
+escaping. These commands treat any text after the command name, that
+is not a command option (@pxref{Command Options}), as the filename,
+even if the filename contains whitespace or quote characters. In the
+following example the user is adding @w{@file{/path/that contains/two
+spaces/}} to the auto-load safe-path (@pxref{add-auto-load-safe-path}):
+
+@smallexample
+(@value{GDBP}) add-auto-load-safe-path /path/that contains/two spaces/
+@end smallexample
+
+A small number of commands require that filenames containing
+whitespace or quote characters are either quoted, or have the special
+characters escaped with a backslash. Commands that support this style
+are marked as such in the manual, any command not marked as accepting
+quoting and escaping of its filename argument, does not accept this
+filename argument style.
+
+For example, to load the file @file{/path/with spaces/to/a file} with
+the @code{file} command (@pxref{Files, ,Commands to Specify Files}),
+you can escape the whitespace characters with a backslash:
+
+@smallexample
+(@value{GDBP}) file /path/with\ spaces/to/a\ file
+@end smallexample
+
+Alternatively the entire filename can be wrapped in quotes, in which
+case no backlsashes are needed, for example:
+
+@smallexample
+(@value{GDBP}) file "/path/with spaces/to/a file"
+@end smallexample
+
@node Command Options
@section Command options
@@ -21615,6 +21666,9 @@
to run. You can change the value of this variable, for both @value{GDBN}
and your program, using the @code{path} command.
+The @var{filename} argument supports escaping and quoting, see
+@ref{Filename Arguments,,Filenames As Command Arguments}.
+
@cindex unlinked object files
@cindex patching object files
You can load unlinked object @file{.o} files into @value{GDBN} using
@@ -21637,6 +21691,9 @@
if necessary to locate your program. Omitting @var{filename} means to
discard information on the executable file.
+The @var{filename} argument supports escaping and quoting, see
+@ref{Filename Arguments,,Filenames As Command Arguments}.
+
@kindex symbol-file
@item symbol-file @r{[} @var{filename} @r{[} -o @var{offset} @r{]]}
Read symbol table information from file @var{filename}. @env{PATH} is
@@ -21660,6 +21717,9 @@
@code{symbol-file} does not repeat if you press @key{RET} again after
executing it once.
+The @var{filename} argument supports escaping and quoting, see
+@ref{Filename Arguments,,Filenames As Command Arguments}.
+
When @value{GDBN} is configured for a particular environment, it
understands debugging information in whatever format is the standard
generated for that environment; you may use either a @sc{gnu} compiler, or
@@ -21754,6 +21814,9 @@
@code{add-symbol-file} command any number of times; the new symbol data
thus read is kept in addition to the old.
+The @var{filename} argument supports escaping and quoting, see
+@ref{Filename Arguments,,Filenames As Command Arguments}.
+
Changes can be reverted using the command @code{remove-symbol-file}.
@cindex relocatable object files, reading symbols from
@@ -21813,6 +21876,9 @@
@code{remove-symbol-file} does not repeat if you press @key{RET} after using it.
+The @var{filename} argument supports escaping and quoting, see
+@ref{Filename Arguments,,Filenames As Command Arguments}.
+
@kindex add-symbol-file-from-memory
@cindex @code{syscall DSO}
@cindex load symbols from memory
--
2.25.4
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCHv2 1/8] gdb/doc: document how filename arguments are formatted
2024-04-20 9:10 ` [PATCHv2 1/8] gdb/doc: document how filename arguments are formatted Andrew Burgess
@ 2024-04-20 9:44 ` Eli Zaretskii
2024-04-27 10:01 ` Andrew Burgess
0 siblings, 1 reply; 50+ messages in thread
From: Eli Zaretskii @ 2024-04-20 9:44 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches, aburgess, lsix
> From: Andrew Burgess <aburgess@redhat.com>
> Cc: Andrew Burgess <aburgess@redhat.com>,
> Lancelot SIX <lsix@lancelotsix.com>,
> Eli Zaretskii <eliz@gnu.org>
> Date: Sat, 20 Apr 2024 10:10:01 +0100
>
> In the following commits I intend to improve GDB's filename
> completion. However, how filenames should be completed is a little
> complex because GDB is not consistent with how it expects filename
> arguments to be formatted.
>
> This commit documents the current state of GDB when it comes to
> formatting filename arguments.
>
> Currently GDB will not correctly complete filenames inline with this
> documentation; GDB will either fail to complete, or complete
> incorrectly (i.e. the result of completion will not then be accepted
> by GDB). However, later commits in this series will fix completion.
> ---
> gdb/doc/gdb.texinfo | 66 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 66 insertions(+)
Thanks, a few comments below.
> +@node Filename Arguments
> +@section Filenames As Command Arguments
Perhaps add some index entry here? Like this, for example:
@cindex file names, quoting and escaping
> +If the filename does include whitespace, double quotes, or single
> +quotes then @value{GDBN} has two approaches for how these filenames
^
A comma missing there.
> +should be formatted, which format to use depends on which command is
> +being used. ^
I'd replace that comma with a semi-colon.
> +Alternatively the entire filename can be wrapped in quotes, in which
> +case no backlsashes are needed, for example:
> +
> +@smallexample
> +(@value{GDBP}) file "/path/with spaces/to/a file"
> +@end smallexample
is quoting "like this" the only supported style of quoting, or do we
also support quoting 'like this'? If the latter, we should say so in
the manual. I would also suggest to describe how to quote file names
with embedded quote characters, instead of leaving this to the
reader's imagination and creativity.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCHv2 1/8] gdb/doc: document how filename arguments are formatted
2024-04-20 9:44 ` Eli Zaretskii
@ 2024-04-27 10:01 ` Andrew Burgess
2024-04-27 10:06 ` Eli Zaretskii
0 siblings, 1 reply; 50+ messages in thread
From: Andrew Burgess @ 2024-04-27 10:01 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches, lsix
Eli,
Thanks for your feedback.
I've addressed the issues you pointed out, and have added some
additional text at the end to better describe the escaping of quote
characters and of backslash itself.
Let me know what you think.
Thanks,
Andrew
---
commit 4d1a140481d8e7262d631cf10756a0d698f75477
Author: Andrew Burgess <aburgess@redhat.com>
Date: Wed Apr 17 14:47:49 2024 +0100
gdb/doc: document how filename arguments are formatted
In the following commits I intend to improve GDB's filename
completion. However, how filenames should be completed is a little
complex because GDB is not consistent with how it expects filename
arguments to be formatted.
This commit documents the current state of GDB when it comes to
formatting filename arguments.
Currently GDB will not correctly complete filenames inline with this
documentation; GDB will either fail to complete, or complete
incorrectly (i.e. the result of completion will not then be accepted
by GDB). However, later commits in this series will fix completion.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index b2e9faac82d..4ba77b56b8f 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -1752,6 +1752,7 @@
* Command Syntax:: How to give commands to @value{GDBN}
* Command Settings:: How to change default behavior of commands
* Completion:: Command completion
+* Filename Arguments:: Filenames As Command Arguments
* Command Options:: Command options
* Help:: How to ask @value{GDBN} for help
@end menu
@@ -2123,6 +2124,68 @@
@}
@end smallexample
+@node Filename Arguments
+@section Filenames As Command Arguments
+@cindex file names, quoting and escaping
+
+When passing filenames (or directory names) as arguments to a command,
+if the filename argument does not include any whitespace, double
+quotes, or single quotes, then for all commands the filename can be
+written as a simple string, for example:
+
+@smallexample
+(@value{GDBP}) file /path/to/some/file
+@end smallexample
+
+If the filename does include whitespace, double quotes, or single
+quotes, then @value{GDBN} has two approaches for how these filenames
+should be formatted; which format to use depends on which command is
+being used.
+
+Most @value{GDBN} commands don't require, or support, quoting and
+escaping. These commands treat any text after the command name, that
+is not a command option (@pxref{Command Options}), as the filename,
+even if the filename contains whitespace or quote characters. In the
+following example the user is adding @w{@file{/path/that contains/two
+spaces/}} to the auto-load safe-path (@pxref{add-auto-load-safe-path}):
+
+@smallexample
+(@value{GDBP}) add-auto-load-safe-path /path/that contains/two spaces/
+@end smallexample
+
+A small number of commands require that filenames containing
+whitespace or quote characters are either quoted, or have the special
+characters escaped with a backslash. Commands that support this style
+are marked as such in the manual, any command not marked as accepting
+quoting and escaping of its filename argument, does not accept this
+filename argument style.
+
+For example, to load the file @file{/path/with spaces/to/a file} with
+the @code{file} command (@pxref{Files, ,Commands to Specify Files}),
+you can escape the whitespace characters with a backslash:
+
+@smallexample
+(@value{GDBP}) file /path/with\ spaces/to/a\ file
+@end smallexample
+
+Alternatively the entire filename can be wrapped in either single or
+double quotes, in which case no backlsashes are needed, for example:
+
+@smallexample
+(@value{GDBP}) symbol-file "/path/with spaces/to/a file"
+(@value{GDBP}) exec-file '/path/with spaces/to/a file'
+@end smallexample
+
+It is possible to include a quote character within a quoted filename
+by escaping it with a backslash, for example, within a filename
+surrounded by double quotes, a double quote character should be
+escaped with a backslash, but a single quote character should not be
+escaped. Within a single quoted string a single quote character needs
+to be escaped, but a double quote character does not.
+
+A literal backslash character can also be included by escaping it with
+a backslash.
+
@node Command Options
@section Command options
@@ -21615,6 +21678,9 @@
to run. You can change the value of this variable, for both @value{GDBN}
and your program, using the @code{path} command.
+The @var{filename} argument supports escaping and quoting, see
+@ref{Filename Arguments,,Filenames As Command Arguments}.
+
@cindex unlinked object files
@cindex patching object files
You can load unlinked object @file{.o} files into @value{GDBN} using
@@ -21637,6 +21703,9 @@
if necessary to locate your program. Omitting @var{filename} means to
discard information on the executable file.
+The @var{filename} argument supports escaping and quoting, see
+@ref{Filename Arguments,,Filenames As Command Arguments}.
+
@kindex symbol-file
@item symbol-file @r{[} @var{filename} @r{[} -o @var{offset} @r{]]}
Read symbol table information from file @var{filename}. @env{PATH} is
@@ -21660,6 +21729,9 @@
@code{symbol-file} does not repeat if you press @key{RET} again after
executing it once.
+The @var{filename} argument supports escaping and quoting, see
+@ref{Filename Arguments,,Filenames As Command Arguments}.
+
When @value{GDBN} is configured for a particular environment, it
understands debugging information in whatever format is the standard
generated for that environment; you may use either a @sc{gnu} compiler, or
@@ -21754,6 +21826,9 @@
@code{add-symbol-file} command any number of times; the new symbol data
thus read is kept in addition to the old.
+The @var{filename} argument supports escaping and quoting, see
+@ref{Filename Arguments,,Filenames As Command Arguments}.
+
Changes can be reverted using the command @code{remove-symbol-file}.
@cindex relocatable object files, reading symbols from
@@ -21813,6 +21888,9 @@
@code{remove-symbol-file} does not repeat if you press @key{RET} after using it.
+The @var{filename} argument supports escaping and quoting, see
+@ref{Filename Arguments,,Filenames As Command Arguments}.
+
@kindex add-symbol-file-from-memory
@cindex @code{syscall DSO}
@cindex load symbols from memory
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCHv2 1/8] gdb/doc: document how filename arguments are formatted
2024-04-27 10:01 ` Andrew Burgess
@ 2024-04-27 10:06 ` Eli Zaretskii
2024-04-29 9:10 ` Andrew Burgess
0 siblings, 1 reply; 50+ messages in thread
From: Eli Zaretskii @ 2024-04-27 10:06 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches, lsix
> From: Andrew Burgess <aburgess@redhat.com>
> Cc: gdb-patches@sourceware.org, lsix@lancelotsix.com
> Date: Sat, 27 Apr 2024 11:01:23 +0100
>
>
> Eli,
>
> Thanks for your feedback.
>
> I've addressed the issues you pointed out, and have added some
> additional text at the end to better describe the escaping of quote
> characters and of backslash itself.
Thanks.
> +For example, to load the file @file{/path/with spaces/to/a file} with
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This should be in @w, because white space is significant.
OK with that nit fixed.
Approved-By: Eli Zaretskii <eliz@gnu.org>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCHv2 1/8] gdb/doc: document how filename arguments are formatted
2024-04-27 10:06 ` Eli Zaretskii
@ 2024-04-29 9:10 ` Andrew Burgess
0 siblings, 0 replies; 50+ messages in thread
From: Andrew Burgess @ 2024-04-29 9:10 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches, lsix
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Andrew Burgess <aburgess@redhat.com>
>> Cc: gdb-patches@sourceware.org, lsix@lancelotsix.com
>> Date: Sat, 27 Apr 2024 11:01:23 +0100
>>
>>
>> Eli,
>>
>> Thanks for your feedback.
>>
>> I've addressed the issues you pointed out, and have added some
>> additional text at the end to better describe the escaping of quote
>> characters and of backslash itself.
>
> Thanks.
>
>> +For example, to load the file @file{/path/with spaces/to/a file} with
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> This should be in @w, because white space is significant.
>
> OK with that nit fixed.
I added the missing @w and pushed just this patch for now.
Thanks,
Andrew
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCHv2 2/8] gdb: split apart two different types of filename completion
2024-04-20 9:10 ` [PATCHv2 0/8] Further filename completion improvements Andrew Burgess
2024-04-20 9:10 ` [PATCHv2 1/8] gdb/doc: document how filename arguments are formatted Andrew Burgess
@ 2024-04-20 9:10 ` Andrew Burgess
2024-04-20 9:10 ` [PATCHv2 3/8] gdb: improve escaping when completing filenames Andrew Burgess
` (6 subsequent siblings)
8 siblings, 0 replies; 50+ messages in thread
From: Andrew Burgess @ 2024-04-20 9:10 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess, Lancelot SIX, Eli Zaretskii
Unfortunately we have two different types of filename completion in
GDB.
The majority of commands have what I call unquoted filename
completion, this is for commands like 'set logging file ...', 'target
core ...', and 'add-auto-load-safe-path ...'. For these commands
everything after the command name (that is not a command option) is
treated as a single filename. If the filename contains white space
then this does not need to be escaped, nor does the filename need to
be quoted. In fact, the filename argument is not de-quoted, and does
not have any escaping removed, so if a user does try to add such
things, they will be treated as part of the filename. As an example:
(gdb) target core "/path/that contains/some white space"
Will look for a directory calls '"' (double quotes) in the local
directory.
A small number of commands do de-quote and remove escapes from
filename arguments. These command accept what I call quoted and
escaped filenames. Right now these are the commands that specify the
file for GDB to debug, so:
file
exec-file
symbol-file
add-symbol-file
remove-symbol-file
As an example of this in action:
(gdb) file "/path/that contains/some white space"
In this case GDB would load the file:
/path/that contains/some white space
Current filename completion always assumes that filenames can be
quoted, though escaping doesn't work in completion right now. But the
assumption that quoting is allowed is clearly wrong.
This commit splits filename completion into two. There is now a
filename completer for unquoted filenames, and a second completer to
handle quoted and escaped filenames.
To handle completion of unquoted filenames the completion needs to be
performed during the brkchars phase of completion, which is why almost
every place we use filename_completer has to change in this commit.
I've also renamed the 'filename_completer' function to reflect that it
now handles filenames that can be quoted.
The filename completion test has been extended to cover more cases.
The 'advance_to_filename_complete_word_point' function is no longer
used after this commit and so I've deleted it.
---
gdb/auto-load.c | 4 +-
gdb/breakpoint.c | 5 +-
gdb/cli/cli-cmds.c | 8 +-
gdb/cli/cli-decode.c | 12 +-
gdb/cli/cli-dump.c | 6 +-
gdb/compile/compile.c | 3 +-
gdb/completer.c | 61 ++++---
gdb/completer.h | 29 +++-
gdb/corefile.c | 4 +-
gdb/corelow.c | 4 +-
gdb/dwarf2/index-write.c | 3 +-
gdb/exec.c | 7 +-
gdb/guile/scm-cmd.c | 2 +-
gdb/infcmd.c | 15 +-
gdb/inferior.c | 2 +-
gdb/jit.c | 2 +-
gdb/python/py-cmd.c | 2 +-
gdb/record-full.c | 5 +-
gdb/record.c | 2 +-
gdb/skip.c | 2 +-
gdb/source.c | 3 +-
gdb/symfile.c | 7 +-
gdb/target-descriptions.c | 7 +-
gdb/target.c | 4 +-
gdb/target.h | 9 +-
.../gdb.base/filename-completion.exp | 156 +++++++++++++-----
gdb/tracectf.c | 3 +-
gdb/tracefile-tfile.c | 4 +-
28 files changed, 248 insertions(+), 123 deletions(-)
diff --git a/gdb/auto-load.c b/gdb/auto-load.c
index db6d6ae0f73..0d57bf92fb7 100644
--- a/gdb/auto-load.c
+++ b/gdb/auto-load.c
@@ -1630,7 +1630,7 @@ This option has security implications for untrusted inferiors."),
See the commands 'set auto-load safe-path' and 'show auto-load safe-path' to\n\
access the current full list setting."),
&cmdlist);
- set_cmd_completer (cmd, filename_completer);
+ set_cmd_completer_handle_brkchars (cmd, filename_completer_handle_brkchars);
cmd = add_cmd ("add-auto-load-scripts-directory", class_support,
add_auto_load_dir,
@@ -1639,7 +1639,7 @@ access the current full list setting."),
See the commands 'set auto-load scripts-directory' and\n\
'show auto-load scripts-directory' to access the current full list setting."),
&cmdlist);
- set_cmd_completer (cmd, filename_completer);
+ set_cmd_completer_handle_brkchars (cmd, filename_completer_handle_brkchars);
add_setshow_boolean_cmd ("auto-load", class_maintenance,
&debug_auto_load, _("\
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 6d8adc62664..946b44df11c 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -15093,14 +15093,15 @@ This includes all types of breakpoints (breakpoints, watchpoints,\n\
catchpoints, tracepoints). Use the 'source' command in another debug\n\
session to restore them."),
&save_cmdlist);
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
cmd_list_element *save_tracepoints_cmd
= add_cmd ("tracepoints", class_trace, save_tracepoints_command, _("\
Save current tracepoint definitions as a script.\n\
Use the 'source' command in another debug session to restore them."),
&save_cmdlist);
- set_cmd_completer (save_tracepoints_cmd, filename_completer);
+ set_cmd_completer_handle_brkchars (save_tracepoints_cmd,
+ filename_completer_handle_brkchars);
c = add_com_alias ("save-tracepoints", save_tracepoints_cmd, class_trace, 0);
deprecate_cmd (c, "save tracepoints");
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 3afe2178199..0a537ca9661 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -2619,7 +2619,7 @@ The debugger's current working directory specifies where scripts and other\n\
files that can be loaded by GDB are located.\n\
In order to change the inferior's current working directory, the recommended\n\
way is to use the \"set cwd\" command."), &cmdlist);
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
add_com ("echo", class_support, echo_command, _("\
Print a constant string. Give string as argument.\n\
@@ -2785,7 +2785,7 @@ the previous command number shown."),
= add_com ("shell", class_support, shell_command, _("\
Execute the rest of the line as a shell command.\n\
With no arguments, run an inferior shell."));
- set_cmd_completer (shell_cmd, filename_completer);
+ set_cmd_completer_handle_brkchars (shell_cmd, filename_completer_handle_brkchars);
add_com_alias ("!", shell_cmd, class_support, 0);
@@ -2874,7 +2874,7 @@ you must type \"disassemble 'foo.c'::bar\" and not \"disassemble foo.c:bar\"."))
c = add_com ("make", class_support, make_command, _("\
Run the ``make'' program using the rest of the line as arguments."));
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
c = add_cmd ("user", no_class, show_user, _("\
Show definitions of non-python/scheme user defined commands.\n\
Argument is the name of the user defined command.\n\
@@ -2958,5 +2958,5 @@ Note that the file \"%s\" is read automatically in this way\n\
when GDB is started."), GDBINIT).release ();
c = add_cmd ("source", class_support, source_command,
source_help_text, &cmdlist);
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
}
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index d9a2ab40575..a48b18bbd87 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -866,7 +866,8 @@ add_setshow_filename_cmd (const char *name, enum command_class theclass,
nullptr, nullptr, set_func,
show_func, set_list, show_list);
- set_cmd_completer (commands.set, filename_completer);
+ set_cmd_completer_handle_brkchars (commands.set,
+ filename_completer_handle_brkchars);
return commands;
}
@@ -890,7 +891,8 @@ add_setshow_filename_cmd (const char *name, command_class theclass,
nullptr, show_func, set_list,
show_list);
- set_cmd_completer (cmds.set, filename_completer);
+ set_cmd_completer_handle_brkchars (cmds.set,
+ filename_completer_handle_brkchars);
return cmds;
}
@@ -1015,7 +1017,8 @@ add_setshow_optional_filename_cmd (const char *name, enum command_class theclass
nullptr, nullptr, set_func, show_func,
set_list, show_list);
- set_cmd_completer (commands.set, filename_completer);
+ set_cmd_completer_handle_brkchars (commands.set,
+ filename_completer_handle_brkchars);
return commands;
}
@@ -1039,7 +1042,8 @@ add_setshow_optional_filename_cmd (const char *name, command_class theclass,
set_func, get_func, nullptr, show_func,
set_list,show_list);
- set_cmd_completer (cmds.set, filename_completer);
+ set_cmd_completer_handle_brkchars (cmds.set,
+ filename_completer_handle_brkchars);
return cmds;
}
diff --git a/gdb/cli/cli-dump.c b/gdb/cli/cli-dump.c
index 9b44c6edcf2..aaacdd6ede1 100644
--- a/gdb/cli/cli-dump.c
+++ b/gdb/cli/cli-dump.c
@@ -348,7 +348,7 @@ add_dump_command (const char *name,
struct dump_context *d;
c = add_cmd (name, all_commands, descr, &dump_cmdlist);
- c->completer = filename_completer;
+ c->completer_handle_brkchars = filename_completer_handle_brkchars;
d = XNEW (struct dump_context);
d->func = func;
d->mode = FOPEN_WB;
@@ -356,7 +356,7 @@ add_dump_command (const char *name,
c->func = call_dump_func;
c = add_cmd (name, all_commands, descr, &append_cmdlist);
- c->completer = filename_completer;
+ c->completer_handle_brkchars = filename_completer_handle_brkchars;
d = XNEW (struct dump_context);
d->func = func;
d->mode = FOPEN_AB;
@@ -705,6 +705,6 @@ Arguments are FILE OFFSET START END where all except FILE are optional.\n\
OFFSET will be added to the base address of the file (default zero).\n\
If START and END are given, only the file contents within that range\n\
(file relative) will be restored to target memory."));
- c->completer = filename_completer;
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
/* FIXME: completers for other commands. */
}
diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index 2d97a1b2005..062418b3791 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -327,8 +327,7 @@ compile_file_command_completer (struct cmd_list_element *ignore,
(tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_ERROR, group))
return;
- word = advance_to_filename_complete_word_point (tracker, text);
- filename_completer (ignore, tracker, text, word);
+ filename_completer_handle_brkchars (ignore, tracker, text, word);
}
/* Handle the input from the 'compile code' command. The
diff --git a/gdb/completer.c b/gdb/completer.c
index 171d1ca8c0e..c300e42f588 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -203,15 +203,15 @@ noop_completer (struct cmd_list_element *ignore,
{
}
-/* Complete on filenames. */
+/* Generate filename completions of WORD, storing the completions into
+ TRACKER. This is used for generating completions for commands that
+ only accept unquoted filenames as well as for commands that accept
+ quoted and escaped filenames. */
-void
-filename_completer (struct cmd_list_element *ignore,
- completion_tracker &tracker,
- const char *text, const char *word)
+static void
+filename_completer_generate_completions (completion_tracker &tracker,
+ const char *word)
{
- rl_completer_quote_characters = gdb_completer_file_name_quote_characters;
-
int subsequent_name = 0;
while (1)
{
@@ -249,13 +249,23 @@ filename_completer (struct cmd_list_element *ignore,
}
}
-/* The corresponding completer_handle_brkchars
- implementation. */
+/* Complete on filenames. */
+
+void
+filename_maybe_quoted_completer (struct cmd_list_element *ignore,
+ completion_tracker &tracker,
+ const char *text, const char *word)
+{
+ rl_completer_quote_characters = gdb_completer_file_name_quote_characters;
+ filename_completer_generate_completions (tracker, word);
+}
+
+/* The corresponding completer_handle_brkchars implementation. */
static void
-filename_completer_handle_brkchars (struct cmd_list_element *ignore,
- completion_tracker &tracker,
- const char *text, const char *word)
+filename_maybe_quoted_completer_handle_brkchars
+ (struct cmd_list_element *ignore, completion_tracker &tracker,
+ const char *text, const char *word)
{
set_rl_completer_word_break_characters
(gdb_completer_file_name_break_characters);
@@ -263,6 +273,18 @@ filename_completer_handle_brkchars (struct cmd_list_element *ignore,
rl_completer_quote_characters = gdb_completer_file_name_quote_characters;
}
+/* See completer.h. */
+
+void
+filename_completer_handle_brkchars
+ (struct cmd_list_element *ignore, completion_tracker &tracker,
+ const char *text, const char *word)
+{
+ gdb_assert (word == nullptr);
+ tracker.set_use_custom_word_point (true);
+ filename_completer_generate_completions (tracker, text);
+}
+
/* Find the bounds of the current word for completion purposes, and
return a pointer to the end of the word. This mimics (and is a
modified version of) readline's _rl_find_completion_word internal
@@ -443,17 +465,6 @@ advance_to_expression_complete_word_point (completion_tracker &tracker,
/* See completer.h. */
-const char *
-advance_to_filename_complete_word_point (completion_tracker &tracker,
- const char *text)
-{
- const char *brk_chars = gdb_completer_file_name_break_characters;
- const char *quote_chars = gdb_completer_file_name_quote_characters;
- return advance_to_completion_word (tracker, brk_chars, quote_chars, text);
-}
-
-/* See completer.h. */
-
bool
completion_tracker::completes_to_completion_word (const char *word)
{
@@ -1877,8 +1888,8 @@ default_completer_handle_brkchars (struct cmd_list_element *ignore,
completer_handle_brkchars_ftype *
completer_handle_brkchars_func_for_completer (completer_ftype *fn)
{
- if (fn == filename_completer)
- return filename_completer_handle_brkchars;
+ if (fn == filename_maybe_quoted_completer)
+ return filename_maybe_quoted_completer_handle_brkchars;
if (fn == location_completer)
return location_completer_handle_brkchars;
diff --git a/gdb/completer.h b/gdb/completer.h
index 98a12f3907c..fa21156bd1f 100644
--- a/gdb/completer.h
+++ b/gdb/completer.h
@@ -563,12 +563,6 @@ extern completion_result
const char *advance_to_expression_complete_word_point
(completion_tracker &tracker, const char *text);
-/* Assuming TEXT is an filename, find the completion word point for
- TEXT, emulating the algorithm readline uses to find the word
- point. */
-extern const char *advance_to_filename_complete_word_point
- (completion_tracker &tracker, const char *text);
-
extern void noop_completer (struct cmd_list_element *,
completion_tracker &tracker,
const char *, const char *);
@@ -577,6 +571,29 @@ extern void filename_completer (struct cmd_list_element *,
completion_tracker &tracker,
const char *, const char *);
+/* Filename completer that can be registered for the brkchars phase of
+ completion. This should be used by commands that don't allow the
+ filename to be quoted, and whitespace does not need to be escaped.
+
+ NOTE: If you are considering using this function as your commands
+ completer, then consider updating your function to use gdb_argv or
+ extract_string_maybe_quoted to allow for possibly quoted filenames
+ instead. You would then use filename_maybe_quoted_completer for
+ filename completion. The benefit of this is that in the future it is
+ possible to add additional arguments to your new command. */
+
+extern void filename_completer_handle_brkchars
+ (struct cmd_list_element *ignore, completion_tracker &tracker,
+ const char *text, const char *word);
+
+/* Filename completer for commands where the filename argument can be
+ quoted, and whitespace within an unquoted filename should be escaped
+ with a backslash. */
+
+extern void filename_maybe_quoted_completer (struct cmd_list_element *,
+ completion_tracker &tracker,
+ const char *, const char *);
+
extern void expression_completer (struct cmd_list_element *,
completion_tracker &tracker,
const char *, const char *);
diff --git a/gdb/corefile.c b/gdb/corefile.c
index 16cd60f7106..04a1021ee2b 100644
--- a/gdb/corefile.c
+++ b/gdb/corefile.c
@@ -404,9 +404,9 @@ Use FILE as core dump for examining memory and registers.\n\
Usage: core-file FILE\n\
No arg means have no core file. This command has been superseded by the\n\
`target core' and `detach' commands."), &cmdlist);
- set_cmd_completer (core_file_cmd, filename_completer);
+ set_cmd_completer_handle_brkchars (core_file_cmd,
+ filename_completer_handle_brkchars);
-
set_show_commands set_show_gnutarget
= add_setshow_string_noescape_cmd ("gnutarget", class_files,
&gnutarget_string, _("\
diff --git a/gdb/corelow.c b/gdb/corelow.c
index f4e8273d962..ff66d808120 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -1509,7 +1509,9 @@ void _initialize_corelow ();
void
_initialize_corelow ()
{
- add_target (core_target_info, core_target_open, filename_completer);
+ struct cmd_list_element *c
+ = add_target (core_target_info, core_target_open);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
add_cmd ("core-file-backed-mappings", class_maintenance,
maintenance_print_core_file_backed_mappings,
_("Print core file's file-backed mappings."),
diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c
index 3f812285995..4f79c42bbf7 100644
--- a/gdb/dwarf2/index-write.c
+++ b/gdb/dwarf2/index-write.c
@@ -1620,8 +1620,7 @@ gdb_save_index_cmd_completer (struct cmd_list_element *ignore,
(tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, grp))
return;
- word = advance_to_filename_complete_word_point (tracker, text);
- filename_completer (ignore, tracker, text, word);
+ filename_completer_handle_brkchars (ignore, tracker, text, word);
}
/* Implementation of the `save gdb-index' command.
diff --git a/gdb/exec.c b/gdb/exec.c
index 98ad81fb99a..5210e6bc4ea 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -1067,14 +1067,14 @@ and it is the program executed when you use the `run' command.\n\
If FILE cannot be found as specified, your execution directory path\n\
($PATH) is searched for a command of that name.\n\
No arg means to have no executable file and no symbols."), &cmdlist);
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer (c, filename_maybe_quoted_completer);
c = add_cmd ("exec-file", class_files, exec_file_command, _("\
Use FILE as program for getting contents of pure memory.\n\
If FILE cannot be found as specified, your execution directory path\n\
is searched for a command of that name.\n\
No arg means have no executable file."), &cmdlist);
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer (c, filename_maybe_quoted_completer);
add_com ("section", class_files, set_section_command, _("\
Change the base address of section SECTION of the exec file to ADDR.\n\
@@ -1112,5 +1112,6 @@ will be loaded as well."),
show_exec_file_mismatch_command,
&setlist, &showlist);
- add_target (exec_target_info, exec_target_open, filename_completer);
+ c = add_target (exec_target_info, exec_target_open);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
}
diff --git a/gdb/guile/scm-cmd.c b/gdb/guile/scm-cmd.c
index d75d2b63c8c..037813250a9 100644
--- a/gdb/guile/scm-cmd.c
+++ b/gdb/guile/scm-cmd.c
@@ -110,7 +110,7 @@ struct cmdscm_completer
static const struct cmdscm_completer cmdscm_completers[] =
{
{ "COMPLETE_NONE", noop_completer },
- { "COMPLETE_FILENAME", filename_completer },
+ { "COMPLETE_FILENAME", filename_maybe_quoted_completer },
{ "COMPLETE_LOCATION", location_completer },
{ "COMPLETE_COMMAND", command_completer },
{ "COMPLETE_SYMBOL", symbol_completer },
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 10a964a90d7..568dfe3693b 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -3113,7 +3113,8 @@ Follow this command with any number of args, to be passed to the program."),
get_args_value,
show_args_command,
&setlist, &showlist);
- set_cmd_completer (args_set_show.set, filename_completer);
+ set_cmd_completer_handle_brkchars (args_set_show.set,
+ filename_completer_handle_brkchars);
auto cwd_set_show
= add_setshow_string_noescape_cmd ("cwd", class_run, _("\
@@ -3129,7 +3130,8 @@ working directory."),
set_cwd_value, get_inferior_cwd,
show_cwd_command,
&setlist, &showlist);
- set_cmd_completer (cwd_set_show.set, filename_completer);
+ set_cmd_completer_handle_brkchars (cwd_set_show.set,
+ filename_completer_handle_brkchars);
c = add_cmd ("environment", no_class, environment_info, _("\
The environment to give the program, or one variable's value.\n\
@@ -3163,7 +3165,7 @@ This path is equivalent to the $PATH shell variable. It is a list of\n\
directories, separated by colons. These directories are searched to find\n\
fully linked executable files and separately compiled object files as \
needed."));
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
c = add_cmd ("paths", no_class, path_info, _("\
Current search path for finding object files.\n\
@@ -3313,18 +3315,19 @@ Specifying -a and an ignore count simultaneously is an error."));
= add_com ("run", class_run, run_command, _("\
Start debugged program.\n"
RUN_ARGS_HELP));
- set_cmd_completer (run_cmd, filename_completer);
+ set_cmd_completer_handle_brkchars (run_cmd,
+ filename_completer_handle_brkchars);
add_com_alias ("r", run_cmd, class_run, 1);
c = add_com ("start", class_run, start_command, _("\
Start the debugged program stopping at the beginning of the main procedure.\n"
RUN_ARGS_HELP));
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
c = add_com ("starti", class_run, starti_command, _("\
Start the debugged program stopping at the first instruction.\n"
RUN_ARGS_HELP));
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
add_com ("interrupt", class_run, interrupt_command,
_("Interrupt the execution of the debugged program.\n\
diff --git a/gdb/inferior.c b/gdb/inferior.c
index 4e1d789d1ba..c883f020585 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -1112,7 +1112,7 @@ as main program.\n\
By default, the new inferior inherits the current inferior's connection.\n\
If -no-connection is specified, the new inferior begins with\n\
no target connection yet."));
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
add_com ("remove-inferiors", no_class, remove_inferior_command, _("\
Remove inferior ID (or list of IDs).\n\
diff --git a/gdb/jit.c b/gdb/jit.c
index 3843b84b0e6..aa0b5289d36 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -1327,7 +1327,7 @@ Usage: jit-reader-load FILE\n\
Try to load file FILE as a debug info reader (and unwinder) for\n\
JIT compiled code. The file is loaded from " JIT_READER_DIR ",\n\
relocated relative to the GDB executable if required."));
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
c = add_com ("jit-reader-unload", no_class,
jit_reader_unload_command, _("\
diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
index f18a8e8eaa9..dec11f75c08 100644
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -39,7 +39,7 @@ struct cmdpy_completer
static const struct cmdpy_completer completers[] =
{
{ "COMPLETE_NONE", noop_completer },
- { "COMPLETE_FILENAME", filename_completer },
+ { "COMPLETE_FILENAME", filename_maybe_quoted_completer },
{ "COMPLETE_LOCATION", location_completer },
{ "COMPLETE_COMMAND", command_completer },
{ "COMPLETE_SYMBOL", symbol_completer },
diff --git a/gdb/record-full.c b/gdb/record-full.c
index 2e67cf5b428..c13b5b8d644 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -2888,12 +2888,13 @@ _initialize_record_full ()
_("Restore the execution log from a file.\n\
Argument is filename. File must be created with 'record save'."),
&record_full_cmdlist);
- set_cmd_completer (record_full_restore_cmd, filename_completer);
+ set_cmd_completer_handle_brkchars (record_full_restore_cmd,
+ filename_completer_handle_brkchars);
/* Deprecate the old version without "full" prefix. */
c = add_alias_cmd ("restore", record_full_restore_cmd, class_obscure, 1,
&record_cmdlist);
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
deprecate_cmd (c, "record full restore");
add_setshow_prefix_cmd ("full", class_support,
diff --git a/gdb/record.c b/gdb/record.c
index 5b1093dd12e..daa3d5bd1e3 100644
--- a/gdb/record.c
+++ b/gdb/record.c
@@ -820,7 +820,7 @@ A size of \"unlimited\" means unlimited lines. The default is 10."),
Usage: record save [FILENAME]\n\
Default filename is 'gdb_record.PROCESS_ID'."),
&record_cmdlist);
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
cmd_list_element *record_delete_cmd
= add_cmd ("delete", class_obscure, cmd_record_delete,
diff --git a/gdb/skip.c b/gdb/skip.c
index f2818eccb34..c932598d243 100644
--- a/gdb/skip.c
+++ b/gdb/skip.c
@@ -683,7 +683,7 @@ Ignore a file while stepping.\n\
Usage: skip file [FILE-NAME]\n\
If no filename is given, ignore the current file."),
&skiplist);
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
c = add_cmd ("function", class_breakpoint, skip_function_command, _("\
Ignore a function while stepping.\n\
diff --git a/gdb/source.c b/gdb/source.c
index 432301e2a71..7c0cea08aa3 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1916,7 +1916,8 @@ directory in which the source file was compiled into object code.\n\
With no argument, reset the search path to $cdir:$cwd, the default."),
&cmdlist);
- set_cmd_completer (directory_cmd, filename_completer);
+ set_cmd_completer_handle_brkchars (directory_cmd,
+ filename_completer_handle_brkchars);
add_setshow_optional_filename_cmd ("directories",
class_files,
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 2a7d41dc974..b9c21d33ac0 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -3870,7 +3870,7 @@ Usage: symbol-file [-readnow | -readnever] [-o OFF] FILE\n\
OFF is an optional offset which is added to each section address.\n\
The `file' command can also load symbol tables, as well as setting the file\n\
to execute.\n" READNOW_READNEVER_HELP), &cmdlist);
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer (c, filename_maybe_quoted_completer);
c = add_cmd ("add-symbol-file", class_files, add_symbol_file_command, _("\
Load symbols from FILE, assuming FILE has been dynamically loaded.\n\
@@ -3884,7 +3884,7 @@ OFF is an optional offset which is added to the default load addresses\n\
of all sections for which no other address was specified.\n"
READNOW_READNEVER_HELP),
&cmdlist);
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer (c, filename_maybe_quoted_completer);
c = add_cmd ("remove-symbol-file", class_files,
remove_symbol_file_command, _("\
@@ -3894,6 +3894,7 @@ Usage: remove-symbol-file FILENAME\n\
The file to remove can be identified by its filename or by an address\n\
that lies within the boundaries of this symbol file in memory."),
&cmdlist);
+ set_cmd_completer (c, filename_maybe_quoted_completer);
c = add_cmd ("load", class_files, load_command, _("\
Dynamically load FILE into the running program.\n\
@@ -3902,7 +3903,7 @@ Usage: load [FILE] [OFFSET]\n\
An optional load OFFSET may also be given as a literal address.\n\
When OFFSET is provided, FILE must also be provided. FILE can be provided\n\
on its own."), &cmdlist);
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
cmd_list_element *overlay_cmd
= add_basic_prefix_cmd ("overlay", class_support,
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 8aca5cf719b..dead63823d1 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -1762,8 +1762,7 @@ maint_print_c_tdesc_cmd_completer (struct cmd_list_element *ignore,
(tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_ERROR, grp))
return;
- word = advance_to_filename_complete_word_point (tracker, text);
- filename_completer (ignore, tracker, text, word);
+ filename_completer_handle_brkchars (ignore, tracker, text, word);
}
/* Implement the maintenance print xml-tdesc command. */
@@ -1947,7 +1946,7 @@ that feature within an already existing target_desc object."), grp);
cmd = add_cmd ("xml-tdesc", class_maintenance, maint_print_xml_tdesc_cmd, _("\
Print the current target description as an XML file."),
&maintenanceprintlist);
- set_cmd_completer (cmd, filename_completer);
+ set_cmd_completer_handle_brkchars (cmd, filename_completer_handle_brkchars);
cmd = add_cmd ("xml-descriptions", class_maintenance,
maintenance_check_xml_descriptions, _("\
@@ -1956,5 +1955,5 @@ Check the target descriptions created in GDB equal the descriptions\n\
created from XML files in the directory.\n\
The parameter is the directory name."),
&maintenancechecklist);
- set_cmd_completer (cmd, filename_completer);
+ set_cmd_completer_handle_brkchars (cmd, filename_completer_handle_brkchars);
}
diff --git a/gdb/target.c b/gdb/target.c
index 107a84b3ca1..949daf82b0a 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -829,7 +829,7 @@ open_target (const char *args, int from_tty, struct cmd_list_element *command)
/* See target.h. */
-void
+struct cmd_list_element *
add_target (const target_info &t, target_open_ftype *func,
completer_ftype *completer)
{
@@ -853,6 +853,8 @@ information on the arguments for a particular protocol, type\n\
c->func = open_target;
if (completer != NULL)
set_cmd_completer (c, completer);
+
+ return c;
}
/* See target.h. */
diff --git a/gdb/target.h b/gdb/target.h
index c9eaff16346..fe8a9c7a60b 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -2370,11 +2370,12 @@ typedef void target_open_ftype (const char *args, int from_tty);
/* Add the target described by INFO to the list of possible targets
and add a new command 'target $(INFO->shortname)'. Set COMPLETER
- as the command's completer if not NULL. */
+ as the command's completer if not NULL. Return the new target
+ command. */
-extern void add_target (const target_info &info,
- target_open_ftype *func,
- completer_ftype *completer = NULL);
+extern struct cmd_list_element *add_target
+ (const target_info &info, target_open_ftype *func,
+ completer_ftype *completer = NULL);
/* Adds a command ALIAS for the target described by INFO and marks it
deprecated. This is useful for maintaining backwards compatibility
diff --git a/gdb/testsuite/gdb.base/filename-completion.exp b/gdb/testsuite/gdb.base/filename-completion.exp
index b700977cec5..1ccaaff9afc 100644
--- a/gdb/testsuite/gdb.base/filename-completion.exp
+++ b/gdb/testsuite/gdb.base/filename-completion.exp
@@ -23,8 +23,16 @@ load_lib completion-support.exp
#
# root/ [ DIRECTORY ]
# aaa/ [ DIRECTORY ]
+# aa bb [ FILE ]
+# aa cc [ FILE ]
+# aaa/ [ DIRECTORY ]
# bb1/ [ DIRECTORY ]
# bb2/ [ DIRECTORY ]
+# dir 1/ [ DIRECTORY ]
+# unique file [ FILE ]
+# dir 2/ [ DIRECTORY ]
+# file 1 [ FILE ]
+# file 2 [ FILE ]
# cc1/ [ DIRECTORY ]
# cc2 [ FILE ]
proc setup_directory_tree {} {
@@ -36,68 +44,139 @@ proc setup_directory_tree {} {
remote_exec host "mkdir -p ${root}/bb2"
remote_exec host "mkdir -p ${root}/cc1"
remote_exec host "touch ${root}/cc2"
-
remote_exec host "touch \"${root}/aaa/aa bb\""
remote_exec host "touch \"${root}/aaa/aa cc\""
+ remote_exec host "mkdir -p \"${root}/bb2/dir 1\""
+ remote_exec host "mkdir -p \"${root}/bb2/dir 2\""
+ remote_exec host "touch \"${root}/bb2/dir 1/unique file\""
+ remote_exec host "touch \"${root}/bb2/dir 2/file 1\""
+ remote_exec host "touch \"${root}/bb2/dir 2/file 2\""
return $root
}
-# Run filename completetion tests. ROOT is the base directory as
-# returned from setup_directory_tree, though, if ROOT is a
-# sub-directory of the user's home directory ROOT might have been
-# modified to replace the $HOME prefix with a single "~" character.
-proc run_tests { root } {
-
- # Completing 'thread apply all ...' commands uses a custom word
- # point. At one point we had a bug where doing this would break
- # completion of quoted filenames that contained white space.
- test_gdb_complete_unique "thread apply all hel" \
- "thread apply all help" " " false \
- "complete a 'thread apply all' command"
-
- foreach_with_prefix qc [list "" "'" "\""] {
- test_gdb_complete_none "file ${qc}${root}/xx" \
+# Run filename completetion tests for those command that accept quoting and
+# escaping of the filename argument.
+#
+# ROOT is the base directory as returned from setup_directory_tree, though,
+# if ROOT is a sub-directory of the user's home directory ROOT might have
+# been modified to replace the $HOME prefix with a single "~" character.
+proc run_quoting_and_escaping_tests { root } {
+ # Test all the commands which allow quoting of filenames, and
+ # which require whitespace to be escaped in unquoted filenames.
+ foreach_with_prefix cmd { file exec-file symbol-file add-symbol-file \
+ remove-symbol-file } {
+ gdb_start
+
+ # Completing 'thread apply all ...' commands uses a custom word
+ # point. At one point we had a bug where doing this would break
+ # completion of quoted filenames that contained white space.
+ test_gdb_complete_unique "thread apply all hel" \
+ "thread apply all help" " " false \
+ "complete a 'thread apply all' command"
+
+ foreach_with_prefix qc [list "" "'" "\""] {
+ test_gdb_complete_none "$cmd ${qc}${root}/xx" \
+ "expand a non-existent filename"
+
+ test_gdb_complete_unique "$cmd ${qc}${root}/a" \
+ "$cmd ${qc}${root}/aaa/" "" false \
+ "expand a unique filename"
+
+ test_gdb_complete_multiple "$cmd ${qc}${root}/" \
+ "b" "b" {
+ "bb1/"
+ "bb2/"
+ } "" "${qc}" false \
+ "expand multiple directory names"
+
+ test_gdb_complete_multiple "$cmd ${qc}${root}/" \
+ "c" "c" {
+ "cc1/"
+ "cc2"
+ } "" "${qc}" false \
+ "expand mixed directory and file names"
+
+ # GDB does not currently escape word break characters
+ # (e.g. white space) correctly in unquoted filenames.
+ if { $qc ne "" } {
+ set sp " "
+
+ test_gdb_complete_multiple "$cmd ${qc}${root}/aaa/" \
+ "a" "a${sp}" {
+ "aa bb"
+ "aa cc"
+ } "" "${qc}" false \
+ "expand filenames containing spaces"
+ }
+ }
+
+ gdb_exit
+ }
+}
+
+# Run filename completetion tests for a sample of commands that take an
+# unquoted, unescaped filename as an argument. Only a sample of commands
+# are (currently) tested as there's a lot of commands that accept this style
+# of filename argument.
+#
+# ROOT is the base directory as returned from setup_directory_tree, though,
+# if ROOT is a sub-directory of the user's home directory ROOT might have
+# been modified to replace the $HOME prefix with a single "~" character.
+proc run_unquoted_tests { root } {
+ # Test all the commands which allow quoting of filenames, and
+ # which require whitespace to be escaped in unquoted filenames.
+ foreach_with_prefix cmd { "set logging file" "target core" \
+ "add-auto-load-safe-path" } {
+ gdb_start
+
+ test_gdb_complete_none "$cmd ${root}/xx" \
"expand a non-existent filename"
- test_gdb_complete_unique "file ${qc}${root}/a" \
- "file ${qc}${root}/aaa/" "" false \
+ test_gdb_complete_unique "$cmd ${root}/a" \
+ "$cmd ${root}/aaa/" "" false \
"expand a unique filename"
- test_gdb_complete_multiple "file ${qc}${root}/" \
+ test_gdb_complete_unique "$cmd ${root}/bb2/dir 1/uni" \
+ "$cmd ${root}/bb2/dir 1/unique file" " " false \
+ "expand a unique filename containing whitespace"
+
+ test_gdb_complete_multiple "$cmd ${root}/" \
"b" "b" {
"bb1/"
"bb2/"
- } "" "${qc}" false \
+ } "" "" false \
"expand multiple directory names"
- test_gdb_complete_multiple "file ${qc}${root}/" \
+ test_gdb_complete_multiple "$cmd ${root}/" \
"c" "c" {
"cc1/"
"cc2"
- } "" "${qc}" false \
+ } "" "" false \
"expand mixed directory and file names"
- # GDB does not currently escape word break characters
- # (e.g. white space) correctly in unquoted filenames.
- if { $qc ne "" } {
- set sp " "
-
- test_gdb_complete_multiple "file ${qc}${root}/aaa/" \
- "a" "a${sp}" {
- "aa bb"
- "aa cc"
- } "" "${qc}" false \
- "expand filenames containing spaces"
- }
+ test_gdb_complete_multiple "$cmd ${root}/aaa/" \
+ "a" "a " {
+ "aa bb"
+ "aa cc"
+ } "" "" false \
+ "expand filenames containing spaces"
+
+ test_gdb_complete_multiple "$cmd ${root}/bb2/dir 2/" \
+ "fi" "le " {
+ "file 1"
+ "file 2"
+ } "" "" false \
+ "expand filenames containing spaces in path"
+
+ gdb_exit
}
}
-gdb_start
-
set root [setup_directory_tree]
-run_tests $root
+run_quoting_and_escaping_tests $root
+run_unquoted_tests $root
# This test relies on using the $HOME directory. We could make this
# work for remote hosts, but right now, this isn't supported.
@@ -114,7 +193,8 @@ if {![is_remote host]} {
with_test_prefix "with tilde" {
# And rerun the tests.
- run_tests $tilde_root
+ run_quoting_and_escaping_tests $tilde_root
+ run_unquoted_tests $tilde_root
}
}
}
diff --git a/gdb/tracectf.c b/gdb/tracectf.c
index 282a8250ac1..94b8a283ac6 100644
--- a/gdb/tracectf.c
+++ b/gdb/tracectf.c
@@ -1721,6 +1721,7 @@ void
_initialize_ctf ()
{
#if HAVE_LIBBABELTRACE
- add_target (ctf_target_info, ctf_target_open, filename_completer);
+ struct cmd_list_element *c = add_target (ctf_target_info, ctf_target_open);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
#endif
}
diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index 79af963b049..14909daa1cc 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -1119,5 +1119,7 @@ void _initialize_tracefile_tfile ();
void
_initialize_tracefile_tfile ()
{
- add_target (tfile_target_info, tfile_target_open, filename_completer);
+ struct cmd_list_element *c =
+ add_target (tfile_target_info, tfile_target_open);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
}
--
2.25.4
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCHv2 3/8] gdb: improve escaping when completing filenames
2024-04-20 9:10 ` [PATCHv2 0/8] Further filename completion improvements Andrew Burgess
2024-04-20 9:10 ` [PATCHv2 1/8] gdb/doc: document how filename arguments are formatted Andrew Burgess
2024-04-20 9:10 ` [PATCHv2 2/8] gdb: split apart two different types of filename completion Andrew Burgess
@ 2024-04-20 9:10 ` Andrew Burgess
2024-04-20 9:10 ` [PATCHv2 4/8] gdb: move display of completion results into completion_result class Andrew Burgess
` (5 subsequent siblings)
8 siblings, 0 replies; 50+ messages in thread
From: Andrew Burgess @ 2024-04-20 9:10 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess, Lancelot SIX, Eli Zaretskii
This improves quoting and escaping when completing filenames for
commands that allow filenames to be quoted and escaped.
I've struggled a bit trying to split this series into chunks. There's
a lot of dependencies between different parts of the completion
system, and trying to get this working correctly is pretty messy.
This first step is really about implementing 3 readline hooks:
rl_char_is_quoted_p
- Is a particular character quoted within readline's input buffer?
rl_filename_dequoting_function
- Remove quoting characters from a filename.
rl_filename_quoting_function
- Add quoting characters to a filename.
See 'info readline' for full details, but with these hooks connected
up, readline (on behalf of GDB) should do a better job inserting
backslash escapes when completing filenames.
There's still a bunch of stuff that doesn't work after this commit,
mostly around the 'complete' command which of course doesn't go
through readline, so doesn't benefit from all of these new functions
yet, I'll add some of this in a later commit.
Tab completion is now slightly improved though, it is possible to
tab-complete a filename that includes a double or single quote, either
in an unquoted string or within a string surrounded by single or
double quotes, backslash escaping is used when necessary.
There are some additional tests to cover the new functionality.
---
gdb/completer.c | 169 +++++++++++++++++-
.../gdb.base/filename-completion.exp | 36 +++-
2 files changed, 201 insertions(+), 4 deletions(-)
diff --git a/gdb/completer.c b/gdb/completer.c
index c300e42f588..396b1e0136b 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -203,6 +203,159 @@ noop_completer (struct cmd_list_element *ignore,
{
}
+/* Return 1 if the character at EINDEX in STRING is quoted (there is an
+ unclosed quoted string), or if the character at EINDEX is quoted by a
+ backslash. */
+
+static int
+gdb_completer_file_name_char_is_quoted (char *string, int eindex)
+{
+ for (int i = 0; i <= eindex && string[i] != '\0'; )
+ {
+ char c = string[i];
+
+ if (c == '\\')
+ {
+ /* The backslash itself is not quoted. */
+ if (i >= eindex)
+ return 0;
+ ++i;
+ /* But the next character is. */
+ if (i >= eindex)
+ return 1;
+ if (string[i] == '\0')
+ return 0;
+ ++i;
+ continue;
+ }
+ else if (strchr (rl_completer_quote_characters, c) != nullptr)
+ {
+ /* This assumes that extract_string_maybe_quoted can handle a
+ string quoted with character C. Currently this is true as the
+ only characters we put in rl_completer_quote_characters are
+ single and/or double quotes, both of which
+ extract_string_maybe_quoted can handle. */
+ gdb_assert (c == '"' || c == '\'');
+ const char *tmp = &string[i];
+ (void) extract_string_maybe_quoted (&tmp);
+ i = tmp - string;
+
+ /* Consider any character within the string we just skipped over
+ as quoted, though this might not be completely correct; the
+ opening and closing quotes are not themselves quoted. But so
+ far this doesn't seem to have caused any issues. */
+ if (i >= eindex)
+ return 1;
+ }
+ else
+ ++i;
+ }
+
+ return 0;
+}
+
+/* Removing character escaping from FILENAME. QUOTE_CHAR is the quote
+ character around FILENAME or the null-character if there is no quoting
+ around FILENAME. */
+
+static char *
+gdb_completer_file_name_dequote (char *filename, int quote_char)
+{
+ std::string tmp;
+
+ if (quote_char == '\'')
+ {
+ /* There is no backslash escaping within a single quoted string. In
+ this case we can just return the input string. */
+ tmp = filename;
+ }
+ else if (quote_char == '"')
+ {
+ /* Remove escaping from a double quoted string. */
+ for (const char *input = filename;
+ *input != '\0';
+ ++input)
+ {
+ if (input[0] == '\\'
+ && input[1] != '\0'
+ && strchr ("\"\\", input[1]) != nullptr)
+ ++input;
+ tmp += *input;
+ }
+ }
+ else
+ {
+ gdb_assert (quote_char == '\0');
+
+ /* Remove escaping from an unquoted string. */
+ for (const char *input = filename;
+ *input != '\0';
+ ++input)
+ {
+ /* We allow anything to be escaped in an unquoted string. */
+ if (*input == '\\')
+ {
+ ++input;
+ if (*input == '\0')
+ break;
+ }
+
+ tmp += *input;
+ }
+ }
+
+ return strdup (tmp.c_str ());
+}
+
+/* Apply character escaping to the file name in TEXT. QUOTE_PTR points to
+ the quote character surrounding TEXT, or points to the null-character if
+ there are no quotes around TEXT. MATCH_TYPE will be one of the readline
+ constants SINGLE_MATCH or MULTI_MATCH depending on if there is one or
+ many completions. */
+
+static char *
+gdb_completer_file_name_quote (char *text, int match_type ATTRIBUTE_UNUSED,
+ char *quote_ptr)
+{
+ std::string str;
+
+ if (*quote_ptr == '\'')
+ {
+ /* There is no backslash escaping permitted within a single quoted
+ string, so in this case we can just return the input sting. */
+ str = text;
+ }
+ else if (*quote_ptr == '"')
+ {
+ /* Add escaping for a double quoted filename. */
+ for (const char *input = text;
+ *input != '\0';
+ ++input)
+ {
+ if (strchr ("\"\\", *input) != nullptr)
+ str += '\\';
+ str += *input;
+ }
+ }
+ else
+ {
+ gdb_assert (*quote_ptr == '\0');
+
+ /* Add escaping for an unquoted filename. */
+ for (const char *input = text;
+ *input != '\0';
+ ++input)
+ {
+ if (strchr (" \t\n\\\"'", *input)
+ != nullptr)
+ str += '\\';
+ str += *input;
+ }
+ }
+
+ return strdup (str.c_str ());
+}
+
/* Generate filename completions of WORD, storing the completions into
TRACKER. This is used for generating completions for commands that
only accept unquoted filenames as well as for commands that accept
@@ -256,6 +409,7 @@ filename_maybe_quoted_completer (struct cmd_list_element *ignore,
completion_tracker &tracker,
const char *text, const char *word)
{
+ rl_char_is_quoted_p = gdb_completer_file_name_char_is_quoted;
rl_completer_quote_characters = gdb_completer_file_name_quote_characters;
filename_completer_generate_completions (tracker, word);
}
@@ -271,6 +425,7 @@ filename_maybe_quoted_completer_handle_brkchars
(gdb_completer_file_name_break_characters);
rl_completer_quote_characters = gdb_completer_file_name_quote_characters;
+ rl_char_is_quoted_p = gdb_completer_file_name_char_is_quoted;
}
/* See completer.h. */
@@ -1271,6 +1426,7 @@ complete_line_internal_1 (completion_tracker &tracker,
completing file names then we can switch to the file name quote
character set (i.e., both single- and double-quotes). */
rl_completer_quote_characters = gdb_completer_expression_quote_characters;
+ rl_char_is_quoted_p = nullptr;
/* Decide whether to complete on a list of gdb commands or on
symbols. */
@@ -2163,9 +2319,11 @@ completion_tracker::build_completion_result (const char *text,
/* Build replacement word, based on the LCD. */
recompute_lowest_common_denominator ();
- match_list[0]
- = expand_preserving_ws (text, end - start,
- m_lowest_common_denominator);
+ if (rl_filename_completion_desired)
+ match_list[0] = xstrdup (m_lowest_common_denominator);
+ else
+ match_list[0]
+ = expand_preserving_ws (text, end - start, m_lowest_common_denominator);
if (m_lowest_common_denominator_unique)
{
@@ -3028,6 +3186,11 @@ _initialize_completer ()
rl_attempted_completion_function = gdb_rl_attempted_completion_function;
set_rl_completer_word_break_characters (default_word_break_characters ());
+ /* Setup readline globals relating to filename completion. */
+ rl_filename_quote_characters = " \t\n\\\"'";
+ rl_filename_dequoting_function = gdb_completer_file_name_dequote;
+ rl_filename_quoting_function = gdb_completer_file_name_quote;
+
add_setshow_zuinteger_unlimited_cmd ("max-completions", no_class,
&max_completions, _("\
Set maximum number of completion candidates."), _("\
diff --git a/gdb/testsuite/gdb.base/filename-completion.exp b/gdb/testsuite/gdb.base/filename-completion.exp
index 1ccaaff9afc..275140ffd9d 100644
--- a/gdb/testsuite/gdb.base/filename-completion.exp
+++ b/gdb/testsuite/gdb.base/filename-completion.exp
@@ -52,6 +52,9 @@ proc setup_directory_tree {} {
remote_exec host "touch \"${root}/bb2/dir 2/file 1\""
remote_exec host "touch \"${root}/bb2/dir 2/file 2\""
+ remote_exec host "touch \"${root}/bb1/aa\\\"bb\""
+ remote_exec host "touch \"${root}/bb1/aa'bb\""
+
return $root
}
@@ -102,12 +105,43 @@ proc run_quoting_and_escaping_tests { root } {
if { $qc ne "" } {
set sp " "
- test_gdb_complete_multiple "$cmd ${qc}${root}/aaa/" \
+ test_gdb_complete_multiple "file ${qc}${root}/aaa/" \
"a" "a${sp}" {
"aa bb"
"aa cc"
} "" "${qc}" false \
"expand filenames containing spaces"
+
+ test_gdb_complete_multiple "file ${qc}${root}/bb1/" \
+ "a" "a" {
+ "aa\"bb"
+ "aa'bb"
+ } "" "${qc}" false \
+ "expand filenames containing quotes"
+ } else {
+ set sp "\\ "
+
+ test_gdb_complete_tab_multiple "file ${qc}${root}/aaa/a" \
+ "a${sp}" {
+ "aa bb"
+ "aa cc"
+ } false \
+ "expand filenames containing spaces"
+
+ test_gdb_complete_tab_multiple "file ${qc}${root}/bb1/a" \
+ "a" {
+ "aa\"bb"
+ "aa'bb"
+ } false \
+ "expand filenames containing quotes"
+
+ test_gdb_complete_tab_unique "file ${qc}${root}/bb1/aa\\\"" \
+ "file ${qc}${root}/bb1/aa\\\\\"bb${qc}" " " \
+ "expand unique filename containing double quotes"
+
+ test_gdb_complete_tab_unique "file ${qc}${root}/bb1/aa\\'" \
+ "file ${qc}${root}/bb1/aa\\\\'bb${qc}" " " \
+ "expand unique filename containing single quote"
}
}
--
2.25.4
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCHv2 4/8] gdb: move display of completion results into completion_result class
2024-04-20 9:10 ` [PATCHv2 0/8] Further filename completion improvements Andrew Burgess
` (2 preceding siblings ...)
2024-04-20 9:10 ` [PATCHv2 3/8] gdb: improve escaping when completing filenames Andrew Burgess
@ 2024-04-20 9:10 ` Andrew Burgess
2024-04-20 9:10 ` [PATCHv2 5/8] gdb: simplify completion_result::print_matches Andrew Burgess
` (4 subsequent siblings)
8 siblings, 0 replies; 50+ messages in thread
From: Andrew Burgess @ 2024-04-20 9:10 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess, Lancelot SIX, Eli Zaretskii
This commit moves the printing of the 'complete' command results out
of the 'complete_command' function. The printing is now done in a new
member function 'completion_result::print_matches'. At this point,
this is entirely a refactor.
The motivation for this refactor is how 'complete' should print the
completion of filename arguments. In some cases the filename results
need to have escaping added to the output. This escaping needs to be
done immediately prior to printing the result as adding too early will
result in multiple 'complete' results potentially being sorted
incorrectly. See the subsequent commits for more details.
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 0a537ca9661..efde7e18f44 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -423,31 +423,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 396b1e0136b..af1c09b45b1 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -2459,6 +2459,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 fa21156bd1f..200d8a9b3af 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
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCHv2 5/8] gdb: simplify completion_result::print_matches
2024-04-20 9:10 ` [PATCHv2 0/8] Further filename completion improvements Andrew Burgess
` (3 preceding siblings ...)
2024-04-20 9:10 ` [PATCHv2 4/8] gdb: move display of completion results into completion_result class Andrew Burgess
@ 2024-04-20 9:10 ` Andrew Burgess
2024-04-20 9:10 ` [PATCHv2 6/8] gdb: add match formatter mechanism for 'complete' command output Andrew Burgess
` (3 subsequent siblings)
8 siblings, 0 replies; 50+ messages in thread
From: Andrew Burgess @ 2024-04-20 9:10 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess, Lancelot SIX, Eli Zaretskii
Simplify completion_result::print_matches by removing one of the code
paths. Now, every time we call ::print_matches we always add the
trailing quote.
Previously, when using the 'complete' command, if there was only one
result then trailing quote was added in ::build_completion_result, but
when we had multiple results the trailing quote was added in
::print_matches. As a consequence, ::print_matches had to understand
not to add the trailing quote for the single result case.
After this commit we don't add the trailing quote in
::build_completion_result, instead ::print_matches always adds the
trailing quote, which makes ::print_matches simpler.
However, there is a slight problem. When completion is being driven
by readline, and not by the 'complete' command, we still need to
manually add the trailing quote in the single result case, and as the
printing is done by readline we can't add the quote at the time of
printing, and so, in ::build_completion_result, we still add the
trailing quote, but only when completion is being done for readline.
And this does cause a small problem. When completing a filename, if
the completion results in a directory name then, when using the
'complete' command, GDB should not be adding a trailing quote. For
example, if we have the file /tmp/xxx/foo.c, then what we should see
is this:
(gdb) complete file '/tmp/xx
file 'tmp/xxx/
But what we actually see after this commit is this:
(gdb) complete file '/tmp/xx
file 'tmp/xxx/'
Previously we didn't get the trailing quote in this case, as when
there is only a single result, the quote was added in
::build_completion_result, and for filename completion, GDB didn't
know what the quote character was in ::build_completion_result, so no
quote was added. Now that the trailing quote is always added in
::print_matches, and GDB does know the quote character at this point,
so we are now getting the trailing quote, which is not correct.
This is a regression, but really, GDB is now broken in a consistent
way, if we create the file /tmp/xxa/bar.c, then previously if we did
this:
(gdb) complete file '/tmp/xx
file '/tmp/xxa/'
file '/tmp/xxx/'
Notice how we get the trailing quote in this case, this is the before
patch behaviour, and is also wrong.
A later commit will fix things so that the trailing quote is not added
in this filename completion case, but for now I'm going to accept this
small regression.
This change in behaviour caused some failures in one of the completion
tests, I've tweaked the test case to expect the trailing quote as part
of this commit, but will revert this in a later commit in this series.
I've also added an extra test for when the 'complete' command does
complete to a single complete filename, in which case the trailing
quote is expected.
---
gdb/completer.c | 62 +++++++++----------
.../gdb.base/filename-completion.exp | 21 ++++++-
2 files changed, 50 insertions(+), 33 deletions(-)
diff --git a/gdb/completer.c b/gdb/completer.c
index af1c09b45b1..4ba45a35f8f 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -2327,23 +2327,30 @@ completion_tracker::build_completion_result (const char *text,
if (m_lowest_common_denominator_unique)
{
- /* We don't rely on readline appending the quote char as
- delimiter as then readline wouldn't append the ' ' after the
- completion. */
- char buf[2] = { (char) quote_char () };
-
- match_list[0] = reconcat (match_list[0], match_list[0],
- buf, (char *) NULL);
- match_list[1] = NULL;
-
- /* If the tracker wants to, or we already have a space at the
- end of the match, tell readline to skip appending
- another. */
- char *match = match_list[0];
- bool completion_suppress_append
- = (suppress_append_ws ()
- || (match[0] != '\0'
- && match[strlen (match) - 1] == ' '));
+ bool completion_suppress_append;
+
+ if (from_readline ())
+ {
+ /* We don't rely on readline appending the quote char as
+ delimiter as then readline wouldn't append the ' ' after the
+ completion. */
+ char buf[2] = { (char) quote_char (), '\0' };
+
+ match_list[0] = reconcat (match_list[0], match_list[0], buf,
+ (char *) nullptr);
+
+ /* If the tracker wants to, or we already have a space at the end
+ of the match, tell readline to skip appending another. */
+ char *match = match_list[0];
+ completion_suppress_append
+ = (suppress_append_ws ()
+ || (match[0] != '\0'
+ && match[strlen (match) - 1] == ' '));
+ }
+ else
+ completion_suppress_append = false;
+
+ match_list[1] = nullptr;
return completion_result (match_list, 1, completion_suppress_append);
}
@@ -2465,21 +2472,14 @@ 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 ();
+ 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");
- }
- }
+ char buf[2] = { (char) quote_char, '\0' };
+ size_t off = this->number_matches == 1 ? 0 : 1;
+
+ for (size_t i = 0; i < this->number_matches; i++)
+ printf_unfiltered ("%s%s%s\n", prefix.c_str (),
+ this->match_list[i + off], buf);
if (this->number_matches == max_completions)
{
diff --git a/gdb/testsuite/gdb.base/filename-completion.exp b/gdb/testsuite/gdb.base/filename-completion.exp
index 275140ffd9d..3e3b9b29ba4 100644
--- a/gdb/testsuite/gdb.base/filename-completion.exp
+++ b/gdb/testsuite/gdb.base/filename-completion.exp
@@ -82,8 +82,25 @@ proc run_quoting_and_escaping_tests { root } {
test_gdb_complete_none "$cmd ${qc}${root}/xx" \
"expand a non-existent filename"
- test_gdb_complete_unique "$cmd ${qc}${root}/a" \
- "$cmd ${qc}${root}/aaa/" "" false \
+ # The following test is split into separate cmd and tab calls
+ # so we can xfail the cmd version. The cmd version will add a
+ # closing quote, it shouldn't be doing this. This will be
+ # fixed in a later commit.
+ if { $qc ne "" } {
+ setup_xfail "*-*-*"
+ }
+ test_gdb_complete_cmd_unique "file ${qc}${root}/a" \
+ "file ${qc}${root}/aaa/" \
+ "expand a unique directory name"
+
+ if { [readline_is_used] } {
+ test_gdb_complete_tab_unique "file ${qc}${root}/a" \
+ "file ${qc}${root}/aaa/" "" \
+ "expand a unique directory name"
+ }
+
+ test_gdb_complete_unique "file ${qc}${root}/cc2" \
+ "file ${qc}${root}/cc2${qc}" " " false \
"expand a unique filename"
test_gdb_complete_multiple "$cmd ${qc}${root}/" \
--
2.25.4
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCHv2 6/8] gdb: add match formatter mechanism for 'complete' command output
2024-04-20 9:10 ` [PATCHv2 0/8] Further filename completion improvements Andrew Burgess
` (4 preceding siblings ...)
2024-04-20 9:10 ` [PATCHv2 5/8] gdb: simplify completion_result::print_matches Andrew Burgess
@ 2024-04-20 9:10 ` Andrew Burgess
2024-04-20 9:10 ` [PATCHv2 7/8] gdb: apply escaping to filenames in 'complete' results Andrew Burgess
` (2 subsequent siblings)
8 siblings, 0 replies; 50+ messages in thread
From: Andrew Burgess @ 2024-04-20 9:10 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess, Lancelot SIX, Eli Zaretskii
This commit solves a problem that existed prior to the previous
commit, but the previous commit made more common.
When completing a filename with the 'complete' command GDB will always
add a trailing quote character, even if the completion is a directory
name, in which case it would be better if the trailing quote was not
added. Consider:
(gdb) complete file '/tmp/xx
file '/tmp/xxx/'
The completion offered here is really only a partial completion, we've
completed up to the end of the next directory name, but, until we have
a filename then the completion is not finished and the trailing quote
should not be added.
This would match the readline behaviour, e.g.:
(gdb) file '/tmp/xx<TAB>
(gdb) file '/tmp/xxx/
In this case readline completes the directory name, but doesn't add
the trailing quote character.
Remember that the 'complete' command is intended for tools like
e.g. emacs in order that they can emulate GDB's standard readline
completion when implementing a CLI of their own. As such, not adding
the trailing quote in this case matches the readline behaviour, and
seems like the right way to go.
To achieve this, I've added a new function pointer member variable
completion_result::m_match_formatter. This contains a pointer to a
callback function which is used by the 'complete' command to format
each result.
The default behaviour of this callback function is to just append the
quote character (the character from before the completion string) to
the end of the completion result. This matches the current behaviour.
However, for filename completion we override the default value of
m_match_formatter, this new function checks if the completion result
is a directory or not. If the completion result is a directory then
the closing quote is not added, instead we add a trailing '/'
character.
The code to add a trailing '/' character already exists within the
filename_completer function. This is no longer needed in this
location, instead this code is moved into the formatter callback.
Tests are updated to handle the changes in functionality, this removes
an xfail added in the previous commit.
---
gdb/completer.c | 92 ++++++++++++++-----
gdb/completer.h | 45 ++++++++-
.../gdb.base/filename-completion.exp | 68 ++++++++++----
3 files changed, 156 insertions(+), 49 deletions(-)
diff --git a/gdb/completer.c b/gdb/completer.c
index 4ba45a35f8f..ee016cdc7f7 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -45,6 +45,8 @@
#include "completer.h"
+#include <filesystem>
+
/* Forward declarations. */
static const char *completion_find_completion_word (completion_tracker &tracker,
const char *text,
@@ -356,6 +358,28 @@ gdb_completer_file_name_quote (char *text, int match_type ATTRIBUTE_UNUSED,
return strdup (str.c_str ());
}
+/* The function is used to update the completion word MATCH before
+ displaying it to the user in the 'complete' command output. This
+ function is only used for formatting filename or directory names.
+
+ This function checks to see if the completion word MATCH is a directory,
+ in which case a trailing "/" (forward-slash) is added, otherwise
+ QUOTE_CHAR is added as a trailing quote.
+
+ Return the updated completion word as a string. */
+
+static std::string
+filename_match_formatter (const char *match, char quote_char)
+{
+ std::string result (match);
+ if (std::filesystem::is_directory (gdb_tilde_expand (match)))
+ result += "/";
+ else
+ result += quote_char;
+
+ return result;
+}
+
/* Generate filename completions of WORD, storing the completions into
TRACKER. This is used for generating completions for commands that
only accept unquoted filenames as well as for commands that accept
@@ -365,6 +389,8 @@ static void
filename_completer_generate_completions (completion_tracker &tracker,
const char *word)
{
+ tracker.set_match_format_func (filename_match_formatter);
+
int subsequent_name = 0;
while (1)
{
@@ -383,20 +409,6 @@ filename_completer_generate_completions (completion_tracker &tracker,
if (p[strlen (p) - 1] == '~')
continue;
- /* Readline appends a trailing '/' if the completion is a
- directory. If this completion request originated from outside
- readline (e.g. GDB's 'complete' command), then we append the
- trailing '/' ourselves now. */
- if (!tracker.from_readline ())
- {
- std::string expanded = gdb_tilde_expand (p_rl.get ());
- struct stat finfo;
- const bool isdir = (stat (expanded.c_str (), &finfo) == 0
- && S_ISDIR (finfo.st_mode));
- if (isdir)
- p_rl.reset (concat (p_rl.get (), "/", nullptr));
- }
-
tracker.add_completion
(make_completion_match_str (std::move (p_rl), word, word));
}
@@ -1646,10 +1658,25 @@ int max_completions = 200;
/* Initial size of the table. It automagically grows from here. */
#define INITIAL_COMPLETION_HTAB_SIZE 200
+/* The function is used to update the completion word MATCH before
+ displaying it to the user in the 'complete' command output. This
+ default function is used in all cases except those where a completion
+ function overrides this function by calling set_match_format_func.
+
+ This function returns MATCH with QUOTE_CHAR appended. If QUOTE_CHAR is
+ the null-character then the returned string will just contain MATCH. */
+
+static std::string
+default_match_formatter (const char *match, char quote_char)
+{
+ return std::string (match) + quote_char;
+}
+
/* See completer.h. */
completion_tracker::completion_tracker (bool from_readline)
- : m_from_readline (from_readline)
+ : m_from_readline (from_readline),
+ m_match_format_func (default_match_formatter)
{
discard_completions ();
}
@@ -2352,7 +2379,8 @@ completion_tracker::build_completion_result (const char *text,
match_list[1] = nullptr;
- return completion_result (match_list, 1, completion_suppress_append);
+ return completion_result (match_list, 1, completion_suppress_append,
+ m_match_format_func);
}
else
{
@@ -2389,7 +2417,8 @@ completion_tracker::build_completion_result (const char *text,
htab_traverse_noresize (m_entries_hash.get (), func, &builder);
match_list[builder.index] = NULL;
- return completion_result (match_list, builder.index - 1, false);
+ return completion_result (match_list, builder.index - 1, false,
+ m_match_format_func);
}
}
@@ -2397,18 +2426,23 @@ completion_tracker::build_completion_result (const char *text,
completion_result::completion_result ()
: match_list (NULL), number_matches (0),
- completion_suppress_append (false)
+ completion_suppress_append (false),
+ m_match_formatter (default_match_formatter)
{}
/* See completer.h */
completion_result::completion_result (char **match_list_,
size_t number_matches_,
- bool completion_suppress_append_)
+ bool completion_suppress_append_,
+ match_format_func_t match_formatter_)
: match_list (match_list_),
number_matches (number_matches_),
- completion_suppress_append (completion_suppress_append_)
-{}
+ completion_suppress_append (completion_suppress_append_),
+ m_match_formatter (match_formatter_)
+{
+ gdb_assert (m_match_formatter != nullptr);
+}
/* See completer.h */
@@ -2421,10 +2455,12 @@ completion_result::~completion_result ()
completion_result::completion_result (completion_result &&rhs) noexcept
: match_list (rhs.match_list),
- number_matches (rhs.number_matches)
+ number_matches (rhs.number_matches),
+ m_match_formatter (rhs.m_match_formatter)
{
rhs.match_list = NULL;
rhs.number_matches = 0;
+ rhs.m_match_formatter = default_match_formatter;
}
/* See completer.h */
@@ -2474,12 +2510,18 @@ completion_result::print_matches (const std::string &prefix,
{
this->sort_match_list ();
- char buf[2] = { (char) quote_char, '\0' };
size_t off = this->number_matches == 1 ? 0 : 1;
for (size_t i = 0; i < this->number_matches; i++)
- printf_unfiltered ("%s%s%s\n", prefix.c_str (),
- this->match_list[i + off], buf);
+ {
+ gdb_assert (this->m_match_formatter != nullptr);
+ std::string formatted_match
+ = this->m_match_formatter (this->match_list[i + off],
+ (char) quote_char);
+
+ printf_unfiltered ("%s%s\n", prefix.c_str (),
+ formatted_match.c_str ());
+ }
if (this->number_matches == max_completions)
{
diff --git a/gdb/completer.h b/gdb/completer.h
index 200d8a9b3af..71303e2ae9a 100644
--- a/gdb/completer.h
+++ b/gdb/completer.h
@@ -247,12 +247,24 @@ struct completion_match_result
struct completion_result
{
+ /* The type of a function that is used to format completion results when
+ using the 'complete' command. MATCH is the completion word to be
+ printed, and QUOTE_CHAR is a trailing quote character to (possibly)
+ add at the end of MATCH. QUOTE_CHAR can be the null-character in
+ which case no trailing quote should be added.
+
+ Return the possibly modified completion match word which should be
+ presented to the user. */
+ using match_format_func_t = std::string (*) (const char *match,
+ char quote_char);
+
/* Create an empty result. */
completion_result ();
/* Create a result. */
completion_result (char **match_list, size_t number_matches,
- bool completion_suppress_append);
+ bool completion_suppress_append,
+ match_format_func_t match_format_func);
/* Destroy a result. */
~completion_result ();
@@ -274,10 +286,15 @@ struct completion_result
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. */
+ The QUOTE_CHAR is usually 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.
+
+ The QUOTE_CHAR is not always appended to the completion output. For
+ example, filename completions will not append QUOTE_CHAR if the
+ completion is a directory name. This is all handled by calling this
+ function. */
void print_matches (const std::string &prefix, const char *word,
int quote_char);
@@ -305,6 +322,12 @@ struct completion_result
/* Whether readline should suppress appending a whitespace, when
there's only one possible completion. */
bool completion_suppress_append;
+
+private:
+ /* A function which formats a single completion match ready for display
+ as part of the 'complete' command output. Different completion
+ functions can set different formatter functions. */
+ match_format_func_t m_match_formatter;
};
/* Object used by completers to build a completion match list to hand
@@ -441,6 +464,14 @@ class completion_tracker
bool from_readline () const
{ return m_from_readline; }
+ /* Set the function used to format the completion word before displaying
+ it to the user to F, this is used by the 'complete' command. */
+ void set_match_format_func (completion_result::match_format_func_t f)
+ {
+ gdb_assert (f != nullptr);
+ m_match_format_func = f;
+ }
+
private:
/* The type that we place into the m_entries_hash hash table. */
@@ -535,6 +566,10 @@ class completion_tracker
interactively. The 'complete' command is a way to generate completions
not to be displayed by readline. */
bool m_from_readline;
+
+ /* The function used to format the completion word before it is printed
+ in the 'complete' command output. */
+ completion_result::match_format_func_t m_match_format_func;
};
/* Return a string to hand off to readline as a completion match
diff --git a/gdb/testsuite/gdb.base/filename-completion.exp b/gdb/testsuite/gdb.base/filename-completion.exp
index 3e3b9b29ba4..0467d5c425e 100644
--- a/gdb/testsuite/gdb.base/filename-completion.exp
+++ b/gdb/testsuite/gdb.base/filename-completion.exp
@@ -58,6 +58,49 @@ proc setup_directory_tree {} {
return $root
}
+# This proc started as a copy of test_gdb_complete_multiple, however, this
+# version does some extra work. See the original test_gdb_complete_multiple
+# for a description of all the arguments.
+#
+# When using the 'complete' command with filenames, GDB will add a trailing
+# quote for filenames, and a trailing "/" for directory names. As the
+# trailing "/" is also added in the tab-completion output the
+# COMPLETION_LIST will include the "/" character, but the trailing quote is
+# only added when using the 'complete' command.
+#
+# Pass the trailing quote will be passed as END_QUOTE_CHAR, this proc will
+# run the tab completion test, and will then add the trailing quote to those
+# entries in COMPLETION_LIST that don't have a trailing "/" before running
+# the 'complete' command test.
+proc test_gdb_complete_filename_multiple {
+ cmd_prefix completion_word add_completed_line completion_list
+ {start_quote_char ""} {end_quote_char ""} {max_completions false}
+ {testname ""}
+} {
+ if { [readline_is_used] } {
+ test_gdb_complete_tab_multiple "$cmd_prefix$completion_word" \
+ $add_completed_line $completion_list $max_completions $testname
+ }
+
+ if { $start_quote_char eq "" && $end_quote_char ne "" } {
+ set updated_completion_list {}
+
+ foreach entry $completion_list {
+ if {[string range $entry end end] ne "/"} {
+ set entry $entry$end_quote_char
+ }
+ lappend updated_completion_list $entry
+ }
+
+ set completion_list $updated_completion_list
+ set end_quote_char ""
+ }
+
+ test_gdb_complete_cmd_multiple $cmd_prefix $completion_word \
+ $completion_list $start_quote_char $end_quote_char $max_completions \
+ $testname
+}
+
# Run filename completetion tests for those command that accept quoting and
# escaping of the filename argument.
#
@@ -82,35 +125,22 @@ proc run_quoting_and_escaping_tests { root } {
test_gdb_complete_none "$cmd ${qc}${root}/xx" \
"expand a non-existent filename"
- # The following test is split into separate cmd and tab calls
- # so we can xfail the cmd version. The cmd version will add a
- # closing quote, it shouldn't be doing this. This will be
- # fixed in a later commit.
- if { $qc ne "" } {
- setup_xfail "*-*-*"
- }
- test_gdb_complete_cmd_unique "file ${qc}${root}/a" \
- "file ${qc}${root}/aaa/" \
+ test_gdb_complete_unique "file ${qc}${root}/a" \
+ "file ${qc}${root}/aaa/" "" false \
"expand a unique directory name"
- if { [readline_is_used] } {
- test_gdb_complete_tab_unique "file ${qc}${root}/a" \
- "file ${qc}${root}/aaa/" "" \
- "expand a unique directory name"
- }
-
test_gdb_complete_unique "file ${qc}${root}/cc2" \
"file ${qc}${root}/cc2${qc}" " " false \
"expand a unique filename"
- test_gdb_complete_multiple "$cmd ${qc}${root}/" \
+ test_gdb_complete_filename_multiple "$cmd ${qc}${root}/" \
"b" "b" {
"bb1/"
"bb2/"
} "" "${qc}" false \
"expand multiple directory names"
- test_gdb_complete_multiple "$cmd ${qc}${root}/" \
+ test_gdb_complete_filename_multiple "$cmd ${qc}${root}/" \
"c" "c" {
"cc1/"
"cc2"
@@ -122,14 +152,14 @@ proc run_quoting_and_escaping_tests { root } {
if { $qc ne "" } {
set sp " "
- test_gdb_complete_multiple "file ${qc}${root}/aaa/" \
+ test_gdb_complete_filename_multiple "file ${qc}${root}/aaa/" \
"a" "a${sp}" {
"aa bb"
"aa cc"
} "" "${qc}" false \
"expand filenames containing spaces"
- test_gdb_complete_multiple "file ${qc}${root}/bb1/" \
+ test_gdb_complete_filename_multiple "file ${qc}${root}/bb1/" \
"a" "a" {
"aa\"bb"
"aa'bb"
--
2.25.4
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCHv2 7/8] gdb: apply escaping to filenames in 'complete' results
2024-04-20 9:10 ` [PATCHv2 0/8] Further filename completion improvements Andrew Burgess
` (5 preceding siblings ...)
2024-04-20 9:10 ` [PATCHv2 6/8] gdb: add match formatter mechanism for 'complete' command output Andrew Burgess
@ 2024-04-20 9:10 ` Andrew Burgess
2024-04-20 9:10 ` [PATCHv2 8/8] gdb: improve gdb_rl_find_completion_word for quoted words Andrew Burgess
2024-06-05 13:36 ` [PATCHv3 0/7] Further filename completion improvements Andrew Burgess
8 siblings, 0 replies; 50+ messages in thread
From: Andrew Burgess @ 2024-04-20 9:10 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess, Lancelot SIX, Eli Zaretskii
Building on the mechanism added in the previous commit(s), this commit
applies escaping to filenames in the 'complete' command output.
Consider a file: /tmp/xxx/aa"bb -- that is a filename that contains a
double quote, currently the 'complete' command output looks like this:
(gdb) complete file /tmp/xxx/a
file /tmp/xxx/aa"bb
Notice that the double quote in the output is not escaped. If we
passed this same output back to GDB then the double quote will be
treated as the start of a string.
After this commit then the output looks like this:
(gdb) complete file /tmp/xxx/a
file /tmp/xxx/aa\"bb
The double quote is now escaped. If we feed this output back to GDB
then GDB will treat this as a single filename that contains a double
quote, exactly what we want.
To achieve this I've done a little refactoring, splitting out the core
of gdb_completer_file_name_quote, and then added a new call from the
filename_match_formatter function.
There are updates to the tests to cover this new functionality.
---
gdb/completer.c | 98 ++++++++++++++---
.../gdb.base/filename-completion.exp | 100 +++++++++++-------
2 files changed, 144 insertions(+), 54 deletions(-)
diff --git a/gdb/completer.c b/gdb/completer.c
index ee016cdc7f7..9c14e6a10ce 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -309,25 +309,24 @@ gdb_completer_file_name_dequote (char *filename, int quote_char)
return strdup (tmp.c_str ());
}
-/* Apply character escaping to the file name in TEXT. QUOTE_PTR points to
- the quote character surrounding TEXT, or points to the null-character if
- there are no quotes around TEXT. MATCH_TYPE will be one of the readline
- constants SINGLE_MATCH or MULTI_MATCH depending on if there is one or
- many completions. */
+/* Apply character escaping to the filename in TEXT and return a newly
+ allocated buffer containing the possibly updated filename.
+
+ QUOTE_CHAR is the quote character surrounding TEXT, or the
+ null-character if there are no quotes around TEXT. */
static char *
-gdb_completer_file_name_quote (char *text, int match_type ATTRIBUTE_UNUSED,
- char *quote_ptr)
+gdb_completer_file_name_quote_1 (const char *text, char quote_char)
{
std::string str;
- if (*quote_ptr == '\'')
+ if (quote_char == '\'')
{
/* There is no backslash escaping permitted within a single quoted
string, so in this case we can just return the input sting. */
str = text;
}
- else if (*quote_ptr == '"')
+ else if (quote_char == '"')
{
/* Add escaping for a double quoted filename. */
for (const char *input = text;
@@ -341,7 +340,7 @@ gdb_completer_file_name_quote (char *text, int match_type ATTRIBUTE_UNUSED,
}
else
{
- gdb_assert (*quote_ptr == '\0');
+ gdb_assert (quote_char == '\0');
/* Add escaping for an unquoted filename. */
for (const char *input = text;
@@ -358,6 +357,19 @@ gdb_completer_file_name_quote (char *text, int match_type ATTRIBUTE_UNUSED,
return strdup (str.c_str ());
}
+/* Apply character escaping to the filename in TEXT. QUOTE_PTR points to
+ the quote character surrounding TEXT, or points to the null-character if
+ there are no quotes around TEXT. MATCH_TYPE will be one of the readline
+ constants SINGLE_MATCH or MULTI_MATCH depending on if there is one or
+ many completions. */
+
+static char *
+gdb_completer_file_name_quote (char *text, int match_type ATTRIBUTE_UNUSED,
+ char *quote_ptr)
+{
+ return gdb_completer_file_name_quote_1 (text, *quote_ptr);
+}
+
/* The function is used to update the completion word MATCH before
displaying it to the user in the 'complete' command output. This
function is only used for formatting filename or directory names.
@@ -366,12 +378,28 @@ gdb_completer_file_name_quote (char *text, int match_type ATTRIBUTE_UNUSED,
in which case a trailing "/" (forward-slash) is added, otherwise
QUOTE_CHAR is added as a trailing quote.
+ When ADD_ESCAPES is true any special characters (e.g. whitespace,
+ quotes) will be escaped with a backslash. See
+ gdb_completer_file_name_quote_1 for full details on escaping. When
+ ADD_ESCAPES is false then no escaping will be added and MATCH (with the
+ correct trailing character) will be used unmodified.
+
Return the updated completion word as a string. */
static std::string
-filename_match_formatter (const char *match, char quote_char)
+filename_match_formatter_1 (const char *match, char quote_char,
+ bool add_escapes)
{
- std::string result (match);
+ std::string result;
+ if (add_escapes)
+ {
+ gdb::unique_xmalloc_ptr<char> quoted_match
+ (gdb_completer_file_name_quote_1 (match, quote_char));
+ result = quoted_match.get ();
+ }
+ else
+ result = match;
+
if (std::filesystem::is_directory (gdb_tilde_expand (match)))
result += "/";
else
@@ -380,16 +408,52 @@ filename_match_formatter (const char *match, char quote_char)
return result;
}
+/* The formatting function used to format the results of a 'complete'
+ command when the result is a filename, but the filename should not have
+ any escape characters added. Most commands that accept a filename don't
+ expect the filename to be quoted or to contain escape characters.
+
+ See filename_match_formatter_1 for more argument details. */
+
+static std::string
+filename_unquoted_match_formatter (const char *match, char quote_char)
+{
+ return filename_match_formatter_1 (match, quote_char, false);
+}
+
+/* The formatting function used to format the results of a 'complete'
+ command when the result is a filename, and the filename should have any
+ special character (e.g. whitespace, quotes) within it escaped with a
+ backslash. A limited number of commands accept this style of filename
+ argument.
+
+ See filename_match_formatter_1 for more argument details. */
+
+static std::string
+filename_maybe_quoted_match_formatter (const char *match, char quote_char)
+{
+ return filename_match_formatter_1 (match, quote_char, true);
+}
+
/* Generate filename completions of WORD, storing the completions into
TRACKER. This is used for generating completions for commands that
only accept unquoted filenames as well as for commands that accept
- quoted and escaped filenames. */
+ quoted and escaped filenames.
+
+ When QUOTE_MATCHES is true TRACKER will be given a match formatter
+ function which will add escape characters (if needed) in the results.
+ When QUOTE_MATCHES is false the match formatter provided will not add
+ any escaping to the results. */
static void
filename_completer_generate_completions (completion_tracker &tracker,
- const char *word)
+ const char *word,
+ bool quote_matches)
{
- tracker.set_match_format_func (filename_match_formatter);
+ if (quote_matches)
+ tracker.set_match_format_func (filename_maybe_quoted_match_formatter);
+ else
+ tracker.set_match_format_func (filename_unquoted_match_formatter);
int subsequent_name = 0;
while (1)
@@ -423,7 +487,7 @@ filename_maybe_quoted_completer (struct cmd_list_element *ignore,
{
rl_char_is_quoted_p = gdb_completer_file_name_char_is_quoted;
rl_completer_quote_characters = gdb_completer_file_name_quote_characters;
- filename_completer_generate_completions (tracker, word);
+ filename_completer_generate_completions (tracker, word, true);
}
/* The corresponding completer_handle_brkchars implementation. */
@@ -449,7 +513,7 @@ filename_completer_handle_brkchars
{
gdb_assert (word == nullptr);
tracker.set_use_custom_word_point (true);
- filename_completer_generate_completions (tracker, text);
+ filename_completer_generate_completions (tracker, text, false);
}
/* Find the bounds of the current word for completion purposes, and
diff --git a/gdb/testsuite/gdb.base/filename-completion.exp b/gdb/testsuite/gdb.base/filename-completion.exp
index 0467d5c425e..3ded82431c8 100644
--- a/gdb/testsuite/gdb.base/filename-completion.exp
+++ b/gdb/testsuite/gdb.base/filename-completion.exp
@@ -82,10 +82,22 @@ proc test_gdb_complete_filename_multiple {
$add_completed_line $completion_list $max_completions $testname
}
- if { $start_quote_char eq "" && $end_quote_char ne "" } {
+ if { $start_quote_char eq "" } {
set updated_completion_list {}
foreach entry $completion_list {
+ # If ENTRY is quoted with double quotes, then any double
+ # quotes within the entry need to be escaped.
+ if { $end_quote_char eq "\"" } {
+ regsub -all "\"" $entry "\\\"" entry
+ }
+
+ if { $end_quote_char eq "" } {
+ regsub -all " " $entry "\\ " entry
+ regsub -all "\"" $entry "\\\"" entry
+ regsub -all "'" $entry "\\'" entry
+ }
+
if {[string range $entry end end] ne "/"} {
set entry $entry$end_quote_char
}
@@ -147,47 +159,61 @@ proc run_quoting_and_escaping_tests { root } {
} "" "${qc}" false \
"expand mixed directory and file names"
- # GDB does not currently escape word break characters
- # (e.g. white space) correctly in unquoted filenames.
if { $qc ne "" } {
set sp " "
-
- test_gdb_complete_filename_multiple "file ${qc}${root}/aaa/" \
- "a" "a${sp}" {
- "aa bb"
- "aa cc"
- } "" "${qc}" false \
- "expand filenames containing spaces"
-
- test_gdb_complete_filename_multiple "file ${qc}${root}/bb1/" \
- "a" "a" {
- "aa\"bb"
- "aa'bb"
- } "" "${qc}" false \
- "expand filenames containing quotes"
} else {
set sp "\\ "
+ }
+
+ if { $qc eq "'" } {
+ set dq "\""
+ set dq_re "\""
+ } else {
+ set dq "\\\""
+ set dq_re "\\\\\""
+ }
+
+ test_gdb_complete_filename_multiple "file ${qc}${root}/bb2/" \
+ "d" "ir${sp}" {
+ "dir 1/"
+ "dir 2/"
+ } "" "${qc}" false \
+ "expand multiple directory names containing spaces"
- test_gdb_complete_tab_multiple "file ${qc}${root}/aaa/a" \
- "a${sp}" {
- "aa bb"
- "aa cc"
- } false \
- "expand filenames containing spaces"
-
- test_gdb_complete_tab_multiple "file ${qc}${root}/bb1/a" \
- "a" {
- "aa\"bb"
- "aa'bb"
- } false \
- "expand filenames containing quotes"
-
- test_gdb_complete_tab_unique "file ${qc}${root}/bb1/aa\\\"" \
- "file ${qc}${root}/bb1/aa\\\\\"bb${qc}" " " \
- "expand unique filename containing double quotes"
-
- test_gdb_complete_tab_unique "file ${qc}${root}/bb1/aa\\'" \
- "file ${qc}${root}/bb1/aa\\\\'bb${qc}" " " \
+ test_gdb_complete_filename_multiple "file ${qc}${root}/aaa/" \
+ "a" "a${sp}" {
+ "aa bb"
+ "aa cc"
+ } "" "${qc}" false \
+ "expand filenames containing spaces"
+
+ test_gdb_complete_filename_multiple "file ${qc}${root}/bb1/" \
+ "a" "a" {
+ "aa\"bb"
+ "aa'bb"
+ } "" "${qc}" false \
+ "expand filenames containing quotes"
+
+ test_gdb_complete_tab_unique "file ${qc}${root}/bb1/aa${dq}" \
+ "file ${qc}${root}/bb1/aa${dq_re}bb${qc}" " " \
+ "expand unique filename containing double quotes"
+
+ # It is not possible to include a single quote character
+ # within a single quoted string. However, GDB does not do
+ # anything smart if a user tries to do this. Avoid testing
+ # this case. Maybe in the future we'll figure a way to avoid
+ # this situation.
+ if { $qc ne "'" } {
+ if { $qc eq "" } {
+ set sq "\\'"
+ set sq_re "\\\\'"
+ } else {
+ set sq "'"
+ set sq_re "'"
+ }
+
+ test_gdb_complete_tab_unique "file ${qc}${root}/bb1/aa${sq}" \
+ "file ${qc}${root}/bb1/aa${sq_re}bb${qc}" " " \
"expand unique filename containing single quote"
}
}
--
2.25.4
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCHv2 8/8] gdb: improve gdb_rl_find_completion_word for quoted words
2024-04-20 9:10 ` [PATCHv2 0/8] Further filename completion improvements Andrew Burgess
` (6 preceding siblings ...)
2024-04-20 9:10 ` [PATCHv2 7/8] gdb: apply escaping to filenames in 'complete' results Andrew Burgess
@ 2024-04-20 9:10 ` Andrew Burgess
2024-06-05 13:36 ` [PATCHv3 0/7] Further filename completion improvements Andrew Burgess
8 siblings, 0 replies; 50+ messages in thread
From: Andrew Burgess @ 2024-04-20 9:10 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess, Lancelot SIX, Eli Zaretskii
The function gdb_rl_find_completion_word is very similar to the
readline function _rl_find_completion_word, but was either an older
version of that function, or was trimmed when copying to remove code
which was considered unnecessary.
We maintain this copy because the _rl_find_completion_word function is
not part of the public readline API, and we need to replicate the
functionality of that function as part of the 'complete' command.
Within gdb_rl_find_completion_word when looking for the completion
word, if we don't find a unclosed quoted string (which would become
the completion word) then we scan backwards looking for a word break
character. For example, given:
(gdb) complete file /tmp/foo
There is no unclosed quoted string so we end up scanning backwards
from the end looking for a word break character. In this case the
space after 'file' and before '/tmp/foo' is found, so '/tmp/foo'
becomes the completion word.
However, given this:
(gdb) complete file /tmp/foo\"
There is still no unclosed quoted string, however, when we can
backwards the '"' (double quotes) are treated as a word break
character, and so we end up using the empty string as the completion
word.
The readline function _rl_find_completion_word avoids this mistake by
using the rl_char_is_quoted_p hook. This function will return true
for the double quote character as it is preceded by a backslash. An
earlier commit in this series supplied a rl_char_is_quoted_p function
for the filename completion case, however, gdb_rl_find_completion_word
doesn't call rl_char_is_quoted_p so this doesn't help for the
'complete' case.
In this commit I've copied the code to call rl_char_is_quoted_p from
_rl_find_completion_word into gdb_rl_find_completion_word.
This half solves the problem. In the case:
(gdb) complete file /tmp/foo\"
We do now try to complete on the string '/tmp/foo\"', however, when we
reach filename_completer we call back into readline to actually
perform filename completion. However, at this point the WORD variable
points to a string that still contains the backslash. The backslash
isn't part of the actual filename, that's just an escape character.
Our expectation is that readline will remove the backslash when
looking for matching filenames. However, readline contains an
optimisation to avoid unnecessary work trying to remove escape
characters.
The readline variable rl_completion_found_quote is set in the readline
function gen_completion_matches before the generation of completion
matches. This variable is set to true (non-zero) if there is (or
might be) escape characters within the completion word.
The function rl_filename_completion_function, which generates the
filename matches, only removes escape characters when
rl_completion_found_quote is true. When GDB generates completions
through readline (e.g. tab completion) then rl_completion_found_quote
is set correctly.
But when we use the 'complete' command we don't pass through readline,
and so gen_completion_matches is never called and
rl_completion_found_quote is not set. In this case when we call
rl_filename_completion_function readline doesn't remove the escapes
from the completion word, and so in our case above, readline looks for
completions of the exact filename '/tmp/foo\"', that is, the filename
including the backslash.
To work around this problem I've added a new flag to our function
gdb_rl_find_completion_word which is set true when we find any quoting
or escaping. This matches what readline does.
Then in the 'complete' function we can set rl_completion_found_quote
prior to generating completion matches.
With this done the 'complete' command now works correctly when trying
to complete filenames that contain escaped word break characters. The
tests have been updated accordingly.
---
gdb/completer.c | 60 +++++++++++++++----
.../gdb.base/filename-completion.exp | 12 ++--
2 files changed, 54 insertions(+), 18 deletions(-)
diff --git a/gdb/completer.c b/gdb/completer.c
index 9c14e6a10ce..6a3428e2bbd 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -50,7 +50,8 @@
/* Forward declarations. */
static const char *completion_find_completion_word (completion_tracker &tracker,
const char *text,
- int *quote_char);
+ int *quote_char,
+ bool *found_any_quoting);
static void set_rl_completer_word_break_characters (const char *break_chars);
@@ -528,7 +529,9 @@ filename_completer_handle_brkchars
boundaries of the current word. QC, if non-null, is set to the
opening quote character if we found an unclosed quoted substring,
'\0' otherwise. DP, if non-null, is set to the value of the
- delimiter character that caused a word break. */
+ delimiter character that caused a word break. FOUND_ANY_QUOTING, if
+ non-null, is set to true if we found any quote characters (single or
+ double quotes, or a backslash) while finding the completion word. */
struct gdb_rl_completion_word_info
{
@@ -539,7 +542,7 @@ struct gdb_rl_completion_word_info
static const char *
gdb_rl_find_completion_word (struct gdb_rl_completion_word_info *info,
- int *qc, int *dp,
+ int *qc, int *dp, bool *found_any_quoting,
const char *line_buffer)
{
int scan, end, delimiter, pass_next, isbrk;
@@ -561,6 +564,7 @@ gdb_rl_find_completion_word (struct gdb_rl_completion_word_info *info,
end = point;
delimiter = 0;
quote_char = '\0';
+ bool found_quote = false;
brkchars = info->word_break_characters;
@@ -586,6 +590,7 @@ gdb_rl_find_completion_word (struct gdb_rl_completion_word_info *info,
if (quote_char != '\'' && line_buffer[scan] == '\\')
{
pass_next = 1;
+ found_quote = true;
continue;
}
@@ -606,6 +611,7 @@ gdb_rl_find_completion_word (struct gdb_rl_completion_word_info *info,
/* Found start of a quoted substring. */
quote_char = line_buffer[scan];
point = scan + 1;
+ found_quote = true;
}
}
}
@@ -619,8 +625,22 @@ gdb_rl_find_completion_word (struct gdb_rl_completion_word_info *info,
{
scan = line_buffer[point];
- if (strchr (brkchars, scan) != 0)
- break;
+ if (strchr (brkchars, scan) == 0)
+ continue;
+
+ /* Call the application-specific function to tell us whether
+ this word break character is quoted and should be skipped.
+ The const_cast is needed here to comply with the readline
+ API. The only function we register for rl_char_is_quoted_p
+ treats the input buffer as 'const', so we're OK. */
+ if (rl_char_is_quoted_p != nullptr && found_quote
+ && (*rl_char_is_quoted_p) (const_cast<char *> (line_buffer),
+ point))
+ continue;
+
+ /* Convoluted code, but it avoids an n^2 algorithm with calls
+ to char_is_quoted. */
+ break;
}
}
@@ -644,6 +664,8 @@ gdb_rl_find_completion_word (struct gdb_rl_completion_word_info *info,
}
}
+ if (found_any_quoting != nullptr)
+ *found_any_quoting = found_quote;
if (qc != NULL)
*qc = quote_char;
if (dp != NULL)
@@ -670,7 +692,7 @@ advance_to_completion_word (completion_tracker &tracker,
int delimiter;
const char *start
- = gdb_rl_find_completion_word (&info, NULL, &delimiter, text);
+ = gdb_rl_find_completion_word (&info, nullptr, &delimiter, nullptr, text);
tracker.advance_custom_word_point_by (start - text);
@@ -732,7 +754,8 @@ complete_nested_command_line (completion_tracker &tracker, const char *text)
int quote_char = '\0';
const char *word = completion_find_completion_word (tracker, text,
- "e_char);
+ "e_char,
+ nullptr);
if (tracker.use_custom_word_point ())
{
@@ -1950,8 +1973,11 @@ complete (const char *line, char const **word, int *quote_char)
try
{
+ bool found_any_quoting = false;
+
*word = completion_find_completion_word (tracker_handle_brkchars,
- line, quote_char);
+ line, quote_char,
+ &found_any_quoting);
/* Completers that provide a custom word point in the
handle_brkchars phase also compute their completions then.
@@ -1961,6 +1987,12 @@ complete (const char *line, char const **word, int *quote_char)
tracker = &tracker_handle_brkchars;
else
{
+ /* Setting this global matches what readline does within
+ gen_completion_matches. We need this set correctly in case
+ our completion function calls back into readline to perform
+ completion (e.g. filename_completer does this). */
+ rl_completion_found_quote = found_any_quoting;
+
complete_line (tracker_handle_completions, *word, line, strlen (line));
tracker = &tracker_handle_completions;
}
@@ -2235,7 +2267,7 @@ gdb_completion_word_break_characters () noexcept
static const char *
completion_find_completion_word (completion_tracker &tracker, const char *text,
- int *quote_char)
+ int *quote_char, bool *found_any_quoting)
{
size_t point = strlen (text);
@@ -2245,6 +2277,13 @@ completion_find_completion_word (completion_tracker &tracker, const char *text,
{
gdb_assert (tracker.custom_word_point () > 0);
*quote_char = tracker.quote_char ();
+ /* This isn't really correct, we're ignoring the case where we found
+ a backslash escaping a character. However, this isn't an issue
+ right now as we only rely on *FOUND_ANY_QUOTING being set when
+ performing filename completion, which doesn't go through this
+ path. */
+ if (found_any_quoting != nullptr)
+ *found_any_quoting = *quote_char != '\0';
return text + tracker.custom_word_point ();
}
@@ -2254,7 +2293,8 @@ completion_find_completion_word (completion_tracker &tracker, const char *text,
info.quote_characters = rl_completer_quote_characters;
info.basic_quote_characters = rl_basic_quote_characters;
- return gdb_rl_find_completion_word (&info, quote_char, NULL, text);
+ return gdb_rl_find_completion_word (&info, quote_char, nullptr,
+ found_any_quoting, text);
}
/* See completer.h. */
diff --git a/gdb/testsuite/gdb.base/filename-completion.exp b/gdb/testsuite/gdb.base/filename-completion.exp
index 3ded82431c8..78f85ace8a1 100644
--- a/gdb/testsuite/gdb.base/filename-completion.exp
+++ b/gdb/testsuite/gdb.base/filename-completion.exp
@@ -167,10 +167,8 @@ proc run_quoting_and_escaping_tests { root } {
if { $qc eq "'" } {
set dq "\""
- set dq_re "\""
} else {
set dq "\\\""
- set dq_re "\\\\\""
}
test_gdb_complete_filename_multiple "file ${qc}${root}/bb2/" \
@@ -194,8 +192,8 @@ proc run_quoting_and_escaping_tests { root } {
} "" "${qc}" false \
"expand filenames containing quotes"
- test_gdb_complete_tab_unique "file ${qc}${root}/bb1/aa${dq}" \
- "file ${qc}${root}/bb1/aa${dq_re}bb${qc}" " " \
+ test_gdb_complete_unique "file ${qc}${root}/bb1/aa${dq}" \
+ "file ${qc}${root}/bb1/aa${dq}bb${qc}" " " false \
"expand unique filename containing double quotes"
# It is not possible to include a single quote character
@@ -206,14 +204,12 @@ proc run_quoting_and_escaping_tests { root } {
if { $qc ne "'" } {
if { $qc eq "" } {
set sq "\\'"
- set sq_re "\\\\'"
} else {
set sq "'"
- set sq_re "'"
}
- test_gdb_complete_tab_unique "file ${qc}${root}/bb1/aa${sq}" \
- "file ${qc}${root}/bb1/aa${sq_re}bb${qc}" " " \
+ test_gdb_complete_unique "file ${qc}${root}/bb1/aa${sq}" \
+ "file ${qc}${root}/bb1/aa${sq}bb${qc}" " " false \
"expand unique filename containing single quote"
}
}
--
2.25.4
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCHv3 0/7] Further filename completion improvements
2024-04-20 9:10 ` [PATCHv2 0/8] Further filename completion improvements Andrew Burgess
` (7 preceding siblings ...)
2024-04-20 9:10 ` [PATCHv2 8/8] gdb: improve gdb_rl_find_completion_word for quoted words Andrew Burgess
@ 2024-06-05 13:36 ` Andrew Burgess
2024-06-05 13:36 ` [PATCHv3 1/7] gdb: split apart two different types of filename completion Andrew Burgess
` (7 more replies)
8 siblings, 8 replies; 50+ messages in thread
From: Andrew Burgess @ 2024-06-05 13:36 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess
In V3:
- Patch #1 from the v2 series was merged.
- Remaining patches have been rebased and retested.
In V2:
- Patches #1 and #2 are new in this iteration. Patches #3 to #8 are
the patches from V1 rebased onto these two new patches,
- Patch #1 adds documentation for the formatting of filename
arguments, this tries to explain the two diffent ways that GDB
expects filename arguments to be formatted.
- Patch #2 addresses the problem that Lancelot pointed out: some
commands don't expect filename arguments to be quoted, or to
contain escapes. In this patch I split the filename completion in
two so the two different filename argument formats are handled
separately...
- This clears the way for the rest of the series, which updates how
completion works for those filename arguments that do accept
quoting and escaping,
- Patches #3 to #8 are in principle the same as in V1, but there
were some changes after rebasing onto the new patch #2.
---
Andrew Burgess (7):
gdb: split apart two different types of filename completion
gdb: improve escaping when completing filenames
gdb: move display of completion results into completion_result class
gdb: simplify completion_result::print_matches
gdb: add match formatter mechanism for 'complete' command output
gdb: apply escaping to filenames in 'complete' results
gdb: improve gdb_rl_find_completion_word for quoted words
gdb/auto-load.c | 4 +-
gdb/breakpoint.c | 5 +-
gdb/cli/cli-cmds.c | 34 +-
gdb/cli/cli-decode.c | 12 +-
gdb/cli/cli-dump.c | 6 +-
gdb/compile/compile.c | 3 +-
gdb/completer.c | 501 +++++++++++++++---
gdb/completer.h | 79 ++-
gdb/corefile.c | 4 +-
gdb/corelow.c | 4 +-
gdb/dwarf2/index-write.c | 3 +-
gdb/exec.c | 7 +-
gdb/guile/scm-cmd.c | 2 +-
gdb/infcmd.c | 15 +-
gdb/inferior.c | 2 +-
gdb/jit.c | 2 +-
gdb/python/py-cmd.c | 2 +-
gdb/record-full.c | 5 +-
gdb/record.c | 2 +-
gdb/skip.c | 2 +-
gdb/source.c | 3 +-
gdb/symfile.c | 7 +-
gdb/target-descriptions.c | 7 +-
gdb/target.c | 4 +-
gdb/target.h | 9 +-
.../gdb.base/filename-completion.exp | 257 +++++++--
gdb/tracectf.c | 3 +-
gdb/tracefile-tfile.c | 4 +-
28 files changed, 791 insertions(+), 197 deletions(-)
base-commit: 2db414c36b4f030782c2c8a24c916c3033261af0
--
2.25.4
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCHv3 1/7] gdb: split apart two different types of filename completion
2024-06-05 13:36 ` [PATCHv3 0/7] Further filename completion improvements Andrew Burgess
@ 2024-06-05 13:36 ` Andrew Burgess
2024-06-06 16:18 ` Tom Tromey
2024-06-05 13:36 ` [PATCHv3 2/7] gdb: improve escaping when completing filenames Andrew Burgess
` (6 subsequent siblings)
7 siblings, 1 reply; 50+ messages in thread
From: Andrew Burgess @ 2024-06-05 13:36 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess
Unfortunately we have two different types of filename completion in
GDB.
The majority of commands have what I call unquoted filename
completion, this is for commands like 'set logging file ...', 'target
core ...', and 'add-auto-load-safe-path ...'. For these commands
everything after the command name (that is not a command option) is
treated as a single filename. If the filename contains white space
then this does not need to be escaped, nor does the filename need to
be quoted. In fact, the filename argument is not de-quoted, and does
not have any escaping removed, so if a user does try to add such
things, they will be treated as part of the filename. As an example:
(gdb) target core "/path/that contains/some white space"
Will look for a directory calls '"' (double quotes) in the local
directory.
A small number of commands do de-quote and remove escapes from
filename arguments. These command accept what I call quoted and
escaped filenames. Right now these are the commands that specify the
file for GDB to debug, so:
file
exec-file
symbol-file
add-symbol-file
remove-symbol-file
As an example of this in action:
(gdb) file "/path/that contains/some white space"
In this case GDB would load the file:
/path/that contains/some white space
Current filename completion always assumes that filenames can be
quoted, though escaping doesn't work in completion right now. But the
assumption that quoting is allowed is clearly wrong.
This commit splits filename completion into two. There is now a
filename completer for unquoted filenames, and a second completer to
handle quoted and escaped filenames.
To handle completion of unquoted filenames the completion needs to be
performed during the brkchars phase of completion, which is why almost
every place we use filename_completer has to change in this commit.
I've also renamed the 'filename_completer' function to reflect that it
now handles filenames that can be quoted.
The filename completion test has been extended to cover more cases.
The 'advance_to_filename_complete_word_point' function is no longer
used after this commit and so I've deleted it.
---
gdb/auto-load.c | 4 +-
gdb/breakpoint.c | 5 +-
gdb/cli/cli-cmds.c | 8 +-
gdb/cli/cli-decode.c | 12 +-
gdb/cli/cli-dump.c | 6 +-
gdb/compile/compile.c | 3 +-
gdb/completer.c | 61 ++++---
gdb/completer.h | 29 +++-
gdb/corefile.c | 4 +-
gdb/corelow.c | 4 +-
gdb/dwarf2/index-write.c | 3 +-
gdb/exec.c | 7 +-
gdb/guile/scm-cmd.c | 2 +-
gdb/infcmd.c | 15 +-
gdb/inferior.c | 2 +-
gdb/jit.c | 2 +-
gdb/python/py-cmd.c | 2 +-
gdb/record-full.c | 5 +-
gdb/record.c | 2 +-
gdb/skip.c | 2 +-
gdb/source.c | 3 +-
gdb/symfile.c | 7 +-
gdb/target-descriptions.c | 7 +-
gdb/target.c | 4 +-
gdb/target.h | 9 +-
.../gdb.base/filename-completion.exp | 156 +++++++++++++-----
gdb/tracectf.c | 3 +-
gdb/tracefile-tfile.c | 4 +-
28 files changed, 248 insertions(+), 123 deletions(-)
diff --git a/gdb/auto-load.c b/gdb/auto-load.c
index afc98eb1f58..51c14e7bb02 100644
--- a/gdb/auto-load.c
+++ b/gdb/auto-load.c
@@ -1629,7 +1629,7 @@ This option has security implications for untrusted inferiors."),
See the commands 'set auto-load safe-path' and 'show auto-load safe-path' to\n\
access the current full list setting."),
&cmdlist);
- set_cmd_completer (cmd, filename_completer);
+ set_cmd_completer_handle_brkchars (cmd, filename_completer_handle_brkchars);
cmd = add_cmd ("add-auto-load-scripts-directory", class_support,
add_auto_load_dir,
@@ -1638,7 +1638,7 @@ access the current full list setting."),
See the commands 'set auto-load scripts-directory' and\n\
'show auto-load scripts-directory' to access the current full list setting."),
&cmdlist);
- set_cmd_completer (cmd, filename_completer);
+ set_cmd_completer_handle_brkchars (cmd, filename_completer_handle_brkchars);
add_setshow_boolean_cmd ("auto-load", class_maintenance,
&debug_auto_load, _("\
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index a973518ac5f..578d515cc5a 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -15094,14 +15094,15 @@ This includes all types of breakpoints (breakpoints, watchpoints,\n\
catchpoints, tracepoints). Use the 'source' command in another debug\n\
session to restore them."),
&save_cmdlist);
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
cmd_list_element *save_tracepoints_cmd
= add_cmd ("tracepoints", class_trace, save_tracepoints_command, _("\
Save current tracepoint definitions as a script.\n\
Use the 'source' command in another debug session to restore them."),
&save_cmdlist);
- set_cmd_completer (save_tracepoints_cmd, filename_completer);
+ set_cmd_completer_handle_brkchars (save_tracepoints_cmd,
+ filename_completer_handle_brkchars);
c = add_com_alias ("save-tracepoints", save_tracepoints_cmd, class_trace, 0);
deprecate_cmd (c, "save tracepoints");
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 3af794cebaf..4c2a443246d 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -2638,7 +2638,7 @@ The debugger's current working directory specifies where scripts and other\n\
files that can be loaded by GDB are located.\n\
In order to change the inferior's current working directory, the recommended\n\
way is to use the \"set cwd\" command."), &cmdlist);
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
add_com ("echo", class_support, echo_command, _("\
Print a constant string. Give string as argument.\n\
@@ -2804,7 +2804,7 @@ the previous command number shown."),
= add_com ("shell", class_support, shell_command, _("\
Execute the rest of the line as a shell command.\n\
With no arguments, run an inferior shell."));
- set_cmd_completer (shell_cmd, filename_completer);
+ set_cmd_completer_handle_brkchars (shell_cmd, filename_completer_handle_brkchars);
add_com_alias ("!", shell_cmd, class_support, 0);
@@ -2893,7 +2893,7 @@ you must type \"disassemble 'foo.c'::bar\" and not \"disassemble foo.c:bar\"."))
c = add_com ("make", class_support, make_command, _("\
Run the ``make'' program using the rest of the line as arguments."));
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
c = add_cmd ("user", no_class, show_user, _("\
Show definitions of non-python/scheme user defined commands.\n\
Argument is the name of the user defined command.\n\
@@ -2977,5 +2977,5 @@ Note that the file \"%s\" is read automatically in this way\n\
when GDB is started."), GDBINIT).release ();
c = add_cmd ("source", class_support, source_command,
source_help_text, &cmdlist);
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
}
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index d9a2ab40575..a48b18bbd87 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -866,7 +866,8 @@ add_setshow_filename_cmd (const char *name, enum command_class theclass,
nullptr, nullptr, set_func,
show_func, set_list, show_list);
- set_cmd_completer (commands.set, filename_completer);
+ set_cmd_completer_handle_brkchars (commands.set,
+ filename_completer_handle_brkchars);
return commands;
}
@@ -890,7 +891,8 @@ add_setshow_filename_cmd (const char *name, command_class theclass,
nullptr, show_func, set_list,
show_list);
- set_cmd_completer (cmds.set, filename_completer);
+ set_cmd_completer_handle_brkchars (cmds.set,
+ filename_completer_handle_brkchars);
return cmds;
}
@@ -1015,7 +1017,8 @@ add_setshow_optional_filename_cmd (const char *name, enum command_class theclass
nullptr, nullptr, set_func, show_func,
set_list, show_list);
- set_cmd_completer (commands.set, filename_completer);
+ set_cmd_completer_handle_brkchars (commands.set,
+ filename_completer_handle_brkchars);
return commands;
}
@@ -1039,7 +1042,8 @@ add_setshow_optional_filename_cmd (const char *name, command_class theclass,
set_func, get_func, nullptr, show_func,
set_list,show_list);
- set_cmd_completer (cmds.set, filename_completer);
+ set_cmd_completer_handle_brkchars (cmds.set,
+ filename_completer_handle_brkchars);
return cmds;
}
diff --git a/gdb/cli/cli-dump.c b/gdb/cli/cli-dump.c
index 9b44c6edcf2..aaacdd6ede1 100644
--- a/gdb/cli/cli-dump.c
+++ b/gdb/cli/cli-dump.c
@@ -348,7 +348,7 @@ add_dump_command (const char *name,
struct dump_context *d;
c = add_cmd (name, all_commands, descr, &dump_cmdlist);
- c->completer = filename_completer;
+ c->completer_handle_brkchars = filename_completer_handle_brkchars;
d = XNEW (struct dump_context);
d->func = func;
d->mode = FOPEN_WB;
@@ -356,7 +356,7 @@ add_dump_command (const char *name,
c->func = call_dump_func;
c = add_cmd (name, all_commands, descr, &append_cmdlist);
- c->completer = filename_completer;
+ c->completer_handle_brkchars = filename_completer_handle_brkchars;
d = XNEW (struct dump_context);
d->func = func;
d->mode = FOPEN_AB;
@@ -705,6 +705,6 @@ Arguments are FILE OFFSET START END where all except FILE are optional.\n\
OFFSET will be added to the base address of the file (default zero).\n\
If START and END are given, only the file contents within that range\n\
(file relative) will be restored to target memory."));
- c->completer = filename_completer;
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
/* FIXME: completers for other commands. */
}
diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index 88531a21781..2c772fddaf5 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -327,8 +327,7 @@ compile_file_command_completer (struct cmd_list_element *ignore,
(tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_ERROR, group))
return;
- word = advance_to_filename_complete_word_point (tracker, text);
- filename_completer (ignore, tracker, text, word);
+ filename_completer_handle_brkchars (ignore, tracker, text, word);
}
/* Handle the input from the 'compile code' command. The
diff --git a/gdb/completer.c b/gdb/completer.c
index f1f44109bdc..640dfc29a8d 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -203,15 +203,15 @@ noop_completer (struct cmd_list_element *ignore,
{
}
-/* Complete on filenames. */
+/* Generate filename completions of WORD, storing the completions into
+ TRACKER. This is used for generating completions for commands that
+ only accept unquoted filenames as well as for commands that accept
+ quoted and escaped filenames. */
-void
-filename_completer (struct cmd_list_element *ignore,
- completion_tracker &tracker,
- const char *text, const char *word)
+static void
+filename_completer_generate_completions (completion_tracker &tracker,
+ const char *word)
{
- rl_completer_quote_characters = gdb_completer_file_name_quote_characters;
-
int subsequent_name = 0;
while (1)
{
@@ -249,13 +249,23 @@ filename_completer (struct cmd_list_element *ignore,
}
}
-/* The corresponding completer_handle_brkchars
- implementation. */
+/* Complete on filenames. */
+
+void
+filename_maybe_quoted_completer (struct cmd_list_element *ignore,
+ completion_tracker &tracker,
+ const char *text, const char *word)
+{
+ rl_completer_quote_characters = gdb_completer_file_name_quote_characters;
+ filename_completer_generate_completions (tracker, word);
+}
+
+/* The corresponding completer_handle_brkchars implementation. */
static void
-filename_completer_handle_brkchars (struct cmd_list_element *ignore,
- completion_tracker &tracker,
- const char *text, const char *word)
+filename_maybe_quoted_completer_handle_brkchars
+ (struct cmd_list_element *ignore, completion_tracker &tracker,
+ const char *text, const char *word)
{
set_rl_completer_word_break_characters
(gdb_completer_file_name_break_characters);
@@ -263,6 +273,18 @@ filename_completer_handle_brkchars (struct cmd_list_element *ignore,
rl_completer_quote_characters = gdb_completer_file_name_quote_characters;
}
+/* See completer.h. */
+
+void
+filename_completer_handle_brkchars
+ (struct cmd_list_element *ignore, completion_tracker &tracker,
+ const char *text, const char *word)
+{
+ gdb_assert (word == nullptr);
+ tracker.set_use_custom_word_point (true);
+ filename_completer_generate_completions (tracker, text);
+}
+
/* Find the bounds of the current word for completion purposes, and
return a pointer to the end of the word. This mimics (and is a
modified version of) readline's _rl_find_completion_word internal
@@ -443,17 +465,6 @@ advance_to_expression_complete_word_point (completion_tracker &tracker,
/* See completer.h. */
-const char *
-advance_to_filename_complete_word_point (completion_tracker &tracker,
- const char *text)
-{
- const char *brk_chars = gdb_completer_file_name_break_characters;
- const char *quote_chars = gdb_completer_file_name_quote_characters;
- return advance_to_completion_word (tracker, brk_chars, quote_chars, text);
-}
-
-/* See completer.h. */
-
bool
completion_tracker::completes_to_completion_word (const char *word)
{
@@ -1877,8 +1888,8 @@ default_completer_handle_brkchars (struct cmd_list_element *ignore,
completer_handle_brkchars_ftype *
completer_handle_brkchars_func_for_completer (completer_ftype *fn)
{
- if (fn == filename_completer)
- return filename_completer_handle_brkchars;
+ if (fn == filename_maybe_quoted_completer)
+ return filename_maybe_quoted_completer_handle_brkchars;
if (fn == location_completer)
return location_completer_handle_brkchars;
diff --git a/gdb/completer.h b/gdb/completer.h
index 98a12f3907c..fa21156bd1f 100644
--- a/gdb/completer.h
+++ b/gdb/completer.h
@@ -563,12 +563,6 @@ extern completion_result
const char *advance_to_expression_complete_word_point
(completion_tracker &tracker, const char *text);
-/* Assuming TEXT is an filename, find the completion word point for
- TEXT, emulating the algorithm readline uses to find the word
- point. */
-extern const char *advance_to_filename_complete_word_point
- (completion_tracker &tracker, const char *text);
-
extern void noop_completer (struct cmd_list_element *,
completion_tracker &tracker,
const char *, const char *);
@@ -577,6 +571,29 @@ extern void filename_completer (struct cmd_list_element *,
completion_tracker &tracker,
const char *, const char *);
+/* Filename completer that can be registered for the brkchars phase of
+ completion. This should be used by commands that don't allow the
+ filename to be quoted, and whitespace does not need to be escaped.
+
+ NOTE: If you are considering using this function as your commands
+ completer, then consider updating your function to use gdb_argv or
+ extract_string_maybe_quoted to allow for possibly quoted filenames
+ instead. You would then use filename_maybe_quoted_completer for
+ filename completion. The benefit of this is that in the future it is
+ possible to add additional arguments to your new command. */
+
+extern void filename_completer_handle_brkchars
+ (struct cmd_list_element *ignore, completion_tracker &tracker,
+ const char *text, const char *word);
+
+/* Filename completer for commands where the filename argument can be
+ quoted, and whitespace within an unquoted filename should be escaped
+ with a backslash. */
+
+extern void filename_maybe_quoted_completer (struct cmd_list_element *,
+ completion_tracker &tracker,
+ const char *, const char *);
+
extern void expression_completer (struct cmd_list_element *,
completion_tracker &tracker,
const char *, const char *);
diff --git a/gdb/corefile.c b/gdb/corefile.c
index 044c084ab6f..b0a48e1aead 100644
--- a/gdb/corefile.c
+++ b/gdb/corefile.c
@@ -406,9 +406,9 @@ Use FILE as core dump for examining memory and registers.\n\
Usage: core-file FILE\n\
No arg means have no core file. This command has been superseded by the\n\
`target core' and `detach' commands."), &cmdlist);
- set_cmd_completer (core_file_cmd, filename_completer);
+ set_cmd_completer_handle_brkchars (core_file_cmd,
+ filename_completer_handle_brkchars);
-
set_show_commands set_show_gnutarget
= add_setshow_string_noescape_cmd ("gnutarget", class_files,
&gnutarget_string, _("\
diff --git a/gdb/corelow.c b/gdb/corelow.c
index 49da9be07ef..1c4f62bdf6b 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -1520,7 +1520,9 @@ void _initialize_corelow ();
void
_initialize_corelow ()
{
- add_target (core_target_info, core_target_open, filename_completer);
+ struct cmd_list_element *c
+ = add_target (core_target_info, core_target_open);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
add_cmd ("core-file-backed-mappings", class_maintenance,
maintenance_print_core_file_backed_mappings,
_("Print core file's file-backed mappings."),
diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c
index 2a50e3b6c2d..03cff7b88cd 100644
--- a/gdb/dwarf2/index-write.c
+++ b/gdb/dwarf2/index-write.c
@@ -1621,8 +1621,7 @@ gdb_save_index_cmd_completer (struct cmd_list_element *ignore,
(tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, grp))
return;
- word = advance_to_filename_complete_word_point (tracker, text);
- filename_completer (ignore, tracker, text, word);
+ filename_completer_handle_brkchars (ignore, tracker, text, word);
}
/* Implementation of the `save gdb-index' command.
diff --git a/gdb/exec.c b/gdb/exec.c
index 88915260d60..cb522a3fc26 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -1065,14 +1065,14 @@ and it is the program executed when you use the `run' command.\n\
If FILE cannot be found as specified, your execution directory path\n\
($PATH) is searched for a command of that name.\n\
No arg means to have no executable file and no symbols."), &cmdlist);
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer (c, filename_maybe_quoted_completer);
c = add_cmd ("exec-file", class_files, exec_file_command, _("\
Use FILE as program for getting contents of pure memory.\n\
If FILE cannot be found as specified, your execution directory path\n\
is searched for a command of that name.\n\
No arg means have no executable file."), &cmdlist);
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer (c, filename_maybe_quoted_completer);
add_com ("section", class_files, set_section_command, _("\
Change the base address of section SECTION of the exec file to ADDR.\n\
@@ -1110,5 +1110,6 @@ will be loaded as well."),
show_exec_file_mismatch_command,
&setlist, &showlist);
- add_target (exec_target_info, exec_target_open, filename_completer);
+ c = add_target (exec_target_info, exec_target_open);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
}
diff --git a/gdb/guile/scm-cmd.c b/gdb/guile/scm-cmd.c
index 2a5507686b0..8255529a2fe 100644
--- a/gdb/guile/scm-cmd.c
+++ b/gdb/guile/scm-cmd.c
@@ -110,7 +110,7 @@ struct cmdscm_completer
static const struct cmdscm_completer cmdscm_completers[] =
{
{ "COMPLETE_NONE", noop_completer },
- { "COMPLETE_FILENAME", filename_completer },
+ { "COMPLETE_FILENAME", filename_maybe_quoted_completer },
{ "COMPLETE_LOCATION", location_completer },
{ "COMPLETE_COMMAND", command_completer },
{ "COMPLETE_SYMBOL", symbol_completer },
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 69db665da75..c769603873a 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -3113,7 +3113,8 @@ Follow this command with any number of args, to be passed to the program."),
get_args_value,
show_args_command,
&setlist, &showlist);
- set_cmd_completer (args_set_show.set, filename_completer);
+ set_cmd_completer_handle_brkchars (args_set_show.set,
+ filename_completer_handle_brkchars);
auto cwd_set_show
= add_setshow_string_noescape_cmd ("cwd", class_run, _("\
@@ -3129,7 +3130,8 @@ working directory."),
set_cwd_value, get_inferior_cwd,
show_cwd_command,
&setlist, &showlist);
- set_cmd_completer (cwd_set_show.set, filename_completer);
+ set_cmd_completer_handle_brkchars (cwd_set_show.set,
+ filename_completer_handle_brkchars);
c = add_cmd ("environment", no_class, environment_info, _("\
The environment to give the program, or one variable's value.\n\
@@ -3163,7 +3165,7 @@ This path is equivalent to the $PATH shell variable. It is a list of\n\
directories, separated by colons. These directories are searched to find\n\
fully linked executable files and separately compiled object files as \
needed."));
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
c = add_cmd ("paths", no_class, path_info, _("\
Current search path for finding object files.\n\
@@ -3313,18 +3315,19 @@ Specifying -a and an ignore count simultaneously is an error."));
= add_com ("run", class_run, run_command, _("\
Start debugged program.\n"
RUN_ARGS_HELP));
- set_cmd_completer (run_cmd, filename_completer);
+ set_cmd_completer_handle_brkchars (run_cmd,
+ filename_completer_handle_brkchars);
add_com_alias ("r", run_cmd, class_run, 1);
c = add_com ("start", class_run, start_command, _("\
Start the debugged program stopping at the beginning of the main procedure.\n"
RUN_ARGS_HELP));
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
c = add_com ("starti", class_run, starti_command, _("\
Start the debugged program stopping at the first instruction.\n"
RUN_ARGS_HELP));
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
add_com ("interrupt", class_run, interrupt_command,
_("Interrupt the execution of the debugged program.\n\
diff --git a/gdb/inferior.c b/gdb/inferior.c
index 0522cb5c14d..b46b638c361 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -1112,7 +1112,7 @@ as main program.\n\
By default, the new inferior inherits the current inferior's connection.\n\
If -no-connection is specified, the new inferior begins with\n\
no target connection yet."));
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
add_com ("remove-inferiors", no_class, remove_inferior_command, _("\
Remove inferior ID (or list of IDs).\n\
diff --git a/gdb/jit.c b/gdb/jit.c
index 797be95a8da..3179f3aef19 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -1328,7 +1328,7 @@ Usage: jit-reader-load FILE\n\
Try to load file FILE as a debug info reader (and unwinder) for\n\
JIT compiled code. The file is loaded from " JIT_READER_DIR ",\n\
relocated relative to the GDB executable if required."));
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
c = add_com ("jit-reader-unload", no_class,
jit_reader_unload_command, _("\
diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
index f83b45dd210..4deeb35eba0 100644
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -39,7 +39,7 @@ struct cmdpy_completer
static const struct cmdpy_completer completers[] =
{
{ "COMPLETE_NONE", noop_completer },
- { "COMPLETE_FILENAME", filename_completer },
+ { "COMPLETE_FILENAME", filename_maybe_quoted_completer },
{ "COMPLETE_LOCATION", location_completer },
{ "COMPLETE_COMMAND", command_completer },
{ "COMPLETE_SYMBOL", symbol_completer },
diff --git a/gdb/record-full.c b/gdb/record-full.c
index eb62d186fa5..122f4b97735 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -2891,12 +2891,13 @@ _initialize_record_full ()
_("Restore the execution log from a file.\n\
Argument is filename. File must be created with 'record save'."),
&record_full_cmdlist);
- set_cmd_completer (record_full_restore_cmd, filename_completer);
+ set_cmd_completer_handle_brkchars (record_full_restore_cmd,
+ filename_completer_handle_brkchars);
/* Deprecate the old version without "full" prefix. */
c = add_alias_cmd ("restore", record_full_restore_cmd, class_obscure, 1,
&record_cmdlist);
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
deprecate_cmd (c, "record full restore");
add_setshow_prefix_cmd ("full", class_support,
diff --git a/gdb/record.c b/gdb/record.c
index b25445713fd..8c3cd7c1e4a 100644
--- a/gdb/record.c
+++ b/gdb/record.c
@@ -821,7 +821,7 @@ A size of \"unlimited\" means unlimited lines. The default is 10."),
Usage: record save [FILENAME]\n\
Default filename is 'gdb_record.PROCESS_ID'."),
&record_cmdlist);
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
cmd_list_element *record_delete_cmd
= add_cmd ("delete", class_obscure, cmd_record_delete,
diff --git a/gdb/skip.c b/gdb/skip.c
index 72f4efd0e29..c9b312195af 100644
--- a/gdb/skip.c
+++ b/gdb/skip.c
@@ -684,7 +684,7 @@ Ignore a file while stepping.\n\
Usage: skip file [FILE-NAME]\n\
If no filename is given, ignore the current file."),
&skiplist);
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
c = add_cmd ("function", class_breakpoint, skip_function_command, _("\
Ignore a function while stepping.\n\
diff --git a/gdb/source.c b/gdb/source.c
index 24a8769da91..77016293d63 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1916,7 +1916,8 @@ directory in which the source file was compiled into object code.\n\
With no argument, reset the search path to $cdir:$cwd, the default."),
&cmdlist);
- set_cmd_completer (directory_cmd, filename_completer);
+ set_cmd_completer_handle_brkchars (directory_cmd,
+ filename_completer_handle_brkchars);
add_setshow_optional_filename_cmd ("directories",
class_files,
diff --git a/gdb/symfile.c b/gdb/symfile.c
index f7f5be5a39a..7d89bda8759 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -3851,7 +3851,7 @@ Usage: symbol-file [-readnow | -readnever] [-o OFF] FILE\n\
OFF is an optional offset which is added to each section address.\n\
The `file' command can also load symbol tables, as well as setting the file\n\
to execute.\n" READNOW_READNEVER_HELP), &cmdlist);
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer (c, filename_maybe_quoted_completer);
c = add_cmd ("add-symbol-file", class_files, add_symbol_file_command, _("\
Load symbols from FILE, assuming FILE has been dynamically loaded.\n\
@@ -3865,7 +3865,7 @@ OFF is an optional offset which is added to the default load addresses\n\
of all sections for which no other address was specified.\n"
READNOW_READNEVER_HELP),
&cmdlist);
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer (c, filename_maybe_quoted_completer);
c = add_cmd ("remove-symbol-file", class_files,
remove_symbol_file_command, _("\
@@ -3875,6 +3875,7 @@ Usage: remove-symbol-file FILENAME\n\
The file to remove can be identified by its filename or by an address\n\
that lies within the boundaries of this symbol file in memory."),
&cmdlist);
+ set_cmd_completer (c, filename_maybe_quoted_completer);
c = add_cmd ("load", class_files, load_command, _("\
Dynamically load FILE into the running program.\n\
@@ -3883,7 +3884,7 @@ Usage: load [FILE] [OFFSET]\n\
An optional load OFFSET may also be given as a literal address.\n\
When OFFSET is provided, FILE must also be provided. FILE can be provided\n\
on its own."), &cmdlist);
- set_cmd_completer (c, filename_completer);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
cmd_list_element *overlay_cmd
= add_basic_prefix_cmd ("overlay", class_support,
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 4f210441623..6e7925b01b4 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -1762,8 +1762,7 @@ maint_print_c_tdesc_cmd_completer (struct cmd_list_element *ignore,
(tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_ERROR, grp))
return;
- word = advance_to_filename_complete_word_point (tracker, text);
- filename_completer (ignore, tracker, text, word);
+ filename_completer_handle_brkchars (ignore, tracker, text, word);
}
/* Implement the maintenance print xml-tdesc command. */
@@ -1947,7 +1946,7 @@ that feature within an already existing target_desc object."), grp);
cmd = add_cmd ("xml-tdesc", class_maintenance, maint_print_xml_tdesc_cmd, _("\
Print the current target description as an XML file."),
&maintenanceprintlist);
- set_cmd_completer (cmd, filename_completer);
+ set_cmd_completer_handle_brkchars (cmd, filename_completer_handle_brkchars);
cmd = add_cmd ("xml-descriptions", class_maintenance,
maintenance_check_xml_descriptions, _("\
@@ -1956,5 +1955,5 @@ Check the target descriptions created in GDB equal the descriptions\n\
created from XML files in the directory.\n\
The parameter is the directory name."),
&maintenancechecklist);
- set_cmd_completer (cmd, filename_completer);
+ set_cmd_completer_handle_brkchars (cmd, filename_completer_handle_brkchars);
}
diff --git a/gdb/target.c b/gdb/target.c
index 276e215945e..58c1b8e7971 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -841,7 +841,7 @@ open_target (const char *args, int from_tty, struct cmd_list_element *command)
/* See target.h. */
-void
+struct cmd_list_element *
add_target (const target_info &t, target_open_ftype *func,
completer_ftype *completer)
{
@@ -865,6 +865,8 @@ information on the arguments for a particular protocol, type\n\
c->func = open_target;
if (completer != NULL)
set_cmd_completer (c, completer);
+
+ return c;
}
/* See target.h. */
diff --git a/gdb/target.h b/gdb/target.h
index 81de4a678c3..5aa1f455aea 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -2403,11 +2403,12 @@ typedef void target_open_ftype (const char *args, int from_tty);
/* Add the target described by INFO to the list of possible targets
and add a new command 'target $(INFO->shortname)'. Set COMPLETER
- as the command's completer if not NULL. */
+ as the command's completer if not NULL. Return the new target
+ command. */
-extern void add_target (const target_info &info,
- target_open_ftype *func,
- completer_ftype *completer = NULL);
+extern struct cmd_list_element *add_target
+ (const target_info &info, target_open_ftype *func,
+ completer_ftype *completer = NULL);
/* Adds a command ALIAS for the target described by INFO and marks it
deprecated. This is useful for maintaining backwards compatibility
diff --git a/gdb/testsuite/gdb.base/filename-completion.exp b/gdb/testsuite/gdb.base/filename-completion.exp
index b700977cec5..1ccaaff9afc 100644
--- a/gdb/testsuite/gdb.base/filename-completion.exp
+++ b/gdb/testsuite/gdb.base/filename-completion.exp
@@ -23,8 +23,16 @@ load_lib completion-support.exp
#
# root/ [ DIRECTORY ]
# aaa/ [ DIRECTORY ]
+# aa bb [ FILE ]
+# aa cc [ FILE ]
+# aaa/ [ DIRECTORY ]
# bb1/ [ DIRECTORY ]
# bb2/ [ DIRECTORY ]
+# dir 1/ [ DIRECTORY ]
+# unique file [ FILE ]
+# dir 2/ [ DIRECTORY ]
+# file 1 [ FILE ]
+# file 2 [ FILE ]
# cc1/ [ DIRECTORY ]
# cc2 [ FILE ]
proc setup_directory_tree {} {
@@ -36,68 +44,139 @@ proc setup_directory_tree {} {
remote_exec host "mkdir -p ${root}/bb2"
remote_exec host "mkdir -p ${root}/cc1"
remote_exec host "touch ${root}/cc2"
-
remote_exec host "touch \"${root}/aaa/aa bb\""
remote_exec host "touch \"${root}/aaa/aa cc\""
+ remote_exec host "mkdir -p \"${root}/bb2/dir 1\""
+ remote_exec host "mkdir -p \"${root}/bb2/dir 2\""
+ remote_exec host "touch \"${root}/bb2/dir 1/unique file\""
+ remote_exec host "touch \"${root}/bb2/dir 2/file 1\""
+ remote_exec host "touch \"${root}/bb2/dir 2/file 2\""
return $root
}
-# Run filename completetion tests. ROOT is the base directory as
-# returned from setup_directory_tree, though, if ROOT is a
-# sub-directory of the user's home directory ROOT might have been
-# modified to replace the $HOME prefix with a single "~" character.
-proc run_tests { root } {
-
- # Completing 'thread apply all ...' commands uses a custom word
- # point. At one point we had a bug where doing this would break
- # completion of quoted filenames that contained white space.
- test_gdb_complete_unique "thread apply all hel" \
- "thread apply all help" " " false \
- "complete a 'thread apply all' command"
-
- foreach_with_prefix qc [list "" "'" "\""] {
- test_gdb_complete_none "file ${qc}${root}/xx" \
+# Run filename completetion tests for those command that accept quoting and
+# escaping of the filename argument.
+#
+# ROOT is the base directory as returned from setup_directory_tree, though,
+# if ROOT is a sub-directory of the user's home directory ROOT might have
+# been modified to replace the $HOME prefix with a single "~" character.
+proc run_quoting_and_escaping_tests { root } {
+ # Test all the commands which allow quoting of filenames, and
+ # which require whitespace to be escaped in unquoted filenames.
+ foreach_with_prefix cmd { file exec-file symbol-file add-symbol-file \
+ remove-symbol-file } {
+ gdb_start
+
+ # Completing 'thread apply all ...' commands uses a custom word
+ # point. At one point we had a bug where doing this would break
+ # completion of quoted filenames that contained white space.
+ test_gdb_complete_unique "thread apply all hel" \
+ "thread apply all help" " " false \
+ "complete a 'thread apply all' command"
+
+ foreach_with_prefix qc [list "" "'" "\""] {
+ test_gdb_complete_none "$cmd ${qc}${root}/xx" \
+ "expand a non-existent filename"
+
+ test_gdb_complete_unique "$cmd ${qc}${root}/a" \
+ "$cmd ${qc}${root}/aaa/" "" false \
+ "expand a unique filename"
+
+ test_gdb_complete_multiple "$cmd ${qc}${root}/" \
+ "b" "b" {
+ "bb1/"
+ "bb2/"
+ } "" "${qc}" false \
+ "expand multiple directory names"
+
+ test_gdb_complete_multiple "$cmd ${qc}${root}/" \
+ "c" "c" {
+ "cc1/"
+ "cc2"
+ } "" "${qc}" false \
+ "expand mixed directory and file names"
+
+ # GDB does not currently escape word break characters
+ # (e.g. white space) correctly in unquoted filenames.
+ if { $qc ne "" } {
+ set sp " "
+
+ test_gdb_complete_multiple "$cmd ${qc}${root}/aaa/" \
+ "a" "a${sp}" {
+ "aa bb"
+ "aa cc"
+ } "" "${qc}" false \
+ "expand filenames containing spaces"
+ }
+ }
+
+ gdb_exit
+ }
+}
+
+# Run filename completetion tests for a sample of commands that take an
+# unquoted, unescaped filename as an argument. Only a sample of commands
+# are (currently) tested as there's a lot of commands that accept this style
+# of filename argument.
+#
+# ROOT is the base directory as returned from setup_directory_tree, though,
+# if ROOT is a sub-directory of the user's home directory ROOT might have
+# been modified to replace the $HOME prefix with a single "~" character.
+proc run_unquoted_tests { root } {
+ # Test all the commands which allow quoting of filenames, and
+ # which require whitespace to be escaped in unquoted filenames.
+ foreach_with_prefix cmd { "set logging file" "target core" \
+ "add-auto-load-safe-path" } {
+ gdb_start
+
+ test_gdb_complete_none "$cmd ${root}/xx" \
"expand a non-existent filename"
- test_gdb_complete_unique "file ${qc}${root}/a" \
- "file ${qc}${root}/aaa/" "" false \
+ test_gdb_complete_unique "$cmd ${root}/a" \
+ "$cmd ${root}/aaa/" "" false \
"expand a unique filename"
- test_gdb_complete_multiple "file ${qc}${root}/" \
+ test_gdb_complete_unique "$cmd ${root}/bb2/dir 1/uni" \
+ "$cmd ${root}/bb2/dir 1/unique file" " " false \
+ "expand a unique filename containing whitespace"
+
+ test_gdb_complete_multiple "$cmd ${root}/" \
"b" "b" {
"bb1/"
"bb2/"
- } "" "${qc}" false \
+ } "" "" false \
"expand multiple directory names"
- test_gdb_complete_multiple "file ${qc}${root}/" \
+ test_gdb_complete_multiple "$cmd ${root}/" \
"c" "c" {
"cc1/"
"cc2"
- } "" "${qc}" false \
+ } "" "" false \
"expand mixed directory and file names"
- # GDB does not currently escape word break characters
- # (e.g. white space) correctly in unquoted filenames.
- if { $qc ne "" } {
- set sp " "
-
- test_gdb_complete_multiple "file ${qc}${root}/aaa/" \
- "a" "a${sp}" {
- "aa bb"
- "aa cc"
- } "" "${qc}" false \
- "expand filenames containing spaces"
- }
+ test_gdb_complete_multiple "$cmd ${root}/aaa/" \
+ "a" "a " {
+ "aa bb"
+ "aa cc"
+ } "" "" false \
+ "expand filenames containing spaces"
+
+ test_gdb_complete_multiple "$cmd ${root}/bb2/dir 2/" \
+ "fi" "le " {
+ "file 1"
+ "file 2"
+ } "" "" false \
+ "expand filenames containing spaces in path"
+
+ gdb_exit
}
}
-gdb_start
-
set root [setup_directory_tree]
-run_tests $root
+run_quoting_and_escaping_tests $root
+run_unquoted_tests $root
# This test relies on using the $HOME directory. We could make this
# work for remote hosts, but right now, this isn't supported.
@@ -114,7 +193,8 @@ if {![is_remote host]} {
with_test_prefix "with tilde" {
# And rerun the tests.
- run_tests $tilde_root
+ run_quoting_and_escaping_tests $tilde_root
+ run_unquoted_tests $tilde_root
}
}
}
diff --git a/gdb/tracectf.c b/gdb/tracectf.c
index 282a8250ac1..94b8a283ac6 100644
--- a/gdb/tracectf.c
+++ b/gdb/tracectf.c
@@ -1721,6 +1721,7 @@ void
_initialize_ctf ()
{
#if HAVE_LIBBABELTRACE
- add_target (ctf_target_info, ctf_target_open, filename_completer);
+ struct cmd_list_element *c = add_target (ctf_target_info, ctf_target_open);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
#endif
}
diff --git a/gdb/tracefile-tfile.c b/gdb/tracefile-tfile.c
index eb879c17970..5a1aa0bc9a1 100644
--- a/gdb/tracefile-tfile.c
+++ b/gdb/tracefile-tfile.c
@@ -1120,5 +1120,7 @@ void _initialize_tracefile_tfile ();
void
_initialize_tracefile_tfile ()
{
- add_target (tfile_target_info, tfile_target_open, filename_completer);
+ struct cmd_list_element *c =
+ add_target (tfile_target_info, tfile_target_open);
+ set_cmd_completer_handle_brkchars (c, filename_completer_handle_brkchars);
}
--
2.25.4
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCHv3 1/7] gdb: split apart two different types of filename completion
2024-06-05 13:36 ` [PATCHv3 1/7] gdb: split apart two different types of filename completion Andrew Burgess
@ 2024-06-06 16:18 ` Tom Tromey
0 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2024-06-06 16:18 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
Andrew> Unfortunately we have two different types of filename completion in
Andrew> GDB.
I am coming into this thread pretty late, so sorry if this has already
been discussed -- but did you consider changing gdb to have a single way
to pass a filename to a command?
It seems to me that a change like this in a major release would be fine.
And, it would only really affect the minority of users who use weird
filenames.
I suspect the current unquoted filename cases can't really handle
everything correctly. For instance, IIRC the gdb command processor
removes trailing whitespace -- so files like that are impossible to give
to some commands.
Tom
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCHv3 2/7] gdb: improve escaping when completing filenames
2024-06-05 13:36 ` [PATCHv3 0/7] Further filename completion improvements Andrew Burgess
2024-06-05 13:36 ` [PATCHv3 1/7] gdb: split apart two different types of filename completion Andrew Burgess
@ 2024-06-05 13:36 ` Andrew Burgess
2024-06-05 13:36 ` [PATCHv3 3/7] gdb: move display of completion results into completion_result class Andrew Burgess
` (5 subsequent siblings)
7 siblings, 0 replies; 50+ messages in thread
From: Andrew Burgess @ 2024-06-05 13:36 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess
This improves quoting and escaping when completing filenames for
commands that allow filenames to be quoted and escaped.
I've struggled a bit trying to split this series into chunks. There's
a lot of dependencies between different parts of the completion
system, and trying to get this working correctly is pretty messy.
This first step is really about implementing 3 readline hooks:
rl_char_is_quoted_p
- Is a particular character quoted within readline's input buffer?
rl_filename_dequoting_function
- Remove quoting characters from a filename.
rl_filename_quoting_function
- Add quoting characters to a filename.
See 'info readline' for full details, but with these hooks connected
up, readline (on behalf of GDB) should do a better job inserting
backslash escapes when completing filenames.
There's still a bunch of stuff that doesn't work after this commit,
mostly around the 'complete' command which of course doesn't go
through readline, so doesn't benefit from all of these new functions
yet, I'll add some of this in a later commit.
Tab completion is now slightly improved though, it is possible to
tab-complete a filename that includes a double or single quote, either
in an unquoted string or within a string surrounded by single or
double quotes, backslash escaping is used when necessary.
There are some additional tests to cover the new functionality.
---
gdb/completer.c | 169 +++++++++++++++++-
.../gdb.base/filename-completion.exp | 36 +++-
2 files changed, 201 insertions(+), 4 deletions(-)
diff --git a/gdb/completer.c b/gdb/completer.c
index 640dfc29a8d..74b3ec6ca14 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -203,6 +203,159 @@ noop_completer (struct cmd_list_element *ignore,
{
}
+/* Return 1 if the character at EINDEX in STRING is quoted (there is an
+ unclosed quoted string), or if the character at EINDEX is quoted by a
+ backslash. */
+
+static int
+gdb_completer_file_name_char_is_quoted (char *string, int eindex)
+{
+ for (int i = 0; i <= eindex && string[i] != '\0'; )
+ {
+ char c = string[i];
+
+ if (c == '\\')
+ {
+ /* The backslash itself is not quoted. */
+ if (i >= eindex)
+ return 0;
+ ++i;
+ /* But the next character is. */
+ if (i >= eindex)
+ return 1;
+ if (string[i] == '\0')
+ return 0;
+ ++i;
+ continue;
+ }
+ else if (strchr (rl_completer_quote_characters, c) != nullptr)
+ {
+ /* This assumes that extract_string_maybe_quoted can handle a
+ string quoted with character C. Currently this is true as the
+ only characters we put in rl_completer_quote_characters are
+ single and/or double quotes, both of which
+ extract_string_maybe_quoted can handle. */
+ gdb_assert (c == '"' || c == '\'');
+ const char *tmp = &string[i];
+ (void) extract_string_maybe_quoted (&tmp);
+ i = tmp - string;
+
+ /* Consider any character within the string we just skipped over
+ as quoted, though this might not be completely correct; the
+ opening and closing quotes are not themselves quoted. But so
+ far this doesn't seem to have caused any issues. */
+ if (i >= eindex)
+ return 1;
+ }
+ else
+ ++i;
+ }
+
+ return 0;
+}
+
+/* Removing character escaping from FILENAME. QUOTE_CHAR is the quote
+ character around FILENAME or the null-character if there is no quoting
+ around FILENAME. */
+
+static char *
+gdb_completer_file_name_dequote (char *filename, int quote_char)
+{
+ std::string tmp;
+
+ if (quote_char == '\'')
+ {
+ /* There is no backslash escaping within a single quoted string. In
+ this case we can just return the input string. */
+ tmp = filename;
+ }
+ else if (quote_char == '"')
+ {
+ /* Remove escaping from a double quoted string. */
+ for (const char *input = filename;
+ *input != '\0';
+ ++input)
+ {
+ if (input[0] == '\\'
+ && input[1] != '\0'
+ && strchr ("\"\\", input[1]) != nullptr)
+ ++input;
+ tmp += *input;
+ }
+ }
+ else
+ {
+ gdb_assert (quote_char == '\0');
+
+ /* Remove escaping from an unquoted string. */
+ for (const char *input = filename;
+ *input != '\0';
+ ++input)
+ {
+ /* We allow anything to be escaped in an unquoted string. */
+ if (*input == '\\')
+ {
+ ++input;
+ if (*input == '\0')
+ break;
+ }
+
+ tmp += *input;
+ }
+ }
+
+ return strdup (tmp.c_str ());
+}
+
+/* Apply character escaping to the file name in TEXT. QUOTE_PTR points to
+ the quote character surrounding TEXT, or points to the null-character if
+ there are no quotes around TEXT. MATCH_TYPE will be one of the readline
+ constants SINGLE_MATCH or MULTI_MATCH depending on if there is one or
+ many completions. */
+
+static char *
+gdb_completer_file_name_quote (char *text, int match_type ATTRIBUTE_UNUSED,
+ char *quote_ptr)
+{
+ std::string str;
+
+ if (*quote_ptr == '\'')
+ {
+ /* There is no backslash escaping permitted within a single quoted
+ string, so in this case we can just return the input sting. */
+ str = text;
+ }
+ else if (*quote_ptr == '"')
+ {
+ /* Add escaping for a double quoted filename. */
+ for (const char *input = text;
+ *input != '\0';
+ ++input)
+ {
+ if (strchr ("\"\\", *input) != nullptr)
+ str += '\\';
+ str += *input;
+ }
+ }
+ else
+ {
+ gdb_assert (*quote_ptr == '\0');
+
+ /* Add escaping for an unquoted filename. */
+ for (const char *input = text;
+ *input != '\0';
+ ++input)
+ {
+ if (strchr (" \t\n\\\"'", *input)
+ != nullptr)
+ str += '\\';
+ str += *input;
+ }
+ }
+
+ return strdup (str.c_str ());
+}
+
/* Generate filename completions of WORD, storing the completions into
TRACKER. This is used for generating completions for commands that
only accept unquoted filenames as well as for commands that accept
@@ -256,6 +409,7 @@ filename_maybe_quoted_completer (struct cmd_list_element *ignore,
completion_tracker &tracker,
const char *text, const char *word)
{
+ rl_char_is_quoted_p = gdb_completer_file_name_char_is_quoted;
rl_completer_quote_characters = gdb_completer_file_name_quote_characters;
filename_completer_generate_completions (tracker, word);
}
@@ -271,6 +425,7 @@ filename_maybe_quoted_completer_handle_brkchars
(gdb_completer_file_name_break_characters);
rl_completer_quote_characters = gdb_completer_file_name_quote_characters;
+ rl_char_is_quoted_p = gdb_completer_file_name_char_is_quoted;
}
/* See completer.h. */
@@ -1271,6 +1426,7 @@ complete_line_internal_1 (completion_tracker &tracker,
completing file names then we can switch to the file name quote
character set (i.e., both single- and double-quotes). */
rl_completer_quote_characters = gdb_completer_expression_quote_characters;
+ rl_char_is_quoted_p = nullptr;
/* Decide whether to complete on a list of gdb commands or on
symbols. */
@@ -2163,9 +2319,11 @@ completion_tracker::build_completion_result (const char *text,
/* Build replacement word, based on the LCD. */
recompute_lowest_common_denominator ();
- match_list[0]
- = expand_preserving_ws (text, end - start,
- m_lowest_common_denominator);
+ if (rl_filename_completion_desired)
+ match_list[0] = xstrdup (m_lowest_common_denominator);
+ else
+ match_list[0]
+ = expand_preserving_ws (text, end - start, m_lowest_common_denominator);
if (m_lowest_common_denominator_unique)
{
@@ -3028,6 +3186,11 @@ _initialize_completer ()
rl_attempted_completion_function = gdb_rl_attempted_completion_function;
set_rl_completer_word_break_characters (default_word_break_characters ());
+ /* Setup readline globals relating to filename completion. */
+ rl_filename_quote_characters = " \t\n\\\"'";
+ rl_filename_dequoting_function = gdb_completer_file_name_dequote;
+ rl_filename_quoting_function = gdb_completer_file_name_quote;
+
add_setshow_zuinteger_unlimited_cmd ("max-completions", no_class,
&max_completions, _("\
Set maximum number of completion candidates."), _("\
diff --git a/gdb/testsuite/gdb.base/filename-completion.exp b/gdb/testsuite/gdb.base/filename-completion.exp
index 1ccaaff9afc..275140ffd9d 100644
--- a/gdb/testsuite/gdb.base/filename-completion.exp
+++ b/gdb/testsuite/gdb.base/filename-completion.exp
@@ -52,6 +52,9 @@ proc setup_directory_tree {} {
remote_exec host "touch \"${root}/bb2/dir 2/file 1\""
remote_exec host "touch \"${root}/bb2/dir 2/file 2\""
+ remote_exec host "touch \"${root}/bb1/aa\\\"bb\""
+ remote_exec host "touch \"${root}/bb1/aa'bb\""
+
return $root
}
@@ -102,12 +105,43 @@ proc run_quoting_and_escaping_tests { root } {
if { $qc ne "" } {
set sp " "
- test_gdb_complete_multiple "$cmd ${qc}${root}/aaa/" \
+ test_gdb_complete_multiple "file ${qc}${root}/aaa/" \
"a" "a${sp}" {
"aa bb"
"aa cc"
} "" "${qc}" false \
"expand filenames containing spaces"
+
+ test_gdb_complete_multiple "file ${qc}${root}/bb1/" \
+ "a" "a" {
+ "aa\"bb"
+ "aa'bb"
+ } "" "${qc}" false \
+ "expand filenames containing quotes"
+ } else {
+ set sp "\\ "
+
+ test_gdb_complete_tab_multiple "file ${qc}${root}/aaa/a" \
+ "a${sp}" {
+ "aa bb"
+ "aa cc"
+ } false \
+ "expand filenames containing spaces"
+
+ test_gdb_complete_tab_multiple "file ${qc}${root}/bb1/a" \
+ "a" {
+ "aa\"bb"
+ "aa'bb"
+ } false \
+ "expand filenames containing quotes"
+
+ test_gdb_complete_tab_unique "file ${qc}${root}/bb1/aa\\\"" \
+ "file ${qc}${root}/bb1/aa\\\\\"bb${qc}" " " \
+ "expand unique filename containing double quotes"
+
+ test_gdb_complete_tab_unique "file ${qc}${root}/bb1/aa\\'" \
+ "file ${qc}${root}/bb1/aa\\\\'bb${qc}" " " \
+ "expand unique filename containing single quote"
}
}
--
2.25.4
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCHv3 3/7] gdb: move display of completion results into completion_result class
2024-06-05 13:36 ` [PATCHv3 0/7] Further filename completion improvements Andrew Burgess
2024-06-05 13:36 ` [PATCHv3 1/7] gdb: split apart two different types of filename completion Andrew Burgess
2024-06-05 13:36 ` [PATCHv3 2/7] gdb: improve escaping when completing filenames Andrew Burgess
@ 2024-06-05 13:36 ` Andrew Burgess
2024-06-06 16:19 ` Tom Tromey
2024-06-05 13:36 ` [PATCHv3 4/7] gdb: simplify completion_result::print_matches Andrew Burgess
` (4 subsequent siblings)
7 siblings, 1 reply; 50+ messages in thread
From: Andrew Burgess @ 2024-06-05 13:36 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess
This commit moves the printing of the 'complete' command results out
of the 'complete_command' function. The printing is now done in a new
member function 'completion_result::print_matches'. At this point,
this is entirely a refactor.
The motivation for this refactor is how 'complete' should print the
completion of filename arguments. In some cases the filename results
need to have escaping added to the output. This escaping needs to be
done immediately prior to printing the result as adding too early will
result in multiple 'complete' results potentially being sorted
incorrectly. See the subsequent commits for more details.
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 4c2a443246d..1a35a6df484 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -423,31 +423,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 74b3ec6ca14..dcff9edbea0 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -2459,6 +2459,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 fa21156bd1f..200d8a9b3af 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
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCHv3 3/7] gdb: move display of completion results into completion_result class
2024-06-05 13:36 ` [PATCHv3 3/7] gdb: move display of completion results into completion_result class Andrew Burgess
@ 2024-06-06 16:19 ` Tom Tromey
0 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2024-06-06 16:19 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
Andrew> This commit moves the printing of the 'complete' command results out
Andrew> of the 'complete_command' function. The printing is now done in a new
Andrew> member function 'completion_result::print_matches'. At this point,
Andrew> this is entirely a refactor.
Andrew> The motivation for this refactor is how 'complete' should print the
Andrew> completion of filename arguments. In some cases the filename results
Andrew> need to have escaping added to the output. This escaping needs to be
Andrew> done immediately prior to printing the result as adding too early will
Andrew> result in multiple 'complete' results potentially being sorted
Andrew> incorrectly. See the subsequent commits for more details.
Andrew> There should be no user visible changes after this commit.
IMO it would be fine to push this one separately if you were so inclined.
Approved-By: Tom Tromey <tom@tromey.com>
Tom
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCHv3 4/7] gdb: simplify completion_result::print_matches
2024-06-05 13:36 ` [PATCHv3 0/7] Further filename completion improvements Andrew Burgess
` (2 preceding siblings ...)
2024-06-05 13:36 ` [PATCHv3 3/7] gdb: move display of completion results into completion_result class Andrew Burgess
@ 2024-06-05 13:36 ` Andrew Burgess
2024-06-05 13:36 ` [PATCHv3 5/7] gdb: add match formatter mechanism for 'complete' command output Andrew Burgess
` (3 subsequent siblings)
7 siblings, 0 replies; 50+ messages in thread
From: Andrew Burgess @ 2024-06-05 13:36 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess
Simplify completion_result::print_matches by removing one of the code
paths. Now, every time we call ::print_matches we always add the
trailing quote.
Previously, when using the 'complete' command, if there was only one
result then trailing quote was added in ::build_completion_result, but
when we had multiple results the trailing quote was added in
::print_matches. As a consequence, ::print_matches had to understand
not to add the trailing quote for the single result case.
After this commit we don't add the trailing quote in
::build_completion_result, instead ::print_matches always adds the
trailing quote, which makes ::print_matches simpler.
However, there is a slight problem. When completion is being driven
by readline, and not by the 'complete' command, we still need to
manually add the trailing quote in the single result case, and as the
printing is done by readline we can't add the quote at the time of
printing, and so, in ::build_completion_result, we still add the
trailing quote, but only when completion is being done for readline.
And this does cause a small problem. When completing a filename, if
the completion results in a directory name then, when using the
'complete' command, GDB should not be adding a trailing quote. For
example, if we have the file /tmp/xxx/foo.c, then what we should see
is this:
(gdb) complete file '/tmp/xx
file 'tmp/xxx/
But what we actually see after this commit is this:
(gdb) complete file '/tmp/xx
file 'tmp/xxx/'
Previously we didn't get the trailing quote in this case, as when
there is only a single result, the quote was added in
::build_completion_result, and for filename completion, GDB didn't
know what the quote character was in ::build_completion_result, so no
quote was added. Now that the trailing quote is always added in
::print_matches, and GDB does know the quote character at this point,
so we are now getting the trailing quote, which is not correct.
This is a regression, but really, GDB is now broken in a consistent
way, if we create the file /tmp/xxa/bar.c, then previously if we did
this:
(gdb) complete file '/tmp/xx
file '/tmp/xxa/'
file '/tmp/xxx/'
Notice how we get the trailing quote in this case, this is the before
patch behaviour, and is also wrong.
A later commit will fix things so that the trailing quote is not added
in this filename completion case, but for now I'm going to accept this
small regression.
This change in behaviour caused some failures in one of the completion
tests, I've tweaked the test case to expect the trailing quote as part
of this commit, but will revert this in a later commit in this series.
I've also added an extra test for when the 'complete' command does
complete to a single complete filename, in which case the trailing
quote is expected.
---
gdb/completer.c | 62 +++++++++----------
.../gdb.base/filename-completion.exp | 21 ++++++-
2 files changed, 50 insertions(+), 33 deletions(-)
diff --git a/gdb/completer.c b/gdb/completer.c
index dcff9edbea0..1fa82ecad87 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -2327,23 +2327,30 @@ completion_tracker::build_completion_result (const char *text,
if (m_lowest_common_denominator_unique)
{
- /* We don't rely on readline appending the quote char as
- delimiter as then readline wouldn't append the ' ' after the
- completion. */
- char buf[2] = { (char) quote_char () };
-
- match_list[0] = reconcat (match_list[0], match_list[0],
- buf, (char *) NULL);
- match_list[1] = NULL;
-
- /* If the tracker wants to, or we already have a space at the
- end of the match, tell readline to skip appending
- another. */
- char *match = match_list[0];
- bool completion_suppress_append
- = (suppress_append_ws ()
- || (match[0] != '\0'
- && match[strlen (match) - 1] == ' '));
+ bool completion_suppress_append;
+
+ if (from_readline ())
+ {
+ /* We don't rely on readline appending the quote char as
+ delimiter as then readline wouldn't append the ' ' after the
+ completion. */
+ char buf[2] = { (char) quote_char (), '\0' };
+
+ match_list[0] = reconcat (match_list[0], match_list[0], buf,
+ (char *) nullptr);
+
+ /* If the tracker wants to, or we already have a space at the end
+ of the match, tell readline to skip appending another. */
+ char *match = match_list[0];
+ completion_suppress_append
+ = (suppress_append_ws ()
+ || (match[0] != '\0'
+ && match[strlen (match) - 1] == ' '));
+ }
+ else
+ completion_suppress_append = false;
+
+ match_list[1] = nullptr;
return completion_result (match_list, 1, completion_suppress_append);
}
@@ -2465,21 +2472,14 @@ 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 ();
+ 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");
- }
- }
+ char buf[2] = { (char) quote_char, '\0' };
+ size_t off = this->number_matches == 1 ? 0 : 1;
+
+ for (size_t i = 0; i < this->number_matches; i++)
+ printf_unfiltered ("%s%s%s\n", prefix.c_str (),
+ this->match_list[i + off], buf);
if (this->number_matches == max_completions)
{
diff --git a/gdb/testsuite/gdb.base/filename-completion.exp b/gdb/testsuite/gdb.base/filename-completion.exp
index 275140ffd9d..3e3b9b29ba4 100644
--- a/gdb/testsuite/gdb.base/filename-completion.exp
+++ b/gdb/testsuite/gdb.base/filename-completion.exp
@@ -82,8 +82,25 @@ proc run_quoting_and_escaping_tests { root } {
test_gdb_complete_none "$cmd ${qc}${root}/xx" \
"expand a non-existent filename"
- test_gdb_complete_unique "$cmd ${qc}${root}/a" \
- "$cmd ${qc}${root}/aaa/" "" false \
+ # The following test is split into separate cmd and tab calls
+ # so we can xfail the cmd version. The cmd version will add a
+ # closing quote, it shouldn't be doing this. This will be
+ # fixed in a later commit.
+ if { $qc ne "" } {
+ setup_xfail "*-*-*"
+ }
+ test_gdb_complete_cmd_unique "file ${qc}${root}/a" \
+ "file ${qc}${root}/aaa/" \
+ "expand a unique directory name"
+
+ if { [readline_is_used] } {
+ test_gdb_complete_tab_unique "file ${qc}${root}/a" \
+ "file ${qc}${root}/aaa/" "" \
+ "expand a unique directory name"
+ }
+
+ test_gdb_complete_unique "file ${qc}${root}/cc2" \
+ "file ${qc}${root}/cc2${qc}" " " false \
"expand a unique filename"
test_gdb_complete_multiple "$cmd ${qc}${root}/" \
--
2.25.4
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCHv3 5/7] gdb: add match formatter mechanism for 'complete' command output
2024-06-05 13:36 ` [PATCHv3 0/7] Further filename completion improvements Andrew Burgess
` (3 preceding siblings ...)
2024-06-05 13:36 ` [PATCHv3 4/7] gdb: simplify completion_result::print_matches Andrew Burgess
@ 2024-06-05 13:36 ` Andrew Burgess
2024-06-06 16:14 ` Tom Tromey
2024-06-05 13:36 ` [PATCHv3 6/7] gdb: apply escaping to filenames in 'complete' results Andrew Burgess
` (2 subsequent siblings)
7 siblings, 1 reply; 50+ messages in thread
From: Andrew Burgess @ 2024-06-05 13:36 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess
This commit solves a problem that existed prior to the previous
commit, but the previous commit made more common.
When completing a filename with the 'complete' command GDB will always
add a trailing quote character, even if the completion is a directory
name, in which case it would be better if the trailing quote was not
added. Consider:
(gdb) complete file '/tmp/xx
file '/tmp/xxx/'
The completion offered here is really only a partial completion, we've
completed up to the end of the next directory name, but, until we have
a filename then the completion is not finished and the trailing quote
should not be added.
This would match the readline behaviour, e.g.:
(gdb) file '/tmp/xx<TAB>
(gdb) file '/tmp/xxx/
In this case readline completes the directory name, but doesn't add
the trailing quote character.
Remember that the 'complete' command is intended for tools like
e.g. emacs in order that they can emulate GDB's standard readline
completion when implementing a CLI of their own. As such, not adding
the trailing quote in this case matches the readline behaviour, and
seems like the right way to go.
To achieve this, I've added a new function pointer member variable
completion_result::m_match_formatter. This contains a pointer to a
callback function which is used by the 'complete' command to format
each result.
The default behaviour of this callback function is to just append the
quote character (the character from before the completion string) to
the end of the completion result. This matches the current behaviour.
However, for filename completion we override the default value of
m_match_formatter, this new function checks if the completion result
is a directory or not. If the completion result is a directory then
the closing quote is not added, instead we add a trailing '/'
character.
The code to add a trailing '/' character already exists within the
filename_completer function. This is no longer needed in this
location, instead this code is moved into the formatter callback.
Tests are updated to handle the changes in functionality, this removes
an xfail added in the previous commit.
---
gdb/completer.c | 92 ++++++++++++++-----
gdb/completer.h | 45 ++++++++-
.../gdb.base/filename-completion.exp | 68 ++++++++++----
3 files changed, 156 insertions(+), 49 deletions(-)
diff --git a/gdb/completer.c b/gdb/completer.c
index 1fa82ecad87..fc33f5a742d 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -45,6 +45,8 @@
#include "completer.h"
+#include <filesystem>
+
/* Forward declarations. */
static const char *completion_find_completion_word (completion_tracker &tracker,
const char *text,
@@ -356,6 +358,28 @@ gdb_completer_file_name_quote (char *text, int match_type ATTRIBUTE_UNUSED,
return strdup (str.c_str ());
}
+/* The function is used to update the completion word MATCH before
+ displaying it to the user in the 'complete' command output. This
+ function is only used for formatting filename or directory names.
+
+ This function checks to see if the completion word MATCH is a directory,
+ in which case a trailing "/" (forward-slash) is added, otherwise
+ QUOTE_CHAR is added as a trailing quote.
+
+ Return the updated completion word as a string. */
+
+static std::string
+filename_match_formatter (const char *match, char quote_char)
+{
+ std::string result (match);
+ if (std::filesystem::is_directory (gdb_tilde_expand (match)))
+ result += "/";
+ else
+ result += quote_char;
+
+ return result;
+}
+
/* Generate filename completions of WORD, storing the completions into
TRACKER. This is used for generating completions for commands that
only accept unquoted filenames as well as for commands that accept
@@ -365,6 +389,8 @@ static void
filename_completer_generate_completions (completion_tracker &tracker,
const char *word)
{
+ tracker.set_match_format_func (filename_match_formatter);
+
int subsequent_name = 0;
while (1)
{
@@ -383,20 +409,6 @@ filename_completer_generate_completions (completion_tracker &tracker,
if (p[strlen (p) - 1] == '~')
continue;
- /* Readline appends a trailing '/' if the completion is a
- directory. If this completion request originated from outside
- readline (e.g. GDB's 'complete' command), then we append the
- trailing '/' ourselves now. */
- if (!tracker.from_readline ())
- {
- std::string expanded = gdb_tilde_expand (p_rl.get ());
- struct stat finfo;
- const bool isdir = (stat (expanded.c_str (), &finfo) == 0
- && S_ISDIR (finfo.st_mode));
- if (isdir)
- p_rl.reset (concat (p_rl.get (), "/", nullptr));
- }
-
tracker.add_completion
(make_completion_match_str (std::move (p_rl), word, word));
}
@@ -1646,10 +1658,25 @@ int max_completions = 200;
/* Initial size of the table. It automagically grows from here. */
#define INITIAL_COMPLETION_HTAB_SIZE 200
+/* The function is used to update the completion word MATCH before
+ displaying it to the user in the 'complete' command output. This
+ default function is used in all cases except those where a completion
+ function overrides this function by calling set_match_format_func.
+
+ This function returns MATCH with QUOTE_CHAR appended. If QUOTE_CHAR is
+ the null-character then the returned string will just contain MATCH. */
+
+static std::string
+default_match_formatter (const char *match, char quote_char)
+{
+ return std::string (match) + quote_char;
+}
+
/* See completer.h. */
completion_tracker::completion_tracker (bool from_readline)
- : m_from_readline (from_readline)
+ : m_from_readline (from_readline),
+ m_match_format_func (default_match_formatter)
{
discard_completions ();
}
@@ -2352,7 +2379,8 @@ completion_tracker::build_completion_result (const char *text,
match_list[1] = nullptr;
- return completion_result (match_list, 1, completion_suppress_append);
+ return completion_result (match_list, 1, completion_suppress_append,
+ m_match_format_func);
}
else
{
@@ -2389,7 +2417,8 @@ completion_tracker::build_completion_result (const char *text,
htab_traverse_noresize (m_entries_hash.get (), func, &builder);
match_list[builder.index] = NULL;
- return completion_result (match_list, builder.index - 1, false);
+ return completion_result (match_list, builder.index - 1, false,
+ m_match_format_func);
}
}
@@ -2397,18 +2426,23 @@ completion_tracker::build_completion_result (const char *text,
completion_result::completion_result ()
: match_list (NULL), number_matches (0),
- completion_suppress_append (false)
+ completion_suppress_append (false),
+ m_match_formatter (default_match_formatter)
{}
/* See completer.h */
completion_result::completion_result (char **match_list_,
size_t number_matches_,
- bool completion_suppress_append_)
+ bool completion_suppress_append_,
+ match_format_func_t match_formatter_)
: match_list (match_list_),
number_matches (number_matches_),
- completion_suppress_append (completion_suppress_append_)
-{}
+ completion_suppress_append (completion_suppress_append_),
+ m_match_formatter (match_formatter_)
+{
+ gdb_assert (m_match_formatter != nullptr);
+}
/* See completer.h */
@@ -2421,10 +2455,12 @@ completion_result::~completion_result ()
completion_result::completion_result (completion_result &&rhs) noexcept
: match_list (rhs.match_list),
- number_matches (rhs.number_matches)
+ number_matches (rhs.number_matches),
+ m_match_formatter (rhs.m_match_formatter)
{
rhs.match_list = NULL;
rhs.number_matches = 0;
+ rhs.m_match_formatter = default_match_formatter;
}
/* See completer.h */
@@ -2474,12 +2510,18 @@ completion_result::print_matches (const std::string &prefix,
{
this->sort_match_list ();
- char buf[2] = { (char) quote_char, '\0' };
size_t off = this->number_matches == 1 ? 0 : 1;
for (size_t i = 0; i < this->number_matches; i++)
- printf_unfiltered ("%s%s%s\n", prefix.c_str (),
- this->match_list[i + off], buf);
+ {
+ gdb_assert (this->m_match_formatter != nullptr);
+ std::string formatted_match
+ = this->m_match_formatter (this->match_list[i + off],
+ (char) quote_char);
+
+ printf_unfiltered ("%s%s\n", prefix.c_str (),
+ formatted_match.c_str ());
+ }
if (this->number_matches == max_completions)
{
diff --git a/gdb/completer.h b/gdb/completer.h
index 200d8a9b3af..71303e2ae9a 100644
--- a/gdb/completer.h
+++ b/gdb/completer.h
@@ -247,12 +247,24 @@ struct completion_match_result
struct completion_result
{
+ /* The type of a function that is used to format completion results when
+ using the 'complete' command. MATCH is the completion word to be
+ printed, and QUOTE_CHAR is a trailing quote character to (possibly)
+ add at the end of MATCH. QUOTE_CHAR can be the null-character in
+ which case no trailing quote should be added.
+
+ Return the possibly modified completion match word which should be
+ presented to the user. */
+ using match_format_func_t = std::string (*) (const char *match,
+ char quote_char);
+
/* Create an empty result. */
completion_result ();
/* Create a result. */
completion_result (char **match_list, size_t number_matches,
- bool completion_suppress_append);
+ bool completion_suppress_append,
+ match_format_func_t match_format_func);
/* Destroy a result. */
~completion_result ();
@@ -274,10 +286,15 @@ struct completion_result
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. */
+ The QUOTE_CHAR is usually 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.
+
+ The QUOTE_CHAR is not always appended to the completion output. For
+ example, filename completions will not append QUOTE_CHAR if the
+ completion is a directory name. This is all handled by calling this
+ function. */
void print_matches (const std::string &prefix, const char *word,
int quote_char);
@@ -305,6 +322,12 @@ struct completion_result
/* Whether readline should suppress appending a whitespace, when
there's only one possible completion. */
bool completion_suppress_append;
+
+private:
+ /* A function which formats a single completion match ready for display
+ as part of the 'complete' command output. Different completion
+ functions can set different formatter functions. */
+ match_format_func_t m_match_formatter;
};
/* Object used by completers to build a completion match list to hand
@@ -441,6 +464,14 @@ class completion_tracker
bool from_readline () const
{ return m_from_readline; }
+ /* Set the function used to format the completion word before displaying
+ it to the user to F, this is used by the 'complete' command. */
+ void set_match_format_func (completion_result::match_format_func_t f)
+ {
+ gdb_assert (f != nullptr);
+ m_match_format_func = f;
+ }
+
private:
/* The type that we place into the m_entries_hash hash table. */
@@ -535,6 +566,10 @@ class completion_tracker
interactively. The 'complete' command is a way to generate completions
not to be displayed by readline. */
bool m_from_readline;
+
+ /* The function used to format the completion word before it is printed
+ in the 'complete' command output. */
+ completion_result::match_format_func_t m_match_format_func;
};
/* Return a string to hand off to readline as a completion match
diff --git a/gdb/testsuite/gdb.base/filename-completion.exp b/gdb/testsuite/gdb.base/filename-completion.exp
index 3e3b9b29ba4..0467d5c425e 100644
--- a/gdb/testsuite/gdb.base/filename-completion.exp
+++ b/gdb/testsuite/gdb.base/filename-completion.exp
@@ -58,6 +58,49 @@ proc setup_directory_tree {} {
return $root
}
+# This proc started as a copy of test_gdb_complete_multiple, however, this
+# version does some extra work. See the original test_gdb_complete_multiple
+# for a description of all the arguments.
+#
+# When using the 'complete' command with filenames, GDB will add a trailing
+# quote for filenames, and a trailing "/" for directory names. As the
+# trailing "/" is also added in the tab-completion output the
+# COMPLETION_LIST will include the "/" character, but the trailing quote is
+# only added when using the 'complete' command.
+#
+# Pass the trailing quote will be passed as END_QUOTE_CHAR, this proc will
+# run the tab completion test, and will then add the trailing quote to those
+# entries in COMPLETION_LIST that don't have a trailing "/" before running
+# the 'complete' command test.
+proc test_gdb_complete_filename_multiple {
+ cmd_prefix completion_word add_completed_line completion_list
+ {start_quote_char ""} {end_quote_char ""} {max_completions false}
+ {testname ""}
+} {
+ if { [readline_is_used] } {
+ test_gdb_complete_tab_multiple "$cmd_prefix$completion_word" \
+ $add_completed_line $completion_list $max_completions $testname
+ }
+
+ if { $start_quote_char eq "" && $end_quote_char ne "" } {
+ set updated_completion_list {}
+
+ foreach entry $completion_list {
+ if {[string range $entry end end] ne "/"} {
+ set entry $entry$end_quote_char
+ }
+ lappend updated_completion_list $entry
+ }
+
+ set completion_list $updated_completion_list
+ set end_quote_char ""
+ }
+
+ test_gdb_complete_cmd_multiple $cmd_prefix $completion_word \
+ $completion_list $start_quote_char $end_quote_char $max_completions \
+ $testname
+}
+
# Run filename completetion tests for those command that accept quoting and
# escaping of the filename argument.
#
@@ -82,35 +125,22 @@ proc run_quoting_and_escaping_tests { root } {
test_gdb_complete_none "$cmd ${qc}${root}/xx" \
"expand a non-existent filename"
- # The following test is split into separate cmd and tab calls
- # so we can xfail the cmd version. The cmd version will add a
- # closing quote, it shouldn't be doing this. This will be
- # fixed in a later commit.
- if { $qc ne "" } {
- setup_xfail "*-*-*"
- }
- test_gdb_complete_cmd_unique "file ${qc}${root}/a" \
- "file ${qc}${root}/aaa/" \
+ test_gdb_complete_unique "file ${qc}${root}/a" \
+ "file ${qc}${root}/aaa/" "" false \
"expand a unique directory name"
- if { [readline_is_used] } {
- test_gdb_complete_tab_unique "file ${qc}${root}/a" \
- "file ${qc}${root}/aaa/" "" \
- "expand a unique directory name"
- }
-
test_gdb_complete_unique "file ${qc}${root}/cc2" \
"file ${qc}${root}/cc2${qc}" " " false \
"expand a unique filename"
- test_gdb_complete_multiple "$cmd ${qc}${root}/" \
+ test_gdb_complete_filename_multiple "$cmd ${qc}${root}/" \
"b" "b" {
"bb1/"
"bb2/"
} "" "${qc}" false \
"expand multiple directory names"
- test_gdb_complete_multiple "$cmd ${qc}${root}/" \
+ test_gdb_complete_filename_multiple "$cmd ${qc}${root}/" \
"c" "c" {
"cc1/"
"cc2"
@@ -122,14 +152,14 @@ proc run_quoting_and_escaping_tests { root } {
if { $qc ne "" } {
set sp " "
- test_gdb_complete_multiple "file ${qc}${root}/aaa/" \
+ test_gdb_complete_filename_multiple "file ${qc}${root}/aaa/" \
"a" "a${sp}" {
"aa bb"
"aa cc"
} "" "${qc}" false \
"expand filenames containing spaces"
- test_gdb_complete_multiple "file ${qc}${root}/bb1/" \
+ test_gdb_complete_filename_multiple "file ${qc}${root}/bb1/" \
"a" "a" {
"aa\"bb"
"aa'bb"
--
2.25.4
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCHv3 6/7] gdb: apply escaping to filenames in 'complete' results
2024-06-05 13:36 ` [PATCHv3 0/7] Further filename completion improvements Andrew Burgess
` (4 preceding siblings ...)
2024-06-05 13:36 ` [PATCHv3 5/7] gdb: add match formatter mechanism for 'complete' command output Andrew Burgess
@ 2024-06-05 13:36 ` Andrew Burgess
2024-06-05 13:36 ` [PATCHv3 7/7] gdb: improve gdb_rl_find_completion_word for quoted words Andrew Burgess
2024-06-06 16:24 ` [PATCHv3 0/7] Further filename completion improvements Tom Tromey
7 siblings, 0 replies; 50+ messages in thread
From: Andrew Burgess @ 2024-06-05 13:36 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess
Building on the mechanism added in the previous commit(s), this commit
applies escaping to filenames in the 'complete' command output.
Consider a file: /tmp/xxx/aa"bb -- that is a filename that contains a
double quote, currently the 'complete' command output looks like this:
(gdb) complete file /tmp/xxx/a
file /tmp/xxx/aa"bb
Notice that the double quote in the output is not escaped. If we
passed this same output back to GDB then the double quote will be
treated as the start of a string.
After this commit then the output looks like this:
(gdb) complete file /tmp/xxx/a
file /tmp/xxx/aa\"bb
The double quote is now escaped. If we feed this output back to GDB
then GDB will treat this as a single filename that contains a double
quote, exactly what we want.
To achieve this I've done a little refactoring, splitting out the core
of gdb_completer_file_name_quote, and then added a new call from the
filename_match_formatter function.
There are updates to the tests to cover this new functionality.
---
gdb/completer.c | 98 ++++++++++++++---
.../gdb.base/filename-completion.exp | 100 +++++++++++-------
2 files changed, 144 insertions(+), 54 deletions(-)
diff --git a/gdb/completer.c b/gdb/completer.c
index fc33f5a742d..d0c542413e3 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -309,25 +309,24 @@ gdb_completer_file_name_dequote (char *filename, int quote_char)
return strdup (tmp.c_str ());
}
-/* Apply character escaping to the file name in TEXT. QUOTE_PTR points to
- the quote character surrounding TEXT, or points to the null-character if
- there are no quotes around TEXT. MATCH_TYPE will be one of the readline
- constants SINGLE_MATCH or MULTI_MATCH depending on if there is one or
- many completions. */
+/* Apply character escaping to the filename in TEXT and return a newly
+ allocated buffer containing the possibly updated filename.
+
+ QUOTE_CHAR is the quote character surrounding TEXT, or the
+ null-character if there are no quotes around TEXT. */
static char *
-gdb_completer_file_name_quote (char *text, int match_type ATTRIBUTE_UNUSED,
- char *quote_ptr)
+gdb_completer_file_name_quote_1 (const char *text, char quote_char)
{
std::string str;
- if (*quote_ptr == '\'')
+ if (quote_char == '\'')
{
/* There is no backslash escaping permitted within a single quoted
string, so in this case we can just return the input sting. */
str = text;
}
- else if (*quote_ptr == '"')
+ else if (quote_char == '"')
{
/* Add escaping for a double quoted filename. */
for (const char *input = text;
@@ -341,7 +340,7 @@ gdb_completer_file_name_quote (char *text, int match_type ATTRIBUTE_UNUSED,
}
else
{
- gdb_assert (*quote_ptr == '\0');
+ gdb_assert (quote_char == '\0');
/* Add escaping for an unquoted filename. */
for (const char *input = text;
@@ -358,6 +357,19 @@ gdb_completer_file_name_quote (char *text, int match_type ATTRIBUTE_UNUSED,
return strdup (str.c_str ());
}
+/* Apply character escaping to the filename in TEXT. QUOTE_PTR points to
+ the quote character surrounding TEXT, or points to the null-character if
+ there are no quotes around TEXT. MATCH_TYPE will be one of the readline
+ constants SINGLE_MATCH or MULTI_MATCH depending on if there is one or
+ many completions. */
+
+static char *
+gdb_completer_file_name_quote (char *text, int match_type ATTRIBUTE_UNUSED,
+ char *quote_ptr)
+{
+ return gdb_completer_file_name_quote_1 (text, *quote_ptr);
+}
+
/* The function is used to update the completion word MATCH before
displaying it to the user in the 'complete' command output. This
function is only used for formatting filename or directory names.
@@ -366,12 +378,28 @@ gdb_completer_file_name_quote (char *text, int match_type ATTRIBUTE_UNUSED,
in which case a trailing "/" (forward-slash) is added, otherwise
QUOTE_CHAR is added as a trailing quote.
+ When ADD_ESCAPES is true any special characters (e.g. whitespace,
+ quotes) will be escaped with a backslash. See
+ gdb_completer_file_name_quote_1 for full details on escaping. When
+ ADD_ESCAPES is false then no escaping will be added and MATCH (with the
+ correct trailing character) will be used unmodified.
+
Return the updated completion word as a string. */
static std::string
-filename_match_formatter (const char *match, char quote_char)
+filename_match_formatter_1 (const char *match, char quote_char,
+ bool add_escapes)
{
- std::string result (match);
+ std::string result;
+ if (add_escapes)
+ {
+ gdb::unique_xmalloc_ptr<char> quoted_match
+ (gdb_completer_file_name_quote_1 (match, quote_char));
+ result = quoted_match.get ();
+ }
+ else
+ result = match;
+
if (std::filesystem::is_directory (gdb_tilde_expand (match)))
result += "/";
else
@@ -380,16 +408,52 @@ filename_match_formatter (const char *match, char quote_char)
return result;
}
+/* The formatting function used to format the results of a 'complete'
+ command when the result is a filename, but the filename should not have
+ any escape characters added. Most commands that accept a filename don't
+ expect the filename to be quoted or to contain escape characters.
+
+ See filename_match_formatter_1 for more argument details. */
+
+static std::string
+filename_unquoted_match_formatter (const char *match, char quote_char)
+{
+ return filename_match_formatter_1 (match, quote_char, false);
+}
+
+/* The formatting function used to format the results of a 'complete'
+ command when the result is a filename, and the filename should have any
+ special character (e.g. whitespace, quotes) within it escaped with a
+ backslash. A limited number of commands accept this style of filename
+ argument.
+
+ See filename_match_formatter_1 for more argument details. */
+
+static std::string
+filename_maybe_quoted_match_formatter (const char *match, char quote_char)
+{
+ return filename_match_formatter_1 (match, quote_char, true);
+}
+
/* Generate filename completions of WORD, storing the completions into
TRACKER. This is used for generating completions for commands that
only accept unquoted filenames as well as for commands that accept
- quoted and escaped filenames. */
+ quoted and escaped filenames.
+
+ When QUOTE_MATCHES is true TRACKER will be given a match formatter
+ function which will add escape characters (if needed) in the results.
+ When QUOTE_MATCHES is false the match formatter provided will not add
+ any escaping to the results. */
static void
filename_completer_generate_completions (completion_tracker &tracker,
- const char *word)
+ const char *word,
+ bool quote_matches)
{
- tracker.set_match_format_func (filename_match_formatter);
+ if (quote_matches)
+ tracker.set_match_format_func (filename_maybe_quoted_match_formatter);
+ else
+ tracker.set_match_format_func (filename_unquoted_match_formatter);
int subsequent_name = 0;
while (1)
@@ -423,7 +487,7 @@ filename_maybe_quoted_completer (struct cmd_list_element *ignore,
{
rl_char_is_quoted_p = gdb_completer_file_name_char_is_quoted;
rl_completer_quote_characters = gdb_completer_file_name_quote_characters;
- filename_completer_generate_completions (tracker, word);
+ filename_completer_generate_completions (tracker, word, true);
}
/* The corresponding completer_handle_brkchars implementation. */
@@ -449,7 +513,7 @@ filename_completer_handle_brkchars
{
gdb_assert (word == nullptr);
tracker.set_use_custom_word_point (true);
- filename_completer_generate_completions (tracker, text);
+ filename_completer_generate_completions (tracker, text, false);
}
/* Find the bounds of the current word for completion purposes, and
diff --git a/gdb/testsuite/gdb.base/filename-completion.exp b/gdb/testsuite/gdb.base/filename-completion.exp
index 0467d5c425e..3ded82431c8 100644
--- a/gdb/testsuite/gdb.base/filename-completion.exp
+++ b/gdb/testsuite/gdb.base/filename-completion.exp
@@ -82,10 +82,22 @@ proc test_gdb_complete_filename_multiple {
$add_completed_line $completion_list $max_completions $testname
}
- if { $start_quote_char eq "" && $end_quote_char ne "" } {
+ if { $start_quote_char eq "" } {
set updated_completion_list {}
foreach entry $completion_list {
+ # If ENTRY is quoted with double quotes, then any double
+ # quotes within the entry need to be escaped.
+ if { $end_quote_char eq "\"" } {
+ regsub -all "\"" $entry "\\\"" entry
+ }
+
+ if { $end_quote_char eq "" } {
+ regsub -all " " $entry "\\ " entry
+ regsub -all "\"" $entry "\\\"" entry
+ regsub -all "'" $entry "\\'" entry
+ }
+
if {[string range $entry end end] ne "/"} {
set entry $entry$end_quote_char
}
@@ -147,47 +159,61 @@ proc run_quoting_and_escaping_tests { root } {
} "" "${qc}" false \
"expand mixed directory and file names"
- # GDB does not currently escape word break characters
- # (e.g. white space) correctly in unquoted filenames.
if { $qc ne "" } {
set sp " "
-
- test_gdb_complete_filename_multiple "file ${qc}${root}/aaa/" \
- "a" "a${sp}" {
- "aa bb"
- "aa cc"
- } "" "${qc}" false \
- "expand filenames containing spaces"
-
- test_gdb_complete_filename_multiple "file ${qc}${root}/bb1/" \
- "a" "a" {
- "aa\"bb"
- "aa'bb"
- } "" "${qc}" false \
- "expand filenames containing quotes"
} else {
set sp "\\ "
+ }
+
+ if { $qc eq "'" } {
+ set dq "\""
+ set dq_re "\""
+ } else {
+ set dq "\\\""
+ set dq_re "\\\\\""
+ }
+
+ test_gdb_complete_filename_multiple "file ${qc}${root}/bb2/" \
+ "d" "ir${sp}" {
+ "dir 1/"
+ "dir 2/"
+ } "" "${qc}" false \
+ "expand multiple directory names containing spaces"
- test_gdb_complete_tab_multiple "file ${qc}${root}/aaa/a" \
- "a${sp}" {
- "aa bb"
- "aa cc"
- } false \
- "expand filenames containing spaces"
-
- test_gdb_complete_tab_multiple "file ${qc}${root}/bb1/a" \
- "a" {
- "aa\"bb"
- "aa'bb"
- } false \
- "expand filenames containing quotes"
-
- test_gdb_complete_tab_unique "file ${qc}${root}/bb1/aa\\\"" \
- "file ${qc}${root}/bb1/aa\\\\\"bb${qc}" " " \
- "expand unique filename containing double quotes"
-
- test_gdb_complete_tab_unique "file ${qc}${root}/bb1/aa\\'" \
- "file ${qc}${root}/bb1/aa\\\\'bb${qc}" " " \
+ test_gdb_complete_filename_multiple "file ${qc}${root}/aaa/" \
+ "a" "a${sp}" {
+ "aa bb"
+ "aa cc"
+ } "" "${qc}" false \
+ "expand filenames containing spaces"
+
+ test_gdb_complete_filename_multiple "file ${qc}${root}/bb1/" \
+ "a" "a" {
+ "aa\"bb"
+ "aa'bb"
+ } "" "${qc}" false \
+ "expand filenames containing quotes"
+
+ test_gdb_complete_tab_unique "file ${qc}${root}/bb1/aa${dq}" \
+ "file ${qc}${root}/bb1/aa${dq_re}bb${qc}" " " \
+ "expand unique filename containing double quotes"
+
+ # It is not possible to include a single quote character
+ # within a single quoted string. However, GDB does not do
+ # anything smart if a user tries to do this. Avoid testing
+ # this case. Maybe in the future we'll figure a way to avoid
+ # this situation.
+ if { $qc ne "'" } {
+ if { $qc eq "" } {
+ set sq "\\'"
+ set sq_re "\\\\'"
+ } else {
+ set sq "'"
+ set sq_re "'"
+ }
+
+ test_gdb_complete_tab_unique "file ${qc}${root}/bb1/aa${sq}" \
+ "file ${qc}${root}/bb1/aa${sq_re}bb${qc}" " " \
"expand unique filename containing single quote"
}
}
--
2.25.4
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCHv3 7/7] gdb: improve gdb_rl_find_completion_word for quoted words
2024-06-05 13:36 ` [PATCHv3 0/7] Further filename completion improvements Andrew Burgess
` (5 preceding siblings ...)
2024-06-05 13:36 ` [PATCHv3 6/7] gdb: apply escaping to filenames in 'complete' results Andrew Burgess
@ 2024-06-05 13:36 ` Andrew Burgess
2024-06-06 16:24 ` [PATCHv3 0/7] Further filename completion improvements Tom Tromey
7 siblings, 0 replies; 50+ messages in thread
From: Andrew Burgess @ 2024-06-05 13:36 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess
The function gdb_rl_find_completion_word is very similar to the
readline function _rl_find_completion_word, but was either an older
version of that function, or was trimmed when copying to remove code
which was considered unnecessary.
We maintain this copy because the _rl_find_completion_word function is
not part of the public readline API, and we need to replicate the
functionality of that function as part of the 'complete' command.
Within gdb_rl_find_completion_word when looking for the completion
word, if we don't find a unclosed quoted string (which would become
the completion word) then we scan backwards looking for a word break
character. For example, given:
(gdb) complete file /tmp/foo
There is no unclosed quoted string so we end up scanning backwards
from the end looking for a word break character. In this case the
space after 'file' and before '/tmp/foo' is found, so '/tmp/foo'
becomes the completion word.
However, given this:
(gdb) complete file /tmp/foo\"
There is still no unclosed quoted string, however, when we can
backwards the '"' (double quotes) are treated as a word break
character, and so we end up using the empty string as the completion
word.
The readline function _rl_find_completion_word avoids this mistake by
using the rl_char_is_quoted_p hook. This function will return true
for the double quote character as it is preceded by a backslash. An
earlier commit in this series supplied a rl_char_is_quoted_p function
for the filename completion case, however, gdb_rl_find_completion_word
doesn't call rl_char_is_quoted_p so this doesn't help for the
'complete' case.
In this commit I've copied the code to call rl_char_is_quoted_p from
_rl_find_completion_word into gdb_rl_find_completion_word.
This half solves the problem. In the case:
(gdb) complete file /tmp/foo\"
We do now try to complete on the string '/tmp/foo\"', however, when we
reach filename_completer we call back into readline to actually
perform filename completion. However, at this point the WORD variable
points to a string that still contains the backslash. The backslash
isn't part of the actual filename, that's just an escape character.
Our expectation is that readline will remove the backslash when
looking for matching filenames. However, readline contains an
optimisation to avoid unnecessary work trying to remove escape
characters.
The readline variable rl_completion_found_quote is set in the readline
function gen_completion_matches before the generation of completion
matches. This variable is set to true (non-zero) if there is (or
might be) escape characters within the completion word.
The function rl_filename_completion_function, which generates the
filename matches, only removes escape characters when
rl_completion_found_quote is true. When GDB generates completions
through readline (e.g. tab completion) then rl_completion_found_quote
is set correctly.
But when we use the 'complete' command we don't pass through readline,
and so gen_completion_matches is never called and
rl_completion_found_quote is not set. In this case when we call
rl_filename_completion_function readline doesn't remove the escapes
from the completion word, and so in our case above, readline looks for
completions of the exact filename '/tmp/foo\"', that is, the filename
including the backslash.
To work around this problem I've added a new flag to our function
gdb_rl_find_completion_word which is set true when we find any quoting
or escaping. This matches what readline does.
Then in the 'complete' function we can set rl_completion_found_quote
prior to generating completion matches.
With this done the 'complete' command now works correctly when trying
to complete filenames that contain escaped word break characters. The
tests have been updated accordingly.
---
gdb/completer.c | 60 +++++++++++++++----
.../gdb.base/filename-completion.exp | 12 ++--
2 files changed, 54 insertions(+), 18 deletions(-)
diff --git a/gdb/completer.c b/gdb/completer.c
index d0c542413e3..dbcc888e2a5 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -50,7 +50,8 @@
/* Forward declarations. */
static const char *completion_find_completion_word (completion_tracker &tracker,
const char *text,
- int *quote_char);
+ int *quote_char,
+ bool *found_any_quoting);
static void set_rl_completer_word_break_characters (const char *break_chars);
@@ -528,7 +529,9 @@ filename_completer_handle_brkchars
boundaries of the current word. QC, if non-null, is set to the
opening quote character if we found an unclosed quoted substring,
'\0' otherwise. DP, if non-null, is set to the value of the
- delimiter character that caused a word break. */
+ delimiter character that caused a word break. FOUND_ANY_QUOTING, if
+ non-null, is set to true if we found any quote characters (single or
+ double quotes, or a backslash) while finding the completion word. */
struct gdb_rl_completion_word_info
{
@@ -539,7 +542,7 @@ struct gdb_rl_completion_word_info
static const char *
gdb_rl_find_completion_word (struct gdb_rl_completion_word_info *info,
- int *qc, int *dp,
+ int *qc, int *dp, bool *found_any_quoting,
const char *line_buffer)
{
int scan, end, delimiter, pass_next, isbrk;
@@ -561,6 +564,7 @@ gdb_rl_find_completion_word (struct gdb_rl_completion_word_info *info,
end = point;
delimiter = 0;
quote_char = '\0';
+ bool found_quote = false;
brkchars = info->word_break_characters;
@@ -586,6 +590,7 @@ gdb_rl_find_completion_word (struct gdb_rl_completion_word_info *info,
if (quote_char != '\'' && line_buffer[scan] == '\\')
{
pass_next = 1;
+ found_quote = true;
continue;
}
@@ -606,6 +611,7 @@ gdb_rl_find_completion_word (struct gdb_rl_completion_word_info *info,
/* Found start of a quoted substring. */
quote_char = line_buffer[scan];
point = scan + 1;
+ found_quote = true;
}
}
}
@@ -619,8 +625,22 @@ gdb_rl_find_completion_word (struct gdb_rl_completion_word_info *info,
{
scan = line_buffer[point];
- if (strchr (brkchars, scan) != 0)
- break;
+ if (strchr (brkchars, scan) == 0)
+ continue;
+
+ /* Call the application-specific function to tell us whether
+ this word break character is quoted and should be skipped.
+ The const_cast is needed here to comply with the readline
+ API. The only function we register for rl_char_is_quoted_p
+ treats the input buffer as 'const', so we're OK. */
+ if (rl_char_is_quoted_p != nullptr && found_quote
+ && (*rl_char_is_quoted_p) (const_cast<char *> (line_buffer),
+ point))
+ continue;
+
+ /* Convoluted code, but it avoids an n^2 algorithm with calls
+ to char_is_quoted. */
+ break;
}
}
@@ -644,6 +664,8 @@ gdb_rl_find_completion_word (struct gdb_rl_completion_word_info *info,
}
}
+ if (found_any_quoting != nullptr)
+ *found_any_quoting = found_quote;
if (qc != NULL)
*qc = quote_char;
if (dp != NULL)
@@ -670,7 +692,7 @@ advance_to_completion_word (completion_tracker &tracker,
int delimiter;
const char *start
- = gdb_rl_find_completion_word (&info, NULL, &delimiter, text);
+ = gdb_rl_find_completion_word (&info, nullptr, &delimiter, nullptr, text);
tracker.advance_custom_word_point_by (start - text);
@@ -732,7 +754,8 @@ complete_nested_command_line (completion_tracker &tracker, const char *text)
int quote_char = '\0';
const char *word = completion_find_completion_word (tracker, text,
- "e_char);
+ "e_char,
+ nullptr);
if (tracker.use_custom_word_point ())
{
@@ -1950,8 +1973,11 @@ complete (const char *line, char const **word, int *quote_char)
try
{
+ bool found_any_quoting = false;
+
*word = completion_find_completion_word (tracker_handle_brkchars,
- line, quote_char);
+ line, quote_char,
+ &found_any_quoting);
/* Completers that provide a custom word point in the
handle_brkchars phase also compute their completions then.
@@ -1961,6 +1987,12 @@ complete (const char *line, char const **word, int *quote_char)
tracker = &tracker_handle_brkchars;
else
{
+ /* Setting this global matches what readline does within
+ gen_completion_matches. We need this set correctly in case
+ our completion function calls back into readline to perform
+ completion (e.g. filename_completer does this). */
+ rl_completion_found_quote = found_any_quoting;
+
complete_line (tracker_handle_completions, *word, line, strlen (line));
tracker = &tracker_handle_completions;
}
@@ -2235,7 +2267,7 @@ gdb_completion_word_break_characters () noexcept
static const char *
completion_find_completion_word (completion_tracker &tracker, const char *text,
- int *quote_char)
+ int *quote_char, bool *found_any_quoting)
{
size_t point = strlen (text);
@@ -2245,6 +2277,13 @@ completion_find_completion_word (completion_tracker &tracker, const char *text,
{
gdb_assert (tracker.custom_word_point () > 0);
*quote_char = tracker.quote_char ();
+ /* This isn't really correct, we're ignoring the case where we found
+ a backslash escaping a character. However, this isn't an issue
+ right now as we only rely on *FOUND_ANY_QUOTING being set when
+ performing filename completion, which doesn't go through this
+ path. */
+ if (found_any_quoting != nullptr)
+ *found_any_quoting = *quote_char != '\0';
return text + tracker.custom_word_point ();
}
@@ -2254,7 +2293,8 @@ completion_find_completion_word (completion_tracker &tracker, const char *text,
info.quote_characters = rl_completer_quote_characters;
info.basic_quote_characters = rl_basic_quote_characters;
- return gdb_rl_find_completion_word (&info, quote_char, NULL, text);
+ return gdb_rl_find_completion_word (&info, quote_char, nullptr,
+ found_any_quoting, text);
}
/* See completer.h. */
diff --git a/gdb/testsuite/gdb.base/filename-completion.exp b/gdb/testsuite/gdb.base/filename-completion.exp
index 3ded82431c8..78f85ace8a1 100644
--- a/gdb/testsuite/gdb.base/filename-completion.exp
+++ b/gdb/testsuite/gdb.base/filename-completion.exp
@@ -167,10 +167,8 @@ proc run_quoting_and_escaping_tests { root } {
if { $qc eq "'" } {
set dq "\""
- set dq_re "\""
} else {
set dq "\\\""
- set dq_re "\\\\\""
}
test_gdb_complete_filename_multiple "file ${qc}${root}/bb2/" \
@@ -194,8 +192,8 @@ proc run_quoting_and_escaping_tests { root } {
} "" "${qc}" false \
"expand filenames containing quotes"
- test_gdb_complete_tab_unique "file ${qc}${root}/bb1/aa${dq}" \
- "file ${qc}${root}/bb1/aa${dq_re}bb${qc}" " " \
+ test_gdb_complete_unique "file ${qc}${root}/bb1/aa${dq}" \
+ "file ${qc}${root}/bb1/aa${dq}bb${qc}" " " false \
"expand unique filename containing double quotes"
# It is not possible to include a single quote character
@@ -206,14 +204,12 @@ proc run_quoting_and_escaping_tests { root } {
if { $qc ne "'" } {
if { $qc eq "" } {
set sq "\\'"
- set sq_re "\\\\'"
} else {
set sq "'"
- set sq_re "'"
}
- test_gdb_complete_tab_unique "file ${qc}${root}/bb1/aa${sq}" \
- "file ${qc}${root}/bb1/aa${sq_re}bb${qc}" " " \
+ test_gdb_complete_unique "file ${qc}${root}/bb1/aa${sq}" \
+ "file ${qc}${root}/bb1/aa${sq}bb${qc}" " " false \
"expand unique filename containing single quote"
}
}
--
2.25.4
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCHv3 0/7] Further filename completion improvements
2024-06-05 13:36 ` [PATCHv3 0/7] Further filename completion improvements Andrew Burgess
` (6 preceding siblings ...)
2024-06-05 13:36 ` [PATCHv3 7/7] gdb: improve gdb_rl_find_completion_word for quoted words Andrew Burgess
@ 2024-06-06 16:24 ` Tom Tromey
7 siblings, 0 replies; 50+ messages in thread
From: Tom Tromey @ 2024-06-06 16:24 UTC (permalink / raw)
To: Andrew Burgess; +Cc: gdb-patches
>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
Andrew> In V3:
Andrew> - Patch #1 from the v2 series was merged.
Andrew> - Remaining patches have been rebased and retested.
I skimmed through this series looking for red flags. I didn't really
review it though. It seems to me like it is a good improvement and I'd
be happy to see it go in.
thanks,
Tom
^ permalink raw reply [flat|nested] 50+ messages in thread