Re: [PATCH] [gdb/tui] Handle unicode chars in prompt

[Tom de Vries <tdevries@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 1441 bytes --]

On 5/30/23 19:03, Tom Tromey wrote:
>>> In TUI, the prompt is written out by tui_puts_internal, which outputs one byte
>>> at a time using waddch, which apparantly breaks multi-byte char support.
>>> Fix this by detecting multi-byte chars in tui_puts_internal, and
>>> printing them using
>>> waddnstr.
> 
>> FWIW, I just came across this commit, which seems relevant:
> 
> Tom> Note that tui_puts_internal remains.  It is needed to handle computing
> Tom> the start line of the readline prompt, which is difficult to do
> Tom> properly in the case where redisplaying can also cause the command
> Tom> window to scroll.  This might be possible to implement by reverting to
> Tom> single "character" output, by using mbsrtowcs for its side effects to
> Tom> find character boundaries in the input.  I have not attempted this.
> Tom> ...
> 
> I no longer remember what made this difficult.  I wonder if it's
> possible to simply emit as many characters as possible in a single call,
> and then use getyx to figure out the length of the prompt after it has
> been fully displayed.  If the prompt wraps or if it takes multiple
> lines, offhand it seems fine to just pick whatever the final column
> happens to be.

I've given that a try, and that seems to work.

I also realized that we don't cover wrapping prompts in the testsuite, 
so I wrote a test-case ( 
https://sourceware.org/pipermail/gdb-patches/2023-May/199950.html ).

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-tui-Handle-unicode-chars-in-prompt.patch --]
[-- Type: text/x-patch, Size: 6736 bytes --]

From 734e62fb3db7a25a69db0fa25f8820fc2ba88bb7 Mon Sep 17 00:00:00 2001
From: Tom de Vries <tdevries@suse.de>
Date: Wed, 24 May 2023 19:54:34 +0200
Subject: [PATCH] [gdb/tui] Handle unicode chars in prompt
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Let's try to set the prompt using a unicode character, say '❯', aka U+276F
(heavy right-pointing angle quotation mark ornament).

This works fine on an xterm with CLI (with X marking the position of the
blinking cursor):
...
$ gdb -q -ex "set prompt GDB❯ "
GDB❯ X
...
but with TUI:
...
$ gdb -q -tui -ex "set prompt GDB❯ "
...
we get instead:
...
GDB  GDB  X
...

We can use the test-case gdb.tui/unicode-prompt.exp to get more details, using
tuiterm.

With Term::dump_screen we have:
...
   16 (gdb) set prompt GDB❯
   17 GDB❯ GDB❯ GDB❯ set prompt (gdb)
   18 (gdb)
...
and with Term::dump_screen_with_attrs (summarizing using attribute sets <attrs1>
and <attrs2>):
...
   16 (gdb) set prompt GDB❯
   17 GDB<attrs1>❯<attrs2> GDB<attrs1>❯<attrs2> GDB<attrs1>❯<attrs2> set prompt (gdb)
   18 (gdb)
...
where:
...
<attrs1> == <reverse:1><invisible:1><blinking:1><intensity:bold>
<attrs2> == <reverse:0><invisible:0><blinking:0><intensity:normal>
...

This explains why we didn't see the unicode char on xterm: it's hidden
because the invisible attribute is set.

So, there seem to be two problems:
- the attributes are incorrect, and
- the prompt is repeated a couple of times.

In TUI, the prompt is written out by tui_puts_internal, which outputs one byte
at a time using waddch, which apparantly breaks multi-byte char support.

In contrast, tui_puts splits up the string using separators "\n\1\2\033\t" and
prints out the bits inbetween using waddnstr.

Fix this by:
- factoring out new function tui_puts_1 out of tui_puts, and
- using it in tui_puts_internal.

Tested on x86_64-linux.

Reported-By: wuzy01@qq.com

PR tui/28800
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28800
---
 gdb/testsuite/gdb.tui/unicode-prompt.exp | 45 ++++++++++++++
 gdb/tui/tui-io.c                         | 76 ++++++++++++------------
 2 files changed, 82 insertions(+), 39 deletions(-)
 create mode 100644 gdb/testsuite/gdb.tui/unicode-prompt.exp

