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

[Andrew Burgess <aburgess@redhat.com>]

Lancelot SIX <lsix@lancelotsix.com> writes:

> On Fri, Mar 29, 2024 at 03:14:07PM +0300, Eli Zaretskii wrote:
>> > From: Andrew Burgess <aburgess@redhat.com>
>> > Cc: Andrew Burgess <aburgess@redhat.com>
>> > Date: Fri, 29 Mar 2024 11:42:28 +0000
>> > 
>> > When using the 'complete' command to complete file names we have some
>> > problems.  The suggested completion should include any required
>> > escaping, so if there is a filename '/tmp/aa"bb' (without the single
>> > quotes), then this should be displayed in the completion output like:
>> > 
>> >   (gdb) complete file /tmp/aa
>> >   file /tmp/aa\"bb
>> 
>> Why should it be displayed with the backslash?  And would the
>> completed name, if passed to a command, also have the backslash added?
>> If so, it could cause some commands to fail; basically, you are adding
>> shell file-name semantics into commands that don't work via the shell.
>
> Hi,
>
> The "file" command currently expects "shell file-name semantics":
>
>     $ gdb -q
>     (gdb) file test/aa bb/a.out
>     test/aa: No such file or directory.
>     (gdb) file "test/aa bb/a.out"
>     Reading symbols from test/aa bb/a.out...
>     (gdb) file test/aa\ bb/a.out
>     Load new symbol table from "test/aa bb/a.out"? (y or n) y
>     Reading symbols from test/aa bb/a.out...
>     (gdb)
>
> The problem Andrew is trying to solve is that the current completer for
> this command completes to something that is not a valid input.
>
> Moreover, the completer as it is currently is is not entirely functional
> for commands that expect "raw" filenames either.  If you take the same
> directory structure, it can complete up to "test/aa bb", but can’t
> complete the directory:
>
>     $ gdb -q
>     (gdb) complete file test/aa
>     file test/aa bb
>     (gdb) complete file test/aa bb
>     (gdb) complete file test/aa bb/
>     (gdb)
>
> Where I do agree with you is that GDB is inconsistent: some command
> expect "shell escaped filenames", and some do not.  For example, "set
> logging file" will take its argument verbatim and use that:
>
>     (gdb) set logging file "/tmp/test/aa bb/file.txt"
>     (gdb) show logging file
>     The current logfile is ""/tmp/test/aa bb/file.txt"".
>     (gdb) set logging file /tmp/test/aa\ bb/file.txt
>     (gdb) show logging file
>     The current logfile is "/tmp/test/aa\ bb/file.txt".
>
> Part of the issue you are raising is that both commands use the same
> completer, so if the completer works for some commands, it will be wrong
> for the others.  I have not reached the end of this series so I am not
> sure if this is addressed (I do not expect it is), but I guess we should
> either:
> 1) have 2 filename completers, and make sure that each function using a
>    filename completer uses the appropriate one, or
> 2) have GDB be consistent regarding how commands consume filenames.
>
> I would prefer consistency, but implementing 2 would strictly speaking
> be an interface breaking change, so it needs careful consideration.

I wasn't aware of this difference.  I would also rather just have
consistent parsing.

The 'set logging file' command gets away with this parsing because the
filename is always the last thing on the command line, but for other
commands this is not always the case.

We could probably arrange a mostly backward compatible solution for this
case.  I'll see if I can spot any other problem cases.

Thanks,
Andrew