Keep the cursor on its line when the window is resized - #32
Merged
Conversation
A resize could move the cursor off the line it was on, in both directions, and the next write then destroyed content the application never touched. The cursor's position is YBase + Y, and each direction changed one half without the other: Shrinking rows clamped Y into the new viewport and left YBase alone, so the cursor jumped backwards onto earlier content. A prompt at row 22 in a window shrunk to ten rows landed on absolute row 9. Growing rows clamped YBase down to pull scrollback into view and left Y alone, so the cursor slid up the content by exactly the lines YBase gave back -- once per resize event, and a live drag is many events. Both now move the two halves together: the shrink pushes its overflow into scrollback rather than clamping, and the grow adds back to Y whatever YBase returns. A viewport that was following the tail keeps following it. This is easy to miss because nothing throws and shells hide it. A shell redraws its prompt on every SIGWINCH, so it repaints over its own damage while dragging; what is left is a window that looks fine. It shows up wherever the terminal contains something that does NOT repaint itself -- I found it with a Sixel image, which kept a prompt-shaped hole per redraw and made the pattern obvious, but the same writes land on any scrollback content under a plain text session. The tests are text-only, since the bug has nothing to do with graphics: one per direction, plus a ladder that drives a shrink-and-grow round trip with a redraw between every step and asserts that twenty earlier lines still read back exactly. Two of the three fail against main. Full suite: 852 passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new shrink overflow logic triggers incorrectly when newRows == 0, which can mutate YBase during a 0-row resize and effectively scroll content unexpectedly.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes a terminal buffer resize bug where changing the viewport height could move the cursor onto a different absolute line (YBase + Y), causing subsequent writes (e.g., shell redraws during live resize) to overwrite content the application never touched.
Changes:
- Adjust cursor
Ywhen growing rows so it compensates forYBaseclamping, preserving the cursor’s absolute line. - When shrinking rows, push cursor overflow into scrollback (via
YBase/Yshift) instead of clampingYalone. - Add regression tests covering shrink, grow, and a multi-step resize “ladder” with redraws.
File summaries
| File | Description |
|---|---|
| src/XTerm.NET/Buffer/TerminalBuffer.cs | Updates resize logic to keep the cursor on the same absolute content line across row resizes. |
| src/XTerm.NET.Tests/Buffer/ResizeEdgeCaseTests.cs | Adds targeted tests to prevent cursor/contents corruption regressions during resize. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Caught in review on this PR. A zero-row viewport's bottom row is not -1, but "newRows - 1" says it is, which made the overflow test true for every cursor and the overflow itself one line too large. A resize with nothing visible still moved the buffer: the cursor's content row went from 20 to 21, and the line that came back at the top when rows were restored was the wrong one. Zero rows is a real case in this codebase rather than a theoretical one -- a buffer can be constructed with none and brought to life by a later resize, which ResizeEdgeCaseTests already covers. Flooring the bottom at zero fixes the arithmetic rather than special-casing it, and the cursor's content row now survives a trip down to zero rows and back -- which neither the original code nor the first version of this fix managed. 853 tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The shrink-path YBase/Y shift can leave a previously tail-following viewport no longer at the tail, which appears to contradict the PR’s stated “following the tail keeps following it” behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
Second review catch on this PR. The screen is the last `rows` lines of the buffer, so shrinking has to move the difference into scrollback. Shifting only enough to bring the cursor back on screen left the rest BELOW the screen -- and the viewport tops out at YBase, so scrolling could never reach those lines again. Measured on a 24-row window shrunk to 10 with the cursor six rows up: YBase landed at 8 where its maximum was 14, stranding six lines. A shrink whose viewport was following the tail now takes all the room there is, bounded by the cursor, which must not end up above the screen. A viewport the user had scrolled away from is left alone and still shifts by the minimum, since yanking it to the bottom would be its own bug. Doing this unconditionally broke two reflow tests and deserves recording: a column reflow can move the cursor without the cursor ever leaving the screen, and those cases want YBase left where it is. The guard on the cursor actually overflowing is what separates them. 853 tests pass, plus a regression asserting all three properties together -- the cursor still on its line, the screen reaching the end of the buffer, and a tail-following viewport still at the tail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 27, 2026
tomlm
approved these changes
Aug 27, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A resize can move the cursor off the line it was on, in both directions, and the next write then destroys content the application never touched.
The bug
The cursor's position is
YBase + Y. Each direction of a row resize changed one half without the other:Yinto the new viewport and leftYBasealone, so the cursor jumped backwards onto earlier content. A prompt at row 22 in a window shrunk to ten rows lands on absolute row 9.YBasedown to pull scrollback into view and leftYalone, so the cursor slid up the content by exactly the linesYBasegave back — once per resize event, and a live drag is many events.Reproduced as a ladder: cursor at absolute row 38, then 35, 29, 23, 17, 11 across five grow steps of a drag.
The fix
Both directions now move the two halves together. The shrink pushes its overflow into scrollback rather than clamping; the grow adds back to
YwhateverYBasereturns. A viewport that was following the tail keeps following it.25 lines in
TerminalBuffer.Resize.Why this survived
Nothing throws, and shells hide it. A shell redraws its prompt on every
SIGWINCH, so it repaints over its own damage while you drag — what you're left with is a window that looks fine. The damage only shows where the terminal contains something that does not repaint itself.I found it with a Sixel image, which kept a prompt-shaped hole per redraw and made the pattern obvious. It has nothing to do with graphics though — the same writes land on scrollback content in a plain text session, which is why the tests here are text-only.
Tests
Three added to the existing
ResizeEdgeCaseTests: one per direction, plus a ladder that drives a shrink-and-grow round trip with a redraw between every step and asserts twenty earlier lines still read back exactly.Two of the three fail against
main. Full suite passes at 852.