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

Add support for the DECSWT (Set Window Title) escape sequence #16804

Merged
merged 8 commits into from
Mar 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/actions/spelling/expect/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,7 @@
DECSTGLT
DECSTR
DECSWL
DECSWT
DECTABSR
DECTCEM
DECXCPR
Expand Down Expand Up @@ -796,7 +797,7 @@
Hostx
HPA
hpcon
HPCON

Check warning on line 800 in .github/actions/spelling/expect/expect.txt

View workflow job for this annotation

GitHub Actions / Spell checking

`HPCON` is ignored by check spelling because another more general variant is also in expect. (ignored-expect-variant)
hpen
HPR
HProvider
Expand Down Expand Up @@ -1910,7 +1911,7 @@
UPDATEDISPLAY
UPDOWN
UPKEY
UPSS

Check warning on line 1914 in .github/actions/spelling/expect/expect.txt

View workflow job for this annotation

GitHub Actions / Spell checking

`UPSS` is ignored by check spelling because another more general variant is also in expect. (ignored-expect-variant)
upss
uregex
URegular
Expand Down
5 changes: 3 additions & 2 deletions src/host/outputStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,12 +159,13 @@ void ConhostInternalGetSet::WarningBell()
// Routine Description:
// - Sets the title of the console window.
// Arguments:
// - 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);
Comment on lines -162 to +168
Copy link
Collaborator Author

@j4james j4james Mar 3, 2024

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, probably vestiges of the pre-wstring times :)

}

// Routine Description:
Expand Down
2 changes: 1 addition & 1 deletion src/terminal/adapter/ITermDispatch.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class Microsoft::Console::VirtualTerminal::ITermDispatch
virtual bool ReverseLineFeed() = 0; // RI
virtual bool BackIndex() = 0; // DECBI
virtual bool ForwardIndex() = 0; // DECFI
virtual bool SetWindowTitle(std::wstring_view title) = 0; // OscWindowTitle
virtual bool SetWindowTitle(std::wstring_view title) = 0; // DECSWT, OscWindowTitle
virtual bool HorizontalTabSet() = 0; // HTS
virtual bool ForwardTab(const VTInt numTabs) = 0; // CHT, HT
virtual bool BackwardsTab(const VTInt numTabs) = 0; // CBT
Expand Down
2 changes: 1 addition & 1 deletion src/terminal/adapter/adaptDispatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2644,7 +2644,7 @@ bool AdaptDispatch::ForwardIndex()
// Routine Description:
// - OSC Set Window Title - Sets the title of the window
// Arguments:
// - title - The string to set the title to. Must be null terminated.
// - title - The string to set the title to.
// Return Value:
// - True.
bool AdaptDispatch::SetWindowTitle(std::wstring_view title)
Expand Down
2 changes: 1 addition & 1 deletion src/terminal/adapter/adaptDispatch.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ namespace Microsoft::Console::VirtualTerminal
bool ReverseLineFeed() override; // RI
bool BackIndex() override; // DECBI
bool ForwardIndex() override; // DECFI
bool SetWindowTitle(const std::wstring_view title) override; // OSCWindowTitle
bool SetWindowTitle(const std::wstring_view title) override; // DECSWT, OSCWindowTitle
bool HorizontalTabSet() override; // HTS
bool ForwardTab(const VTInt numTabs) override; // CHT, HT
bool BackwardsTab(const VTInt numTabs) override; // CBT
Expand Down
2 changes: 1 addition & 1 deletion src/terminal/adapter/termDispatch.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class Microsoft::Console::VirtualTerminal::TermDispatch : public Microsoft::Cons
bool ReverseLineFeed() override { return false; } // RI
bool BackIndex() override { return false; } // DECBI
bool ForwardIndex() override { return false; } // DECFI
bool SetWindowTitle(std::wstring_view /*title*/) override { return false; } // OscWindowTitle
bool SetWindowTitle(std::wstring_view /*title*/) override { return false; } // DECSWT, OscWindowTitle
bool HorizontalTabSet() override { return false; } // HTS
bool ForwardTab(const VTInt /*numTabs*/) override { return false; } // CHT, HT
bool BackwardsTab(const VTInt /*numTabs*/) override { return false; } // CBT
Expand Down
4 changes: 1 addition & 3 deletions src/terminal/parser/IStateMachineEngine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,7 @@ namespace Microsoft::Console::VirtualTerminal

virtual bool ActionIgnore() = 0;

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;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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.


virtual bool ActionSs3Dispatch(const wchar_t wch, const VTParameters parameters) = 0;

