Skip to content

Conversation

@lhecker
Copy link
Member

@lhecker lhecker commented Apr 24, 2025

RenderSettings already stores DECSCNM (reversed screen),
so it only makes sense to also store DECSET 2026 there.

Validation Steps Performed

@lhecker lhecker requested a review from j4james April 24, 2025 12:49
@j4james
Copy link
Collaborator

j4james commented Apr 24, 2025

Still needs to be reset here:

diff --git a/src/renderer/base/RenderSettings.cpp b/src/renderer/base/RenderSettings.cpp
index 371b591b3..2e8ae8d0c 100644
--- a/src/renderer/base/RenderSettings.cpp
+++ b/src/renderer/base/RenderSettings.cpp
@@ -46,9 +46,9 @@ void RenderSettings::RestoreDefaultSettings() noexcept
 {
     _colorTable = _defaultColorTable;
     _colorAliasIndices = _defaultColorAliasIndices;
-    // For now, DECSCNM is the only render mode we need to reset. The others are
-    // all user preferences that can't be changed programmatically.
-    _renderMode.reset(Mode::ScreenReversed);
+    // DECSCNM and Synchronized Output are the only render modes we need to reset.
+    // The others are all user preferences that can't be changed programmatically.
+    _renderMode.reset(Mode::ScreenReversed, Mode::SynchronizedOutput);
 }
 
 // Routine Description:

I know this is unlikely to effect most people, but it matters to me because it causes Windows Terminal to register a failure in one of my test frameworks when verifying RIS correctness.

@lhecker
Copy link
Member Author

lhecker commented Apr 24, 2025

Ah thanks, I forgot about RIS.

Copy link
Collaborator

@j4james j4james left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@DHowett DHowett merged commit a2d8068 into main Apr 24, 2025
19 checks passed
@DHowett DHowett deleted the dev/lhecker/8331-synchronized-output2 branch April 24, 2025 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants