Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly fix ConPTY buffer corking #16793

Merged
merged 1 commit into from
Mar 28, 2024
Merged

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
20 checks passed
@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: Rejected
Status: Rejected
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