[PATCH] Add completion styling

[PATCH] Add completion styling

[Tom Tromey]

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.

gdb/ChangeLog
2020-04-08  Tom Tromey  <tom@tromey.com>

	* NEWS: Add entry for completion styling.
	* completer.c (_rl_completion_prefix_display_length): Move
	declaration earlier.
	(gdb_fnprint): Use completion_style.
	(gdb_display_match_list_1): Likewise.
	* cli/cli-style.c (completion_style): New global.
	(_initialize_cli_style): Register new global.
	* cli/cli-style.h (completion_style): Declare.

gdb/doc/ChangeLog
2020-04-08  Tom Tromey  <tom@tromey.com>

	* gdb.texinfo (Output Styling): Mention "completion" styling.
	(Editing): Mention readline completion styling.

gdb/testsuite/ChangeLog
2020-04-08  Tom Tromey  <tom@tromey.com>

	* gdb.base/style.exp: Add completion styling test.
	* lib/gdb-utils.exp (style): Add "completion" style.
---
 gdb/ChangeLog                    | 11 ++++++++++
 gdb/NEWS                         |  6 ++++++
 gdb/cli/cli-style.c              | 11 ++++++++++
 gdb/cli/cli-style.h              |  3 +++
 gdb/completer.c                  | 35 +++++++++++++++++++++++++-------
 gdb/doc/ChangeLog                |  5 +++++
 gdb/doc/gdb.texinfo              |  8 ++++++++
 gdb/testsuite/ChangeLog          |  5 +++++
 gdb/testsuite/gdb.base/style.exp | 12 +++++++++++
 gdb/testsuite/lib/gdb-utils.exp  |  1 +
 10 files changed, 90 insertions(+), 7 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 6657f6fadce..331b9967a7b 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -55,6 +55,12 @@ show exec-file-mismatch -- Show exec-file-mismatch handling (ask|warn|off).
   whether to load the process executable file; if 'warn', just display
   a warning; if 'off', don't attempt to detect a mismatch.
 
+set style completion foreground COLOR
+set style completion background COLOR
+set style completion intensity VALUE
+  Control the styling of completions.  The common prefix of completion
+  candidates will be shown with this style.
+
 tui new-layout NAME WINDOW WEIGHT [WINDOW WEIGHT]...
   Define a new TUI layout, specifying its name and the windows that
   will be displayed.
diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c
index d2d9928acd5..efada5e2ca9 100644
--- a/gdb/cli/cli-style.c
+++ b/gdb/cli/cli-style.c
@@ -98,6 +98,10 @@ cli_style_option metadata_style ("metadata", ui_file_style::DIM);
 
 /* See cli-style.h.  */
 
+cli_style_option completion_style ("completion", ui_file_style::MAGENTA);
+
+/* See cli-style.h.  */
+
 cli_style_option::cli_style_option (const char *name,
 				    ui_file_style::basic_color fg)
   : changed (name),
@@ -409,6 +413,13 @@ Configure metadata colors and display intensity\n\
 The \"metadata\" style is used when GDB displays information about\n\
 your data, for example \"<unavailable>\""), false);
 
