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

[Lancelot SIX <lsix@lancelotsix.com>]

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.

My experience is that I tend to use "file" / "add-symbol-file" more than
other commands dealing with filenames, so I would prefer to have a
completer that works for those.  Also, the current completer does not
completely work for commands that consume "raw" filenames anyway, so
having a completer that works properly for some commands rather than for
none sounds appealing to me.  That being said, this is just my 2 cent, I
am looking forward to hearing other people’s thoughts on the subject.

Best,
Lancelot.

> 
> So I'm not sure I understand what is being intended here, and I'm
> afraid you will uncover a lot of problems as you go with this (as you
> already discovered).
> 
> Apologies if I'm missing something important here.