diff --git a/gdb/testsuite/gdb.tui/unicode-prompt.exp b/gdb/testsuite/gdb.tui/unicode-prompt.exp
new file mode 100644
index 00000000000..7cd38731e83
--- /dev/null
+++ b/gdb/testsuite/gdb.tui/unicode-prompt.exp
@@ -0,0 +1,45 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test a prompt with a unicode char in TUI.
+
+require allow_tui_tests
+
+tuiterm_env
+
+save_vars { env(LC_ALL) } {
+    # Override "C" setting from default_gdb_init.
+    setenv LC_ALL "C.UTF-8"
+
+    Term::clean_restart 24 80
+
+    if {![Term::enter_tui]} {
+	unsupported "TUI not supported"
+	return
+    }
+
+    set unicode_char "\u276F"
+
+    set prompt "GDB$unicode_char "
+    set prompt_re [string_to_regexp $prompt]
+
+    # Set new prompt.
+    send_gdb "set prompt $prompt\n"
+    # Set old prompt back.
+    send_gdb "set prompt (gdb) \n"
+
+    gdb_assert { [Term::wait_for "^${prompt_re}set prompt $gdb_prompt "] } \
+	"prompt with unicode char"
+}
diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index a1eadcd937d..40b40717dbd 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -451,16 +451,18 @@ tui_write (const char *buf, size_t length)
   tui_puts (copy.c_str ());
 }
 
-/* Print a string in the curses command window.  The output is
-   buffered.  It is up to the caller to refresh the screen if
-   necessary.  */
+/* Print a STRING in the curses command window W.  Set *SAW_NL to true if the
+   STRING contains a newline.  */
 
-void
-tui_puts (const char *string, WINDOW *w)
+static void
+tui_puts_1 (const char *string, WINDOW *w, bool *saw_nl)
 {
   if (w == nullptr)
     w = TUI_CMD_WIN->handle.get ();
 
+  if (saw_nl != nullptr)
+    *saw_nl = false;
+
   while (true)
     {
       const char *next = strpbrk (string, "\n\1\2\033\t");
@@ -485,6 +487,10 @@ tui_puts (const char *string, WINDOW *w)
 	  break;
 
 	case '\n':
+	  if (saw_nl != nullptr)
+	    *saw_nl = true;
+	  /* FALLTHROUGH */
+
 	case '\t':
 	  do_tui_putc (w, c);
 	  ++next;
@@ -504,57 +510,49 @@ tui_puts (const char *string, WINDOW *w)
 	  break;
 
 	default:
-	  gdb_assert_not_reached ("missing case in tui_puts");
+	  gdb_assert_not_reached ("missing case in tui_puts_1");
 	}
 
       string = next;
     }
+}
+
+/* Print a string in the curses command window.  The output is
+   buffered.  It is up to the caller to refresh the screen if
+   necessary.  */
+
+void
+tui_puts (const char *string, WINDOW *w)
+{
+  if (w == nullptr)
+    w = TUI_CMD_WIN->handle.get ();
+
+  tui_puts_1 (string, w, nullptr);
 
   if (TUI_CMD_WIN != nullptr && w == TUI_CMD_WIN->handle.get ())
     update_cmdwin_start_line ();
 }
 
+/* Print a STRING in the curses command window W.  Update HEIGHT according to
+   line wraps.  */
+
 static void
 tui_puts_internal (WINDOW *w, const char *string, int *height)
 {
-  char c;
-  int prev_col = 0;
-  bool saw_nl = false;
-
-  while ((c = *string++) != 0)
-    {
-      if (c == '\n')
-	saw_nl = true;
+  bool saw_nl;
+  int prev_line = getcury (w);
 
-      if (c == '\1' || c == '\2')
-	{
-	  /* Ignore these, they are readline escape-marking
-	     sequences.  */
-	}
-      else
-	{
-	  if (c == '\033')
-	    {
-	      size_t bytes_read = apply_ansi_escape (w, string - 1);
-	      if (bytes_read > 0)
-		{
-		  string = string + bytes_read - 1;
-		  continue;
-		}
-	    }
-	  do_tui_putc (w, c);
+  tui_puts_1 (string, w, &saw_nl);
 
-	  if (height != nullptr)
-	    {
-	      int col = getcurx (w);
-	      if (col <= prev_col)
-		++*height;
-	      prev_col = col;
-	    }
-	}
+  if (height != nullptr)
+    {
+      int line = getcury (w);
+      *height += line - prev_line;
     }
+
   if (TUI_CMD_WIN != nullptr && w == TUI_CMD_WIN->handle.get ())
     update_cmdwin_start_line ();
+
   if (saw_nl)
     wrefresh (w);
 }

base-commit: 768d1d879be2d134e049521f28d4d5e03b69bafc
-- 
2.35.3