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

@@ -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,12 +78,7 @@ 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:
+	if _fd >= 0:
-		push_error("Resizing must be done using positive cols and rows.")
-
-	if _fd < 0:
-		return
-
 		PTYUnix.new().resize(_fd, cols, rows)
 
 
@@ -135,8 +116,8 @@ func fork(
 	file: String = OS.get_environment("SHELL"),
 	args: PoolStringArray = PoolStringArray(),
 	cwd = LibuvUtils.get_cwd(),
-	p_cols: int = DEFAULT_COLS,
+	cols: int = DEFAULT_COLS,
-	p_rows: int = DEFAULT_ROWS,
+	rows: int = DEFAULT_ROWS,
 	uid: int = -1,
 	gid: int = -1,
 	utf8 = true

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:
+func resize(cols = _cols, rows = _rows) -> void:
-	_pty_native.resize(cols, rows)
+	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,
+	cols: int = _cols,
-	p_rows: int = DEFAULT_ROWS,
+	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

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 (?<rows>[0-9]+).*columns (?<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
 

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"
+}