[PATCH 1/2] gdb/testsuite: add links for handled control sequences in lib/tuiterm.exp

[PATCH 1/2] gdb/testsuite: add links for handled control sequences in lib/tuiterm.exp

[Simon Marchi]

This code can be a bit cryptic for those who don't know terminal control
sequences very well.  This patch adds links for all the handled
sequences, so it's easy to get some doc to follow the code.

I linked to a VT510 manual, because I think it's well formatted and easy
to read.  There's only the repeat sequence (_csi_b) which I haven't
found in it, it looks to be xterm-specific or something.

I also tried to use the sequence names as they are in the manual.

gdb/testsuite/ChangeLog:

	* lib/tuiterm.exp: Add links in comments.

Change-Id: I670b947a238e5e9bcab7c476a20eb3c31cf2909d
---
 gdb/testsuite/lib/tuiterm.exp | 54 +++++++++++++++++++++++++++++------
 1 file changed, 45 insertions(+), 9 deletions(-)

diff --git a/gdb/testsuite/lib/tuiterm.exp b/gdb/testsuite/lib/tuiterm.exp
index 749212449302..c2a547214bd5 100644
--- a/gdb/testsuite/lib/tuiterm.exp
+++ b/gdb/testsuite/lib/tuiterm.exp
@@ -123,7 +123,9 @@ namespace eval Term {
 	set _cur_x 0
     }
 
