From f774c903fe9e4edd0ea1677646b7976d60ff9f41 Mon Sep 17 00:00:00 2001 From: Leroy Hopson Date: Fri, 12 Aug 2022 10:24:54 +1200 Subject: [PATCH] Ensure initial PTY size matches Terminal (if any) If a PTY has a terminal_path set to a valid Terminal, then ensure that the initial cols and rows of PTY match the cols and rows of the Terminal when calling fork() or open(), otherwise PTY will output wrong-sized data for the Terminal until resized. Fixes #56. --- addons/godot_xterm/nodes/pty/unix/pty_unix.gd | 27 +--- addons/godot_xterm/pty.gd | 52 +++++-- test/platform/unix/unix.test.gd | 55 ++++++++ test/scenes/pty_and_terminal.tscn | 131 ++++++++++++++++++ 4 files changed, 231 insertions(+), 34 deletions(-) create mode 100644 test/scenes/pty_and_terminal.tscn diff --git a/addons/godot_xterm/nodes/pty/unix/pty_unix.gd b/addons/godot_xterm/nodes/pty/unix/pty_unix.gd index 163c5e4..3f12859 100644 --- a/addons/godot_xterm/nodes/pty/unix/pty_unix.gd +++ b/addons/godot_xterm/nodes/pty/unix/pty_unix.gd @@ -46,12 +46,6 @@ enum Signal { # The process ID. var _pid: int -# The column size in characters. -var cols: int = DEFAULT_COLS setget set_cols - -# The row size in characters. -var rows: int = DEFAULT_ROWS setget set_rows - # Environment to be set for the child program. var env := DEFAULT_ENV @@ -71,14 +65,6 @@ var _fd: int = -1 var _exit_cb: FuncRef -func set_cols(value: int): - resize(value, rows) - - -func set_rows(value: int): - resize(cols, value) - - # Writes data to the socket. # data: The data to write. func write(data) -> void: @@ -92,13 +78,8 @@ func write(data) -> void: func resize(cols: int, rows: int) -> void: - if cols <= 0 or rows <= 0 or cols == NAN or rows == NAN or cols == INF or rows == INF: - push_error("Resizing must be done using positive cols and rows.") - - if _fd < 0: - return - - PTYUnix.new().resize(_fd, cols, rows) + if _fd >= 0: + PTYUnix.new().resize(_fd, cols, rows) func kill(signum: int = Signal.SIGHUP) -> void: @@ -135,8 +116,8 @@ func fork( file: String = OS.get_environment("SHELL"), args: PoolStringArray = PoolStringArray(), cwd = LibuvUtils.get_cwd(), - p_cols: int = DEFAULT_COLS, - p_rows: int = DEFAULT_ROWS, + cols: int = DEFAULT_COLS, + rows: int = DEFAULT_ROWS, uid: int = -1, gid: int = -1, utf8 = true diff --git a/addons/godot_xterm/pty.gd b/addons/godot_xterm/pty.gd index ae3defe..11185cc 100644 --- a/addons/godot_xterm/pty.gd +++ b/addons/godot_xterm/pty.gd @@ -28,10 +28,10 @@ export(NodePath) var terminal_path := NodePath() setget set_terminal_path var _terminal: _Terminal = null setget _set_terminal # The column size in characters. -export(int) var cols: int = DEFAULT_COLS setget set_cols +export(int) var cols: int = DEFAULT_COLS setget set_cols, get_cols # The row size in characters. -export(int) var rows: int = DEFAULT_ROWS setget set_rows +export(int) var rows: int = DEFAULT_ROWS setget set_rows, get_rows # Environment to be set for the child program. export(Dictionary) var env := DEFAULT_ENV @@ -41,6 +41,8 @@ export(Dictionary) var env := DEFAULT_ENV # former taking precedence in the case of conflicts. export(bool) var use_os_env := true +var _cols := DEFAULT_COLS +var _rows := DEFAULT_ROWS var _pty_native: _PTYNative @@ -58,12 +60,25 @@ func _init(): add_child(_pty_native) +func _ready(): + if terminal_path and not _terminal: + set_terminal_path(terminal_path) + + func set_cols(value: int): - resize(value, rows) + resize(value, _rows) + + +func get_cols() -> int: + return _cols func set_rows(value: int): - resize(cols, value) + resize(_cols, value) + + +func get_rows() -> int: + return _rows func set_terminal_path(value := NodePath()): @@ -87,7 +102,7 @@ func _set_terminal(value: _Terminal): return # Connect the new terminal. - # FIXME! resize(terminal.get_cols(), terminal.get_rows()) + resize(_terminal.cols, _terminal.rows) if not _terminal.is_connected("size_changed", self, "resizev"): _terminal.connect("size_changed", self, "resizev") if not _terminal.is_connected("data_sent", self, "write"): @@ -105,13 +120,20 @@ func write(data) -> void: # Resizes the dimensions of the pty. # cols: The number of columns. # rows: The number of rows. -func resize(cols, rows = null) -> void: - _pty_native.resize(cols, rows) +func resize(cols = _cols, rows = _rows) -> void: + if not _valid_size(cols, rows): + push_error("Size of cols/rows must be a positive integer.") + return + + _cols = cols + _rows = rows + + _pty_native.resize(_cols, _rows) # Same as resize() but takes a Vector2. func resizev(size: Vector2) -> void: - resize(size.x, size.y) + resize(int(size.x), int(size.y)) # Kill the pty. @@ -133,13 +155,17 @@ func fork( file: String = OS.get_environment("SHELL"), args: PoolStringArray = PoolStringArray(), cwd = _LibuvUtils.get_cwd(), - p_cols: int = DEFAULT_COLS, - p_rows: int = DEFAULT_ROWS, + cols: int = _cols, + rows: int = _rows, uid: int = -1, gid: int = -1, utf8 = true ) -> int: - return _pty_native.fork(file, args, cwd, p_cols, p_rows, uid, gid, utf8) + resize(cols, rows) # Ensures error message is printed if cols/rows are invalid. + if not _valid_size(cols, rows): + return ERR_INVALID_PARAMETER + + return _pty_native.fork(file, args, cwd, _cols, _rows, uid, gid, utf8) func open(cols: int = DEFAULT_COLS, rows: int = DEFAULT_ROWS) -> Array: @@ -156,3 +182,7 @@ func _on_pty_native_data_received(data): func _on_pty_native_exited(exit_code: int, signum: int) -> void: emit_signal("exited", exit_code, signum) + + +static func _valid_size(cols: int, rows: int) -> bool: + return cols > 0 and rows > 0 and cols != NAN and rows != NAN and cols != INF and rows != INF diff --git a/test/platform/unix/unix.test.gd b/test/platform/unix/unix.test.gd index d2d8e4f..c2f8c40 100644 --- a/test/platform/unix/unix.test.gd +++ b/test/platform/unix/unix.test.gd @@ -167,6 +167,61 @@ class Helper: return {rows = int(size.x), cols = int(size.y)} +class TestPTYSize: + extends "res://addons/gut/test.gd" + # Tests to check that psuedoterminal size (as reported by the stty command) + # matches the size of the Terminal node. Uses various scene tree layouts with + # Terminal and PTY nodes in different places. + # See: https://github.com/lihop/godot-xterm/issues/56 + + const PTY := preload("res://addons/godot_xterm/pty.gd") + const Terminal := preload("res://addons/godot_xterm/terminal.gd") + + var pty: PTY + var terminal: Terminal + var scene: Node + var regex := RegEx.new() + + func before_all(): + regex.compile(".*rows (?[0-9]+).*columns (?[0-9]+).*") + + func before_each(): + scene = add_child_autofree(preload("res://test/scenes/pty_and_terminal.tscn").instance()) + + func test_correct_stty_reports_correct_size(): + for s in [ + "PTYChild", + "PTYSiblingAbove", + "PTYSiblingBelow", + "PTYCousinAbove", + "PTYCousinBelow", + "PTYCousinAbove2", + "PTYCousinBelow2" + ]: + pty = scene.get_node(s).find_node("PTY") + terminal = scene.get_node(s).find_node("Terminal") + + pty.call_deferred("fork", OS.get_environment("SHELL")) + pty.call_deferred("write", "stty -a | head -n1\n") + var output := "" + while not "rows" in output and not "columns" in output: + output = (yield(pty, "data_received")).get_string_from_utf8() + var regex_match = regex.search(output) + var stty_rows = int(regex_match.get_string("rows")) + var stty_cols = int(regex_match.get_string("columns")) + + assert_eq( + stty_rows, + terminal.rows, + "Expected stty to report correct number of rows for layout '%s'" % s + ) + assert_eq( + stty_cols, + terminal.cols, + "Expected stty to report correct number of columns for layout '%s'" % s + ) + + class LinuxHelper: extends Helper diff --git a/test/scenes/pty_and_terminal.tscn b/test/scenes/pty_and_terminal.tscn new file mode 100644 index 0000000..0cc79b7 --- /dev/null +++ b/test/scenes/pty_and_terminal.tscn @@ -0,0 +1,131 @@ +[gd_scene load_steps=3 format=2] + +[ext_resource path="res://addons/godot_xterm/terminal.gd" type="Script" id=1] +[ext_resource path="res://addons/godot_xterm/pty.gd" type="Script" id=2] + +[node name="PTYandTerminal" type="Node"] + +[node name="PTYChild" type="Node" parent="."] + +[node name="Terminal" type="Control" parent="PTYChild"] +anchor_right = 1.0 +anchor_bottom = 1.0 +script = ExtResource( 1 ) + +[node name="PTY" type="Node" parent="PTYChild/Terminal"] +script = ExtResource( 2 ) +terminal_path = NodePath("..") +env = { +"COLORTERM": "truecolor", +"TERM": "xterm-256color" +} + +[node name="PTYSiblingAbove" type="Node" parent="."] + +[node name="PTY" type="Node" parent="PTYSiblingAbove"] +script = ExtResource( 2 ) +terminal_path = NodePath("../Terminal") +env = { +"COLORTERM": "truecolor", +"TERM": "xterm-256color" +} + +[node name="Terminal" type="Control" parent="PTYSiblingAbove"] +anchor_right = 1.0 +anchor_bottom = 1.0 +script = ExtResource( 1 ) + +[node name="PTYSiblingBelow" type="Node" parent="."] + +[node name="Terminal" type="Control" parent="PTYSiblingBelow"] +anchor_right = 1.0 +anchor_bottom = 1.0 +script = ExtResource( 1 ) + +[node name="PTY" type="Node" parent="PTYSiblingBelow"] +script = ExtResource( 2 ) +terminal_path = NodePath("../Terminal") +env = { +"COLORTERM": "truecolor", +"TERM": "xterm-256color" +} + +[node name="PTYCousinAbove" type="Node" parent="."] + +[node name="Node" type="Node" parent="PTYCousinAbove"] + +[node name="PTY" type="Node" parent="PTYCousinAbove/Node"] +script = ExtResource( 2 ) +terminal_path = NodePath("../../Terminal") +env = { +"COLORTERM": "truecolor", +"TERM": "xterm-256color" +} + +[node name="Terminal" type="Control" parent="PTYCousinAbove"] +anchor_right = 1.0 +anchor_bottom = 1.0 +script = ExtResource( 1 ) + +[node name="PTYCousinBelow" type="Node" parent="."] + +[node name="Terminal" type="Control" parent="PTYCousinBelow"] +anchor_right = 1.0 +anchor_bottom = 1.0 +script = ExtResource( 1 ) + +[node name="Node" type="Node" parent="PTYCousinBelow"] + +[node name="PTY" type="Node" parent="PTYCousinBelow/Node"] +script = ExtResource( 2 ) +terminal_path = NodePath("../../Terminal") +env = { +"COLORTERM": "truecolor", +"TERM": "xterm-256color" +} + +[node name="PTYCousinAbove2" type="Node" parent="."] + +[node name="Node" type="Node" parent="PTYCousinAbove2"] + +[node name="Node" type="Node" parent="PTYCousinAbove2/Node"] + +[node name="PTY" type="Node" parent="PTYCousinAbove2/Node/Node"] +script = ExtResource( 2 ) +terminal_path = NodePath("../../../Control/Terminal") +env = { +"COLORTERM": "truecolor", +"TERM": "xterm-256color" +} + +[node name="Control" type="Control" parent="PTYCousinAbove2"] +anchor_right = 1.0 +anchor_bottom = 1.0 + +[node name="Terminal" type="Control" parent="PTYCousinAbove2/Control"] +anchor_right = 1.0 +anchor_bottom = 1.0 +script = ExtResource( 1 ) + +[node name="PTYCousinBelow2" type="Node" parent="."] + +[node name="Control" type="Control" parent="PTYCousinBelow2"] +anchor_right = 1.0 +anchor_bottom = 1.0 + +[node name="Terminal" type="Control" parent="PTYCousinBelow2/Control"] +anchor_right = 1.0 +anchor_bottom = 1.0 +script = ExtResource( 1 ) + +[node name="Node" type="Node" parent="PTYCousinBelow2"] + +[node name="Node" type="Node" parent="PTYCousinBelow2/Node"] + +[node name="PTY" type="Node" parent="PTYCousinBelow2/Node/Node"] +script = ExtResource( 2 ) +terminal_path = NodePath("../../../Control/Terminal") +env = { +"COLORTERM": "truecolor", +"TERM": "xterm-256color" +}