Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: gdb-patches@sourceware.org
Subject: [pushed] Fix PR tui/21216: TUI line breaks regression
Date: Wed, 08 Mar 2017 00:19:00 -0000	[thread overview]
Message-ID: <1488932352-10885-3-git-send-email-palves@redhat.com> (raw)

Commit d7e747318f4d04 ("Eliminate make_cleanup_ui_file_delete / make
ui_file a class hierarchy") regressed the TUI's command window.
Newlines miss doing a "carriage return", resulting in output like:

~~~~~~~~~~~~~~~~~~
(gdb) helpList of classes of commands:

                                      aliases -- Aliases of other commands
                                                                          breakpoints -- Making program stop at certain points
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Before the commit mentioned above, the default ui_file->to_write
implementation had a hack that would defer into the ui_file->to_fputs
method.  The TUI's ui_file did not implement the to_write method, so
all writes would end up going to the ncurses window via tui_file_fputs
-> tui_puts.

After the commit above, the hack is gone, but the TUI's ui_file still
does not implement the ui_file::write method.  Since tui_file inherits
from stdio_file, writing to a tui_file ends up doing fwrite on the
FILE stream the TUI is "associated" with, via stdio_file::write,
instead of writing to the ncurses window.

The fix is to have tui_file override the "write" method.

New test included.

gdb/ChangeLog:
2017-03-08  Pedro Alves  <palves@redhat.com>

	PR tui/21216
	* tui/tui-file.c (tui_file::write): New.
	* tui/tui-file.h (tui_file): Override "write".
	* tui/tui-io.c (do_tui_putc, update_start_line): New functions,
	factored out from ...
	(tui_puts): ... here.
	(tui_putc): Use them.
	(tui_write): New function.
	* tui/tui-io.h (tui_write): Declare.

gdb/testsuite/ChangeLog:
2017-03-08  Pedro Alves  <palves@redhat.com>

	PR tui/21216
	* gdb.tui/tui-nl-filtered-output.exp: New file.
---
 gdb/ChangeLog                                    |  12 +++
 gdb/testsuite/ChangeLog                          |   5 +
 gdb/testsuite/gdb.tui/tui-nl-filtered-output.exp |  57 ++++++++++++
 gdb/tui/tui-file.c                               |  10 ++
 gdb/tui/tui-file.h                               |   3 +-
 gdb/tui/tui-io.c                                 | 114 +++++++++++++++--------
 gdb/tui/tui-io.h                                 |   4 +
 7 files changed, 165 insertions(+), 40 deletions(-)
 create mode 100644 gdb/testsuite/gdb.tui/tui-nl-filtered-output.exp

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3a156ad..432cdcc 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,15 @@
+2017-03-08  Pedro Alves  <palves@redhat.com>
+
+	PR tui/21216
+	* tui/tui-file.c (tui_file::write): New.
+	* tui/tui-file.h (tui_file): Override "write".
+	* tui/tui-io.c (do_tui_putc, update_start_line): New functions,
+	factored out from ...
+	(tui_puts): ... here.
+	(tui_putc): Use them.
+	(tui_write): New function.
+	* tui/tui-io.h (tui_write): Declare.
+
 2017-03-07  Sergio Durigan Junior  <sergiodj@redhat.com>
 
 	* Makefile.in (SFILES): Replace "environ.c" with
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index f986520..0654bef 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,10 @@
 2017-03-08  Pedro Alves  <palves@redhat.com>
 
+	PR tui/21216
+	* gdb.tui/tui-nl-filtered-output.exp: New file.
+
+2017-03-08  Pedro Alves  <palves@redhat.com>
+
 	* gdb.base/completion.exp: Move TUI completion tests to ...
 	* gdb.tui/completion.exp: ... this new file.
 
diff --git a/gdb/testsuite/gdb.tui/tui-nl-filtered-output.exp b/gdb/testsuite/gdb.tui/tui-nl-filtered-output.exp
new file mode 100644
index 0000000..d1f56f2
--- /dev/null
+++ b/gdb/testsuite/gdb.tui/tui-nl-filtered-output.exp
@@ -0,0 +1,57 @@
+# Copyright 2017 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/>.
+
+# Regression test for PR tui/21216 - TUI line breaks regression.
+#
+# Tests that newlines in filtered output force a "carriage return" in
+# the TUI command window.  With a broken gdb, instead of:
+#
+#  (gdb) printf "hello\nworld\n"
+#  hello
+#  world
+#  (gdb)
+#
+# we'd get:
+#
+#  (gdb) printf "hello\nworld\n"hello
+#                                    world
+#
+#  (gdb)
+
+gdb_exit
+gdb_start
+
+if {[skip_tui_tests]} {
+    return
+}
+
+# Enable the TUI.
+
+set test "tui enable"
+gdb_test_multiple "tui enable" $test {
+    -re "$gdb_prompt $" {
+	pass $test
+    }
+}
+
+# Make sure filtering/pagination is enabled, but make the window high
+# enough that we don't paginate in practice.
+gdb_test_no_output "set pagination on"
+gdb_test_no_output "set height 2000"
+
+gdb_test \
+    {printf "hello\nworld\n"} \
+    "hello\r\nworld" \
+    "correct line breaks"
diff --git a/gdb/tui/tui-file.c b/gdb/tui/tui-file.c
index 2f895b7..80a31f8 100644
--- a/gdb/tui/tui-file.c
+++ b/gdb/tui/tui-file.c
@@ -44,6 +44,16 @@ tui_file::puts (const char *linebuffer)
 }
 
 void
