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

[Eli Zaretskii <eliz@gnu.org>]

> From: Andrew Burgess <aburgess@redhat.com>
> Cc: lsix@lancelotsix.com, gdb-patches@sourceware.org
> Date: Fri, 12 Apr 2024 23:20:33 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > What is "gdb-shell"?  Saying "gdb-shell semantics" is only useful if
> > the term "gdb-shell" is known and understood by many.  Otherwise, we
> > should not use this terminology, because it doesn't explain anything.
> 
> It's a name I made up for the thing you type at after a (gdb) prompt.
> 
> The point is, it's not a "shell", it's GDB.  It has its own rules.

Do we have those rules spelled out somewhere?  In the context of this
discussion, the pertinent rules are those for quoting in file names.
Are they documented?  And does this patch series modify those rules in
some way that users will need to know?

I guess this part:

>   + We are agreed that there's clearly a documentation issue.  However,
>     I would argue that the documentation is missing for current
>     functionality (i.e. the current rules for quoting and escaping file
>     names).  We wouldn't document this stuff as part of "command
>     completion", that doesn't really make sense I think.  I will start
>     working on a separate patch to try and improve the documentation for
>     how file names are quoted / escaped currently,

means the answer is NO.  So yes, it would be good to document those
rules in the manual (it would also be useful when someone would like
to fix the bugs in Emacs you mention below).

> OK, so now the bad news.
> 
> My patched GDB doesn't "just work" with emacs.
> 
> Remember though, an unpatched GDB doesn't work either when trying to
> complete a filename with spaces in, so it's not that I've made things
> worse.
> 
> I tracked at least one bug down to this function in gud.el:
> 
>   (defun gud-gdb-completion-at-point ()
>     "Return the data to complete the GDB command before point."
>     (let ((end (point))
>           (start
>            (save-excursion
>              (skip-chars-backward "^ " (comint-line-beginning-position))
>              (point))))
>       ;; FIXME: `gud-gdb-run-command-fetch-lines' has some nasty side-effects on
>       ;; the buffer (via `gud-delete-prompt-marker'): it removes the prompt and
>       ;; then re-adds it later, thus messing up markers and overlays along the
>       ;; way (bug#18282).
>       ;; We use an "insert-before" marker for `start', since it's typically right
>       ;; after the prompt, which works around the problem, but is a hack (and
>       ;; comes with other downsides, e.g. if completion adds text at `start').
>       (list (copy-marker start t) end
>             (completion-table-dynamic
>              (apply-partially gud-gdb-completion-function
>                               (buffer-substring (comint-line-beginning-position)
>                                                 start))))))
>                                                 
> This is trying to figure out everything before the completion word on
> the current line and doing the apply-partial with this context on
> gud-gdb-completion-function.
> 
> So if we are completing 'file /tmp/abc<tab>' then we'd end up doing:
> 
>   (apply-partial gud-gdb-completion-function "file ")
> 
> The completion context is calculated as running from the start of the
> line up to 'start', which is calculated in the 'let'.  And 'start' is
> calculated by starting from 'point' and moving backward to the first
> whitespace.
> 
> That's not going to work for completing any line that contains a space.
> 
> I think this function would need to get smarter to understand about
> escaping and quoting.

Yes.  Would you mind submitting a bug report about that, using the
command "M-x report-emacs-bug"?

> And, finally, some good news.
> 
> If a filename _doesn't_ include any spaces, then as far as I can tell,
> from my limited testing, everything works fine in emacs, and is now
> improved.
> 
> On an unpatched GDB, given a file /tmp/qqq/f1, within emacs:
> 
>   (gdb) file /tmp/qq<tab>
>      => file /tmp/qqq<tab>
>      *Sole Completion*
> 
> That's it.  At this point emacs will not help me complete this at all.
> I have to manually add the '/' and only then will emacs offer the next
> level of completion.
> 
> Compare this to plain GDB:
> 
>   (gdb) file /tmp/qq<tab>
>      => file /tmp/qqq/<tag>
>      => file /tmp/qqq/f1
> 
> With a patched GDB, emacs is now as good as plain GDB.  This is thanks
> to me adding the trailing '/' character in the 'complete' command
> output.
> 
> And on the subject of quotes.  With an unpatched GDB emacs is perfectly
> happy completing a file containing quotes (well, as happy as it is with
> an unquoted file name), e.g.:
> 
>   (gdb) file "/tmp/qq<tab>
>      => file "/tmp/qqq
> 
> But, when I get to the final file completion, emacs doesn't add the
> trailing quote, so:
> 
>   (gdb) file "/tmp/qqq/f<tab>
>     =>  file "/tmp/qqq/f1
> 
> In contrast and *unpatched* GDB at the CLI will add the final quote.
> 
> With a patched GDB, emacs is just as happy handling the quotes, only now
> it adds the final trailing quote on, just like GDB's CLI:
> 
>   (gdb) file "/tmp/qqq/f<tab>
>     =>  file "/tmp/qqq/f1"

SGTM, thanks.

> Are there any concerns that I haven't addressed?  I don't want to miss
> anything that's concerning you.

No, I don't think you've missed anything.  Thanks for working on this
important feature (I happen to be one of those completion junkies, so
any improvements in this area are welcome here).