Skip to content

Commit

Permalink
Remove SetTextAttributes from the ITerminalApi interface (#17685)
Browse files Browse the repository at this point in the history
The only reason we had the `SetTextAttributes` method in `ITerminalApi`
was to allow for conhost to remap the default color attributes when the
VT PowerShell quirk was active. Since that quirk has now been removed,
there's no need for this API anymore.

## References and Relevant Issues

The PowerShell quirk was removed in PR #17666.

## Validation Steps Performed

I've had to update all the attribute tests in adapterTest to manually
check the expected attributes, since those checks were previously being
handled in a `SetTextAttributes` mock which no longer exists.

I've also performed some manual tests of the VT attribute operations to
double check that they're still working as expected.
  • Loading branch information
j4james authored Aug 8, 2024
1 parent 9ab2870 commit edfa3ea
Show file tree
Hide file tree
Showing 10 changed files with 72 additions and 54 deletions.
1 change: 0 additions & 1 deletion src/cascadia/TerminalCore/Terminal.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ class Microsoft::Terminal::Core::Terminal final :
Microsoft::Console::VirtualTerminal::StateMachine& GetStateMachine() noexcept override;
BufferState GetBufferAndViewport() noexcept override;
void SetViewportPosition(const til::point position) noexcept override;
void SetTextAttributes(const TextAttribute& attrs) noexcept override;
void SetSystemMode(const Mode mode, const bool enabled) noexcept override;
bool GetSystemMode(const Mode mode) const noexcept override;
void ReturnAnswerback() override;
Expand Down
5 changes: 0 additions & 5 deletions src/cascadia/TerminalCore/TerminalApi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,6 @@ try
}
CATCH_LOG()

void Terminal::SetTextAttributes(const TextAttribute& attrs) noexcept
{
_activeBuffer().SetCurrentAttributes(attrs);
}

void Terminal::SetSystemMode(const Mode mode, const bool enabled) noexcept
{
_assertLocked();
Expand Down
12 changes: 0 additions & 12 deletions src/host/outputStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,18 +88,6 @@ void ConhostInternalGetSet::SetViewportPosition(const til::point position)
info.UpdateBottom();
}

// Method Description:
// - Sets the current TextAttribute of the active screen buffer. Text
// written to this buffer will be written with these attributes.
// Arguments:
// - attrs: The new TextAttribute to use
// Return Value:
// - <none>
void ConhostInternalGetSet::SetTextAttributes(const TextAttribute& attrs)
{
_io.GetActiveOutputBuffer().SetAttributes(attrs);
}

// Routine Description:
// - Sets the state of one of the system modes.
// Arguments:
Expand Down
2 changes: 0 additions & 2 deletions src/host/outputStream.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal::
BufferState GetBufferAndViewport() override;
void SetViewportPosition(const til::point position) override;

void SetTextAttributes(const TextAttribute& attrs) override;

void SetSystemMode(const Mode mode, const bool enabled) override;
bool GetSystemMode(const Mode mode) const override;

Expand Down
2 changes: 0 additions & 2 deletions src/terminal/adapter/ITerminalApi.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,6 @@ namespace Microsoft::Console::VirtualTerminal

virtual bool IsVtInputEnabled() const = 0;

virtual void SetTextAttributes(const TextAttribute& attrs) = 0;

enum class Mode : size_t
{
AutoWrap,
Expand Down
9 changes: 1 addition & 8 deletions src/terminal/adapter/PageManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,9 @@ const TextAttribute& Page::Attributes() const noexcept
return _buffer.GetCurrentAttributes();
}

void Page::SetAttributes(const TextAttribute& attr, ITerminalApi* api) const
void Page::SetAttributes(const TextAttribute& attr) const noexcept
{
_buffer.SetCurrentAttributes(attr);
// If the api parameter was specified, we need to pass the new attributes
// through to the api. This occurs when there's a potential for the colors
// to be changed, which may require some legacy remapping in conhost.
if (api)
{
api->SetTextAttributes(attr);
}
}

til::size Page::Size() const noexcept
Expand Down
2 changes: 1 addition & 1 deletion src/terminal/adapter/PageManager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ namespace Microsoft::Console::VirtualTerminal
til::CoordType Number() const noexcept;
Cursor& Cursor() const noexcept;
const TextAttribute& Attributes() const noexcept;
void SetAttributes(const TextAttribute& attr, ITerminalApi* api = nullptr) const;
void SetAttributes(const TextAttribute& attr) const noexcept;
til::size Size() const noexcept;
til::CoordType Top() const noexcept;
til::CoordType Bottom() const noexcept;
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 @@ -555,7 +555,7 @@ bool AdaptDispatch::CursorRestoreState()
}

// Restore text attributes.
page.SetAttributes(savedCursorState.Attributes, &_api);
page.SetAttributes(savedCursorState.Attributes);

// Restore designated character sets.
_termOutput.RestoreFrom(savedCursorState.TermOutput);
Expand Down
4 changes: 2 additions & 2 deletions src/terminal/adapter/adaptDispatchGraphics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ bool AdaptDispatch::SetGraphicsRendition(const VTParameters options)
const auto page = _pages.ActivePage();
auto attr = page.Attributes();
_ApplyGraphicsOptions(options, attr);
page.SetAttributes(attr, &_api);
page.SetAttributes(attr);
return true;
}

Expand Down Expand Up @@ -487,6 +487,6 @@ bool AdaptDispatch::PopGraphicsRendition()
{
const auto page = _pages.ActivePage();
const auto& currentAttributes = page.Attributes();
page.SetAttributes(_sgrStack.Pop(currentAttributes), &_api);
page.SetAttributes(_sgrStack.Pop(currentAttributes));
return true;
}
Loading

0 comments on commit edfa3ea

Please sign in to comment.