Skip to content

Properly fix ConPTY buffer corking#16793

Merged
DHowett merged 1 commit intomainfrom
dev/lhecker/16769-conpty-flushing
Mar 28, 2024
Merged

Properly fix ConPTY buffer corking#16793
DHowett merged 1 commit intomainfrom
dev/lhecker/16769-conpty-flushing

Conversation

@lhecker
Copy link
Member

@lhecker lhecker commented Mar 1, 2024

I've found that #16079 was never properly addressed (it still randomly
occurred after even after PR #16349), which later led to the issues
described in #16769 (nushell flickering due to too many flushes).

The crux of the fix is that this brings back the _noFlushOnEnd flag
that was removed in PR #15991. This is then combined with a change to
the cork API: An uncork on VtEngine now only flushes if _Flush
got called while it was corked in the first place.

_noFlushOnEnd prevents us from flushing in between two "unknown"
VT sequences (like soft fonts or FTCS) which prevents them from being
corrupted. The corking prevents the remaining cases of flushing too
often. Long-term, a proper fix would be to pass through VT unmodified.

Closes #16769

@DHowett DHowett added this pull request to the merge queue Mar 28, 2024
Merged via the queue into main with commit 1ede023 Mar 28, 2024
@DHowett DHowett deleted the dev/lhecker/16769-conpty-flushing branch March 28, 2024 20:45
DHowett pushed a commit that referenced this pull request Apr 16, 2024
I've found that #16079 was never properly addressed (it still randomly
occurred after even after PR #16349), which later led to the issues
described in #16769 (nushell flickering due to too many flushes).

The crux of the fix is that this brings back the `_noFlushOnEnd` flag
that was removed in PR #15991. This is then combined with a change to
the cork API: An `uncork` on `VtEngine` now only flushes if `_Flush`
got called while it was corked in the first place.

`_noFlushOnEnd` prevents us from flushing in between two "unknown"
VT sequences (like soft fonts or FTCS) which prevents them from being
corrupted. The corking prevents the remaining cases of flushing too
often. Long-term, a proper fix would be to pass through VT unmodified.

Closes #16769

(cherry picked from commit 1ede023)
Service-Card-Id: 91965217
Service-Version: 1.20
DHowett pushed a commit that referenced this pull request Apr 16, 2024
I've found that #16079 was never properly addressed (it still randomly
occurred after even after PR #16349), which later led to the issues
described in #16769 (nushell flickering due to too many flushes).

The crux of the fix is that this brings back the `_noFlushOnEnd` flag
that was removed in PR #15991. This is then combined with a change to
the cork API: An `uncork` on `VtEngine` now only flushes if `_Flush`
got called while it was corked in the first place.

`_noFlushOnEnd` prevents us from flushing in between two "unknown"
VT sequences (like soft fonts or FTCS) which prevents them from being
corrupted. The corking prevents the remaining cases of flushing too
often. Long-term, a proper fix would be to pass through VT unmodified.

Closes #16769

(cherry picked from commit 1ede023)
Service-Card-Id: 91965216
Service-Version: 1.19
DHowett added a commit that referenced this pull request Apr 30, 2024
DHowett added a commit that referenced this pull request Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: To Cherry Pick

Development

Successfully merging this pull request may close these issues.

Flushing FTCS sequences causes flickering in nushell (again; 1.20+)

3 participants