Add support for OSC 104, 110, 111, 112 and 117 (resets) #18767
Conversation
this makes resetting the background color the opposite of setting it: setting it changes the alias to DEFAULT_BACKGROUND, and resetting it changes it back to INDEX=0 (conhost). As a side effect, the value of DEFAULT_BACKGROUND is also destroyed. One could test this in xterm by using DECAC to set the background to their internal background color index and observing whether it, too, was preserved.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
microsoft-github-policy-service
bot
added
Issue-Task
This comment has been minimized.
This comment has been minimized.
src/renderer/base/RenderSettings.cpp
Outdated
| // Routine Description: | ||
| // - Returns one color table entry to the value saved in SaveDefaultSettings. | ||
| void RenderSettings::RestoreDefaultColorTableEntries(const size_t startIndex, const size_t count) |
There was a problem hiding this comment.
This comment doesn't seem right.
There was a problem hiding this comment.
Sure isn't! Last minute change to take {start, length} rather than just an index :) (thanks!)
|
Thanks for the review, and for the testing! I appreciate it. :) |
src/renderer/base/RenderSettings.cpp
Outdated
| // - Restores a range of color table entries to the values saved in SaveDefaultSettings. | ||
| void RenderSettings::RestoreDefaultColorTableEntries(const size_t startIndex, const size_t count) | ||
| { | ||
| std::copy_n(&_defaultColorTable.at(startIndex), count, &_colorTable.at(startIndex)); |
There was a problem hiding this comment.
This seems like it could use some bounds checks. Perhaps just a std::min together with a soft assert though? Not sure... Alternative we could make it two explicit reset-256-indexed and reset-specific-index functions.
There was a problem hiding this comment.
So, yeah. I feel weird about this function.
Everywhere else in this file we use .at() and allow the SafeExecute handler to catch the exception. No bounds checks, no worries.
I was so averse to adding members to ITermDispatch that I forgot I could add members to RenderSettings with no real cost.
This pull request adds support for resetting the various color table entries and xterm resource values back to their defaults.
Building on the default color table James introduced in #17879, it was relatively straightforward to add support for resetting specific entries.
This implementation cleaves tightly to observed behavior in xterm(379) rather than observed behavior in libvte(0.70.6). They differ in the following ways:
I was unable to verify how these reset commands interact with colors set via
DECAC Assign Colorso I went with the implementation that made the most sense:110also restores the background color alias entry to its pre-DECACvalue; this results in the perceived background color returning to e.g. index 0 in conhost and thebackgroundcolor in Terminal.Refs #18695
Refs #17879
Closes #3719