Add support for the DECSWT (Set Window Title) escape sequence #16804
Conversation
| // - title - The null-terminated string to set as the window title | ||
| // - title - The string to set as the window title | ||
| // Return Value: | ||
| // - <none> | ||
| void ConhostInternalGetSet::SetWindowTitle(std::wstring_view title) | ||
| { | ||
| ServiceLocator::LocateGlobals().getConsoleInformation().SetTitle(title); | ||
| auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); | ||
| gci.SetTitle(title.empty() ? gci.GetOriginalTitle() : title); |
There was a problem hiding this comment.
I'm not sure why the title parameter originally expected to be null-terminated, but that's no longer the case. The CONSOLE_INFORMATION::SetTitle method immediately constructs a std::wstring with that parameter, and in the Terminal::SetWindowTitle implementation, it's assigned to a std::optional<std::wstring> using emplace. Neither of those should need a null-terminated string_view as far as I understand.
There was a problem hiding this comment.
Ah, probably vestiges of the pre-wstring times :)
| virtual bool ActionOscDispatch(const wchar_t wch, | ||
| const size_t parameter, | ||
| const std::wstring_view string) = 0; | ||
| virtual bool ActionOscDispatch(const size_t parameter, const std::wstring_view string) = 0; |
There was a problem hiding this comment.
I dropped the wch parameter from this method because it's never actually used for anything. It just indicated whether the sequence was terminated with a BEL or an ST backslash, but that's of no relevance to anyone.
| std::wstring title; | ||
| success = _GetOscTitle(string, title); | ||
| success = success && _dispatch->SetWindowTitle(title); | ||
| success = _dispatch->SetWindowTitle(string); |
There was a problem hiding this comment.
All that _GetOscTitle was doing was assigning the wstring_view to a wstring, and checking that it wasn't blank. But we don't actually want that blank check, and there's no reason why we can't just pass the wstring_view through to SetWindowTitle as is.
| // - title - The null-terminated string to set as the window title | ||
| // - title - The string to set as the window title | ||
| // Return Value: | ||
| // - <none> | ||
| void ConhostInternalGetSet::SetWindowTitle(std::wstring_view title) | ||
| { | ||
| ServiceLocator::LocateGlobals().getConsoleInformation().SetTitle(title); | ||
| auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); | ||
| gci.SetTitle(title.empty() ? gci.GetOriginalTitle() : title); |
There was a problem hiding this comment.
Ah, probably vestiges of the pre-wstring times :)
This PR adds support for the
OSC 21sequence used on DEC terminals toset the window title. It's just an alias of the
OSC 2title sequenceused by XTerm.
This PR also corrects the handling of blank title sequences, which are
supposed to reset the title to its default value, but were previously
ignored.
Detailed Description of the Pull Request / Additional comments
To handle the blank title parsing correctly, I had to make some changes
to the state machine. Previously it would not have dispatched an
OSCsequence unless it received a semicolon following the
OSCnumber, butwhen there's a blank string, that semicolon should not be required.
I also took this opportunity to simplify the
OSCparsing in the statemachine, and eliminate the
_GetOscTitlemethod which no longer servedany purpose.
Validation Steps Performed
I've manually confirmed that the title sequences are now working as
expected, and added some state machine unit tests covering the blank
value handling for these sequences.
I also had to update one of the existing state machine tests to account
for the changes I made to allow the semicolon to be omitted.
Closes #16783
Closes #16784