+  STYLE_ADD_SETSHOW_COMMANDS (completion_style,
+			      _("\
+Completion display styling.\n\
+Configure completion colors and display intensity\n\
+The \"completion\" style is used when GDB displays possible completions,\n\
+for example in response to the TAB key."), false);
+
   STYLE_ADD_SETSHOW_COMMANDS (tui_border_style,
 			      _("\
 TUI border display styling.\n\
diff --git a/gdb/cli/cli-style.h b/gdb/cli/cli-style.h
index 04009aa3615..d9ed8c28d54 100644
--- a/gdb/cli/cli-style.h
+++ b/gdb/cli/cli-style.h
@@ -126,6 +126,9 @@ extern cli_style_option tui_border_style;
 /* The border style of a TUI window that does have the focus.  */
 extern cli_style_option tui_active_border_style;
 
+/* The completion style.  */
+extern cli_style_option completion_style;
+
 /* True if source styling is enabled.  */
 extern bool source_styling;
 
diff --git a/gdb/completer.c b/gdb/completer.c
index 67dce30fbe3..364362b50a8 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -31,6 +31,7 @@
 #include <algorithm>
 #include "linespec.h"
 #include "cli/cli-decode.h"
+#include "cli/cli-style.h"
 
 /* FIXME: This is needed because of lookup_cmd_1 ().  We should be
    calling a hook instead so we eliminate the CLI dependency.  */
@@ -2702,6 +2703,8 @@ gdb_fnwidth (const char *string)
   return width;
 }
 
+extern int _rl_completion_prefix_display_length;
+
 /* Print TO_PRINT, one matching completion.
    PREFIX_BYTES is number of common prefix bytes.
    Based on readline/complete.c:fnprint.  */
@@ -2710,7 +2713,7 @@ static int
 gdb_fnprint (const char *to_print, int prefix_bytes,
 	     const struct match_list_displayer *displayer)
 {
-  int printed_len, w;
+  int common_prefix_len, printed_len, w;
   const char *s;
 #if defined (HANDLE_MULTIBYTE)
   mbstate_t ps;
@@ -2723,14 +2726,18 @@ gdb_fnprint (const char *to_print, int prefix_bytes,
   memset (&ps, 0, sizeof (mbstate_t));
 #endif
 
-  printed_len = 0;
+  printed_len = common_prefix_len = 0;
 
   /* Don't print only the ellipsis if the common prefix is one of the
      possible completions */
   if (to_print[prefix_bytes] == '\0')
     prefix_bytes = 0;
 
-  if (prefix_bytes)
+  ui_file_style style = completion_style.style ();
+  if (!style.is_default ())
+    displayer->puts (displayer, style.to_ansi ().c_str ());
+
+  if (prefix_bytes && _rl_completion_prefix_display_length > 0)
     {
       char ellipsis;
 
@@ -2739,6 +2746,11 @@ gdb_fnprint (const char *to_print, int prefix_bytes,
 	displayer->putch (displayer, ellipsis);
       printed_len = ELLIPSIS_LEN;
     }
+  else if (prefix_bytes && !style.is_default ())
+    {
+      common_prefix_len = prefix_bytes;
+      prefix_bytes = 0;
+    }
 
   s = to_print + prefix_bytes;
   while (*s)
@@ -2790,6 +2802,12 @@ gdb_fnprint (const char *to_print, int prefix_bytes,
 	  printed_len++;
 #endif
 	}
+      if (common_prefix_len > 0 && (s - to_print) >= common_prefix_len
+	  && !style.is_default ())
+	{
+	  displayer->puts (displayer, ui_file_style ().to_ansi ().c_str ());
+	  common_prefix_len = 0;
+	}
     }
 
   return printed_len;
@@ -2900,7 +2918,6 @@ gdb_complete_get_screenwidth (const struct match_list_displayer *displayer)
   return displayer->width;
 }
 
-extern int _rl_completion_prefix_display_length;
 extern int _rl_print_completions_horizontally;
 
 EXTERN_C int _rl_qsort_string_compare (const void *, const void *);
@@ -2919,19 +2936,23 @@ gdb_display_match_list_1 (char **matches, int len, int max,
   char *temp, *t;
   int page_completions = displayer->height != INT_MAX && pagination_enabled;
 
+  bool want_style = !completion_style.style ().is_default ();
+
   /* Find the length of the prefix common to all items: length as displayed
      characters (common_length) and as a byte index into the matches (sind) */
   common_length = sind = 0;
-  if (_rl_completion_prefix_display_length > 0)
+  if (_rl_completion_prefix_display_length > 0 || want_style)
     {
       t = gdb_printable_part (matches[0]);
       temp = strrchr (t, '/');
       common_length = temp ? gdb_fnwidth (temp) : gdb_fnwidth (t);
       sind = temp ? strlen (temp) : strlen (t);
 
-      if (common_length > _rl_completion_prefix_display_length && common_length > ELLIPSIS_LEN)
+      if (_rl_completion_prefix_display_length > 0
+	  && common_length > _rl_completion_prefix_display_length
+	  && common_length > ELLIPSIS_LEN)
 	max -= common_length - ELLIPSIS_LEN;
-      else
+      else if (!want_style || common_length > max || sind > max)
 	common_length = sind = 0;
     }
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 385c832f222..0176b60e5ff 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -25267,6 +25267,10 @@ This command accepts the current line for execution and fetches the
 next line relative to the current line from the history for editing.
 Any argument is ignored.
 
+Note that @value{GDBN} ignores the Readline
+@code{colored-completion-prefix} setting.  Instead, this is handled
+using the @code{completion} style (@xref{Output Styling}).
+
 @node Command History
 @section Command History
 @cindex command history
@@ -25600,6 +25604,10 @@ general styling to @value{GDBN}.  @xref{TUI Configuration}.
 Control the styling of the active TUI border; that is, the TUI window
 that has the focus.
 
+@item completion
+Control the styling of completion.  When completing, the common prefix
+of completion candidates will be shown with this style.
+
 @end table
 
 @node Numbers
diff --git a/gdb/testsuite/gdb.base/style.exp b/gdb/testsuite/gdb.base/style.exp
index 47ef8c93c7c..d33041e3154 100644
--- a/gdb/testsuite/gdb.base/style.exp
+++ b/gdb/testsuite/gdb.base/style.exp
@@ -152,4 +152,16 @@ save_vars { env(TERM) } {
 	"warning: [style .*? file] is not a directory\\..*"
     gdb_test "show data-directory" \
 	"GDB's data directory is \"[style .*? file]\"\\..*"
+
+    if {[readline_is_used]} {
+	set test "complete print VALUE_"
+	# ESC-? is the readline binding to show all completions.
+	send_gdb "print VALUE_\x1b?"
+	set val [style VALUE_ completion]
+	gdb_test_multiple "" $test {
+	    -re "${val}ONE\[ \t\]+${val}TWO.*$gdb_prompt print VALUE_$" {
+		gdb_test "ONE" " = .*"
+	    }
+	}
+    }
 }
diff --git a/gdb/testsuite/lib/gdb-utils.exp b/gdb/testsuite/lib/gdb-utils.exp
index 9741f0a9591..c6dad81e853 100644
--- a/gdb/testsuite/lib/gdb-utils.exp
+++ b/gdb/testsuite/lib/gdb-utils.exp
@@ -55,6 +55,7 @@ proc style {str style} {
 	variable { set style 36 }
 	address { set style 34 }
 	metadata { set style 2 }
+	completion { set style 35 }
     }
     return "\033\\\[${style}m${str}\033\\\[m"
 }
-- 
2.17.2

Re: [PATCH] Add completion styling

[Eli Zaretskii]

> From: Tom Tromey <tom@tromey.com>
> Date: Wed,  8 Apr 2020 20:41:12 -0600
> Cc: Tom Tromey <tom@tromey.com>
> 
> 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.

Would it make sense to default to the style determined by the user's
colored-completion-prefix setting?

Also, do we want the default styling to be no-styling, or do we want
something else?

> gdb/ChangeLog
> 2020-04-08  Tom Tromey  <tom@tromey.com>
> 
> 	* NEWS: Add entry for completion styling.
> 	* completer.c (_rl_completion_prefix_display_length): Move
> 	declaration earlier.
> 	(gdb_fnprint): Use completion_style.
> 	(gdb_display_match_list_1): Likewise.
> 	* cli/cli-style.c (completion_style): New global.
> 	(_initialize_cli_style): Register new global.
> 	* cli/cli-style.h (completion_style): Declare.
> 
> gdb/doc/ChangeLog
> 2020-04-08  Tom Tromey  <tom@tromey.com>
> 
> 	* gdb.texinfo (Output Styling): Mention "completion" styling.
> 	(Editing): Mention readline completion styling.

The documentation parts are okay, but maybe we should say something
about the default styling of this, even if the default is no styling.

Btw, Emacs solves this problem differently: it uses a distinct styling
for the first character that distinguishes between completion
candidates.  I wonder if that idea is more useful for quickly
realizing what one needs to type next.  Or do we want to follow
Readline here for consistency reasons?

Thanks.

Re: [PATCH] Add completion styling

[Tom Tromey]

>>>>> "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.

Tom

Re: [PATCH] Add completion styling

[Eli Zaretskii]

> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>,  gdb-patches@sourceware.org
> Date: Fri, 24 Apr 2020 13:51:54 -0600
> 
> 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).

Fair enough.

> 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.

But I thought the code disabled it by default?  I did I misread?

> 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.

That would be nice, I think.

Re: [PATCH] Add completion styling

[Tom Tromey]

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> But I thought the code disabled it by default?  I did I misread?

The default is magenta, and it's enabled if $TERM is set properly.

+cli_style_option completion_style ("completion", ui_file_style::MAGENTA);

Tom

Re: [PATCH] Add completion styling

[Tom Tromey]

Tom> I think the Emacs idea is nice.  Maybe we want to let the user control
Tom> the prefix text, the "difference character", and the final completion
Tom> text.

Here's a new version of the patch that implements this idea.

Tom

commit 0af816c447b989a47494b3b1ff970b463b100152
Author: Tom Tromey <tom@tromey.com>
Date:   Sat May 16 13:17:33 2020 -0600

    Add completion styling
    
    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.
    
    This version incorporates an Emacs idea, via Eli: style the prefix,
    the "difference character", and the suffix differently.
    
    gdb/ChangeLog
    2020-05-16  Tom Tromey  <tom@tromey.com>
    
            * NEWS: Add entry for completion styling.
            * completer.c (_rl_completion_prefix_display_length): Move
            declaration earlier.
            (gdb_fnprint): Use completion_style.
            (gdb_display_match_list_1): Likewise.
            * cli/cli-style.c (completion_prefix_style)
            (completion_difference_style, completion_suffix_style): New
            globals.
            (_initialize_cli_style): Register new globals.
            * cli/cli-style.h (completion_prefix_style)
            (completion_difference_style, completion_suffix_style): Declare.
    
    gdb/doc/ChangeLog
    2020-05-16  Tom Tromey  <tom@tromey.com>
    
            * gdb.texinfo (Output Styling): Mention completion styling.
            (Editing): Mention readline completion styling.
    
    gdb/testsuite/ChangeLog
    2020-05-16  Tom Tromey  <tom@tromey.com>
    
            * gdb.base/style.exp: Add completion styling test.
            * lib/gdb-utils.exp (style): Add completion styles.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index def9db59b83..0d056754d98 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,17 @@
+2020-05-16  Tom Tromey  <tom@tromey.com>
+
+	* NEWS: Add entry for completion styling.
+	* completer.c (_rl_completion_prefix_display_length): Move
+	declaration earlier.
+	(gdb_fnprint): Use completion_style.
+	(gdb_display_match_list_1): Likewise.
+	* cli/cli-style.c (completion_prefix_style)
+	(completion_difference_style, completion_suffix_style): New
+	globals.
+	(_initialize_cli_style): Register new globals.
+	* cli/cli-style.h (completion_prefix_style)
+	(completion_difference_style, completion_suffix_style): Declare.
+
 2020-05-16  Tom Tromey  <tom@tromey.com>
 
 	* top.c (quit_force): Update.
diff --git a/gdb/NEWS b/gdb/NEWS
index a059fc7aa0e..1a8458ada8f 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -63,6 +63,17 @@ show exec-file-mismatch -- Show exec-file-mismatch handling (ask|warn|off).
   whether to load the process executable file; if 'warn', just display
   a warning; if 'off', don't attempt to detect a mismatch.
 
+set style completion-prefix foreground COLOR
+set style completion-prefix background COLOR
+set style completion-prefix intensity VALUE
+set style completion-difference foreground COLOR
+set style completion-difference background COLOR
+set style completion-difference intensity VALUE
+set style completion-suffix foreground COLOR
+set style completion-suffix background COLOR
+set style completion-suffix intensity VALUE
+  Control the styling of completions.
+
 tui new-layout NAME WINDOW WEIGHT [WINDOW WEIGHT]...
   Define a new TUI layout, specifying its name and the windows that
   will be displayed.
diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c
index a0c3cc51801..b16b800d152 100644
--- a/gdb/cli/cli-style.c
+++ b/gdb/cli/cli-style.c
@@ -98,6 +98,21 @@ cli_style_option metadata_style ("metadata", ui_file_style::DIM);
 
 /* See cli-style.h.  */
 
+cli_style_option completion_prefix_style ("completion-prefix",
+					  ui_file_style::DIM);
+
+/* See cli-style.h.  */
+
+cli_style_option completion_difference_style ("completion-difference",
+					      ui_file_style::MAGENTA);
+
+/* See cli-style.h.  */
+
+cli_style_option completion_suffix_style ("completion-suffix",
+					  ui_file_style::NONE);
+
+/* See cli-style.h.  */
+
 cli_style_option::cli_style_option (const char *name,
 				    ui_file_style::basic_color fg)
   : changed (name),
@@ -366,6 +381,33 @@ your data, for example \"<unavailable>\""),
 				       &style_set_list, &style_show_list,
 				       false);
 
