Repository navigation
ghostty-next: the wrap edge break reads a compressed history row only when the pair was joined - #29775
Conversation
… when the pair was joined Pins GhosttyNextKit xcframework-84a225487372c53851b186864c867e469e7596d2-apple-v6 (checksum 66e883f7... from its SHA256SUMS) and moves the gitlink and the notices with it. ghostty-next 8444cc1eb..84a225487 on release/cmux-9ce9b7d0-e2 is fork PR #68 only (review note 1 on #29609): breakWrapAbove read the row above before it checked the flags, so IL or DL at the screen's top row restored the last history page from compression on every call, and a region scroll can run per output line. It now decides on the resident row first and reads a compressed row only when the pair was joined. Red: fork run 41240560795 ("insertLines and deleteLines at the top row leave a compressed history page compressed" fails without the fix). Green: fork PR #68 run 41240575661 and the release run 41241826620, which published this kit. docs/ghostty-fork.md (review note 2): every commit ID in the E2 section is now the one on release/cmux-9ce9b7d0-e2, and the section says so.
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
Not verified: macOS jobs did not run: compile and app tests were skipped on CI passes on Written by Shipped in:
|
|
Review (Claude Opus 5.5 review subagent): SHIP at eb7600b Independent review subagent (read-only). It checked that the early return is sound for every state that this pin's own paths make, the moment top_joined is read in IL and DL, the other callers, that the Zig test reaches the compressed path and fails without the fix, the URL and checksum against SHA256SUMS, the gitlink, the notices, and every docs/ghostty-fork.md hash on release/cmux-9ce9b7d0-e2. Non-blocking finding, kept as a follow-up: the new comment in Screen.breakWrapAbove says "every clear clears both", which is not true. Two older paths, clearRows (ED 1, ED 2, DECALN) and cursorScrollAbove with the cursor at row 0, can leave wrap=true on a history row whose active continuation row has no flag. This PR does not create that state. Before this PR, IL or DL at row 0 cleared it by chance; now that stops only when the history page is compressed. The comment fix, and a root fix for those two paths with their own red tests, go in the next fork change. |
|
Rust replay tests on this pin: fleet step 0e1a97219c7ae128a85709bc (Actions run 41250330654): 6 vt_replay tests pass, among them vt_replay_keeps_a_stale_wrap_continuation_row_in_its_column. |
|
"cmux-next swift test" classification (run 41250318057): not caused by this PR.
|
|
Classification of the compile reds on head 69ae06a (after cmux-bot merged feat): base red, not this change.
|
…wrap-edge-history-cost-2
|
I merged the feat tip 33995df into this branch (head 4bf50aa). It brings #29871 (ac5389c), the fix for the NativePopupHost compile base red. The PR's own diff against the merge-base is unchanged: the same 5 files. I removed and added ready-to-land again for this head. The review receipt is for the unchanged diff at eb7600b. |
|
Review (Claude Opus 5.5 review subagent): SHIP at 4bf50aa Re-review after the two base merges (69ae06a by cmux-bot, 4bf50aa by me). The PR's own diff against the merge-base is byte-for-byte the diff reviewed at eb7600b. Neither merge resolved a conflict in the PR's 5 files. Earlier non-blocking findings stay open:
|
…s the cleared rows' edges (#30012) * test(ghostty-vt): ED 2 and DECALN break the soft wrap from scrollback into the cleared screen (red; E2 class) Review of #29775: clearRows resets the cleared rows, but the scrollback row above the screen kept wrap=true. After ED 2 (or DECALN) and a wider resize, reflow joined that row and the new text on row 0 into one line ("abcdeXY"). Red at pin 84a225487 (fleet step e3b896d45e34435553a6e67a on a probe branch with the same tests). * ghostty-next: a clear of rows (ED, DECALN) breaks the soft wrap across the cleared rows' edges Pins GhosttyNextKit xcframework-34e73ba8ba7cb8da0115caeb780065e3a12f2f62-apple-v6 (checksum 32cb09bb... from its SHA256SUMS) and moves the gitlink and the notices with it. ghostty-next 84a225487..34e73ba8b on release/cmux-9ce9b7d0-e2 is the second part of fork PR #68 only: clearRows reset the cleared rows, but the row above the range kept wrap=true (a scrollback row for ED 2 and DECALN) and the row below kept wrap_continuation=true (the cursor row after ED 1). It now breaks both edges after a full clear; a protected clear keeps its row flags. Red: vt_ed2_breaks_the_wrap_from_scrollback_into_the_cleared_screen and vt_decaln_breaks_the_wrap_from_scrollback_into_the_filled_screen fail on 1602522 (pin 84a225487: "abcdeXY" after a resize). Fork red: run 41251885912 (3 of 3 clearRows Zig tests fail without the fix). Fork green: PR #68 run 41251888717, broad run 41254020927 (2297 of 2297 libghostty-vt tests) and the release run 41255186701, which published this kit.
…essed history row only when the pair was joined (port of #29775) Port 89dd169 of feat PR #29775 (squash 1e327b2). The pin is resolved for this line: ghostty-next release/cmux-ef6ce279-e2 at 7f8d846b2 (this line's previous pin 3a82309b9 plus only fork PR #68 part 1, the same commits as feat's 8444cc1eb..84a225487), GhosttyNextKit xcframework-7f8d846b24cf0f994feaebddfe6376f3ee38c595-apple-v6. PR #30015. Port review: Review (Opus 5.5 review subagent): SHIP at 89dd169. Fork red: run 41240560795 (the compressed history page test fails without the fix); green in this release's run 41260252846. Journey r3-e2-reattach (kit c370e63) passed 3 of 3 on port build 6d6a0fcc090a0347f7b7142d (cmux19 03:39Z, 03:40Z, 03:41Z).
Summary
Screen.breakWrapAboveread the row above before it checked the flags. So IL or DL at the screen's top row restored the last history page from compression on every call. Under load, a region scroll can run once per output line.wrap_continuation, or for IL/DL the top row's flag read before the rows move). It reads a row in a compressed history page only when the pair was joined.xcframework-84a225487372c53851b186864c867e469e7596d2-apple-v6, onrelease/cmux-9ce9b7d0-e2. That release is 8444cc1eb (the ghostty-next: a scroll region scroll breaks the soft wrap across its edges (VT fuzz finding E2) #29609 pin) plus only fix cmux crashing issue #68. The gitlink,package-notices.jsonandvt-link-graph.jsonmove with the pin.docs/ghostty-fork.mdis now a release-branch ID, and the section says so.Testing
Observed / Not run
66e883f7130352862fd3dd4b46cf0a6d22d50750bf633f3589831bf9b2f4f99f, which is the checksum in Package.swift.Changelog
none
Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Fixes a performance issue in
ghostty-nextwhereScreen.breakWrapAboveread a compressed history row before checking flags, so IL or DL at the screen's top row restored the last history page from compression on every call. The function now decides on the resident row first and reads a compressed row only when the pair was joined.GhosttyNextKitxcframework-84a225487372c53851b186864c867e469e7596d2-apple-v6(checksum66e883f7...); the gitlink,package-notices.json, andvt-link-graph.jsonmove with it.docs/ghostty-fork.mdnow states that every commit ID in the E2 section is a release-branch ID.Written for commit 4bf50aa. Summary will update on new commits.