Re: [PATCH] Add completion styling

[Tom Tromey <tom@tromey.com>]

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