Skip to content

Commit

Permalink
Add support for chaining OSC 10-12 (#8999)
Browse files Browse the repository at this point in the history 
This adds the support for chaining OSC 10-12, allowing users to set all
of them at once.

**BREAKING CHANGE**
Before this PR, the OSC 10/11/12 command will only be dispatched iff the
first color is valid. This is no longer true. The new implementation
strictly follows xterm's behavior. Each color is treated independently.
For example, `\e]10;invalid;white\e\\` is effectively `\e]11;white\e\\`.

## Validation Steps Performed

Tests added. Manually tested.

Main OSC color tracking issue: #942
OSC 4 & Initial OSC 10-12 PR: #7578

Closes one item in #942
  • Loading branch information
@skyline75489
skyline75489 authored Feb 4, 2021
1 parent 45bee07 commit 779354d
Show file tree
Hide file tree
Showing 2 changed files with 122 additions and 50 deletions.
84 changes: 49 additions & 35 deletions src/terminal/parser/OutputStateMachineEngine.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
using namespace Microsoft::Console;
using namespace Microsoft::Console::VirtualTerminal;

// the console uses 0xffffffff as an "invalid color" value
constexpr COLORREF INVALID_COLOR = 0xffffffff;

// takes ownership of pDispatch
OutputStateMachineEngine::OutputStateMachineEngine(std::unique_ptr<ITermDispatch> pDispatch) :
_dispatch(std::move(pDispatch)),
Expand Down Expand Up @@ -672,36 +675,52 @@ bool OutputStateMachineEngine::ActionOscDispatch(const wchar_t /*wch*/,
break;
}
case OscActionCodes::SetForegroundColor:
{
std::vector<DWORD> colors;
success = _GetOscSetColor(string, colors);
if (success && colors.size() > 0)
{
success = _dispatch->SetDefaultForeground(til::at(colors, 0));
}
TermTelemetry::Instance().Log(TermTelemetry::Codes::OSCFG);
break;
}
case OscActionCodes::SetBackgroundColor:
{
std::vector<DWORD> colors;
success = _GetOscSetColor(string, colors);
if (success && colors.size() > 0)
{
success = _dispatch->SetDefaultBackground(til::at(colors, 0));
}
TermTelemetry::Instance().Log(TermTelemetry::Codes::OSCBG);
break;
}
case OscActionCodes::SetCursorColor:
{
std::vector<DWORD> colors;
success = _GetOscSetColor(string, colors);
if (success && colors.size() > 0)
if (success)
{
success = _dispatch->SetCursorColor(til::at(colors, 0));
size_t commandIndex = parameter;
size_t colorIndex = 0;

if (commandIndex == OscActionCodes::SetForegroundColor && colors.size() > colorIndex)
{
const auto color = til::at(colors, colorIndex);
if (color != INVALID_COLOR)
{
success = success && _dispatch->SetDefaultForeground(color);
}
TermTelemetry::Instance().Log(TermTelemetry::Codes::OSCFG);
commandIndex++;
colorIndex++;
}

if (commandIndex == OscActionCodes::SetBackgroundColor && colors.size() > colorIndex)
{
const auto color = til::at(colors, colorIndex);
if (color != INVALID_COLOR)
{
success = success && _dispatch->SetDefaultBackground(color);
}
TermTelemetry::Instance().Log(TermTelemetry::Codes::OSCBG);
commandIndex++;
colorIndex++;
}

if (commandIndex == OscActionCodes::SetCursorColor && colors.size() > colorIndex)
{
const auto color = til::at(colors, colorIndex);
if (color != INVALID_COLOR)
{
success = success && _dispatch->SetCursorColor(color);
}
TermTelemetry::Instance().Log(TermTelemetry::Codes::OSCSCC);
commandIndex++;
colorIndex++;
}
}
TermTelemetry::Instance().Log(TermTelemetry::Codes::OSCSCC);
break;
}
case OscActionCodes::SetClipboard:
Expand All @@ -718,7 +737,7 @@ bool OutputStateMachineEngine::ActionOscDispatch(const wchar_t /*wch*/,
}
case OscActionCodes::ResetCursorColor:
{
success = _dispatch->SetCursorColor(0xffffffff);
success = _dispatch->SetCursorColor(INVALID_COLOR);
TermTelemetry::Instance().Log(TermTelemetry::Codes::OSCRCC);
break;
}
Expand Down Expand Up @@ -947,19 +966,15 @@ bool OutputStateMachineEngine::_ParseHyperlink(const std::wstring_view string,
// with accumulated Ps. For example "OSC 10;color1;color2" is effectively an "OSC 10;color1"
// and an "OSC 11;color2".
//
// However, we do not support the chaining of OSC 10-17 yet. Right now only the first parameter
// will take effect.
// Arguments:
// - string - the Osc String to parse
// - rgbs - receives the colors that we parsed in the format: 0x00BBGGRR
// Return Value:
// - True if the first table index and color was parsed successfully. False otherwise.
// - True if at least one color was parsed successfully. False otherwise.
bool OutputStateMachineEngine::_GetOscSetColor(const std::wstring_view string,
std::vector<DWORD>& rgbs) const noexcept
try
{
bool success = false;

const auto parts = Utils::SplitString(string, L';');
if (parts.size() < 1)
{
Expand All @@ -973,17 +988,16 @@ try
if (colorOptional.has_value())
{
newRgbs.push_back(colorOptional.value());
// Only mark the parsing as a success iff the first color is a success.
if (i == 0)
{
success = true;
}
}
else
{
newRgbs.push_back(INVALID_COLOR);
}
}

rgbs.swap(newRgbs);

return success;
return rgbs.size() > 0;
}
CATCH_LOG_RETURN_FALSE()

Expand Down
88 changes: 73 additions & 15 deletions src/terminal/parser/ut_parser/OutputEngineTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1052,6 +1052,8 @@ class StatefulDispatch final : public TermDispatch
_defaultForegroundColor{ RGB(0, 0, 0) },
_setDefaultBackground(false),
_defaultBackgroundColor{ RGB(0, 0, 0) },
_setDefaultCursorColor(false),
_defaultCursorColor{ RGB(0, 0, 0) },
_hyperlinkMode{ false },
_options{ s_cMaxOptions, static_cast<DispatchTypes::GraphicsOptions>(s_uiGraphicsCleared) }, // fill with cleared option
_colorTable{},
Expand Down Expand Up @@ -1441,6 +1443,13 @@ class StatefulDispatch final : public TermDispatch
return true;
}

bool SetCursorColor(const DWORD color) noexcept override
{
_setDefaultCursorColor = true;
_defaultCursorColor = color;
return true;
}

bool SetClipboard(std::wstring_view content) noexcept override
{
_copyContent = { content.begin(), content.end() };
Expand Down Expand Up @@ -1527,6 +1536,8 @@ class StatefulDispatch final : public TermDispatch
DWORD _defaultForegroundColor;
bool _setDefaultBackground;
DWORD _defaultBackgroundColor;
bool _setDefaultCursorColor;
DWORD _defaultCursorColor;
bool _setColorTableEntry;
bool _hyperlinkMode;
std::wstring _copyContent;
Expand Down Expand Up @@ -2846,6 +2857,7 @@ class StateMachineExternalTest final
auto engine = std::make_unique<OutputStateMachineEngine>(std::move(dispatch));
StateMachine mach(std::move(engine));

// Single param
mach.ProcessString(L"\033]10;rgb:1/1/1\033\\");
VERIFY_IS_TRUE(pDispatch->_setDefaultForeground);
VERIFY_ARE_EQUAL(RGB(0x11, 0x11, 0x11), pDispatch->_defaultForegroundColor);
Expand Down Expand Up @@ -2876,20 +2888,34 @@ class StateMachineExternalTest final

pDispatch->ClearState();

// Multiple params.
// Multiple params
mach.ProcessString(L"\033]10;#111;rgb:2/2/2\033\\");
VERIFY_IS_TRUE(pDispatch->_setDefaultForeground);
VERIFY_ARE_EQUAL(RGB(0x10, 0x10, 0x10), pDispatch->_defaultForegroundColor);
VERIFY_IS_TRUE(pDispatch->_setDefaultBackground);
VERIFY_ARE_EQUAL(RGB(0x22, 0x22, 0x22), pDispatch->_defaultBackgroundColor);

pDispatch->ClearState();

mach.ProcessString(L"\033]10;#111;DarkOrange\033\\");
VERIFY_IS_TRUE(pDispatch->_setDefaultForeground);
VERIFY_ARE_EQUAL(RGB(0x10, 0x10, 0x10), pDispatch->_defaultForegroundColor);
VERIFY_IS_TRUE(pDispatch->_setDefaultBackground);
VERIFY_ARE_EQUAL(RGB(255, 140, 0), pDispatch->_defaultBackgroundColor);

pDispatch->ClearState();

// Partially valid sequences. Only the first color works.
mach.ProcessString(L"\033]10;#111;DarkOrange;rgb:2/2/2\033\\");
VERIFY_IS_TRUE(pDispatch->_setDefaultForeground);
VERIFY_ARE_EQUAL(RGB(0x10, 0x10, 0x10), pDispatch->_defaultForegroundColor);
VERIFY_IS_TRUE(pDispatch->_setDefaultBackground);
VERIFY_ARE_EQUAL(RGB(255, 140, 0), pDispatch->_defaultBackgroundColor);
VERIFY_IS_TRUE(pDispatch->_setDefaultCursorColor);
VERIFY_ARE_EQUAL(RGB(0x22, 0x22, 0x22), pDispatch->_defaultCursorColor);

pDispatch->ClearState();

// Partially valid multi-param sequences.
mach.ProcessString(L"\033]10;#111;\033\\");
VERIFY_IS_TRUE(pDispatch->_setDefaultForeground);
VERIFY_ARE_EQUAL(RGB(0x10, 0x10, 0x10), pDispatch->_defaultForegroundColor);
Expand All @@ -2899,33 +2925,38 @@ class StateMachineExternalTest final
mach.ProcessString(L"\033]10;#111;rgb:\033\\");
VERIFY_IS_TRUE(pDispatch->_setDefaultForeground);
VERIFY_ARE_EQUAL(RGB(0x10, 0x10, 0x10), pDispatch->_defaultForegroundColor);
VERIFY_IS_FALSE(pDispatch->_setDefaultBackground);

pDispatch->ClearState();

mach.ProcessString(L"\033]10;#111;#2\033\\");
VERIFY_IS_TRUE(pDispatch->_setDefaultForeground);
VERIFY_ARE_EQUAL(RGB(0x10, 0x10, 0x10), pDispatch->_defaultForegroundColor);
VERIFY_IS_FALSE(pDispatch->_setDefaultBackground);

pDispatch->ClearState();

// Invalid sequences.
mach.ProcessString(L"\033]10;rgb:1/1/\033\\");
mach.ProcessString(L"\033]10;;rgb:1/1/1\033\\");
VERIFY_IS_FALSE(pDispatch->_setDefaultForeground);
VERIFY_IS_TRUE(pDispatch->_setDefaultBackground);
VERIFY_ARE_EQUAL(RGB(0x11, 0x11, 0x11), pDispatch->_defaultBackgroundColor);

pDispatch->ClearState();

mach.ProcessString(L"\033]10;#1\033\\");
mach.ProcessString(L"\033]10;#1;rgb:1/1/1\033\\");
VERIFY_IS_FALSE(pDispatch->_setDefaultForeground);
VERIFY_IS_TRUE(pDispatch->_setDefaultBackground);
VERIFY_ARE_EQUAL(RGB(0x11, 0x11, 0x11), pDispatch->_defaultBackgroundColor);

pDispatch->ClearState();

// Invalid multi-param sequences.
mach.ProcessString(L"\033]10;;rgb:1/1/1\033\\");
// Invalid sequences.
mach.ProcessString(L"\033]10;rgb:1/1/\033\\");
VERIFY_IS_FALSE(pDispatch->_setDefaultForeground);

pDispatch->ClearState();

mach.ProcessString(L"\033]10;#1;rgb:1/1/1\033\\");
mach.ProcessString(L"\033]10;#1\033\\");
VERIFY_IS_FALSE(pDispatch->_setDefaultForeground);

pDispatch->ClearState();
Expand All @@ -2944,6 +2975,7 @@ class StateMachineExternalTest final

pDispatch->ClearState();

// Single param
mach.ProcessString(L"\033]11;rgb:12/34/56\033\\");
VERIFY_IS_TRUE(pDispatch->_setDefaultBackground);
VERIFY_ARE_EQUAL(RGB(0x12, 0x34, 0x56), pDispatch->_defaultBackgroundColor);
Expand All @@ -2968,7 +3000,30 @@ class StateMachineExternalTest final

pDispatch->ClearState();

// Partially valid sequences. Only the first color works.
// Multiple params
mach.ProcessString(L"\033]11;#111;rgb:2/2/2\033\\");
VERIFY_IS_TRUE(pDispatch->_setDefaultBackground);
VERIFY_ARE_EQUAL(RGB(0x10, 0x10, 0x10), pDispatch->_defaultBackgroundColor);
VERIFY_ARE_EQUAL(RGB(0x22, 0x22, 0x22), pDispatch->_defaultCursorColor);

pDispatch->ClearState();

mach.ProcessString(L"\033]11;#111;DarkOrange\033\\");
VERIFY_IS_TRUE(pDispatch->_setDefaultBackground);
VERIFY_ARE_EQUAL(RGB(0x10, 0x10, 0x10), pDispatch->_defaultBackgroundColor);
VERIFY_ARE_EQUAL(RGB(255, 140, 0), pDispatch->_defaultCursorColor);

pDispatch->ClearState();

mach.ProcessString(L"\033]11;#111;DarkOrange;rgb:2/2/2\033\\");
VERIFY_IS_TRUE(pDispatch->_setDefaultBackground);
VERIFY_ARE_EQUAL(RGB(0x10, 0x10, 0x10), pDispatch->_defaultBackgroundColor);
VERIFY_ARE_EQUAL(RGB(255, 140, 0), pDispatch->_defaultCursorColor);
// The third param is out of range.

pDispatch->ClearState();

// Partially valid multi-param sequences.
mach.ProcessString(L"\033]11;#111;\033\\");
VERIFY_IS_TRUE(pDispatch->_setDefaultBackground);
VERIFY_ARE_EQUAL(RGB(0x10, 0x10, 0x10), pDispatch->_defaultBackgroundColor);
Expand All @@ -2987,24 +3042,27 @@ class StateMachineExternalTest final

pDispatch->ClearState();

// Invalid sequences.
mach.ProcessString(L"\033]11;rgb:1/1/\033\\");
mach.ProcessString(L"\033]11;;rgb:1/1/1\033\\");
VERIFY_IS_FALSE(pDispatch->_setDefaultBackground);
VERIFY_IS_TRUE(pDispatch->_setDefaultCursorColor);
VERIFY_ARE_EQUAL(RGB(0x11, 0x11, 0x11), pDispatch->_defaultCursorColor);

pDispatch->ClearState();

mach.ProcessString(L"\033]11;#1\033\\");
mach.ProcessString(L"\033]11;#1;rgb:1/1/1\033\\");
VERIFY_IS_FALSE(pDispatch->_setDefaultBackground);
VERIFY_IS_TRUE(pDispatch->_setDefaultCursorColor);
VERIFY_ARE_EQUAL(RGB(0x11, 0x11, 0x11), pDispatch->_defaultCursorColor);

pDispatch->ClearState();

// Invalid multi-param sequences.
mach.ProcessString(L"\033]11;;rgb:1/1/1\033\\");
// Invalid sequences.
mach.ProcessString(L"\033]11;rgb:1/1/\033\\");
VERIFY_IS_FALSE(pDispatch->_setDefaultBackground);

pDispatch->ClearState();

mach.ProcessString(L"\033]11;#1;rgb:1/1/1\033\\");
mach.ProcessString(L"\033]11;#1\033\\");
VERIFY_IS_FALSE(pDispatch->_setDefaultBackground);

pDispatch->ClearState();
Expand Down

0 comments on commit 779354d

Please sign in to comment.