Expand Down
5 changes: 1 addition & 4 deletions src/terminal/parser/InputStateMachineEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -538,14 +538,11 @@ bool InputStateMachineEngine::ActionIgnore() noexcept
// - Triggers the OscDispatch action to indicate that the listener should handle a control sequence.
// These sequences perform various API-type commands that can include many parameters.
// Arguments:
// - wch - Character to dispatch. This will be a BEL or ST char.
// - parameter - identifier of the OSC action to perform
// - string - OSC string we've collected. NOT null terminated.
// Return Value:
// - true if we handled the dispatch.
bool InputStateMachineEngine::ActionOscDispatch(const wchar_t /*wch*/,
const size_t /*parameter*/,
const std::wstring_view /*string*/) noexcept
bool InputStateMachineEngine::ActionOscDispatch(const size_t /*parameter*/, const std::wstring_view /*string*/) noexcept
{
return false;
}
Expand Down
4 changes: 1 addition & 3 deletions src/terminal/parser/InputStateMachineEngine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,7 @@ namespace Microsoft::Console::VirtualTerminal

bool ActionIgnore() noexcept override;

bool ActionOscDispatch(const wchar_t wch,
const size_t parameter,
const std::wstring_view string) noexcept override;
bool ActionOscDispatch(const size_t parameter, const std::wstring_view string) noexcept override;

bool ActionSs3Dispatch(const wchar_t wch, const VTParameters parameters) override;

Expand Down
25 changes: 3 additions & 22 deletions src/terminal/parser/OutputStateMachineEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -766,14 +766,11 @@ bool OutputStateMachineEngine::ActionIgnore() noexcept
// - Triggers the OscDispatch action to indicate that the listener should handle a control sequence.
// These sequences perform various API-type commands that can include many parameters.
// Arguments:
// - wch - Character to dispatch. This will be a BEL or ST char.
// - parameter - identifier of the OSC action to perform
// - string - OSC string we've collected. NOT null terminated.
// Return Value:
// - true if we handled the dispatch.
bool OutputStateMachineEngine::ActionOscDispatch(const wchar_t /*wch*/,
const size_t parameter,
const std::wstring_view string)
bool OutputStateMachineEngine::ActionOscDispatch(const size_t parameter, const std::wstring_view string)
{
auto success = false;

Expand All @@ -782,10 +779,9 @@ bool OutputStateMachineEngine::ActionOscDispatch(const wchar_t /*wch*/,
case OscActionCodes::SetIconAndWindowTitle:
case OscActionCodes::SetWindowIcon:
case OscActionCodes::SetWindowTitle:
case OscActionCodes::DECSWT_SetWindowTitle:
{
std::wstring title;
success = _GetOscTitle(string, title);
success = success && _dispatch->SetWindowTitle(title);
success = _dispatch->SetWindowTitle(string);
Comment on lines -786 to +784
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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.

break;
}
case OscActionCodes::SetColor:
Expand Down Expand Up @@ -932,21 +928,6 @@ bool OutputStateMachineEngine::ActionSs3Dispatch(const wchar_t /*wch*/, const VT
return false;
}

// Routine Description:
// - Null terminates, then returns, the string that we've collected as part of the OSC string.
// Arguments:
// - string - Osc String input
// - title - Where to place the Osc String to use as a title.
// Return Value:
// - True if there was a title to output. (a title with length=0 is still valid)
bool OutputStateMachineEngine::_GetOscTitle(const std::wstring_view string,
std::wstring& title) const
{
title = string;

return !string.empty();
}

// Routine Description:
// - OSC 4 ; c ; spec ST
// c: the index of the ansi color table
Expand Down
8 changes: 2 additions & 6 deletions src/terminal/parser/OutputStateMachineEngine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,7 @@ namespace Microsoft::Console::VirtualTerminal

bool ActionIgnore() noexcept override;

bool ActionOscDispatch(const wchar_t wch,
const size_t parameter,
const std::wstring_view string) override;
bool ActionOscDispatch(const size_t parameter, const std::wstring_view string) override;

bool ActionSs3Dispatch(const wchar_t wch, const VTParameters parameters) noexcept override;

Expand Down Expand Up @@ -217,6 +215,7 @@ namespace Microsoft::Console::VirtualTerminal
SetForegroundColor = 10,
SetBackgroundColor = 11,
SetCursorColor = 12,
DECSWT_SetWindowTitle = 21,
SetClipboard = 52,
ResetForegroundColor = 110, // Not implemented
ResetBackgroundColor = 111, // Not implemented
Expand All @@ -226,9 +225,6 @@ namespace Microsoft::Console::VirtualTerminal
ITerm2Action = 1337,
};

bool _GetOscTitle(const std::wstring_view string,
std::wstring& title) const;

bool _GetOscSetColorTable(const std::wstring_view string,
std::vector<size_t>& tableIndexes,
std::vector<DWORD>& rgbs) const;
Expand Down
32 changes: 20 additions & 12 deletions src/terminal/parser/stateMachine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -685,17 +685,17 @@ void StateMachine::_ActionOscPut(const wchar_t wch)
}

