* [PATCH 0/2] tui input related fixes @ 2021-01-22 19:08 Andrew Burgess 2021-01-22 19:08 ` [PATCH 1/2] gdb/testsuite: fix implementation of delete line in tuiterm.exp Andrew Burgess 2021-01-22 19:08 ` [PATCH 2/2] gdb/tui: fix issue with handling the return character Andrew Burgess 0 siblings, 2 replies; 6+ messages in thread From: Andrew Burgess @ 2021-01-22 19:08 UTC (permalink / raw) To: gdb-patches This started as a desire to support some limited scrolling in our tuiterm.exp library. Turns out there's a bug in GDB's TUI input handling AND the tuiterm library. Patch #1 fixes tuiterm.exp. Patch #2 fixes GDB. --- Andrew Burgess (2): gdb/testsuite: fix implementation of delete line in tuiterm.exp gdb/tui: fix issue with handling the return character gdb/ChangeLog | 10 +++ gdb/testsuite/ChangeLog | 14 ++++ gdb/testsuite/gdb.tui/scroll.exp | 52 +++++++++++++ .../gdb.tui/tui-layout-asm-short-prog.exp | 4 +- gdb/testsuite/lib/tuiterm.exp | 49 ++++++++----- gdb/tui/tui-interp.c | 23 +++++- gdb/tui/tui-io.c | 73 ++++++++++--------- gdb/tui/tui-io.h | 5 ++ 8 files changed, 177 insertions(+), 53 deletions(-) create mode 100644 gdb/testsuite/gdb.tui/scroll.exp -- 2.25.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] gdb/testsuite: fix implementation of delete line in tuiterm.exp 2021-01-22 19:08 [PATCH 0/2] tui input related fixes Andrew Burgess @ 2021-01-22 19:08 ` Andrew Burgess 2021-02-06 19:17 ` Tom Tromey 2021-01-22 19:08 ` [PATCH 2/2] gdb/tui: fix issue with handling the return character Andrew Burgess 1 sibling, 1 reply; 6+ messages in thread From: Andrew Burgess @ 2021-01-22 19:08 UTC (permalink / raw) To: gdb-patches The implementation of the delete line escape sequence in tuiterm.exp was wrong. Delete should take a count and then delete COUNT lines at the current cursor location, all remaining lines in the scroll region are moved up to replace the deleted lines, with blank lines being added at the end of the scroll region. It's not clear to me what "scroll region" means here (or at least how that is defined), but for now I'm just treating the whole screen as the scroll region, which seems to work fine. In contrast the current broken implementation deletes COUNT lines at the cursor location moving the next COUNT lines up to fill the gap. The rest of the screen is then cleared. gdb/testsuite/ChangeLog: * gdb.tui/scroll.exp: New file. * gdb.tui/tui-layout-asm-short-prog.exp: Update expected results. * lib/tuiterm.exp (Term::_csi_M): Delete count lines, scroll remaining lines up. (Term::check_region_contents): New proc. (Term::check_box_contents): Use check_region_contents. --- gdb/testsuite/ChangeLog | 9 +++ gdb/testsuite/gdb.tui/scroll.exp | 62 +++++++++++++++++++ .../gdb.tui/tui-layout-asm-short-prog.exp | 4 +- gdb/testsuite/lib/tuiterm.exp | 49 ++++++++++----- 4 files changed, 106 insertions(+), 18 deletions(-) create mode 100644 gdb/testsuite/gdb.tui/scroll.exp diff --git a/gdb/testsuite/gdb.tui/scroll.exp b/gdb/testsuite/gdb.tui/scroll.exp new file mode 100644 index 00000000000..4566bd8f5c7 --- /dev/null +++ b/gdb/testsuite/gdb.tui/scroll.exp @@ -0,0 +1,62 @@ +# Copyright 2021 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/>. + +# Check scrolling in the command window. This test only covers the +# case where scrolling in the command window is caused by issuing many +# non-inferior related commands, as once the inferior is given control +# the terminal settings are modified and our tuiterm library really +# gets confused. + +tuiterm_env + +standard_testfile tui-layout.c + +if {[build_executable "failed to prepare" ${testfile} ${srcfile}] == -1} { + return -1 +} + +Term::clean_restart 24 80 $testfile +if {![Term::enter_tui]} { + unsupported "TUI not supported" + return +} + +for {set i 0} {$i < 10} {incr i 1} { + Term::command "p $i" +} + +# Now check that the contents of the command window are as expected. +# +# Well, we would if there wasn't a massive bug in GDB!! The command +# window contents will not be exactly what you'd expect after this +# test has run. +# +# The expected output pattern given here is crafted so that it matches +# those bits of the GDB output that will be correct, and ignores those +# parts of the output that are known to be incorrect. +# +# If/when GDB is fixed it is expected that this test will continue to +# pass, though it is possible that at that point the pattern here +# could be improved. +Term::check_region_contents "check cmd window" 0 16 80 8 \ + [multi_line \ + "\[^\r\n\]*\\\$7 = 6\[^\r\n\]+" \ + "\\(gdb\\)\[^\r\n\]+" \ + "\[^\r\n\]*\\\$8 = 7\[^\r\n\]+" \ + "\\(gdb\\)\[^\r\n\]+" \ + "\[^\r\n\]*\\\$9 = 8\[^\r\n\]+" \ + "\\(gdb\\)\[^\r\n\]+" \ + "\[^\r\n\]*\\\$10 = 9\[^\r\n\]+" \ + "\\(gdb\\)"] diff --git a/gdb/testsuite/gdb.tui/tui-layout-asm-short-prog.exp b/gdb/testsuite/gdb.tui/tui-layout-asm-short-prog.exp index 7d3c8e1f519..a7d4d91d5ca 100644 --- a/gdb/testsuite/gdb.tui/tui-layout-asm-short-prog.exp +++ b/gdb/testsuite/gdb.tui/tui-layout-asm-short-prog.exp @@ -43,7 +43,9 @@ set first_line [Term::get_line 1] # instruction in the program. Term::command "+ 13" Term::check_box_contents "check asm box contents again" 0 0 80 15 \ - "^ *$hex\[^\n\]+\n +\n" + [multi_line \ + "^ *$hex\[^\r\n\]+" \ + "\\s+"] # Now scroll backward again, we should return to the start of the # program. diff --git a/gdb/testsuite/lib/tuiterm.exp b/gdb/testsuite/lib/tuiterm.exp index 4160586b615..3f6271ef0ea 100644 --- a/gdb/testsuite/lib/tuiterm.exp +++ b/gdb/testsuite/lib/tuiterm.exp @@ -367,16 +367,15 @@ namespace eval Term { variable _chars set y $_cur_row - set next_y [expr {$y + 1}] - while {$count > 0 && $next_y < $_rows} { + set next_y [expr {$y + $count}] + while {$next_y < $_rows} { for {set x 0} {$x < $_cols} {incr x} { set _chars($x,$y) $_chars($x,$next_y) } incr y incr next_y - incr count -1 } - _clear_lines $next_y $_rows + _clear_lines $y $_rows } } @@ -789,6 +788,33 @@ namespace eval Term { } } + # Check that the region of the screen described by X, Y, WIDTH, + # and HEIGHT match REGEXP. This is like check_contents except + # only part of the screen is checked. This can be used to check + # the contents within a box (though check_box_contents is a better + # choice for boxes with a border). + proc check_region_contents { test_name x y width height regexp } { + variable _chars + + # Now grab the contents of the box, join each line together + # with a '\r\n' sequence and match against REGEXP. + set result "" + for {set yy $y} {$yy < [expr {$y + $height}]} {incr yy} { + if {$yy > $y} { + # Add the end of line sequence only if this isn't the + # first line. + append result "\r\n" + } + for {set xx $x} {$xx < [expr {$x + $width}]} {incr xx} { + append result [lindex $_chars($xx,$yy) 0] + } + } + + if {![gdb_assert {[regexp -- $regexp $result]} $test_name]} { + dump_screen + } + } + # Check the contents of a box on the screen. This is a little # like check_contents, but doens't check the whole screen # contents, only the contents of a single box. This procedure @@ -805,19 +831,8 @@ namespace eval Term { return } - # Now grab the contents of the box, join each line together - # with a newline character and match against REGEXP. - set result "" - for {set yy [expr {$y + 1}]} {$yy < [expr {$y + $height - 1}]} {incr yy} { - for {set xx [expr {$x + 1}]} {$xx < [expr {$x + $width - 1}]} {incr xx} { - append result [lindex $_chars($xx,$yy) 0] - } - append result "\n" - } - - if {![gdb_assert {[regexp -- $regexp $result]} $test_name]} { - dump_screen - } + check_region_contents $test_name [expr {$x + 1}] [expr {$y + 1}] \ + [expr {$width - 2}] [expr {$height - 2}] $regexp } # A debugging function to dump the current screen, with line -- 2.25.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] gdb/testsuite: fix implementation of delete line in tuiterm.exp 2021-01-22 19:08 ` [PATCH 1/2] gdb/testsuite: fix implementation of delete line in tuiterm.exp Andrew Burgess @ 2021-02-06 19:17 ` Tom Tromey 0 siblings, 0 replies; 6+ messages in thread From: Tom Tromey @ 2021-02-06 19:17 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes: Andrew> * gdb.tui/scroll.exp: New file. Andrew> * gdb.tui/tui-layout-asm-short-prog.exp: Update expected results. Andrew> * lib/tuiterm.exp (Term::_csi_M): Delete count lines, scroll Andrew> remaining lines up. Andrew> (Term::check_region_contents): New proc. Andrew> (Term::check_box_contents): Use check_region_contents. Looks good. Thank you for doing this. Tom ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] gdb/tui: fix issue with handling the return character 2021-01-22 19:08 [PATCH 0/2] tui input related fixes Andrew Burgess 2021-01-22 19:08 ` [PATCH 1/2] gdb/testsuite: fix implementation of delete line in tuiterm.exp Andrew Burgess @ 2021-01-22 19:08 ` Andrew Burgess 2021-01-27 13:41 ` Andrew Burgess 1 sibling, 1 reply; 6+ messages in thread From: Andrew Burgess @ 2021-01-22 19:08 UTC (permalink / raw) To: gdb-patches My initial goal was to fix our gdb/testsuite/lib/tuiterm.exp such that it would correctly support (some limited) scrolling of the command window. What I observe is that when sending commands to the tui command window in a test script with: Term::command "p 1" The command window would be left looking like this: (gdb) (gdb) p 1$1 = 1 (gdb) When I would have expected it to look like this: (gdb) p 1 $1 = 1 (gdb) Obviously a bug in our tuiterm.exp library, right??? Wrong! Turns out there's a bug in GDB. If in GDB I enable the tui and then type (slowly) the 'p 1\r' (the \r is pressing the return key at the end of the string), then you do indeed get the "expected" terminal output. However, if instead I copy the 'p 1\r' string and paste it into the tui in one go then I now see the same corrupted output as we do when using tuiterm.exp. It turns out the problem is that GDB fails when handling lots of input arriving quickly with a \r (or \n) on the end. The reason for this bug is as follows: When the tui is active the terminal is in no-echo mode, so characters sent to the terminal are not echoed out again. This means that when the user types \r, this is not echoed to the terminal. The characters read in are passed to readline and \r indicates that the command line is complete and ready to be processed. However, the \r is not included in readlines command buffer, and is NOT printed by readline when is displays its buffer to the screen. So, in GDB we have to manually spot the \r when it is read in and update the display. Printing a newline character to the output and moving the cursor to the next line. This is done in tui_getc_1. Now readline tries to reduce the number of write calls. So if we very quickly (as in paste in one go) the text 'p 1' to readline (this time with no \r on the end), then readline will fetch the fist character and add it to its internal buffer. But before printing the character out readline checks to see if there's more input incoming. As we pasted multiple characters, then yes, readline sees the ' ' and adds this to its buffer, and finally the '1', this too is added to the buffer. Now if at this point we take a break, readline sees there is no more input available, and so prints its buffer out. Now when we press \r the code in tui_getc_1 kicks in, adds a \n to the output and moves the cursor to the next line. But, if instead we paste 'p 1\r' in one go then readline adds 'p 1' to its buffer as before, but now it sees that there is still more input available. Now it fetches the '\r', but this triggers the newline behaviour, we print '\n' and move to the next line - however readline has not printed its buffer yet! So finally we end up on the next line. There's no more input available so readline prints its buffer, then GDB gets passed the buffer, handles it, and prints the result. The solution I think is to put of our special newline insertion code until we know that readline has finished printing its buffer. Handily we know when this is - the next thing readline does is pass us the command line buffer for processing. So all we need to do is hook in to the command line processing, and before we pass the command line to GDB's internals we do all of the magic print a newline and move the cursor to the next line stuff. Luckily, GDB's interpreter mechanism already provides the hooks we need to do this. So all I do here is move the newline printing code from tui_getc_1 into a new function, setup a new input_handler hook for the tui, and call my new newline printing function. After this I can enable the tui and paste in 'p 1\r' and see the correct output. Also the tuiterm.exp library will now see non-corrupted output. gdb/ChangeLog: * tui/tui-interp.c (tui_command_line_handler): New function. (tui_interp::resume): Register tui_command_line_handler as the input_handler. * tui/tui-io.c (tui_inject_newline_into_command_window): New function. (tui_getc_1): Delete handling of '\n' and '\r'. * tui-io.h (tui_inject_newline_into_command_window): Declare. gdb/testsuite/ChangeLog: * gdb.tui/scroll.exp: Tighten expected results. Remove comment about bug in GDB. --- gdb/ChangeLog | 10 +++++ gdb/testsuite/ChangeLog | 5 +++ gdb/testsuite/gdb.tui/scroll.exp | 28 ++++-------- gdb/tui/tui-interp.c | 23 +++++++++- gdb/tui/tui-io.c | 73 +++++++++++++++++--------------- gdb/tui/tui-io.h | 5 +++ 6 files changed, 90 insertions(+), 54 deletions(-) diff --git a/gdb/testsuite/gdb.tui/scroll.exp b/gdb/testsuite/gdb.tui/scroll.exp index 4566bd8f5c7..f71d052da16 100644 --- a/gdb/testsuite/gdb.tui/scroll.exp +++ b/gdb/testsuite/gdb.tui/scroll.exp @@ -38,25 +38,15 @@ for {set i 0} {$i < 10} {incr i 1} { } # Now check that the contents of the command window are as expected. -# -# Well, we would if there wasn't a massive bug in GDB!! The command -# window contents will not be exactly what you'd expect after this -# test has run. -# -# The expected output pattern given here is crafted so that it matches -# those bits of the GDB output that will be correct, and ignores those -# parts of the output that are known to be incorrect. -# -# If/when GDB is fixed it is expected that this test will continue to -# pass, though it is possible that at that point the pattern here -# could be improved. Term::check_region_contents "check cmd window" 0 16 80 8 \ [multi_line \ - "\[^\r\n\]*\\\$7 = 6\[^\r\n\]+" \ - "\\(gdb\\)\[^\r\n\]+" \ - "\[^\r\n\]*\\\$8 = 7\[^\r\n\]+" \ - "\\(gdb\\)\[^\r\n\]+" \ - "\[^\r\n\]*\\\$9 = 8\[^\r\n\]+" \ - "\\(gdb\\)\[^\r\n\]+" \ - "\[^\r\n\]*\\\$10 = 9\[^\r\n\]+" \ + "\\\$7 = 6\\s+" \ + "\\(gdb\\) p 7\\s+" \ + "\\\$8 = 7\\s+" \ + "\\(gdb\\) p 8\\s+" \ + "\\\$9 = 8\\s+" \ + "\\(gdb\\) p 9\\s+" \ + "\\\$10 = 9\\s+" \ "\\(gdb\\)"] + +Term::dump_screen diff --git a/gdb/tui/tui-interp.c b/gdb/tui/tui-interp.c index 331e97ff11f..f70e1a7b4c2 100644 --- a/gdb/tui/tui-interp.c +++ b/gdb/tui/tui-interp.c @@ -247,6 +247,27 @@ tui_interp::init (bool top_level) tui_ensure_readline_initialized (); } +/* Used as the command handler for the tui. */ + +static void +tui_command_line_handler (gdb::unique_xmalloc_ptr<char> &&rl) +{ + /* When a tui enabled GDB is running in either tui mode or cli mode then + it is always the tui interpreter that is in use. As a result we end + up in here even in standard cli mode. + + We only need to do any special actions when the tui is in use + though. When the tui is active the users return is not echoed to the + screen as a result the display will not automatically move us to the + next line. Here we manually insert a newline character and move the + cursor. */ + if (tui_active) + tui_inject_newline_into_command_window (); + + /* Now perform GDB's standard CLI command line handling. */ + command_line_handler (std::move (rl)); +} + void tui_interp::resume () { @@ -266,7 +287,7 @@ tui_interp::resume () gdb_setup_readline (1); - ui->input_handler = command_line_handler; + ui->input_handler = tui_command_line_handler; if (stream != NULL) tui_old_uiout->set_stream (gdb_stdout); diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c index 0a33d53e869..a2be4d4353e 100644 --- a/gdb/tui/tui-io.c +++ b/gdb/tui/tui-io.c @@ -991,6 +991,45 @@ tui_dispatch_ctrl_char (unsigned int ch) return 0; } +/* See tui-io.h. */ + +void +tui_inject_newline_into_command_window () +{ + gdb_assert (tui_active); + + WINDOW *w= TUI_CMD_WIN->handle.get (); + + /* When hitting return with an empty input, gdb executes the last + command. If we emit a newline, this fills up the command window + with empty lines with gdb prompt at beginning. Instead of that, + stay on the same line but provide a visual effect to show the + user we recognized the command. */ + if (rl_end == 0 && !gdb_in_secondary_prompt_p (current_ui)) + { + wmove (w, getcury (w), 0); + + /* Clear the line. This will blink the gdb prompt since + it will be redrawn at the same line. */ + wclrtoeol (w); + wrefresh (w); + napms (20); + } + else + { + /* Move cursor to the end of the command line before emitting the + newline. We need to do so because when ncurses outputs a newline + it truncates any text that appears past the end of the cursor. */ + int px, py; + getyx (w, py, px); + px += rl_end - rl_point; + py += px / TUI_CMD_WIN->width; + px %= TUI_CMD_WIN->width; + wmove (w, py, px); + tui_putc ('\n'); + } +} + /* Main worker for tui_getc. Get a character from the command window. This is called from the readline package, but wrapped in a try/catch by tui_getc. */ @@ -1010,40 +1049,6 @@ tui_getc_1 (FILE *fp) ch = gdb_wgetch (w); - /* The \n must be echoed because it will not be printed by - readline. */ - if (ch == '\n' || ch == '\r') - { - /* When hitting return with an empty input, gdb executes the last - command. If we emit a newline, this fills up the command window - with empty lines with gdb prompt at beginning. Instead of that, - stay on the same line but provide a visual effect to show the - user we recognized the command. */ - if (rl_end == 0 && !gdb_in_secondary_prompt_p (current_ui)) - { - wmove (w, getcury (w), 0); - - /* Clear the line. This will blink the gdb prompt since - it will be redrawn at the same line. */ - wclrtoeol (w); - wrefresh (w); - napms (20); - } - else - { - /* Move cursor to the end of the command line before emitting the - newline. We need to do so because when ncurses outputs a newline - it truncates any text that appears past the end of the cursor. */ - int px, py; - getyx (w, py, px); - px += rl_end - rl_point; - py += px / TUI_CMD_WIN->width; - px %= TUI_CMD_WIN->width; - wmove (w, py, px); - tui_putc ('\n'); - } - } - /* Handle prev/next/up/down here. */ ch = tui_dispatch_ctrl_char (ch); diff --git a/gdb/tui/tui-io.h b/gdb/tui/tui-io.h index 8faf5b3d2ab..760532140f1 100644 --- a/gdb/tui/tui-io.h +++ b/gdb/tui/tui-io.h @@ -55,4 +55,9 @@ extern void tui_apply_style (WINDOW *w, ui_file_style style); extern struct ui_out *tui_out; extern cli_ui_out *tui_old_uiout; +/* This should be called when the user has entered a command line in tui + mode. Inject the newline into the output and move the cursor to the + next line. */ +extern void tui_inject_newline_into_command_window (); + #endif /* TUI_TUI_IO_H */ -- 2.25.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] gdb/tui: fix issue with handling the return character 2021-01-22 19:08 ` [PATCH 2/2] gdb/tui: fix issue with handling the return character Andrew Burgess @ 2021-01-27 13:41 ` Andrew Burgess 2021-02-06 19:27 ` Tom Tromey 0 siblings, 1 reply; 6+ messages in thread From: Andrew Burgess @ 2021-01-27 13:41 UTC (permalink / raw) To: gdb-patches Updated version of this patch includes additional testing. Everything else is unchanged. thanks, Andrew --- commit 46c2f5a962797536b22ec7118842c4ced2d45793 Author: Andrew Burgess <andrew.burgess@embecosm.com> Date: Fri Jan 22 17:40:19 2021 +0000 gdb/tui: fix issue with handling the return character My initial goal was to fix our gdb/testsuite/lib/tuiterm.exp such that it would correctly support (some limited) scrolling of the command window. What I observe is that when sending commands to the tui command window in a test script with: Term::command "p 1" The command window would be left looking like this: (gdb) (gdb) p 1$1 = 1 (gdb) When I would have expected it to look like this: (gdb) p 1 $1 = 1 (gdb) Obviously a bug in our tuiterm.exp library, right??? Wrong! Turns out there's a bug in GDB. If in GDB I enable the tui and then type (slowly) the 'p 1\r' (the \r is pressing the return key at the end of the string), then you do indeed get the "expected" terminal output. However, if instead I copy the 'p 1\r' string and paste it into the tui in one go then I now see the same corrupted output as we do when using tuiterm.exp. It turns out the problem is that GDB fails when handling lots of input arriving quickly with a \r (or \n) on the end. The reason for this bug is as follows: When the tui is active the terminal is in no-echo mode, so characters sent to the terminal are not echoed out again. This means that when the user types \r, this is not echoed to the terminal. The characters read in are passed to readline and \r indicates that the command line is complete and ready to be processed. However, the \r is not included in readlines command buffer, and is NOT printed by readline when is displays its buffer to the screen. So, in GDB we have to manually spot the \r when it is read in and update the display. Printing a newline character to the output and moving the cursor to the next line. This is done in tui_getc_1. Now readline tries to reduce the number of write calls. So if we very quickly (as in paste in one go) the text 'p 1' to readline (this time with no \r on the end), then readline will fetch the fist character and add it to its internal buffer. But before printing the character out readline checks to see if there's more input incoming. As we pasted multiple characters, then yes, readline sees the ' ' and adds this to its buffer, and finally the '1', this too is added to the buffer. Now if at this point we take a break, readline sees there is no more input available, and so prints its buffer out. Now when we press \r the code in tui_getc_1 kicks in, adds a \n to the output and moves the cursor to the next line. But, if instead we paste 'p 1\r' in one go then readline adds 'p 1' to its buffer as before, but now it sees that there is still more input available. Now it fetches the '\r', but this triggers the newline behaviour, we print '\n' and move to the next line - however readline has not printed its buffer yet! So finally we end up on the next line. There's no more input available so readline prints its buffer, then GDB gets passed the buffer, handles it, and prints the result. The solution I think is to put of our special newline insertion code until we know that readline has finished printing its buffer. Handily we know when this is - the next thing readline does is pass us the command line buffer for processing. So all we need to do is hook in to the command line processing, and before we pass the command line to GDB's internals we do all of the magic print a newline and move the cursor to the next line stuff. Luckily, GDB's interpreter mechanism already provides the hooks we need to do this. So all I do here is move the newline printing code from tui_getc_1 into a new function, setup a new input_handler hook for the tui, and call my new newline printing function. After this I can enable the tui and paste in 'p 1\r' and see the correct output. Also the tuiterm.exp library will now see non-corrupted output. gdb/ChangeLog: * tui/tui-interp.c (tui_command_line_handler): New function. (tui_interp::resume): Register tui_command_line_handler as the input_handler. * tui/tui-io.c (tui_inject_newline_into_command_window): New function. (tui_getc_1): Delete handling of '\n' and '\r'. * tui-io.h (tui_inject_newline_into_command_window): Declare. gdb/testsuite/ChangeLog: * gdb.tui/scroll.exp: Tighten expected results. Remove comment about bug in GDB, update expected results, and add more tests. diff --git a/gdb/testsuite/gdb.tui/scroll.exp b/gdb/testsuite/gdb.tui/scroll.exp index 4566bd8f5c7..2e85e4e2503 100644 --- a/gdb/testsuite/gdb.tui/scroll.exp +++ b/gdb/testsuite/gdb.tui/scroll.exp @@ -38,25 +38,35 @@ for {set i 0} {$i < 10} {incr i 1} { } # Now check that the contents of the command window are as expected. -# -# Well, we would if there wasn't a massive bug in GDB!! The command -# window contents will not be exactly what you'd expect after this -# test has run. -# -# The expected output pattern given here is crafted so that it matches -# those bits of the GDB output that will be correct, and ignores those -# parts of the output that are known to be incorrect. -# -# If/when GDB is fixed it is expected that this test will continue to -# pass, though it is possible that at that point the pattern here -# could be improved. Term::check_region_contents "check cmd window" 0 16 80 8 \ [multi_line \ - "\[^\r\n\]*\\\$7 = 6\[^\r\n\]+" \ - "\\(gdb\\)\[^\r\n\]+" \ - "\[^\r\n\]*\\\$8 = 7\[^\r\n\]+" \ - "\\(gdb\\)\[^\r\n\]+" \ - "\[^\r\n\]*\\\$9 = 8\[^\r\n\]+" \ - "\\(gdb\\)\[^\r\n\]+" \ - "\[^\r\n\]*\\\$10 = 9\[^\r\n\]+" \ + "\\\$7 = 6\\s+" \ + "\\(gdb\\) p 7\\s+" \ + "\\\$8 = 7\\s+" \ + "\\(gdb\\) p 8\\s+" \ + "\\\$9 = 8\\s+" \ + "\\(gdb\\) p 9\\s+" \ + "\\\$10 = 9\\s+" \ + "\\(gdb\\)"] + +# Now create a new layout where the CMD window is at the top of the +# screen. Sitch to this layout and ensure that scrolling still works +# as expected. +Term::command "tui new-layout flip cmd 1 src 1" +Term::command "layout flip" + +for {set i 10} {$i < 20} {incr i 1} { + Term::command "p $i" +} + +# Now check that the contents of the command window are as expected. +Term::check_region_contents "check cmd window in flip layout" 0 0 80 8 \ + [multi_line \ + "\\\$17 = 16\\s+" \ + "\\(gdb\\) p 17\\s+" \ + "\\\$18 = 17\\s+" \ + "\\(gdb\\) p 18\\s+" \ + "\\\$19 = 18\\s+" \ + "\\(gdb\\) p 19\\s+" \ + "\\\$20 = 19\\s+" \ "\\(gdb\\)"] diff --git a/gdb/tui/tui-interp.c b/gdb/tui/tui-interp.c index 331e97ff11f..f70e1a7b4c2 100644 --- a/gdb/tui/tui-interp.c +++ b/gdb/tui/tui-interp.c @@ -247,6 +247,27 @@ tui_interp::init (bool top_level) tui_ensure_readline_initialized (); } +/* Used as the command handler for the tui. */ + +static void +tui_command_line_handler (gdb::unique_xmalloc_ptr<char> &&rl) +{ + /* When a tui enabled GDB is running in either tui mode or cli mode then + it is always the tui interpreter that is in use. As a result we end + up in here even in standard cli mode. + + We only need to do any special actions when the tui is in use + though. When the tui is active the users return is not echoed to the + screen as a result the display will not automatically move us to the + next line. Here we manually insert a newline character and move the + cursor. */ + if (tui_active) + tui_inject_newline_into_command_window (); + + /* Now perform GDB's standard CLI command line handling. */ + command_line_handler (std::move (rl)); +} + void tui_interp::resume () { @@ -266,7 +287,7 @@ tui_interp::resume () gdb_setup_readline (1); - ui->input_handler = command_line_handler; + ui->input_handler = tui_command_line_handler; if (stream != NULL) tui_old_uiout->set_stream (gdb_stdout); diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c index 0a33d53e869..a2be4d4353e 100644 --- a/gdb/tui/tui-io.c +++ b/gdb/tui/tui-io.c @@ -991,6 +991,45 @@ tui_dispatch_ctrl_char (unsigned int ch) return 0; } +/* See tui-io.h. */ + +void +tui_inject_newline_into_command_window () +{ + gdb_assert (tui_active); + + WINDOW *w= TUI_CMD_WIN->handle.get (); + + /* When hitting return with an empty input, gdb executes the last + command. If we emit a newline, this fills up the command window + with empty lines with gdb prompt at beginning. Instead of that, + stay on the same line but provide a visual effect to show the + user we recognized the command. */ + if (rl_end == 0 && !gdb_in_secondary_prompt_p (current_ui)) + { + wmove (w, getcury (w), 0); + + /* Clear the line. This will blink the gdb prompt since + it will be redrawn at the same line. */ + wclrtoeol (w); + wrefresh (w); + napms (20); + } + else + { + /* Move cursor to the end of the command line before emitting the + newline. We need to do so because when ncurses outputs a newline + it truncates any text that appears past the end of the cursor. */ + int px, py; + getyx (w, py, px); + px += rl_end - rl_point; + py += px / TUI_CMD_WIN->width; + px %= TUI_CMD_WIN->width; + wmove (w, py, px); + tui_putc ('\n'); + } +} + /* Main worker for tui_getc. Get a character from the command window. This is called from the readline package, but wrapped in a try/catch by tui_getc. */ @@ -1010,40 +1049,6 @@ tui_getc_1 (FILE *fp) ch = gdb_wgetch (w); - /* The \n must be echoed because it will not be printed by - readline. */ - if (ch == '\n' || ch == '\r') - { - /* When hitting return with an empty input, gdb executes the last - command. If we emit a newline, this fills up the command window - with empty lines with gdb prompt at beginning. Instead of that, - stay on the same line but provide a visual effect to show the - user we recognized the command. */ - if (rl_end == 0 && !gdb_in_secondary_prompt_p (current_ui)) - { - wmove (w, getcury (w), 0); - - /* Clear the line. This will blink the gdb prompt since - it will be redrawn at the same line. */ - wclrtoeol (w); - wrefresh (w); - napms (20); - } - else - { - /* Move cursor to the end of the command line before emitting the - newline. We need to do so because when ncurses outputs a newline - it truncates any text that appears past the end of the cursor. */ - int px, py; - getyx (w, py, px); - px += rl_end - rl_point; - py += px / TUI_CMD_WIN->width; - px %= TUI_CMD_WIN->width; - wmove (w, py, px); - tui_putc ('\n'); - } - } - /* Handle prev/next/up/down here. */ ch = tui_dispatch_ctrl_char (ch); diff --git a/gdb/tui/tui-io.h b/gdb/tui/tui-io.h index 8faf5b3d2ab..760532140f1 100644 --- a/gdb/tui/tui-io.h +++ b/gdb/tui/tui-io.h @@ -55,4 +55,9 @@ extern void tui_apply_style (WINDOW *w, ui_file_style style); extern struct ui_out *tui_out; extern cli_ui_out *tui_old_uiout; +/* This should be called when the user has entered a command line in tui + mode. Inject the newline into the output and move the cursor to the + next line. */ +extern void tui_inject_newline_into_command_window (); + #endif /* TUI_TUI_IO_H */ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] gdb/tui: fix issue with handling the return character 2021-01-27 13:41 ` Andrew Burgess @ 2021-02-06 19:27 ` Tom Tromey 0 siblings, 0 replies; 6+ messages in thread From: Tom Tromey @ 2021-02-06 19:27 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes: Andrew> gdb/ChangeLog: Andrew> * tui/tui-interp.c (tui_command_line_handler): New function. Andrew> (tui_interp::resume): Register tui_command_line_handler as the Andrew> input_handler. Andrew> * tui/tui-io.c (tui_inject_newline_into_command_window): New Andrew> function. Andrew> (tui_getc_1): Delete handling of '\n' and '\r'. Andrew> * tui-io.h (tui_inject_newline_into_command_window): Declare. Andrew> gdb/testsuite/ChangeLog: Andrew> * gdb.tui/scroll.exp: Tighten expected results. Remove comment Andrew> about bug in GDB, update expected results, and add more tests. Thank you for tracking this down. That was a great explanation. It looks good to me. Tom ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-02-06 19:27 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-22 19:08 [PATCH 0/2] tui input related fixes Andrew Burgess 2021-01-22 19:08 ` [PATCH 1/2] gdb/testsuite: fix implementation of delete line in tuiterm.exp Andrew Burgess 2021-02-06 19:17 ` Tom Tromey 2021-01-22 19:08 ` [PATCH 2/2] gdb/tui: fix issue with handling the return character Andrew Burgess 2021-01-27 13:41 ` Andrew Burgess 2021-02-06 19:27 ` Tom Tromey
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).