-    # Make room for characters.
+    # Insert Character.
+    #
+    # https://vt100.net/docs/vt510-rm/ICH.html
     proc _csi_@ {args} {
 	set n [_default [lindex $args 0] 1]
 	variable _cur_x
@@ -139,6 +141,8 @@ namespace eval Term {
     }
 
     # Cursor Up.
+    #
+    # https://vt100.net/docs/vt510-rm/CUU.html
     proc _csi_A {args} {
 	variable _cur_y
 	set arg [_default [lindex $args 0] 1]
@@ -146,6 +150,8 @@ namespace eval Term {
     }
 
     # Cursor Down.
+    #
+    # https://vt100.net/docs/vt510-rm/CUD.html
     proc _csi_B {args} {
 	variable _cur_y
 	variable _rows
@@ -154,6 +160,8 @@ namespace eval Term {
     }
 
     # Cursor Forward.
+    #
+    # https://vt100.net/docs/vt510-rm/CUF.html
     proc _csi_C {args} {
 	variable _cur_x
 	variable _cols
@@ -161,7 +169,9 @@ namespace eval Term {
 	set _cur_x [expr {min ($_cur_x + $arg, $_cols)}]
     }
 
-    # Cursor Back.
+    # Cursor Backward.
+    #
+    # https://vt100.net/docs/vt510-rm/CUB.html
     proc _csi_D {args} {
 	variable _cur_x
 	set arg [_default [lindex $args 0] 1]
@@ -169,6 +179,8 @@ namespace eval Term {
     }
 
     # Cursor Next Line.
+    #
+    # https://vt100.net/docs/vt510-rm/CNL.html
     proc _csi_E {args} {
 	variable _cur_x
 	variable _cur_y
@@ -179,6 +191,8 @@ namespace eval Term {
     }
 
     # Cursor Previous Line.
+    #
+    # https://vt100.net/docs/vt510-rm/CPL.html
     proc _csi_F {args} {
 	variable _cur_x
 	variable _cur_y
@@ -189,6 +203,8 @@ namespace eval Term {
     }
 
     # Cursor Horizontal Absolute.
+    #
+    # https://vt100.net/docs/vt510-rm/CHA.html
     proc _csi_G {args} {
 	variable _cur_x
 	variable _cols
@@ -196,7 +212,9 @@ namespace eval Term {
 	set _cur_x [expr {min ($arg - 1, $_cols)}]
     }
 
-    # Move cursor (don't know the official name of this one).
+    # Cursor Position.
+    #
+    # https://vt100.net/docs/vt510-rm/CUP.html
     proc _csi_H {args} {
 	variable _cur_x
 	variable _cur_y
@@ -204,7 +222,9 @@ namespace eval Term {
 	set _cur_x [expr {[_default [lindex $args 1] 1] - 1}]
     }
 
-    # Cursor Forward Tabulation.
+    # Cursor Horizontal Forward Tabulation.
+    #
+    # https://vt100.net/docs/vt510-rm/CHT.html
     proc _csi_I {args} {
 	set n [_default [lindex $args 0] 1]
 	variable _cur_x
@@ -215,7 +235,9 @@ namespace eval Term {
 	}
     }
 
-    # Erase.
+    # Erase in Display.
+    #
+    # https://vt100.net/docs/vt510-rm/ED.html
     proc _csi_J {args} {
 	variable _cur_x
 	variable _cur_y
@@ -233,7 +255,9 @@ namespace eval Term {
 	}
     }
 
-    # Erase Line.
+    # Erase in Line.
+    #
+    # https://vt100.net/docs/vt510-rm/EL.html
     proc _csi_K {args} {
 	variable _cur_x
 	variable _cur_y
@@ -249,7 +273,9 @@ namespace eval Term {
 	}
     }
 
-    # Delete lines.
+    # Delete line.
+    #
+    # https://vt100.net/docs/vt510-rm/DL.html
     proc _csi_M {args} {
 	variable _cur_y
 	variable _rows
@@ -270,6 +296,8 @@ namespace eval Term {
     }
 
     # Erase chars.
+    #
+    # https://vt100.net/docs/vt510-rm/ECH.html
     proc _csi_X {args} {
 	set n [_default [lindex $args 0] 1]
 	# Erase characters but don't move cursor.
@@ -285,7 +313,9 @@ namespace eval Term {
 	}
     }
 
-    # Backward tab stops.
+    # Cursor Backward Tabulation.
+    #
+    # https://vt100.net/docs/vt510-rm/CBT.html
     proc _csi_Z {args} {
 	set n [_default [lindex $args 0] 1]
 	variable _cur_x
@@ -293,19 +323,25 @@ namespace eval Term {
     }
 
     # Repeat.
+    #
+    # https://www.xfree86.org/current/ctlseqs.html (See `(REP)`)
     proc _csi_b {args} {
 	variable _last_char
 	set n [_default [lindex $args 0] 1]
 	_insert [string repeat $_last_char $n]
     }
 
-    # Line Position Absolute.
+    # Vertical Line Position Absolute.
+    #
+    # https://vt100.net/docs/vt510-rm/VPA.html
     proc _csi_d {args} {
 	variable _cur_y
 	set _cur_y [expr {[_default [lindex $args 0] 1] - 1}]
     }
 
     # Select Graphic Rendition.
+    #
+    # https://vt100.net/docs/vt510-rm/SGR.html
     proc _csi_m {args} {
 	variable _attrs
 	foreach item $args {
-- 
2.29.2

[PATCH 2/2] gdb/testsuite: rename _cur_x/_cur_y to _cur_col/_cur_row in lib/tuiterm.exp

[Simon Marchi]

I am having trouble remembering which of _cur_x/_cur_y is columns and
which is rows, so renaming them helps.  We already have _rows and _cols
to represent the terminal size, so I think that makes sense to name the
"_cur" variables accordingly.

gdb/testsuite/ChangeLog:

	* lib/tuiterm.exp: Rename _cur_x/_cur_y to _cur_col/_cur_row.

Change-Id: I6abd3cdfdb295d8abde12dcd5f0ae09f18f07967
---
 gdb/testsuite/lib/tuiterm.exp | 176 +++++++++++++++++-----------------
 1 file changed, 90 insertions(+), 86 deletions(-)

diff --git a/gdb/testsuite/lib/tuiterm.exp b/gdb/testsuite/lib/tuiterm.exp
index c2a547214bd5..dcc535863ac8 100644
--- a/gdb/testsuite/lib/tuiterm.exp
+++ b/gdb/testsuite/lib/tuiterm.exp
@@ -46,12 +46,16 @@ proc tuiterm_env_finish { } {
 }
 
 namespace eval Term {
+    # Size of the terminal.
     variable _rows
     variable _cols
+
+    # Buffer / contents of the terminal.
     variable _chars
 
-    variable _cur_x
-    variable _cur_y
+    # Position of the cursor.
+    variable _cur_col
+    variable _cur_row
 
     variable _attrs
 
@@ -94,33 +98,33 @@ namespace eval Term {
 
     # Backspace.
     proc _ctl_0x08 {} {
-	variable _cur_x
-	incr _cur_x -1
-	if {$_cur_x < 0} {
-	    variable _cur_y
+	variable _cur_col
+	incr _cur_col -1
+	if {$_cur_col < 0} {
+	    variable _cur_row
 	    variable _cols
-	    set _cur_x [expr {$_cols - 1}]
-	    incr _cur_y -1
-	    if {$_cur_y < 0} {
-		set _cur_y 0
+	    set _cur_col [expr {$_cols - 1}]
+	    incr _cur_row -1
+	    if {$_cur_row < 0} {
+		set _cur_row 0
 	    }
 	}
     }
 
     # Linefeed.
     proc _ctl_0x0a {} {
-	variable _cur_y
+	variable _cur_row
 	variable _rows
-	incr _cur_y 1
-	if {$_cur_y >= $_rows} {
+	incr _cur_row 1
+	if {$_cur_row >= $_rows} {
 	    error "FIXME scroll"
 	}
     }
 
     # Carriage return.
     proc _ctl_0x0d {} {
-	variable _cur_x
-	set _cur_x 0
+	variable _cur_col
+	set _cur_col 0
     }
 
     # Insert Character.
@@ -128,13 +132,13 @@ namespace eval Term {
     # https://vt100.net/docs/vt510-rm/ICH.html
     proc _csi_@ {args} {
 	set n [_default [lindex $args 0] 1]
-	variable _cur_x
-	variable _cur_y
+	variable _cur_col
+	variable _cur_row
 	variable _chars
-	set in_x $_cur_x
-	set out_x [expr {$_cur_x + $n}]
+	set in_x $_cur_col
+	set out_x [expr {$_cur_col + $n}]
 	for {set i 0} {$i < $n} {incr i} {
-	    set _chars($out_x,$_cur_y) $_chars($in_x,$_cur_y)
+	    set _chars($out_x,$_cur_row) $_chars($in_x,$_cur_row)
 	    incr in_x
 	    incr out_x
 	}
@@ -144,82 +148,82 @@ namespace eval Term {
     #
     # https://vt100.net/docs/vt510-rm/CUU.html
     proc _csi_A {args} {
-	variable _cur_y
+	variable _cur_row
 	set arg [_default [lindex $args 0] 1]
-	set _cur_y [expr {max ($_cur_y - $arg, 0)}]
+	set _cur_row [expr {max ($_cur_row - $arg, 0)}]
     }
 
     # Cursor Down.
     #
     # https://vt100.net/docs/vt510-rm/CUD.html
     proc _csi_B {args} {
-	variable _cur_y
+	variable _cur_row
 	variable _rows
 	set arg [_default [lindex $args 0] 1]
-	set _cur_y [expr {min ($_cur_y + $arg, $_rows)}]
+	set _cur_row [expr {min ($_cur_row + $arg, $_rows)}]
     }
 
     # Cursor Forward.
     #
     # https://vt100.net/docs/vt510-rm/CUF.html
     proc _csi_C {args} {
-	variable _cur_x
+	variable _cur_col
 	variable _cols
 	set arg [_default [lindex $args 0] 1]
-	set _cur_x [expr {min ($_cur_x + $arg, $_cols)}]
+	set _cur_col [expr {min ($_cur_col + $arg, $_cols)}]
     }
 
     # Cursor Backward.
     #
     # https://vt100.net/docs/vt510-rm/CUB.html
     proc _csi_D {args} {
-	variable _cur_x
+	variable _cur_col
 	set arg [_default [lindex $args 0] 1]
-	set _cur_x [expr {max ($_cur_x - $arg, 0)}]
+	set _cur_col [expr {max ($_cur_col - $arg, 0)}]
     }
 
     # Cursor Next Line.
     #
     # https://vt100.net/docs/vt510-rm/CNL.html
     proc _csi_E {args} {
-	variable _cur_x
-	variable _cur_y
+	variable _cur_col
+	variable _cur_row
 	variable _rows
 	set arg [_default [lindex $args 0] 1]
-	set _cur_x 0
-	set _cur_y [expr {min ($_cur_y + $arg, $_rows)}]
+	set _cur_col 0
+	set _cur_row [expr {min ($_cur_row + $arg, $_rows)}]
     }
 
     # Cursor Previous Line.
     #
     # https://vt100.net/docs/vt510-rm/CPL.html
     proc _csi_F {args} {
-	variable _cur_x
-	variable _cur_y
+	variable _cur_col
+	variable _cur_row
 	variable _rows
 	set arg [_default [lindex $args 0] 1]
-	set _cur_x 0
-	set _cur_y [expr {max ($_cur_y - $arg, 0)}]
+	set _cur_col 0
+	set _cur_row [expr {max ($_cur_row - $arg, 0)}]
     }
 
     # Cursor Horizontal Absolute.
     #
     # https://vt100.net/docs/vt510-rm/CHA.html
     proc _csi_G {args} {
-	variable _cur_x
+	variable _cur_col
 	variable _cols
 	set arg [_default [lindex $args 0] 1]
-	set _cur_x [expr {min ($arg - 1, $_cols)}]
+	set _cur_col [expr {min ($arg - 1, $_cols)}]
     }
 
     # Cursor Position.
     #
     # https://vt100.net/docs/vt510-rm/CUP.html
     proc _csi_H {args} {
-	variable _cur_x
-	variable _cur_y
-	set _cur_y [expr {[_default [lindex $args 0] 1] - 1}]
-	set _cur_x [expr {[_default [lindex $args 1] 1] - 1}]
+	variable _cur_col
+	variable _cur_row
+	set _cur_row [expr {[_default [lindex $args 0] 1] - 1}]
+	set _cur_col [expr {[_default [lindex $args 1] 1] - 1}]
     }
 
     # Cursor Horizontal Forward Tabulation.
@@ -227,11 +231,11 @@ namespace eval Term {
     # https://vt100.net/docs/vt510-rm/CHT.html
     proc _csi_I {args} {
 	set n [_default [lindex $args 0] 1]
-	variable _cur_x
+	variable _cur_col
 	variable _cols
-	incr _cur_x [expr {$n * 8 - $_cur_x % 8}]
-	if {$_cur_x >= $_cols} {
-	    set _cur_x [expr {$_cols - 1}]
+	incr _cur_col [expr {$n * 8 - $_cur_col % 8}]
+	if {$_cur_col >= $_cols} {
+	    set _cur_col [expr {$_cols - 1}]
 	}
     }
 
@@ -239,17 +243,17 @@ namespace eval Term {
     #
     # https://vt100.net/docs/vt510-rm/ED.html
     proc _csi_J {args} {
-	variable _cur_x
-	variable _cur_y
+	variable _cur_col
+	variable _cur_row
 	variable _rows
 	variable _cols
 	set arg [_default [lindex $args 0] 0]
 	if {$arg == 0} {
-	    _clear_in_line $_cur_x $_cols $_cur_y
-	    _clear_lines [expr {$_cur_y + 1}] $_rows
+	    _clear_in_line $_cur_col $_cols $_cur_row
+	    _clear_lines [expr {$_cur_row + 1}] $_rows
 	} elseif {$arg == 1} {
-	    _clear_lines 0 [expr {$_cur_y - 1}]
-	    _clear_in_line 0 $_cur_x $_cur_y
+	    _clear_lines 0 [expr {$_cur_row - 1}]
+	    _clear_in_line 0 $_cur_col $_cur_row
 	} elseif {$arg == 2} {
 	    _clear_lines 0 $_rows
 	}
@@ -259,17 +263,17 @@ namespace eval Term {
     #
     # https://vt100.net/docs/vt510-rm/EL.html
     proc _csi_K {args} {
-	variable _cur_x
-	variable _cur_y
+	variable _cur_col
+	variable _cur_row
 	variable _cols
 	set arg [_default [lindex $args 0] 0]
 	if {$arg == 0} {
 	    # From cursor to end.
-	    _clear_in_line $_cur_x $_cols $_cur_y
+	    _clear_in_line $_cur_col $_cols $_cur_row
 	} elseif {$arg == 1} {
-	    _clear_in_line 0 $_cur_x $_cur_y
+	    _clear_in_line 0 $_cur_col $_cur_row
 	} elseif {$arg == 2} {
-	    _clear_in_line 0 $_cols $_cur_y
+	    _clear_in_line 0 $_cols $_cur_row
 	}
     }
 
@@ -277,12 +281,12 @@ namespace eval Term {
     #
     # https://vt100.net/docs/vt510-rm/DL.html
     proc _csi_M {args} {
-	variable _cur_y
+	variable _cur_row
 	variable _rows
 	variable _cols
 	variable _chars
 	set count [_default [lindex $args 0] 1]
-	set y $_cur_y
+	set y $_cur_row
 	set next_y [expr {$y + 1}]
 	while {$count > 0 && $next_y < $_rows} {
 	    for {set x 0} {$x < $_cols} {incr x} {
@@ -301,14 +305,14 @@ namespace eval Term {
     proc _csi_X {args} {
 	set n [_default [lindex $args 0] 1]
 	# Erase characters but don't move cursor.
-	variable _cur_x
-	variable _cur_y
+	variable _cur_col
+	variable _cur_row
 	variable _attrs
 	variable _chars
 	set lattr [array get _attrs]
-	set x $_cur_x
+	set x $_cur_col
 	for {set i 0} {$i < $n} {incr i} {
-	    set _chars($x,$_cur_y) [list " " $lattr]
+	    set _chars($x,$_cur_row) [list " " $lattr]
 	    incr x
 	}
     }
@@ -318,8 +322,8 @@ namespace eval Term {
     # https://vt100.net/docs/vt510-rm/CBT.html
     proc _csi_Z {args} {
 	set n [_default [lindex $args 0] 1]
-	variable _cur_x
-	set _cur_x [expr {max (int (($_cur_x - 1) / 8) * 8 - ($n - 1) * 8, 0)}]
+	variable _cur_col
+	set _cur_col [expr {max (int (($_cur_col - 1) / 8) * 8 - ($n - 1) * 8, 0)}]
     }
 
     # Repeat.
@@ -335,8 +339,8 @@ namespace eval Term {
     #
     # https://vt100.net/docs/vt510-rm/VPA.html
     proc _csi_d {args} {
-	variable _cur_y
-	set _cur_y [expr {[_default [lindex $args 0] 1] - 1}]
+	variable _cur_row
+	set _cur_row [expr {[_default [lindex $args 0] 1] - 1}]
     }
 
     # Select Graphic Rendition.
@@ -393,20 +397,20 @@ namespace eval Term {
     # Insert string at the cursor location.
     proc _insert {str} {
 	verbose "INSERT <<$str>>"
-	variable _cur_x
-	variable _cur_y
+	variable _cur_col
+	variable _cur_row
 	variable _rows
 	variable _cols
 	variable _attrs
 	variable _chars
 	set lattr [array get _attrs]
 	foreach char [split $str {}] {
-	    set _chars($_cur_x,$_cur_y) [list $char $lattr]
-	    incr _cur_x
-	    if {$_cur_x >= $_cols} {
-		set _cur_x 0
-		incr _cur_y
-		if {$_cur_y >= $_rows} {
+	    set _chars($_cur_col,$_cur_row) [list $char $lattr]
+	    incr _cur_col
+	    if {$_cur_col >= $_cols} {
+		set _cur_col 0
+		incr _cur_row
+		if {$_cur_row >= $_rows} {
 		    error "FIXME scroll"
 		}
 	    }
@@ -420,15 +424,15 @@ namespace eval Term {
 
 	variable _rows
 	variable _cols
-	variable _cur_x
-	variable _cur_y
+	variable _cur_col
+	variable _cur_row
 	variable _attrs
 	variable _resize_count
 
 	set _rows $rows
 	set _cols $cols
-	set _cur_x 0
-	set _cur_y 0
+	set _cur_col 0
+	set _cur_row 0
 	set _resize_count 0
 	array set _attrs {
 	    intensity normal
@@ -447,8 +451,8 @@ namespace eval Term {
     proc wait_for {wait_for} {
 	global expect_out
 	global gdb_prompt
-	variable _cur_x
-	variable _cur_y
+	variable _cur_col
+	variable _cur_row
 
 	set prompt_wait_for "$gdb_prompt \$"
 
@@ -487,9 +491,9 @@ namespace eval Term {
 	    # isn't reliable to check this only after an insertion,
 	    # because curses may make "unusual" redrawing decisions.
 	    if {$wait_for == "$prompt_wait_for"} {
-		set prev [get_line $_cur_y $_cur_x]
+		set prev [get_line $_cur_row $_cur_col]
 	    } else {
-		set prev [get_line $_cur_y]
+		set prev [get_line $_cur_row]
 	    }
 	    if {[regexp -- $wait_for $prev]} {
 		if {$wait_for == "$prompt_wait_for"} {
@@ -609,9 +613,9 @@ namespace eval Term {
 
     # Get the text just before the cursor.
     proc get_current_line {} {
-	variable _cur_x
-	variable _cur_y
-	return [get_line $_cur_y $_cur_x]
+	variable _cur_col
+	variable _cur_row
+	return [get_line $_cur_row $_cur_col]
     }
 
     # Helper function for check_box.  Returns empty string if the box
-- 
2.29.2

[PATCH 3/2] gdb/testsuite: improve logging in lib/tuiterm.exp

[Simon Marchi]

Here's a bonus patch that applies on top of the other two.

While debugging TUI test cases, it's hard to know what exactly is
happening in the little mind of the terminal emulator.  Add some logging
for all input processing.  Right now I'm interested in seeing what
happens to the cursor position, so made it so all operations log the
"before" and "after" cursor position.  It should help see if any
operation is not behaving as expected, w.r.t. the cursor position.

Here are some examples of the logging found in gdb.log with this patch
applied:

    +++ Inserting string '+|'
    +++   Inserted char '+', cursor: (0, 79) -> (1, 0)
    +++   Inserted char '|', cursor: (1, 0) -> (1, 1)
    +++ Inserted string '+|', cursor: (0, 79) -> (1, 1)
    +++ Cursor Horizontal Absolute (80), cursor: (1, 1) -> (1, 79)

In the last line, note that the argument is 80 and we move to 79, that's
because the position in the argument to the control sequence is 1-based,
while our indexing is 0-based.

gdb/testsuite/ChangeLog:

	* lib/tuiterm.exp (_log, _log_cur): New, use throughout.

Change-Id: Ibf570d4b2867729ce65bea8c193343a8a846170d
---
 gdb/testsuite/lib/tuiterm.exp | 442 +++++++++++++++++++++-------------
 1 file changed, 274 insertions(+), 168 deletions(-)

diff --git a/gdb/testsuite/lib/tuiterm.exp b/gdb/testsuite/lib/tuiterm.exp
index dcc535863ac8..a77c8d39e546 100644
--- a/gdb/testsuite/lib/tuiterm.exp
+++ b/gdb/testsuite/lib/tuiterm.exp
@@ -63,6 +63,23 @@ namespace eval Term {
 
     variable _resize_count
 
+    proc _log { what } {
+	verbose -log "+++ $what"
+    }
+
+    # Call BODY, then log WHAT along with the original and new cursor position.
+    proc _log_cur { what body } {
+	variable _cur_row
+	variable _cur_col
+
+	set orig_cur_row $_cur_row
+	set orig_cur_col $_cur_col
+
+	uplevel $body
+
+	_log "$what, cursor: ($orig_cur_row, $orig_cur_col) -> ($_cur_row, $_cur_col)"
+    }
+
     # If ARG is empty, return DEF: otherwise ARG.  This is useful for
     # defaulting arguments in CSIs.
     proc _default {arg def} {
@@ -98,33 +115,43 @@ namespace eval Term {
 
     # Backspace.
     proc _ctl_0x08 {} {
-	variable _cur_col
-	incr _cur_col -1
-	if {$_cur_col < 0} {
-	    variable _cur_row
-	    variable _cols
-	    set _cur_col [expr {$_cols - 1}]
-	    incr _cur_row -1
-	    if {$_cur_row < 0} {
-		set _cur_row 0
+	_log_cur "Backspace" {
+	    variable _cur_col
+
+	    incr _cur_col -1
+	    if {$_cur_col < 0} {
+		variable _cur_row
+		variable _cols
+
+		set _cur_col [expr {$_cols - 1}]
+		incr _cur_row -1
+		if {$_cur_row < 0} {
+		    set _cur_row 0
+		}
 	    }
 	}
     }
 
     # Linefeed.
     proc _ctl_0x0a {} {
-	variable _cur_row
-	variable _rows
-	incr _cur_row 1
-	if {$_cur_row >= $_rows} {
-	    error "FIXME scroll"
+	_log_cur "Line feed" {
+	    variable _cur_row
+	    variable _rows
+
+	    incr _cur_row 1
+	    if {$_cur_row >= $_rows} {
+		error "FIXME scroll"
+	    }
 	}
     }
 
     # Carriage return.
     proc _ctl_0x0d {} {
-	variable _cur_col
-	set _cur_col 0
+	_log_cur "Carriage return" {
+	    variable _cur_col
+
+	    set _cur_col 0
+	}
     }
 
     # Insert Character.
@@ -132,15 +159,19 @@ namespace eval Term {
     # https://vt100.net/docs/vt510-rm/ICH.html
     proc _csi_@ {args} {
 	set n [_default [lindex $args 0] 1]
-	variable _cur_col
-	variable _cur_row
-	variable _chars
-	set in_x $_cur_col
-	set out_x [expr {$_cur_col + $n}]
-	for {set i 0} {$i < $n} {incr i} {
-	    set _chars($out_x,$_cur_row) $_chars($in_x,$_cur_row)
-	    incr in_x
-	    incr out_x
+
+	_log_cur "Insert Character ($n)" {
+	    variable _cur_col
+	    variable _cur_row
+	    variable _chars
+
+	    set in_x $_cur_col
+	    set out_x [expr {$_cur_col + $n}]
+	    for {set i 0} {$i < $n} {incr i} {
+		set _chars($out_x,$_cur_row) $_chars($in_x,$_cur_row)
+		incr in_x
+		incr out_x
+	    }
 	}
     }
 
@@ -148,82 +179,116 @@ namespace eval Term {
     #
     # https://vt100.net/docs/vt510-rm/CUU.html
     proc _csi_A {args} {
-	variable _cur_row
 	set arg [_default [lindex $args 0] 1]
-	set _cur_row [expr {max ($_cur_row - $arg, 0)}]
+
+	_log_cur "Cursor Up ($arg)" {
+	    variable _cur_row
+
+	    set _cur_row [expr {max ($_cur_row - $arg, 0)}]
+	}
     }
 
     # Cursor Down.
     #
     # https://vt100.net/docs/vt510-rm/CUD.html
     proc _csi_B {args} {
-	variable _cur_row
-	variable _rows
 	set arg [_default [lindex $args 0] 1]
-	set _cur_row [expr {min ($_cur_row + $arg, $_rows)}]
+
+	_log_cur "Cursor Down ($arg)" {
+	    variable _cur_row
+	    variable _rows
+
+	    set _cur_row [expr {min ($_cur_row + $arg, $_rows)}]
+	}
     }
 
     # Cursor Forward.
     #
     # https://vt100.net/docs/vt510-rm/CUF.html
     proc _csi_C {args} {
-	variable _cur_col
-	variable _cols
 	set arg [_default [lindex $args 0] 1]
-	set _cur_col [expr {min ($_cur_col + $arg, $_cols)}]
+
+	_log_cur "Cursor Forward ($arg)" {
+	    variable _cur_col
+	    variable _cols
+
+	    set _cur_col [expr {min ($_cur_col + $arg, $_cols)}]
+	}
     }
 
     # Cursor Backward.
     #
     # https://vt100.net/docs/vt510-rm/CUB.html
     proc _csi_D {args} {
-	variable _cur_col
 	set arg [_default [lindex $args 0] 1]
-	set _cur_col [expr {max ($_cur_col - $arg, 0)}]
+
+	_log_cur "Cursor Backward ($arg)" {
+	    variable _cur_col
+
+	    set _cur_col [expr {max ($_cur_col - $arg, 0)}]
+	}
     }
 
     # Cursor Next Line.
     #
     # https://vt100.net/docs/vt510-rm/CNL.html
     proc _csi_E {args} {
-	variable _cur_col
-	variable _cur_row
-	variable _rows
 	set arg [_default [lindex $args 0] 1]
-	set _cur_col 0
-	set _cur_row [expr {min ($_cur_row + $arg, $_rows)}]
+
+	_log_cur "Cursor Next Line ($arg)" {
+	    variable _cur_col
+	    variable _cur_row
+	    variable _rows
+
+	    set _cur_col 0
+	    set _cur_row [expr {min ($_cur_row + $arg, $_rows)}]
+	}
     }
 
     # Cursor Previous Line.
     #
     # https://vt100.net/docs/vt510-rm/CPL.html
     proc _csi_F {args} {
-	variable _cur_col
-	variable _cur_row
-	variable _rows
 	set arg [_default [lindex $args 0] 1]
-	set _cur_col 0
-	set _cur_row [expr {max ($_cur_row - $arg, 0)}]
+
+	_log_cur "Cursor Previous Line ($arg)" {
+	    variable _cur_col
+	    variable _cur_row
+	    variable _rows
+
+	    set _cur_col 0
+	    set _cur_row [expr {max ($_cur_row - $arg, 0)}]
+	}
     }
 
     # Cursor Horizontal Absolute.
     #
     # https://vt100.net/docs/vt510-rm/CHA.html
     proc _csi_G {args} {
-	variable _cur_col
-	variable _cols
 	set arg [_default [lindex $args 0] 1]
-	set _cur_col [expr {min ($arg - 1, $_cols)}]
+
+	_log_cur "Cursor Horizontal Absolute ($arg)" {
+	    variable _cur_col
+	    variable _cols
+
+	    set _cur_col [expr {min ($arg - 1, $_cols)}]
+	}
     }
 
     # Cursor Position.
     #
     # https://vt100.net/docs/vt510-rm/CUP.html
     proc _csi_H {args} {
-	variable _cur_col
-	variable _cur_row
-	set _cur_row [expr {[_default [lindex $args 0] 1] - 1}]
-	set _cur_col [expr {[_default [lindex $args 1] 1] - 1}]
+	set row [_default [lindex $args 0] 1]
+	set col [_default [lindex $args 1] 1]
+
+	_log_cur "Cursor Position ($row, $col)" {
+	    variable _cur_col
+	    variable _cur_row
+
+	    set _cur_row [expr {$row - 1}]
+	    set _cur_col [expr {$col - 1}]
+	}
     }
 
     # Cursor Horizontal Forward Tabulation.
@@ -231,11 +296,15 @@ namespace eval Term {
     # https://vt100.net/docs/vt510-rm/CHT.html
     proc _csi_I {args} {
 	set n [_default [lindex $args 0] 1]
-	variable _cur_col
-	variable _cols
-	incr _cur_col [expr {$n * 8 - $_cur_col % 8}]
-	if {$_cur_col >= $_cols} {
-	    set _cur_col [expr {$_cols - 1}]
+
+	_log_cur "Cursor Horizontal Forward Tabulation ($n)" {
+	    variable _cur_col
+	    variable _cols
+
+	    incr _cur_col [expr {$n * 8 - $_cur_col % 8}]
+	    if {$_cur_col >= $_cols} {
+		set _cur_col [expr {$_cols - 1}]
+	    }
 	}
     }
 
@@ -243,19 +312,23 @@ namespace eval Term {
     #
     # https://vt100.net/docs/vt510-rm/ED.html
     proc _csi_J {args} {
-	variable _cur_col
-	variable _cur_row
-	variable _rows
-	variable _cols
 	set arg [_default [lindex $args 0] 0]
-	if {$arg == 0} {
-	    _clear_in_line $_cur_col $_cols $_cur_row
-	    _clear_lines [expr {$_cur_row + 1}] $_rows
-	} elseif {$arg == 1} {
-	    _clear_lines 0 [expr {$_cur_row - 1}]
-	    _clear_in_line 0 $_cur_col $_cur_row
-	} elseif {$arg == 2} {
-	    _clear_lines 0 $_rows
+
+	_log_cur "Erase in Display ($arg)" {
+	    variable _cur_col
+	    variable _cur_row
+	    variable _rows
+	    variable _cols
+
+	    if {$arg == 0} {
+		_clear_in_line $_cur_col $_cols $_cur_row
+		_clear_lines [expr {$_cur_row + 1}] $_rows
+	    } elseif {$arg == 1} {
+		_clear_lines 0 [expr {$_cur_row - 1}]
+		_clear_in_line 0 $_cur_col $_cur_row
+	    } elseif {$arg == 2} {
+		_clear_lines 0 $_rows
+	    }
 	}
     }
 
@@ -263,17 +336,21 @@ namespace eval Term {
     #
     # https://vt100.net/docs/vt510-rm/EL.html
     proc _csi_K {args} {
-	variable _cur_col
-	variable _cur_row
-	variable _cols
 	set arg [_default [lindex $args 0] 0]
-	if {$arg == 0} {
-	    # From cursor to end.
-	    _clear_in_line $_cur_col $_cols $_cur_row
-	} elseif {$arg == 1} {
-	    _clear_in_line 0 $_cur_col $_cur_row
-	} elseif {$arg == 2} {
-	    _clear_in_line 0 $_cols $_cur_row
+
+	_log_cur "Erase in Line ($arg)" {
+	    variable _cur_col
+	    variable _cur_row
+	    variable _cols
+
+	    if {$arg == 0} {
+		# From cursor to end.
+		_clear_in_line $_cur_col $_cols $_cur_row
+	    } elseif {$arg == 1} {
+		_clear_in_line 0 $_cur_col $_cur_row
+	    } elseif {$arg == 2} {
+		_clear_in_line 0 $_cols $_cur_row
+	    }
 	}
     }
 
@@ -281,22 +358,26 @@ namespace eval Term {
     #
     # https://vt100.net/docs/vt510-rm/DL.html
     proc _csi_M {args} {
-	variable _cur_row
-	variable _rows
-	variable _cols
-	variable _chars
 	set count [_default [lindex $args 0] 1]
-	set y $_cur_row
-	set next_y [expr {$y + 1}]
-	while {$count > 0 && $next_y < $_rows} {
-	    for {set x 0} {$x < $_cols} {incr x} {
-		set _chars($x,$y) $_chars($x,$next_y)
+
+	_log_cur "Delete line ($count)" {
+	    variable _cur_row
+	    variable _rows
+	    variable _cols
+	    variable _chars
+
+	    set y $_cur_row
+	    set next_y [expr {$y + 1}]
+	    while {$count > 0 && $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
 	    }
-	    incr y
-	    incr next_y
-	    incr count -1
+	    _clear_lines $next_y $_rows
 	}
-	_clear_lines $next_y $_rows
     }
 
     # Erase chars.
@@ -304,16 +385,20 @@ namespace eval Term {
     # https://vt100.net/docs/vt510-rm/ECH.html
     proc _csi_X {args} {
 	set n [_default [lindex $args 0] 1]
-	# Erase characters but don't move cursor.
-	variable _cur_col
-	variable _cur_row
-	variable _attrs
-	variable _chars
-	set lattr [array get _attrs]
-	set x $_cur_col
-	for {set i 0} {$i < $n} {incr i} {
-	    set _chars($x,$_cur_row) [list " " $lattr]
-	    incr x
+
+	_log_cur "Erase chars ($n)" {
+	    # Erase characters but don't move cursor.
+	    variable _cur_col
+	    variable _cur_row
+	    variable _attrs
+	    variable _chars
+
+	    set lattr [array get _attrs]
+	    set x $_cur_col
+	    for {set i 0} {$i < $n} {incr i} {
+		set _chars($x,$_cur_row) [list " " $lattr]
+		incr x
+	    }
 	}
     }
 
@@ -322,96 +407,117 @@ namespace eval Term {
     # https://vt100.net/docs/vt510-rm/CBT.html
     proc _csi_Z {args} {
 	set n [_default [lindex $args 0] 1]
-	variable _cur_col
-	set _cur_col [expr {max (int (($_cur_col - 1) / 8) * 8 - ($n - 1) * 8, 0)}]
+
+	_log_cur "Cursor Backward Tabulation ($n)" {
+	    variable _cur_col
+
+	    set _cur_col [expr {max (int (($_cur_col - 1) / 8) * 8 - ($n - 1) * 8, 0)}]
+	}
     }
 
     # Repeat.
     #
     # https://www.xfree86.org/current/ctlseqs.html (See `(REP)`)
     proc _csi_b {args} {
-	variable _last_char
 	set n [_default [lindex $args 0] 1]
-	_insert [string repeat $_last_char $n]
+
+	_log_cur "Repeat ($n)" {
+	    variable _last_char
+
+	    _insert [string repeat $_last_char $n]
+	}
     }
 
     # Vertical Line Position Absolute.
     #
     # https://vt100.net/docs/vt510-rm/VPA.html
     proc _csi_d {args} {
-	variable _cur_row
-	set _cur_row [expr {[_default [lindex $args 0] 1] - 1}]
+	set row [_default [lindex $args 0] 1]
+
+	_log_cur "Vertical Line Position Absolute ($row)" {
+	    variable _cur_row
+
+	    set _cur_row [expr {$row - 1}]
+	}
     }
 
     # Select Graphic Rendition.
     #
     # https://vt100.net/docs/vt510-rm/SGR.html
     proc _csi_m {args} {
-	variable _attrs
-	foreach item $args {
-	    switch -exact -- $item {
-		"" - 0 {
-		    set _attrs(intensity) normal
-		    set _attrs(fg) default
-		    set _attrs(bg) default
-		    set _attrs(underline) 0
-		    set _attrs(reverse) 0
-		}
-		1 {
-		    set _attrs(intensity) bold
-		}
-		2 {
-		    set _attrs(intensity) dim
-		}
-		4 {
-		    set _attrs(underline) 1
-		}
-		7 {
-		    set _attrs(reverse) 1
-		}
-		22 {
-		    set _attrs(intensity) normal
-		}
-		24 {
-		    set _attrs(underline) 0
-		}
-		27 {
-		    set _attrs(reverse) 1
-		}
-		30 - 31 - 32 - 33 - 34 - 35 - 36 - 37 {
-		    set _attrs(fg) $item
-		}
-		39 {
-		    set _attrs(fg) default
-		}
-		40 - 41 - 42 - 43 - 44 - 45 - 46 - 47 {
-		    set _attrs(bg) $item
-		}
-		49 {
-		    set _attrs(bg) default
-		}
-	    }
+	_log_cur "Select Graphic Rendition ([join $args {, }])" {
+	  variable _attrs
+
+	  foreach item $args {
+	      switch -exact -- $item {
+		  "" - 0 {
+		      set _attrs(intensity) normal
+		      set _attrs(fg) default
+		      set _attrs(bg) default
+		      set _attrs(underline) 0
+		      set _attrs(reverse) 0
+		  }
+		  1 {
+		      set _attrs(intensity) bold
+		  }
+		  2 {
+		      set _attrs(intensity) dim
+		  }
+		  4 {
+		      set _attrs(underline) 1
+		  }
+		  7 {
+		      set _attrs(reverse) 1
+		  }
+		  22 {
+		      set _attrs(intensity) normal
+		  }
+		  24 {
+		      set _attrs(underline) 0
+		  }
+		  27 {
+		      set _attrs(reverse) 1
+		  }
+		  30 - 31 - 32 - 33 - 34 - 35 - 36 - 37 {
+		      set _attrs(fg) $item
+		  }
+		  39 {
+		      set _attrs(fg) default
+		  }
+		  40 - 41 - 42 - 43 - 44 - 45 - 46 - 47 {
+		      set _attrs(bg) $item
+		  }
+		  49 {
+		      set _attrs(bg) default
+		  }
+	      }
+	  }
 	}
     }
 
     # Insert string at the cursor location.
     proc _insert {str} {
-	verbose "INSERT <<$str>>"
-	variable _cur_col
-	variable _cur_row
-	variable _rows
-	variable _cols
-	variable _attrs
-	variable _chars
-	set lattr [array get _attrs]
-	foreach char [split $str {}] {
-	    set _chars($_cur_col,$_cur_row) [list $char $lattr]
-	    incr _cur_col
-	    if {$_cur_col >= $_cols} {
-		set _cur_col 0
-		incr _cur_row
-		if {$_cur_row >= $_rows} {
-		    error "FIXME scroll"
+	_log_cur "Inserted string '$str'" {
+	    _log "Inserting string '$str'"
+
+	    variable _cur_col
+	    variable _cur_row
+	    variable _rows
+	    variable _cols
+	    variable _attrs
+	    variable _chars
+	    set lattr [array get _attrs]
+	    foreach char [split $str {}] {
+		  _log_cur "  Inserted char '$char'" {
+		    set _chars($_cur_col,$_cur_row) [list $char $lattr]
+		    incr _cur_col
+		    if {$_cur_col >= $_cols} {
+			set _cur_col 0
+			incr _cur_row
+			if {$_cur_row >= $_rows} {
+			    error "FIXME scroll"
+			}
+		    }
 		}
 	    }
 	}
-- 
2.29.2

Re: [PATCH 1/2] gdb/testsuite: add links for handled control sequences in lib/tuiterm.exp

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> gdb/testsuite/ChangeLog:

Simon> 	* lib/tuiterm.exp: Add links in comments.

Nice.  Thank you for doing this.

Tom

Re: [PATCH 2/2] gdb/testsuite: rename _cur_x/_cur_y to _cur_col/_cur_row in lib/tuiterm.exp

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> I am having trouble remembering which of _cur_x/_cur_y is columns and
Simon> which is rows, so renaming them helps.  We already have _rows and _cols
Simon> to represent the terminal size, so I think that makes sense to name the
Simon> "_cur" variables accordingly.

Hah, i have the opposite problem, where "x" and "y" are clear but I
have to think about row/column.

Simon> gdb/testsuite/ChangeLog:

Simon> 	* lib/tuiterm.exp: Rename _cur_x/_cur_y to _cur_col/_cur_row.

Looks good.  Thanks.

Tom

Re: [PATCH 3/2] gdb/testsuite: improve logging in lib/tuiterm.exp

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> Here are some examples of the logging found in gdb.log with this patch
Simon> applied:

Simon>     +++ Inserting string '+|'
Simon>     +++   Inserted char '+', cursor: (0, 79) -> (1, 0)
Simon>     +++   Inserted char '|', cursor: (1, 0) -> (1, 1)
Simon>     +++ Inserted string '+|', cursor: (0, 79) -> (1, 1)
Simon>     +++ Cursor Horizontal Absolute (80), cursor: (1, 1) -> (1, 79)

Simon> In the last line, note that the argument is 80 and we move to 79, that's
Simon> because the position in the argument to the control sequence is 1-based,
Simon> while our indexing is 0-based.

Seems fine though I have some questions.

Simon> +    proc _log { what } {
Simon> +	verbose -log "+++ $what"

wait_for already does logging, but doesn't use -log.  Should it?  Or
maybe the new proc should also just use "verbose"?

Tom

Re: [PATCH 2/2] gdb/testsuite: rename _cur_x/_cur_y to _cur_col/_cur_row in lib/tuiterm.exp

[Simon Marchi]

On 2021-01-11 3:56 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> I am having trouble remembering which of _cur_x/_cur_y is columns and
> Simon> which is rows, so renaming them helps.  We already have _rows and _cols
> Simon> to represent the terminal size, so I think that makes sense to name the
> Simon> "_cur" variables accordingly.
> 
> Hah, i have the opposite problem, where "x" and "y" are clear but I
> have to think about row/column.

Oh well... the important thing is that we use consistent naming!

> Simon> gdb/testsuite/ChangeLog:
> 
> Simon> 	* lib/tuiterm.exp: Rename _cur_x/_cur_y to _cur_col/_cur_row.
> 
> Looks good.  Thanks.
> 
> Tom

Ok, thanks, I'll push it.

Simon

Re: [PATCH 3/2] gdb/testsuite: improve logging in lib/tuiterm.exp

[Simon Marchi]

On 2021-01-11 4:01 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> Here are some examples of the logging found in gdb.log with this patch
> Simon> applied:
> 
> Simon>     +++ Inserting string '+|'
> Simon>     +++   Inserted char '+', cursor: (0, 79) -> (1, 0)
> Simon>     +++   Inserted char '|', cursor: (1, 0) -> (1, 1)
> Simon>     +++ Inserted string '+|', cursor: (0, 79) -> (1, 1)
> Simon>     +++ Cursor Horizontal Absolute (80), cursor: (1, 1) -> (1, 79)
> 
> Simon> In the last line, note that the argument is 80 and we move to 79, that's
> Simon> because the position in the argument to the control sequence is 1-based,
> Simon> while our indexing is 0-based.
> 
> Seems fine though I have some questions.
> 
> Simon> +    proc _log { what } {
> Simon> +	verbose -log "+++ $what"
> 
> wait_for already does logging, but doesn't use -log.  Should it?  Or
> maybe the new proc should also just use "verbose"?


Hmm right.  I would prefer to make it use -log too.  That way it ends
up in gdb.log, it's easy to find.  I would therefore apply this fixup
to the patch.

From 8cc1c5239b5f4d64427834a5f4a828777f7cb974 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Wed, 20 Jan 2021 16:15:46 -0500
Subject: [PATCH] fixup

Change-Id: I378023066bdeb5cbe2b491e8c2b95bae09d29c3e
---
 gdb/testsuite/lib/tuiterm.exp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gdb/testsuite/lib/tuiterm.exp b/gdb/testsuite/lib/tuiterm.exp
index a77c8d39e546..4160586b615c 100644
--- a/gdb/testsuite/lib/tuiterm.exp
+++ b/gdb/testsuite/lib/tuiterm.exp
@@ -567,17 +567,17 @@ namespace eval Term {
 		-re "^\[\x07\x08\x0a\x0d\]" {
 		    scan $expect_out(0,string) %c val
 		    set hexval [format "%02x" $val]
-		    verbose "+++ _ctl_0x${hexval}"
+		    _log "wait_for: _ctl_0x${hexval}"
 		    _ctl_0x${hexval}
 		}
 		-re "^\x1b(\[0-9a-zA-Z\])" {
-		    verbose "+++ unsupported escape"
+		    _log "wait_for: unsupported escape"
 		    error "unsupported escape"
 		}
 		-re "^\x1b\\\[(\[0-9;\]*)(\[a-zA-Z@\])" {
 		    set cmd $expect_out(2,string)
 		    set params [split $expect_out(1,string) ";"]
-		    verbose "+++ _csi_$cmd <<<$expect_out(1,string)>>>"
+		    _log "wait_for: _csi_$cmd <<<$expect_out(1,string)>>>"
 		    eval _csi_$cmd $params
 		}
 		-re "^\[^\x07\x08\x0a\x0d\x1b\]+" {
-- 
2.30.0

Re: [PATCH 3/2] gdb/testsuite: improve logging in lib/tuiterm.exp

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

>> wait_for already does logging, but doesn't use -log.  Should it?  Or
>> maybe the new proc should also just use "verbose"?


Simon> Hmm right.  I would prefer to make it use -log too.  That way it ends
Simon> up in gdb.log, it's easy to find.  I would therefore apply this fixup
Simon> to the patch.

Seems fine to me, thanks.

Tom

Re: [PATCH 3/2] gdb/testsuite: improve logging in lib/tuiterm.exp

[Simon Marchi]

On 2021-01-21 1:45 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
>>> wait_for already does logging, but doesn't use -log.  Should it?  Or
>>> maybe the new proc should also just use "verbose"?
> 
> 
> Simon> Hmm right.  I would prefer to make it use -log too.  That way it ends
> Simon> up in gdb.log, it's easy to find.  I would therefore apply this fixup
> Simon> to the patch.
> 
> Seems fine to me, thanks.
> 
> Tom
> 

Thanks, pushed.

Simon