+  completion_prefix_style.add_setshow_commands (no_class, _("\
+Completion prefix display styling.\n\
+Configure completion prefix colors and display intensity\n\
+The \"completion-prefix\" style is used when GDB displays the shared\n\
+prefix common to the possible completions."),
+						&style_set_list,
+						&style_show_list,
+						false);
+
+  completion_difference_style.add_setshow_commands (no_class, _("\
+Completion difference display styling.\n\
+Configure completion difference colors and display intensity\n\
+The \"completion-difference\" style is used when GDB displays the\n\
+character that differs between the possible completions."),
+						&style_set_list,
+						&style_show_list,
+						false);
+
+  completion_suffix_style.add_setshow_commands (no_class, _("\
+Completion suffix display styling.\n\
+Configure completion suffix colors and display intensity\n\
+The \"completion-suffix\" style is used when GDB displays the suffix\n\
+of the possible completions."),
+						&style_set_list,
+						&style_show_list,
+						false);
+
   tui_border_style.add_setshow_commands (no_class, _("\
 TUI border display styling.\n\
 Configure TUI border colors\n\
diff --git a/gdb/cli/cli-style.h b/gdb/cli/cli-style.h
index 6422e5296a3..c2e0df1b337 100644
--- a/gdb/cli/cli-style.h
+++ b/gdb/cli/cli-style.h
@@ -124,6 +124,15 @@ extern cli_style_option tui_border_style;
 /* The border style of a TUI window that does have the focus.  */
 extern cli_style_option tui_active_border_style;
 
+/* The style for the common prefix of completions.  */
+extern cli_style_option completion_prefix_style;
+
+/* The style for the difference character of completions.  */
+extern cli_style_option completion_difference_style;
+
+/* The style for the suffix of completions.  */
+extern cli_style_option completion_suffix_style;
+
 /* True if source styling is enabled.  */
 extern bool source_styling;
 
diff --git a/gdb/completer.c b/gdb/completer.c
index 71e31cf6df8..bb3a76b25ca 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -31,6 +31,7 @@
 #include <algorithm>
 #include "linespec.h"
 #include "cli/cli-decode.h"
+#include "cli/cli-style.h"
 
 /* FIXME: This is needed because of lookup_cmd_1 ().  We should be
    calling a hook instead so we eliminate the CLI dependency.  */
@@ -2714,6 +2715,8 @@ gdb_fnwidth (const char *string)
   return width;
 }
 
+extern int _rl_completion_prefix_display_length;
+
 /* Print TO_PRINT, one matching completion.
    PREFIX_BYTES is number of common prefix bytes.
    Based on readline/complete.c:fnprint.  */
@@ -2722,7 +2725,7 @@ static int
 gdb_fnprint (const char *to_print, int prefix_bytes,
 	     const struct match_list_displayer *displayer)
 {
-  int printed_len, w;
+  int common_prefix_len, printed_len, w;
   const char *s;
 #if defined (HANDLE_MULTIBYTE)
   mbstate_t ps;
@@ -2735,14 +2738,18 @@ gdb_fnprint (const char *to_print, int prefix_bytes,
   memset (&ps, 0, sizeof (mbstate_t));
 #endif
 
-  printed_len = 0;
+  printed_len = common_prefix_len = 0;
 
   /* Don't print only the ellipsis if the common prefix is one of the
      possible completions */
   if (to_print[prefix_bytes] == '\0')
     prefix_bytes = 0;
 
-  if (prefix_bytes)
+  ui_file_style style = completion_prefix_style.style ();
+  if (!style.is_default ())
+    displayer->puts (displayer, style.to_ansi ().c_str ());
+
+  if (prefix_bytes && _rl_completion_prefix_display_length > 0)
     {
       char ellipsis;
 
@@ -2751,6 +2758,16 @@ gdb_fnprint (const char *to_print, int prefix_bytes,
 	displayer->putch (displayer, ellipsis);
       printed_len = ELLIPSIS_LEN;
     }
+  else if (prefix_bytes && !style.is_default ())
+    {
+      common_prefix_len = prefix_bytes;
+      prefix_bytes = 0;
+    }
+
+  /* There are 3 states: the initial state (#0), when we use the
+     prefix style; the difference state (#1), which lasts a single
+     character; and then the suffix state (#2).  */
+  int state = 0;
 
   s = to_print + prefix_bytes;
   while (*s)
@@ -2802,8 +2819,31 @@ gdb_fnprint (const char *to_print, int prefix_bytes,
 	  printed_len++;
 #endif
 	}
+      if (common_prefix_len > 0 && (s - to_print) >= common_prefix_len)
+	{
+	  if (!style.is_default ())
+	    displayer->puts (displayer, ui_file_style ().to_ansi ().c_str ());
+
+	  ++state;
+	  if (state == 1)
+	    {
+	      common_prefix_len = 1;
+	      style = completion_difference_style.style ();
+	    }
+	  else
+	    {
+	      common_prefix_len = 0;
+	      style = completion_suffix_style.style ();
+	    }
+
+	  if (!style.is_default ())
+	    displayer->puts (displayer, style.to_ansi ().c_str ());
+	}
     }
 
+  if (!style.is_default ())
+    displayer->puts (displayer, ui_file_style ().to_ansi ().c_str ());
+
   return printed_len;
 }
 
@@ -2912,7 +2952,6 @@ gdb_complete_get_screenwidth (const struct match_list_displayer *displayer)
   return displayer->width;
 }
 
-extern int _rl_completion_prefix_display_length;
 extern int _rl_print_completions_horizontally;
 
 EXTERN_C int _rl_qsort_string_compare (const void *, const void *);
@@ -2931,19 +2970,23 @@ gdb_display_match_list_1 (char **matches, int len, int max,
   char *temp, *t;
   int page_completions = displayer->height != INT_MAX && pagination_enabled;
 
+  bool want_style = !completion_prefix_style.style ().is_default ();
+
   /* Find the length of the prefix common to all items: length as displayed
      characters (common_length) and as a byte index into the matches (sind) */
   common_length = sind = 0;
-  if (_rl_completion_prefix_display_length > 0)
+  if (_rl_completion_prefix_display_length > 0 || want_style)
     {
       t = gdb_printable_part (matches[0]);
       temp = strrchr (t, '/');
       common_length = temp ? gdb_fnwidth (temp) : gdb_fnwidth (t);
       sind = temp ? strlen (temp) : strlen (t);
 
-      if (common_length > _rl_completion_prefix_display_length && common_length > ELLIPSIS_LEN)
+      if (_rl_completion_prefix_display_length > 0
+	  && common_length > _rl_completion_prefix_display_length
+	  && common_length > ELLIPSIS_LEN)
 	max -= common_length - ELLIPSIS_LEN;
-      else
+      else if (!want_style || common_length > max || sind > max)
 	common_length = sind = 0;
     }
 
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 0cb4182ef2c..49db684bc5c 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,8 @@
+2020-05-16  Tom Tromey  <tom@tromey.com>
+
+	* gdb.texinfo (Output Styling): Mention completion styling.
+	(Editing): Mention readline completion styling.
+
 2020-05-15  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
 
 	* gdb.texinfo (Help): Document the help and apropos changes.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 8f3301259af..be1f3e0f2c1 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -25270,6 +25270,10 @@ This command accepts the current line for execution and fetches the
 next line relative to the current line from the history for editing.
 Any argument is ignored.
 
+Note that @value{GDBN} ignores the Readline
+@code{colored-completion-prefix} setting.  Instead, this is handled
+using the style settings (@xref{Output Styling}).
+
 @node Command History
 @section Command History
 @cindex command history
@@ -25603,6 +25607,22 @@ general styling to @value{GDBN}.  @xref{TUI Configuration}.
 Control the styling of the active TUI border; that is, the TUI window
 that has the focus.
 
+@item completion-prefix
+Control the styling of the completion prefix.  When completing, the
+common prefix of completion candidates will be shown with this style.
+By default, this style's intensity is dim.
+
+@item completion-difference
+Control the styling of the completion difference character.  When
+completing, the character that differs between different completions
+will be shown using this style.  By default, this style's foreground
+color is magenta.
+
+@item completion-suffix
+Control the styling of the completion suffix.  When completing, the
+suffix of completion candidates will be shown with this style.  By
+default, this style is the same as the default styling.
+
 @end table
 
 @node Numbers
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index c57ddf55cbf..32e32e67068 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2020-05-16  Tom Tromey  <tom@tromey.com>
+
+	* gdb.base/style.exp: Add completion styling test.
+	* lib/gdb-utils.exp (style): Add completion styles.
+
 2020-05-15  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
 
 	* gdb.base/alias.exp: Verify 'help aliases' shows user defined aliases.
diff --git a/gdb/testsuite/gdb.base/style.exp b/gdb/testsuite/gdb.base/style.exp
index 129f1746a39..23a35403bbf 100644
--- a/gdb/testsuite/gdb.base/style.exp
+++ b/gdb/testsuite/gdb.base/style.exp
@@ -167,4 +167,18 @@ save_vars { env(TERM) } {
 	"warning: [style .*? file] is not a directory\\..*"
     gdb_test "show data-directory" \
 	"GDB's data directory is \"[style .*? file]\"\\..*"
+
+    if {[readline_is_used]} {
+	set test "complete print VALUE_"
+	# ESC-? is the readline binding to show all completions.
+	send_gdb "print VALUE_\x1b?"
+	set pfx [style VALUE_ completion-prefix]
+	set d1 [style O completion-difference]
+	set d2 [style T completion-difference]
+	gdb_test_multiple "" $test {
+	    -re "${pfx}${d1}NE\[ \t\]+${pfx}${d2}WO.*$gdb_prompt print VALUE_$" {
+		gdb_test "ONE" " = .*"
+	    }
+	}
+    }
 }
diff --git a/gdb/testsuite/lib/gdb-utils.exp b/gdb/testsuite/lib/gdb-utils.exp
index 9741f0a9591..98bdd7206a4 100644
--- a/gdb/testsuite/lib/gdb-utils.exp
+++ b/gdb/testsuite/lib/gdb-utils.exp
@@ -55,6 +55,8 @@ proc style {str style} {
 	variable { set style 36 }
 	address { set style 34 }
 	metadata { set style 2 }
+	completion-prefix { set style 2 }
+	completion-difference { set style 35 }
     }
     return "\033\\\[${style}m${str}\033\\\[m"
 }

Re: [PATCH] Add completion styling

[Eli Zaretskii]

> From: Tom Tromey <tom@tromey.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  gdb-patches@sourceware.org
> Date: Sat, 16 May 2020 13:18:47 -0600
> 
> commit 0af816c447b989a47494b3b1ff970b463b100152
> Author: Tom Tromey <tom@tromey.com>
> Date:   Sat May 16 13:17:33 2020 -0600
> 
>     Add completion styling
>     
>     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.
>     
>     This version incorporates an Emacs idea, via Eli: style the prefix,
>     the "difference character", and the suffix differently.
>     
>     gdb/ChangeLog
>     2020-05-16  Tom Tromey  <tom@tromey.com>
>     
>             * NEWS: Add entry for completion styling.
>             * completer.c (_rl_completion_prefix_display_length): Move
>             declaration earlier.
>             (gdb_fnprint): Use completion_style.
>             (gdb_display_match_list_1): Likewise.
>             * cli/cli-style.c (completion_prefix_style)
>             (completion_difference_style, completion_suffix_style): New
>             globals.
>             (_initialize_cli_style): Register new globals.
>             * cli/cli-style.h (completion_prefix_style)
>             (completion_difference_style, completion_suffix_style): Declare.
>     
>     gdb/doc/ChangeLog
>     2020-05-16  Tom Tromey  <tom@tromey.com>
>     
>             * gdb.texinfo (Output Styling): Mention completion styling.
>             (Editing): Mention readline completion styling.
>     
>     gdb/testsuite/ChangeLog
>     2020-05-16  Tom Tromey  <tom@tromey.com>
>     
>             * gdb.base/style.exp: Add completion styling test.
>             * lib/gdb-utils.exp (style): Add completion styles.
> 

Thanks, the documentation parts are okay.

Re: [PATCH] Add completion styling

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> I think the Emacs idea is nice.  Maybe we want to let the user control
Tom> the prefix text, the "difference character", and the final completion
Tom> text.

> Here's a new version of the patch that implements this idea.

I'm checking this in.

Tom

Re: [PATCH] Add completion styling

[Philippe Waroquiers]

On Sat, 2020-05-23 at 14:52 -0600, Tom Tromey wrote:
> > > > > > "Tom" == Tom Tromey <tom@tromey.com> writes:
> 
> Tom> I think the Emacs idea is nice.  Maybe we want to let the user control
> Tom> the prefix text, the "difference character", and the final completion
> Tom> text.
> 
> > Here's a new version of the patch that implements this idea.
> 
> I'm checking this in.
I played with it, nice.

A suggestion:
If easy to do, it would be nice to also have coloring in the ambiguous output:
  (gdb) di
  Ambiguous command "di": directory, dis, disa, disable, disassemble, disconnect, display.
(but I guess this might not be easy if the coloring is done by readline,
and the ambiguous list is produced by GDB).

Another suggestion:
I am wondering why completion-difference is not used to color the full set
of characters needed to make a command non-ambiguous.

For example, when completing <di>, it would be nice to color <sc> of disconnect
and color <sp> of display, <sas> of disassemble, ....

The idea is to show what must be typed specifically to have this command,
rather than show one letter but still giving something ambiguous needing
further completion.

Currently, only the 3rd letter is colored: 
e.g.<s> is colored, but it implies then to type <s> then tab
then <c> (and maybe tab again if you want to be sure) before having the command
you want.

Philippe

Re: [PATCH] Add completion styling

[Hannes Domani]

 Am Samstag, 23. Mai 2020, 22:53:10 MESZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:

> >>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
>
> Tom> I think the Emacs idea is nice.  Maybe we want to let the user control
> Tom> the prefix text, the "difference character", and the final completion
> Tom> text.
>
> > Here's a new version of the patch that implements this idea.
>
> I'm checking this in.

I just tried it out on Windows.
It works fine in TUI, but in the CLI it just prints the escape sequences:

(gdb) b
?[2mb?[m?[35ma?[mcktrace    ?[2mb?[m?[35mo?[mokmark     ?[2mb?[m?[35mr?[meak
    ?[2mb?[m?[35mr?[meak-range  ?[2mb?[m?[35mt?[m

Once I added the same gdb_console_fputs call for cli_mld_puts as
stdio_file::puts has, the problem was gone:

diff --git a/gdb/cli-out.c b/gdb/cli-out.c
index e47272ad87..1ddc80811b 100644
--- a/gdb/cli-out.c
+++ b/gdb/cli-out.c
@@ -339,6 +339,8 @@ cli_mld_putch (const struct match_list_displayer *displayer, int ch)
 static void
 cli_mld_puts (const struct match_list_displayer *displayer, const char *s)
 {
+  if (gdb_console_fputs (s, rl_outstream))
+    return;
   fputs (s, rl_outstream);
 }


Hannes

Re: [PATCH] Add completion styling

[Tom de Vries]

On 23-05-2020 22:52, Tom Tromey wrote:
>>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
> 
> Tom> I think the Emacs idea is nice.  Maybe we want to let the user control
> Tom> the prefix text, the "difference character", and the final completion
> Tom> text.
> 
>> Here's a new version of the patch that implements this idea.
> 
> I'm checking this in.

I'm seeing regressions due to this patch, sofar:
...
FAIL: gdb.base/completion.exp: complete (2) 'p 'arg' (timeout)
FAIL: gdb.base/completion.exp: complete 'handle signal' (timeout)
FAIL: gdb.base/completion.exp: complete 'handle keyword' (timeout)
FAIL: gdb.base/completion.exp: complete help aliases (timeout)
FAIL: gdb.base/completion.exp: complete 'p no_var_named_this-arg' (timeout)
FAIL: gdb.base/completion.exp: complete (2) 'p no_var_named_this-arg'
(timeout)
FAIL: gdb.base/completion.exp: complete (2) 'p no_var_named_this-' (timeout)
FAIL: gdb.base/completion.exp: complete 'p values[0].a' (timeout)
FAIL: gdb.base/completion.exp: complete 'p values[0] . a' (timeout)
FAIL: gdb.base/completion.exp: complete 'p &values[0] -> a' (timeout)
FAIL: gdb.base/completion.exp: completion of field in anonymous union
FAIL: gdb.base/completion.exp: complete 'info func marke' (timeout)
FAIL: gdb.base/completion.exp: complete 'set follow-fork-mode' (timeout)
ERROR: internal buffer is full.
ERROR: internal buffer is full.
ERROR: internal buffer is full.
ERROR: internal buffer is full.
ERROR: internal buffer is full.
ERROR: internal buffer is full.
ERROR: internal buffer is full.
ERROR: internal buffer is full.
ERROR: internal buffer is full.
ERROR: internal buffer is full.
ERROR: internal buffer is full.
ERROR: internal buffer is full.
ERROR: internal buffer is full.
ERROR: internal buffer is full.
FAIL: gdb.base/filesym.exp: completion list for "filesym" (timeout)
FAIL: gdb.base/options.exp: cmd=require-delimiter: test-misc: tab
complete "maint test-options require-delimiter -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=require-delimiter: test-flag: tab
complete "maint test-options require-delimiter  -flag -" (second tab)
(timeout)
FAIL: gdb.base/options.exp: cmd=require-delimiter: test-boolean: tab
complete "maint test-options require-delimiter -bool " (second tab)
(timeout)
FAIL: gdb.base/options.exp: cmd=require-delimiter: test-boolean: tab
complete "maint test-options require-delimiter -bool -" (second tab)
(timeout)
FAIL: gdb.base/options.exp: cmd=require-delimiter: test-boolean: tab
complete "maint test-options require-delimiter -bool o" (second tab)
(timeout)
FAIL: gdb.base/options.exp: cmd=require-delimiter: test-uinteger: tab
complete "maint test-options require-delimiter -uinteger " (second tab)
(timeout)
FAIL: gdb.base/options.exp: cmd=require-delimiter: test-uinteger: tab
complete "maint test-options require-delimiter -uinteger 1 " (second
tab) (timeout)
FAIL: gdb.base/options.exp: cmd=require-delimiter: test-uinteger: tab
complete "maint test-options require-delimiter -zuinteger-unlimited "
(second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=require-delimiter: test-uinteger: tab
complete "maint test-options require-delimiter -zuinteger-unlimited -1 "
(second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=require-delimiter: test-uinteger: tab
complete "maint test-options require-delimiter -zuinteger-unlimited 1 "
(second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=require-delimiter: test-enum: tab
complete "maint test-options require-delimiter -enum " (second tab)
(timeout)
FAIL: gdb.base/options.exp: cmd=require-delimiter: test-string: str=STR:
tab complete "maint test-options require-delimiter -string STR -"
(second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=require-delimiter: test-string:
str="STR": tab complete "maint test-options require-delimiter -string
"STR" -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=require-delimiter: test-string:
str=\"STR: tab complete "maint test-options require-delimiter -string
\"STR -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=require-delimiter: test-string:
str='STR': tab complete "maint test-options require-delimiter -string
'STR' -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=require-delimiter: test-string:
str=\'STR: tab complete "maint test-options require-delimiter -string
\'STR -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=require-delimiter: test-string: str="STR
AAA": tab complete "maint test-options require-delimiter -string "STR
AAA" -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=require-delimiter: test-string: str='STR
BBB': tab complete "maint test-options require-delimiter -string 'STR
BBB' -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=require-delimiter: test-string: str="STR
'CCC' DDD": tab complete "maint test-options require-delimiter -string
"STR 'CCC' DDD" -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=require-delimiter: test-string: str='STR
"EEE" FFF': tab complete "maint test-options require-delimiter -string
'STR "EEE" FFF' -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=require-delimiter: test-string: str="STR
\"GGG\" HHH": tab complete "maint test-options require-delimiter -string
"STR \"GGG\" HHH" -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=require-delimiter: test-string: str='STR
\'III\' JJJ': tab complete "maint test-options require-delimiter -string
'STR \'III\' JJJ' -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-error: test-misc: tab
complete "maint test-options unknown-is-error -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-error: test-flag: tab
complete "maint test-options unknown-is-error  -flag -" (second tab)
(timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-error: test-boolean: tab
complete "maint test-options unknown-is-error -bool -" (second tab)
(timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-error: test-boolean: tab
complete "maint test-options unknown-is-error -bool o" (second tab)
(timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-error: test-uinteger: tab
complete "maint test-options unknown-is-error -uinteger " (second tab)
(timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-error: test-uinteger: tab
complete "maint test-options unknown-is-error -zuinteger-unlimited "
(second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-error: test-enum: tab
complete "maint test-options unknown-is-error -enum " (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-error: test-string: str=STR:
tab complete "maint test-options unknown-is-error -string STR -" (second
tab) (timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-error: test-string:
str="STR": tab complete "maint test-options unknown-is-error -string
"STR" -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-error: test-string:
str=\"STR: tab complete "maint test-options unknown-is-error -string
\"STR -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-error: test-string:
str='STR': tab complete "maint test-options unknown-is-error -string
'STR' -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-error: test-string:
str=\'STR: tab complete "maint test-options unknown-is-error -string
\'STR -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-error: test-string: str="STR
AAA": tab complete "maint test-options unknown-is-error -string "STR
AAA" -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-error: test-string: str='STR
BBB': tab complete "maint test-options unknown-is-error -string 'STR
BBB' -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-error: test-string: str="STR
'CCC' DDD": tab complete "maint test-options unknown-is-error -string
"STR 'CCC' DDD" -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-error: test-string: str='STR
"EEE" FFF': tab complete "maint test-options unknown-is-error -string
'STR "EEE" FFF' -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-error: test-string: str="STR
\"GGG\" HHH": tab complete "maint test-options unknown-is-error -string
"STR \"GGG\" HHH" -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-error: test-string: str='STR
\'III\' JJJ': tab complete "maint test-options unknown-is-error -string
'STR \'III\' JJJ' -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-operand: test-misc: tab
complete "maint test-options unknown-is-operand -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-operand: test-flag: tab
complete "maint test-options unknown-is-operand  -flag -" (second tab)
(timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-operand: test-boolean: tab
complete "maint test-options unknown-is-operand -bool -" (second tab)
(timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-operand: test-boolean: tab
complete "maint test-options unknown-is-operand -bool o" (second tab)
(timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-operand: test-uinteger: tab
complete "maint test-options unknown-is-operand -uinteger " (second tab)
(timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-operand: test-uinteger: tab
complete "maint test-options unknown-is-operand -zuinteger-unlimited "
(second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-operand: test-enum: tab
complete "maint test-options unknown-is-operand -enum " (second tab)
(timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-operand: test-string:
str=STR: tab complete "maint test-options unknown-is-operand -string STR
-" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-operand: test-string:
str="STR": tab complete "maint test-options unknown-is-operand -string
"STR" -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-operand: test-string:
str=\"STR: tab complete "maint test-options unknown-is-operand -string
\"STR -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-operand: test-string:
str='STR': tab complete "maint test-options unknown-is-operand -string
'STR' -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-operand: test-string:
str=\'STR: tab complete "maint test-options unknown-is-operand -string
\'STR -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-operand: test-string:
str="STR AAA": tab complete "maint test-options unknown-is-operand
-string "STR AAA" -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-operand: test-string:
str='STR BBB': tab complete "maint test-options unknown-is-operand
-string 'STR BBB' -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-operand: test-string:
str="STR 'CCC' DDD": tab complete "maint test-options unknown-is-operand
-string "STR 'CCC' DDD" -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-operand: test-string:
str='STR "EEE" FFF': tab complete "maint test-options unknown-is-operand
-string 'STR "EEE" FFF' -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-operand: test-string:
str="STR \"GGG\" HHH": tab complete "maint test-options
unknown-is-operand -string "STR \"GGG\" HHH" -" (second tab) (timeout)
FAIL: gdb.base/options.exp: cmd=unknown-is-operand: test-string:
str='STR \'III\' JJJ': tab complete "maint test-options
unknown-is-operand -string 'STR \'III\' JJJ' -" (second tab) (timeout)
FAIL: gdb.base/options.exp: test-print: tab complete "print -" (second
tab) (timeout)
FAIL: gdb.base/options.exp: test-print: tab complete "frame apply all
print -" (second tab) (timeout)
FAIL: gdb.base/options.exp: test-print: tab complete "frame apply 1
print -" (second tab) (timeout)
FAIL: gdb.base/options.exp: test-print: tab complete "frame apply level
0 print -" (second tab) (timeout)
FAIL: gdb.base/options.exp: test-print: tab complete "thread apply all
print -" (second tab) (timeout)
FAIL: gdb.base/options.exp: test-print: tab complete "thread apply 1
print -" (second tab) (timeout)
FAIL: gdb.base/options.exp: test-print: tab complete "thread apply 1
frame apply 1 print -" (second tab) (timeout)
FAIL: gdb.base/options.exp: test-backtrace: tab complete "backtrace -"
(second tab) (timeout)
FAIL: gdb.base/options.exp: test-frame-apply: tab complete "frame apply
all " (second tab) (timeout)
FAIL: gdb.base/options.exp: test-frame-apply: tab complete "frame apply
all -" (second tab) (timeout)
FAIL: gdb.base/options.exp: test-frame-apply: trailing-space: tab
complete "frame apply all -- " (second tab) (timeout)
FAIL: gdb.base/options.exp: test-frame-apply: tab complete "frame apply
all -s " (second tab) (timeout)
FAIL: gdb.base/options.exp: test-frame-apply: tab complete "frame apply
1 " (second tab) (timeout)
FAIL: gdb.base/options.exp: test-frame-apply: tab complete "frame apply
1 -" (second tab) (timeout)
FAIL: gdb.base/options.exp: test-frame-apply: trailing-space: tab
complete "frame apply 1 -- " (second tab) (timeout)
FAIL: gdb.base/options.exp: test-frame-apply: tab complete "frame apply
1 -s " (second tab) (timeout)
FAIL: gdb.base/options.exp: test-frame-apply: tab complete "frame apply
level 0 " (second tab) (timeout)
FAIL: gdb.base/options.exp: test-frame-apply: tab complete "frame apply
level 0 -" (second tab) (timeout)
FAIL: gdb.base/options.exp: test-frame-apply: trailing-space: tab
complete "frame apply level 0 -- " (second tab) (timeout)
FAIL: gdb.base/options.exp: test-frame-apply: tab complete "frame apply
level 0 -s " (second tab) (timeout)
FAIL: gdb.base/options.exp: test-frame-apply: tab complete "faas "
(second tab) (timeout)
FAIL: gdb.base/options.exp: test-frame-apply: tab complete "faas -"
(second tab) (timeout)
FAIL: gdb.base/options.exp: test-frame-apply: trailing-space: tab
complete "faas -- " (second tab) (timeout)
FAIL: gdb.base/options.exp: test-frame-apply: tab complete "faas -s "
(second tab) (timeout)
FAIL: gdb.base/options.exp: test-frame-apply: tab complete "tfaas "
(second tab) (timeout)
FAIL: gdb.base/options.exp: test-frame-apply: tab complete "tfaas -"
(second tab) (timeout)
FAIL: gdb.base/options.exp: test-frame-apply: trailing-space: tab
complete "tfaas -- " (second tab) (timeout)
FAIL: gdb.base/options.exp: test-frame-apply: tab complete "tfaas -s "
(second tab) (timeout)
FAIL: gdb.base/options.exp: test-thread-apply: tab complete "thread
apply all " (second tab) (timeout)
FAIL: gdb.base/options.exp: test-thread-apply: tab complete "thread
apply all -" (second tab) (timeout)
FAIL: gdb.base/options.exp: test-thread-apply: trailing-space: tab
complete "thread apply all -- " (second tab) (timeout)
FAIL: gdb.base/options.exp: test-thread-apply: tab complete "thread
apply all -c " (second tab) (timeout)
FAIL: gdb.base/options.exp: test-thread-apply: tab complete "thread
apply 1 " (second tab) (timeout)
FAIL: gdb.base/options.exp: test-thread-apply: tab complete "thread
apply 1 -" (second tab) (timeout)
FAIL: gdb.base/options.exp: test-thread-apply: trailing-space: tab
complete "thread apply 1 -- " (second tab) (timeout)
FAIL: gdb.base/options.exp: test-thread-apply: tab complete "thread
apply 1 -c " (second tab) (timeout)
FAIL: gdb.base/options.exp: test-thread-apply: tab complete "taas "
(second tab) (timeout)
FAIL: gdb.base/options.exp: test-thread-apply: tab complete "taas -"
(second tab) (timeout)
FAIL: gdb.base/options.exp: test-thread-apply: trailing-space: tab
complete "taas -- " (second tab) (timeout)
FAIL: gdb.base/options.exp: test-thread-apply: tab complete "taas -c "
(second tab) (timeout)
FAIL: gdb.base/options.exp: test-info-threads: tab complete "info
threads " (second tab) (timeout)
FAIL: gdb.base/readline-ask.exp: more message for 01 and 02 (timeout)
FAIL: gdb.base/readline-ask.exp: more message for 03 (timeout)
FAIL: gdb.base/readline-ask.exp: more finish for 04 (timeout)
FAIL: gdb.base/readline-ask.exp: ask message for 01 and 02 (timeout)
FAIL: gdb.base/settings.exp: test-integer zuinteger: tab complete "maint
set test-settings zuinteger" (second tab) (timeout)
FAIL: gdb.base/settings.exp: test-integer zuinteger: tab complete
"maintenance show test-settings zuinteger" (second tab) (timeout)
FAIL: gdb.base/settings.exp: test-boolean: tab complete "maint set
test-settings boolean " (second tab) (timeout)
FAIL: gdb.base/settings.exp: test-auto-boolean: tab complete "maint set
test-settings auto-boolean " (second tab) (timeout)
FAIL: gdb.base/settings.exp: test-enum: tab complete "maint set
test-settings enum " (second tab) (timeout)
FAIL: gdb.base/settings.exp: test-string string: tab complete "maint
show test-settings string" (second tab) (timeout)
FAIL: gdb.base/shell.exp: tab complete "pipe " (second tab) (timeout)
FAIL: gdb.base/shell.exp: tab complete "| " (second tab) (timeout)
FAIL: gdb.base/shell.exp: tab complete "|" (second tab) (timeout)
FAIL: gdb.base/skip.exp: skip delete completion: tab complete "skip
delete " (second tab) (timeout)
FAIL: gdb.base/skip.exp: skip delete completion: tab complete "skip
delete 1" (second tab) (timeout)
FAIL: gdb.base/skip.exp: skip delete completion: tab complete "skip
delete 2 " (second tab) (timeout)
FAIL: gdb.base/skip.exp: skip delete completion: tab complete "skip
delete 2-" (second tab) (timeout)
FAIL: gdb.base/with.exp: completion: tab complete "with print elements
unlimited -- " (second tab) (timeout)
FAIL: gdb.cp/cpcompletion.exp: expression with namespace: tab complete
"p Test_NS" (second tab) (timeout)
FAIL: gdb.cp/cpcompletion.exp: expression with namespace: tab complete
"p Test_NS:" (second tab) (timeout)
FAIL: gdb.cp/cpcompletion.exp: expression with namespace: tab complete
"p Test_NS::" (second tab) (timeout)
...

Thanks,
- Tom

Re: [PATCH] Add completion styling

[Pedro Alves]

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

Re: [PATCH] Add completion styling

[Eli Zaretskii]

> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Sun, 24 May 2020 13:58:02 +0100
> 
> 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()
>                                     ^

Really? not here:

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

But I actually don't understand how did we get these candidates by
typing "main".  Don't we complete by looking for strings that begin
with what the user typed?  I'm probably missing something here.

Re: [PATCH] Add completion styling

[Tom Tromey]

>> Here's a new version of the patch that implements this idea.

Tom> I'm checking this in.

Ok, based on the feedback so far, I am going to back this patch out.  I
can't do it immediately, but probably sometime this weekend.

Tom

Re: [PATCH] Add completion styling

[Pedro Alves]

On 5/24/20 3:42 PM, Eli Zaretskii wrote:
>> Cc: gdb-patches@sourceware.org
>> From: Pedro Alves <palves@redhat.com>
>> Date: Sun, 24 May 2020 13:58:02 +0100
>>
>> 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()
>>                                     ^
> 
> Really? not here:
> 
>  selftests::string_view::cons_1::main()
>                               ^

I don't understand.  Under the "1"?  Why?  
My understanding is that "set style completion-difference" highlights the
first character that follows the string that the user typed, right?
So if you type "b main[TAB]", for the "selftests::string_view::cons_1::main()"
symbol, after "main" comes the open parenthesis.  Just like here,
you get a highlight under the "t" in "maint":

maintenance_translate_address(char const*, int)
    ^

> But I actually don't understand how did we get these candidates by
> typing "main".  Don't we complete by looking for strings that begin
> with what the user typed?  I'm probably missing something here.

You're missing that "b main" sets a breakpoint on every "main"
function in every namespace.  "b -qualified main" disables that.

Thanks,
Pedro Alves

Re: [PATCH] Add completion styling

[Eli Zaretskii]

> Cc: tom@tromey.com, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Sun, 24 May 2020 16:30:07 +0100
> 
> >> while it should highlight here:
> >>
> >> selftests::string_view::cons_1::main()
> >> selftests::string_view::cons_2::main()
> >> selftests::string_view::cons_3::main()
> >>                                     ^
> > 
> > Really? not here:
> > 
> >  selftests::string_view::cons_1::main()
> >                               ^
> 
> I don't understand.  Under the "1"?  Why?  

because that's the first difference between the candidates.

> My understanding is that "set style completion-difference" highlights the
> first character that follows the string that the user typed, right?

No, it highlights the first character from the left that distinguishes
between candidates.  That is the character the user _has_ to type to
narrow his/her choice, potentially resolving it to a single candidate.

> > But I actually don't understand how did we get these candidates by
> > typing "main".  Don't we complete by looking for strings that begin
> > with what the user typed?  I'm probably missing something here.
> 
> You're missing that "b main" sets a breakpoint on every "main"
> function in every namespace.

OK, but then I still stand by my expectations.  Although implementing
that may not be easy given that the candidates don't start with what
the user typed.

Re: [PATCH] Add completion styling

[Pedro Alves]

On 5/24/20 5:29 PM, Eli Zaretskii wrote:
>> Cc: tom@tromey.com, gdb-patches@sourceware.org
>> From: Pedro Alves <palves@redhat.com>
>> Date: Sun, 24 May 2020 16:30:07 +0100
>>
>>>> while it should highlight here:
>>>>
>>>> selftests::string_view::cons_1::main()
>>>> selftests::string_view::cons_2::main()
>>>> selftests::string_view::cons_3::main()
>>>>                                     ^
>>>
>>> Really? not here:
>>>
>>>  selftests::string_view::cons_1::main()
>>>                               ^
>>
>> I don't understand.  Under the "1"?  Why?  
> 
> because that's the first difference between the candidates.
> 
>> My understanding is that "set style completion-difference" highlights the
>> first character that follows the string that the user typed, right?
> 
> No, it highlights the first character from the left that distinguishes
> between candidates.  That is the character the user _has_ to type to
> narrow his/her choice, potentially resolving it to a single candidate.

Well, that's the same thing, but expressed better.

I just didn't realize that you were looking at the whole prefix.
It's obvious to me now what you mean.

> 
>>> But I actually don't understand how did we get these candidates by
>>> typing "main".  Don't we complete by looking for strings that begin
>>> with what the user typed?  I'm probably missing something here.
>>
>> You're missing that "b main" sets a breakpoint on every "main"
>> function in every namespace.
> 
> OK, but then I still stand by my expectations.  Although implementing
> that may not be easy given that the candidates don't start with what
> the user typed.

I know from experience that getting the lower common denominator to
not be a simple common prefix of the completion matches wasn't easy.
But it works now, and GDB has all the info, since it is GDB that informs
GDB about the common prefix, so it should be possible.

Thanks,
Pedro Alves

Re: [PATCH] Add completion styling

[Tom de Vries]

On 24-05-2020 12:16, Tom de Vries wrote:
> On 23-05-2020 22:52, Tom Tromey wrote:
>>>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
>>
>> Tom> I think the Emacs idea is nice.  Maybe we want to let the user control
>> Tom> the prefix text, the "difference character", and the final completion
>> Tom> text.
>>
>>> Here's a new version of the patch that implements this idea.
>>
>> I'm checking this in.
> 
> I'm seeing regressions due to this patch, sofar:
> ...
> FAIL: gdb.base/completion.exp: complete (2) 'p 'arg' (timeout)

FWIW, I've looked into this FAIL and found that it's due to this regexp:
...
            -re "argv.*$gdb_prompt " {
...
trying to match:
...
^[[2marg^[[m^[[35mv^[[m
...

Thanks,
- Tom