Re: [PATCH] Add completion styling

[Pedro Alves <palves@redhat.com>]

On 4/24/20 8:51 PM, Tom Tromey wrote:
>>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
> 
> Thanks for your comments.
> 
>>> Readline has a styling feature for completion -- if it is enabled, the
>>> common prefix of completions will be displayed in a different style.
>>> This doesn't work in gdb, because gdb implements its own completer.
>>>
>>> This patch implements the feature.  However, it doesn't directly use
>>> the Readline feature, because gdb can do a bit better: it can let the
>>> user control the styling using the existing mechanisms.
> 
> Eli> Would it make sense to default to the style determined by the user's
> Eli> colored-completion-prefix setting?
> 
> That's a bit of a pain because readline parses LS_COLORS on its own.
> This doesn't seem to be documented anywhere.  I'm reluctant to try to
> use readline's code, because it is using "_" prefixes; in readline these
> indicate private items and there's at least one bug report about this in
> bugzilla already (some distro made these hidden in the .so and it broke
> gdb).
> 
> Eli> Also, do we want the default styling to be no-styling, or do we want
> Eli> something else?
> 
> I would prefer to enable it by default.  It's easy to disable if one
> prefers.
> 
> Eli> Btw, Emacs solves this problem differently: it uses a distinct styling
> Eli> for the first character that distinguishes between completion
> Eli> candidates.  I wonder if that idea is more useful for quickly
> Eli> realizing what one needs to type next.  Or do we want to follow
> Eli> Readline here for consistency reasons?
> 
> I think the Emacs idea is nice.  Maybe we want to let the user control
> the prefix text, the "difference character", and the final completion
> text.

I'm coming here late, sorry about that.  I have some comments.

As you know, I'm very much in favor of color in completion, mostly because
of the C++ wildmatching feature.  Unfortunately, coloring for that is
doesn't work correctly.  Try "b main[TAB]" when debugging GDB, and you'll
see that GDB highlights the "t" as the first different character here:

selftests::string_view::cons_1::main()
selftests::string_view::cons_2::main()
selftests::string_view::cons_3::main()
    ^

while it should highlight here:

selftests::string_view::cons_1::main()
selftests::string_view::cons_2::main()
selftests::string_view::cons_3::main()
                                    ^

Also, if you enable "set style completion-prefix foreground",
then you should see GDB highlight the common prefix here:

selftests::string_view::cons_1::main()
selftests::string_view::cons_2::main()
selftests::string_view::cons_3::main()
                                ^^^^

though it actually incorrectly highlights here:

selftests::string_view::cons_1::main()
selftests::string_view::cons_2::main()
selftests::string_view::cons_3::main()
^^^^

BTW, it seems like the "dim" default style for "completion-prefix foreground"
doesn't have any effect on my konsole terminal, and neither on xterm.
It does work on a real Linux virtual console.  Seems like "dim" isn't universally
supported?  Maybe we should avoid "dim" in default styles?
I'm using "set style completion-prefix foreground red" here, and it looks
nice.

Also, I noticed that "set style enabled off" does not disable
completion styling.  That seems wrong to me.

Thanks,
Pedro Alves