Fixes DECRQM, XTSAVE and XTRESTORE for modes 47, 1047, 1048, and 1049.
Resolves #14199.
Notes
- The tests circle the same thing. I hope they are neither excessive nor insufficient.
- Consider a
ModeState.getSavedhelper; it would be handy for restoring. - I couldn't find an elegant way to avoid the double lookup in
modeReport. - The terminal formatter is untouched (terminal/formatter.zig#L415).
Reproduction
Compared against xterm 411 via XQuartz. "Expected" is xterm's behaviour. "Observed" is Ghostty's behaviour (this PR).
Modes 47/1047/1049 (DECRQM)
Enter with one mode, then leave with another.
# On the primary screen
printf '\e[?47h'
# On the alternate screen
printf '\e[?1047$p'; cat -v # Expected = Observed: ^[[?1047;1$y
printf '\e[?1049$p'; cat -v # Expected = Observed: ^[[?1049;1$y
printf '\e[?1049l'
# On the primary screen
printf '\e[?1047$p'; cat -v # Expected = Observed: ^[[?1047;2$y
printf '\e[?47$p'; cat -v # Expected = Observed: ^[[?47;2$yMode 1048 (DECRQM)
xterm saves a cursor for each buffer at startup. Ghostty reports reset until something actually saves one.
tput reset # For good measure. Your shell may interfere.
printf '\e[?1048$p'; cat -v # Expected: ^[[?1048;1$y (always set)
# Observed: ^[[?1048;2$y
printf '\e7' # DECSC
printf '\e[?1048$p'; cat -v # Expected = Observed: ^[[?1048;1$yMode 1049 (XTRESTORE does not erase)
Restoring mode 1049 preserves the alternate screen content.
# On the primary screen
printf '\e[?1049h'
# On the alternate screen
lorem --faust -s 5
printf '\e[?1049s'
printf '\e[?1049l'
# On the primary screen
printf '\e[?1049r'
# Expected = Observed: content is preserved (not erased).AI Disclosure
Made with Claude (tests, docs, validation). The core changes, thorough review, fixes, understanding — by meat proxy.
Activity
11
korikhin13d agoUpdate: docs only, snapshot notes reworded.
korikhin13d agoThis is a cleaned-up version of the previous attempt. mitchellh, apologies for that unnecessary review re-request. That kick was actually refreshing: it forced me to come up with a better commit structure and throw away all the bs I could find. I simply needed a fresh pair of eyes. I'd been staring at it for too long. Thank you!
For the record, I understand every line of this change and can explain it at a whiteboard. I tried to make it satisfy the following constraints:
- It should not introduce any kind of synchronization that requires babysitting.
- It should not mess around with the snapshot API or
ModeState. - It should leave room for future modes with special behaviour.
The terminal formatter (formatter.zig#L415) still reads the recorded bits on purpose, because effective values would make it emit ?47h, ?1047h and ?1049h at once on the alternate screen, and ?1048h whenever a cursor happens to be saved. Since the stream no longer records them, the formatter emits none of the four.
Also, I thought about other approaches that would make these inconsistencies unrepresentable, but they all break the second constraint and would mean a major rework. And to be fully transparent, I don't have enough expertise to say how the C API should look here.
Thank you.
korikhin8d agoUpdate:
- Rebased onto the latest
main. - Resolved a conflict with the Zig 0.17 changes to
@typeInfo. - Checked that the build and tests still pass.
This pull request is a draft
Merging is unavailable until it is marked ready for review.
I'm in a different TZ but will try to respond ASAP.