[PATCH] gdb/testsuite/tui: implement _csi_P proc

[PATCH] gdb/testsuite/tui: implement _csi_P proc

[Simon Marchi]

Since commit 3cd522938792 ("Change the pager to a ui_file"), I see these
errors when running gdb.tui/scroll.exp:

    ERROR: invalid command name "_csi_P"
        while executing
    "::gdb_tcl_unknown _csi_P 2"
        ("uplevel" body line 1)
        invoked from within
    "uplevel 1 ::gdb_tcl_unknown $args"
        (procedure "::unknown" line 5)
        invoked from within
    "_csi_P 2"
        ("eval" body line 1)
        invoked from within
    "eval _csi_$cmd $params"

It looks like GDB is emitting a CSI that it did not emit before, the
"Delete character" one:

  https://vt100.net/docs/vt510-rm/DCH.html

Implement it.

When I added the _csi_P but without an actual implementation, I stopped
seeing the error shown above, but I had a FAIL.  After adding the
implementation, the test started passing.

Note that I did not investigate why GDB was suddenly emitting this CSI,
if it makes sense, or if I'm just papering over the problem.

Change-Id: I5bf86b6104d51b0623a26a69df83d1ca9a4851b7
---
 gdb/testsuite/lib/tuiterm.exp | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/gdb/testsuite/lib/tuiterm.exp b/gdb/testsuite/lib/tuiterm.exp
index 38948015e96..94bb2703889 100644
--- a/gdb/testsuite/lib/tuiterm.exp
+++ b/gdb/testsuite/lib/tuiterm.exp
@@ -349,6 +349,36 @@ namespace eval Term {
 	}
     }
 
+    # Delete character.
+    #
+    # https://vt100.net/docs/vt510-rm/DCH.html
+    proc _csi_P {args} {
+	set count [_default [lindex $args 0] 1]
+
+	_log_cur "Delete character ($count)" {
+	    variable _cur_row
+	    variable _cur_col
+	    variable _chars
+	    variable _cols
+
+	    set y $_cur_row
+
+	    # Move all characters right of the cursor COUNT positions left.
+	    for {set x $_cur_col} {$x < $_cols} {incr x} {
+		set from_x [expr $x + $count]
+
+		if { $from_x < $_cols } {
+		    set c $_chars($from_x,$y)
+		} else {
+		    # Insert blank spaces from the right margin.
+		    set c " "
+		}
+
+		set _chars($x,$y) $c
+	    }
+	}
+    }
+
     # Erase chars.
     #
     # https://vt100.net/docs/vt510-rm/ECH.html
-- 
2.35.1

Re: [PATCH] gdb/testsuite/tui: implement _csi_P proc

[Andrew Burgess]

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

> Since commit 3cd522938792 ("Change the pager to a ui_file"), I see these
> errors when running gdb.tui/scroll.exp:
>
>     ERROR: invalid command name "_csi_P"
>         while executing
>     "::gdb_tcl_unknown _csi_P 2"
>         ("uplevel" body line 1)
>         invoked from within
>     "uplevel 1 ::gdb_tcl_unknown $args"
>         (procedure "::unknown" line 5)
>         invoked from within
>     "_csi_P 2"
>         ("eval" body line 1)
>         invoked from within
>     "eval _csi_$cmd $params"
>
> It looks like GDB is emitting a CSI that it did not emit before, the
> "Delete character" one:
>
>   https://vt100.net/docs/vt510-rm/DCH.html
>
> Implement it.
>
> When I added the _csi_P but without an actual implementation, I stopped
> seeing the error shown above, but I had a FAIL.  After adding the
> implementation, the test started passing.
>
> Note that I did not investigate why GDB was suddenly emitting this CSI,
> if it makes sense, or if I'm just papering over the problem.

Maybe I should just go ahead and push this:

  https://sourceware.org/pipermail/gdb-patches/2022-March/186433.html

This includes _csi_P, _csi_L, and _csi_S, plus a bug fix in _csi_K.

As far as I can tell our _csi_P implementations are equivalent.

Thoughts?

Thanks,
Andrew


>
> Change-Id: I5bf86b6104d51b0623a26a69df83d1ca9a4851b7
> ---
>  gdb/testsuite/lib/tuiterm.exp | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git a/gdb/testsuite/lib/tuiterm.exp b/gdb/testsuite/lib/tuiterm.exp
> index 38948015e96..94bb2703889 100644
> --- a/gdb/testsuite/lib/tuiterm.exp
> +++ b/gdb/testsuite/lib/tuiterm.exp
> @@ -349,6 +349,36 @@ namespace eval Term {
>  	}
>      }
>  
> +    # Delete character.
> +    #
> +    # https://vt100.net/docs/vt510-rm/DCH.html
> +    proc _csi_P {args} {
> +	set count [_default [lindex $args 0] 1]
> +
> +	_log_cur "Delete character ($count)" {
> +	    variable _cur_row
> +	    variable _cur_col
> +	    variable _chars
> +	    variable _cols
> +
> +	    set y $_cur_row
> +
> +	    # Move all characters right of the cursor COUNT positions left.
> +	    for {set x $_cur_col} {$x < $_cols} {incr x} {
> +		set from_x [expr $x + $count]
> +
> +		if { $from_x < $_cols } {
> +		    set c $_chars($from_x,$y)
> +		} else {
> +		    # Insert blank spaces from the right margin.
> +		    set c " "
> +		}
> +
> +		set _chars($x,$y) $c
> +	    }
> +	}
> +    }
> +
>      # Erase chars.
>      #
>      # https://vt100.net/docs/vt510-rm/ECH.html
> -- 
> 2.35.1

Re: [PATCH] gdb/testsuite/tui: implement _csi_P proc

[Simon Marchi]

> Maybe I should just go ahead and push this:
> 
>   https://sourceware.org/pipermail/gdb-patches/2022-March/186433.html
> 
> This includes _csi_P, _csi_L, and _csi_S, plus a bug fix in _csi_K.
> 
> As far as I can tell our _csi_P implementations are equivalent.

Oh, I hadn't seen this (I must skip some threads / series, because I
don't have time to read everything).

I wasn't sure if our implementations were equivalent, there seemed to be
an off-by-one difference in the row/column.  I wasn't sure though, so
that prompted me to start writing some unit tests for Term.  I'll send a
patch soon to add them.  After that, you could rebase your patch on top,
add some tests for the control sequences you add support for, so we can
be more confident that the implementation is right.

Simon