Skip to content

Commit e1e09d0

Browse files
committed
This is a fix for the fact that I was treating savedY in two different
ways in the codebase, either a viewport relative value, or a buffer relative value (0..<rows vs yBase offset). I compared this to xterm.js which was the original code for this, and they do not have that discrepancy - either it got fixed there, or I introduced the bug. https://gist.github.com/migueldeicaza/fb7470de4b4c9501c7eb660bc56a3ede Now treats savedY as absolute (yBase-baesd). Also, it now converts absolute savedY back to viewport row (savedY - yBase) and then clamps via restrictCursor(). This should be a more comprehensive fix than the one reported in #467 While auditing I found another difference, softReset now also resets the saved cursor position
1 parent 14b0a2d commit e1e09d0

3 files changed

Lines changed: 55 additions & 11 deletions

File tree

Sources/SwiftTerm/Buffer.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ public final class Buffer {
135135
public var savedX: Int
136136

137137
/**
138-
* This records the saved Y position
138+
* This records the saved absolute Y position (yBase + y)
139139
*/
140140
public var savedY: Int
141141

@@ -370,7 +370,7 @@ public final class Buffer {
370370
public func softReset ()
371371
{
372372
savedAttr = CharData.defaultAttr
373-
savedY = 0
373+
savedY = yBase
374374
savedX = 0
375375
savedCharset = CharSets.defaultCharset
376376
marginRight = cols-1

Sources/SwiftTerm/Terminal.swift

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -757,13 +757,8 @@ open class Terminal {
757757
}
758758

759759
func resizeBuffers(newColumns: Int, newRows: Int) {
760-
// correct the savedY cursor to follow changes to y
761-
let dy = normalBuffer.savedY - normalBuffer.y
762760
normalBuffer.resize (newCols: newColumns, newRows: newRows)
763-
normalBuffer.savedY = normalBuffer.y + dy
764-
765761
altBuffer.resize (newCols: newColumns, newRows: newRows)
766-
767762
}
768763
public func setup (isReset: Bool = false)
769764
{
@@ -2424,16 +2419,16 @@ open class Terminal {
24242419

24252420
func cmdRestoreCursor (_ pars: [Int], _ collect: cstring)
24262421
{
2427-
// Clamp savedX and savedY to valid ranges to prevent abort() in Debug builds.
2428-
// Saved values can become invalid after resize/scroll operations.
2422+
// savedY stores the absolute buffer row (yBase + y). Convert back to viewport-relative.
24292423
buffer.x = min(max(0, buffer.savedX), cols - 1)
2430-
buffer.y = min(max(0, buffer.savedY), rows - 1)
2424+
buffer.y = max(0, buffer.savedY - buffer.yBase)
24312425
curAttr = buffer.savedAttr
24322426
charset = buffer.savedCharset
24332427
originMode = buffer.savedOriginMode
24342428
setMarginMode(buffer.savedMarginMode)
24352429
setWraparound(buffer.savedWraparound)
24362430
reverseWraparound = buffer.savedReverseWraparound
2431+
restrictCursor()
24372432
}
24382433

24392434
//
@@ -2954,7 +2949,7 @@ open class Terminal {
29542949
func cmdSaveCursor (_ pars: [Int], _ collect: cstring)
29552950
{
29562951
buffer.savedX = buffer.x
2957-
buffer.savedY = buffer.y
2952+
buffer.savedY = buffer.yBase + buffer.y
29582953
buffer.savedAttr = curAttr
29592954
buffer.savedCharset = charset
29602955
buffer.savedWraparound = wraparound

Tests/SwiftTermTests/BufferTests.swift

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,40 @@ final class BufferTests: TerminalDelegate {
209209
#expect(terminal.buffer.y == terminal.rows - 1, "y should be clamped to rows-1")
210210
}
211211

212+
/// Test that save cursor stores absolute row coordinates (yBase + y).
213+
@Test func testSaveCursorStoresAbsoluteSavedY() {
214+
let terminal = Terminal(delegate: self, options: TerminalOptions(cols: 80, rows: 6, scrollback: 200))
215+
216+
for i in 0..<30 {
217+
terminal.feed(text: "line \(i)\r\n")
218+
}
219+
#expect(terminal.buffer.yBase > 0, "Expected yBase > 0 after scrolling")
220+
221+
terminal.feed(text: "\u{1b}[2;5H")
222+
let expectedSavedY = terminal.buffer.yBase + terminal.buffer.y
223+
terminal.feed(text: "\u{1b}7")
224+
225+
#expect(terminal.buffer.savedY == expectedSavedY, "savedY should store absolute row")
226+
}
227+
228+
/// Test that restore cursor translates absolute savedY back into viewport-relative y.
229+
@Test func testRestoreCursorUsesAbsoluteSavedY() {
230+
let terminal = Terminal(delegate: self, options: TerminalOptions(cols: 80, rows: 6, scrollback: 200))
231+
232+
for i in 0..<30 {
233+
terminal.feed(text: "line \(i)\r\n")
234+
}
235+
#expect(terminal.buffer.yBase > 0, "Expected yBase > 0 after scrolling")
236+
237+
let targetViewportRow = 1
238+
terminal.buffer.savedX = 0
239+
terminal.buffer.savedY = terminal.buffer.yBase + targetViewportRow
240+
terminal.feed(text: "\u{1b}[6;10H")
241+
terminal.feed(text: "\u{1b}8")
242+
243+
#expect(terminal.buffer.y == targetViewportRow, "restore should convert saved absolute row to viewport row")
244+
}
245+
212246
/// Test that cmdRestoreCursor clamps negative savedY.
213247
@Test func testRestoreCursorClampsNegativeSavedY() {
214248
let terminal = Terminal(delegate: self, options: TerminalOptions(cols: 80, rows: 25))
@@ -239,6 +273,21 @@ final class BufferTests: TerminalDelegate {
239273
#expect(terminal.buffer.x == terminal.cols - 1, "x should be clamped to cols-1")
240274
}
241275

276+
/// Test that DECSTR resets savedY to current yBase.
277+
@Test func testSoftResetResetsSavedYToYBase() {
278+
let terminal = Terminal(delegate: self, options: TerminalOptions(cols: 80, rows: 6, scrollback: 200))
279+
280+
for i in 0..<30 {
281+
terminal.feed(text: "line \(i)\r\n")
282+
}
283+
#expect(terminal.buffer.yBase > 0, "Expected yBase > 0 after scrolling")
284+
285+
terminal.buffer.savedY = 0
286+
terminal.softReset()
287+
288+
#expect(terminal.buffer.savedY == terminal.buffer.yBase, "savedY should reset to yBase")
289+
}
290+
242291
// MARK: - Additional edge case tests
243292

244293
/// Test that clear() works correctly when called directly on a buffer

0 commit comments

Comments
 (0)