// Routine Description:
// - Triggers the CsiDispatch action to indicate that the listener should handle a control sequence.
// - Triggers the OscDispatch action to indicate that the listener should handle a control sequence.
// These sequences perform various API-type commands that can include many parameters.
// Arguments:
// - wch - Character to dispatch.
// - <none>
// Return Value:
// - <none>
void StateMachine::_ActionOscDispatch(const wchar_t wch)
void StateMachine::_ActionOscDispatch()
{
_trace.TraceOnAction(L"OscDispatch");
_trace.DispatchSequenceTrace(_SafeExecute([=]() {
return _engine->ActionOscDispatch(wch, _oscParameter, _oscString);
return _engine->ActionOscDispatch(_oscParameter, _oscString);
}));
}

Expand Down Expand Up @@ -1434,20 +1434,28 @@ void StateMachine::_EventCsiSubParam(const wchar_t wch)
// Routine Description:
// - Processes a character event into an Action that occurs while in the OscParam state.
// Events in this state will:
// 1. Collect numeric values into an Osc Param
// 2. Move to the OscString state on a delimiter
// 3. Ignore everything else.
// 1. Trigger the OSC action associated with the param on an OscTerminator
// 2. If we see a ESC, enter the OscTermination state. We'll wait for one
// more character before we dispatch the (empty) string.
// 3. Collect numeric values into an Osc Param
// 4. Move to the OscString state on a delimiter
// 5. Ignore everything else.
// Arguments:
// - wch - Character that triggered the event
// Return Value:
// - <none>
void StateMachine::_EventOscParam(const wchar_t wch) noexcept
void StateMachine::_EventOscParam(const wchar_t wch)
{
_trace.TraceOnEvent(L"OscParam");
if (_isOscTerminator(wch))
{
_ActionOscDispatch();
_EnterGround();
}
else if (_isEscape(wch))
{
_EnterOscTermination();
}
else if (_isNumericParamValue(wch))
{
_ActionOscParam(wch);
Expand Down Expand Up @@ -1479,7 +1487,7 @@ void StateMachine::_EventOscString(const wchar_t wch)
_trace.TraceOnEvent(L"OscString");
if (_isOscTerminator(wch))
{
_ActionOscDispatch(wch);
_ActionOscDispatch();
_EnterGround();
}
else if (_isEscape(wch))
Expand Down Expand Up @@ -1511,7 +1519,7 @@ void StateMachine::_EventOscTermination(const wchar_t wch)
_trace.TraceOnEvent(L"OscTermination");
if (_isStringTerminatorIndicator(wch))
{
_ActionOscDispatch(wch);
_ActionOscDispatch();
_EnterGround();
}
else
Expand Down Expand Up @@ -1856,8 +1864,8 @@ void StateMachine::ProcessCharacter(const wchar_t wch)
ProcessCharacter(_c1To7Bit(wch));
}
}
// Don't go to escape from the OSC string state - ESC can be used to terminate OSC strings.
else if (_isEscape(wch) && _state != VTStates::OscString)
// Don't go to escape from the OSC string/param states - ESC can be used to terminate OSC strings.
else if (_isEscape(wch) && _state != VTStates::OscString && _state != VTStates::OscParam)
{
_ActionInterrupt();
_EnterEscape();
Expand Down
4 changes: 2 additions & 2 deletions src/terminal/parser/stateMachine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ namespace Microsoft::Console::VirtualTerminal
void _ActionCsiDispatch(const wchar_t wch);
void _ActionOscParam(const wchar_t wch) noexcept;
void _ActionOscPut(const wchar_t wch);
void _ActionOscDispatch(const wchar_t wch);
void _ActionOscDispatch();
void _ActionSs3Dispatch(const wchar_t wch);
void _ActionDcsDispatch(const wchar_t wch);

Expand Down Expand Up @@ -132,7 +132,7 @@ namespace Microsoft::Console::VirtualTerminal
void _EventCsiIgnore(const wchar_t wch);
void _EventCsiParam(const wchar_t wch);
void _EventCsiSubParam(const wchar_t wch);
void _EventOscParam(const wchar_t wch) noexcept;
void _EventOscParam(const wchar_t wch);
void _EventOscString(const wchar_t wch);
void _EventOscTermination(const wchar_t wch);
void _EventSs3Entry(const wchar_t wch);
Expand Down
Loading
Loading