+tui_file::write (const char *buf, long length_buf)
+{
+  tui_write (buf, length_buf);
+  /* gdb_stdout is buffered, and the caller must gdb_flush it at
+     appropriate times.  Other streams are not so buffered.  */
+  if (this != gdb_stdout)
+    tui_refresh_cmd_win ();
+}
+
+void
 tui_file::flush ()
 {
   /* gdb_stdout is buffered.  Other files are always flushed on
diff --git a/gdb/tui/tui-file.h b/gdb/tui/tui-file.h
index aceaab6..c426a83 100644
--- a/gdb/tui/tui-file.h
+++ b/gdb/tui/tui-file.h
@@ -28,8 +28,9 @@ class tui_file : public stdio_file
 public:
   explicit tui_file (FILE *stream);
 
-  void flush () override;
+  void write (const char *buf, long length_buf) override;
   void puts (const char *) override;
+  void flush () override;
 };
 
 #endif
diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index 433762b..ba1ee9a 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -137,58 +137,94 @@ static int tui_readline_pipe[2];
    This may be the main gdb prompt or a secondary prompt.  */
 static char *tui_rl_saved_prompt;
 
+/* Print a character in the curses command window.  The output is
+   buffered.  It is up to the caller to refresh the screen if
+   necessary.  */
+
+static void
+do_tui_putc (WINDOW *w, char c)
+{
+  static int tui_skip_line = -1;
+
+  /* Catch annotation and discard them.  We need two \032 and discard
+     until a \n is seen.  */
+  if (c == '\032')
+    {
+      tui_skip_line++;
+    }
+  else if (tui_skip_line != 1)
+    {
+      tui_skip_line = -1;
+      /* Expand TABs, since ncurses on MS-Windows doesn't.  */
+      if (c == '\t')
+	{
+	  int col;
+
+	  col = getcurx (w);
+	  do
+	    {
+	      waddch (w, ' ');
+	      col++;
+	    }
+	  while ((col % 8) != 0);
+	}
+      else
+	waddch (w, c);
+    }
+  else if (c == '\n')
+    tui_skip_line = -1;
+}
+
+/* Update the cached value of the command window's start line based on
+   the window's current Y coordinate.  */
+
+static void
+update_cmdwin_start_line ()
+{
+  TUI_CMD_WIN->detail.command_info.start_line
+    = getcury (TUI_CMD_WIN->generic.handle);
+}
+
+/* Print a character in the curses command window.  The output is
+   buffered.  It is up to the caller to refresh the screen if
+   necessary.  */
+
 static void
 tui_putc (char c)
 {
-  char buf[2];
+  WINDOW *w = TUI_CMD_WIN->generic.handle;
+
+  do_tui_putc (w, c);
+  update_cmdwin_start_line ();
+}
+
+/* Print LENGTH characters from the buffer pointed to by BUF to the
+   curses command window.  The output is buffered.  It is up to the
+   caller to refresh the screen if necessary.  */
+
+void
+tui_write (const char *buf, size_t length)
+{
+  WINDOW *w = TUI_CMD_WIN->generic.handle;
 
-  buf[0] = c;
-  buf[1] = 0;
-  tui_puts (buf);
+  for (size_t i = 0; i < length; i++)
+    do_tui_putc (w, buf[i]);
+  update_cmdwin_start_line ();
 }
 
-/* Print the 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.  The output is
+   buffered.  It is up to the caller to refresh the screen if
+   necessary.  */
 
 void
 tui_puts (const char *string)
 {
-  static int tui_skip_line = -1;
+  WINDOW *w = TUI_CMD_WIN->generic.handle;
   char c;
-  WINDOW *w;
 
-  w = TUI_CMD_WIN->generic.handle;
   while ((c = *string++) != 0)
-    {
-      /* Catch annotation and discard them.  We need two \032 and
-         discard until a \n is seen.  */
-      if (c == '\032')
-        {
-          tui_skip_line++;
-        }
-      else if (tui_skip_line != 1)
-        {
-          tui_skip_line = -1;
-	  /* Expand TABs, since ncurses on MS-Windows doesn't.  */
-	  if (c == '\t')
-	    {
-	      int col;
-
-	      col = getcurx (w);
-	      do
-		{
-		  waddch (w, ' ');
-		  col++;
-		} while ((col % 8) != 0);
-	    }
-	  else
-	    waddch (w, c);
-        }
-      else if (c == '\n')
-        tui_skip_line = -1;
-    }
-  TUI_CMD_WIN->detail.command_info.start_line = getcury (w);
+    do_tui_putc (w, c);
+  update_cmdwin_start_line ();
 }
 
 /* Readline callback.
diff --git a/gdb/tui/tui-io.h b/gdb/tui/tui-io.h
index e1d5f86..3bd465f 100644
--- a/gdb/tui/tui-io.h
+++ b/gdb/tui/tui-io.h
@@ -28,6 +28,10 @@ class cli_ui_out;
 /* Print the string in the curses command window.  */
 extern void tui_puts (const char *);
 
+/* Print LENGTH characters from the buffer pointed to by BUF to the
+   curses command window.  */
+extern void tui_write (const char *buf, size_t length);
+
 /* Setup the IO for curses or non-curses mode.  */
 extern void tui_setup_io (int mode);
 
-- 
2.5.5


             reply	other threads:[~2017-03-08  0:19 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-08  0:19 Pedro Alves [this message]
2017-03-09 23:04 ` Jan Kratochvil
2017-03-10 12:59   ` Jan Kratochvil
2017-03-10 13:33     ` Pedro Alves
2017-03-10 14:04       ` Jan Kratochvil
2017-03-10 17:20         ` Pedro Alves
2017-03-10 17:27           ` Jan Kratochvil
2017-03-10 18:17             ` Pedro Alves
2017-03-10 18:46               ` Jan Kratochvil
2017-03-10 18:55                 ` Pedro Alves

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1488932352-10885-3-git-send-email-palves@redhat.com \
    --to=palves@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox