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

Let marks be cleared by clear (and friends) #15686

Merged
merged 14 commits into from Jul 18, 2023
Merged
100 changes: 100 additions & 0 deletions src/buffer/out/textBuffer.cpp
Expand Up @@ -2660,6 +2660,9 @@ try
// Set size back to real size as it will be taking over the rendering duties.
newCursor.SetSize(ulSize);

newBuffer._marks = oldBuffer._marks;
newBuffer._trimMarksOutsideBuffer();

return S_OK;
}
CATCH_RETURN()
Expand Down Expand Up @@ -2869,3 +2872,100 @@ PointTree TextBuffer::GetPatterns(const til::CoordType firstRow, const til::Coor
PointTree result(std::move(intervals));
return result;
}

const std::vector<ScrollMark>& TextBuffer::GetMarks() const noexcept
{
return _marks;
}

// Remove all marks between `start` & `end`, inclusive.
void TextBuffer::ClearMarksInRange(
const til::point start,
const til::point end)
{
auto inRange = [&start, &end](const ScrollMark& m) {
return (m.start >= start && m.start <= end) ||
(m.end >= start && m.end <= end);
};

_marks.erase(std::remove_if(_marks.begin(),
_marks.end(),
inRange),
_marks.end());
}
void TextBuffer::ClearAllMarks() noexcept
{
_marks.clear();
}

// Adjust all the marks in the y-direction by `delta`. Positive values move the
// marks down (the positive y direction). Negative values move up. This will
// trim marks that are no longer have a start in the bounds of the buffer
void TextBuffer::ScrollMarks(const int delta)
{
for (auto& mark : _marks)
{
mark.start.y += delta;

// If the mark had sub-regions, then move those pointers too
if (mark.commandEnd.has_value())
{
(*mark.commandEnd).y += delta;
}
if (mark.outputEnd.has_value())
{
(*mark.outputEnd).y += delta;
}
}
_trimMarksOutsideBuffer();
}

void TextBuffer::StartPromptMark(const ScrollMark& m)
{
_marks.push_back(m);
}
void TextBuffer::AddMark(const ScrollMark& m)
{
_marks.insert(_marks.begin(), m);
}

void TextBuffer::_trimMarksOutsideBuffer()
{
const auto height = GetSize().Height();
_marks.erase(std::remove_if(_marks.begin(),
_marks.end(),
[height](const auto& m) {
return (m.start.y < 0) ||
(m.start.y >= height);
}),
_marks.end());
}

void TextBuffer::SetCurrentPromptEnd(const til::point pos) noexcept
{
if (_marks.empty())
{
return;
}
auto& curr{ _marks.back() };
curr.end = pos;
}
void TextBuffer::SetCurrentCommandEnd(const til::point pos)
{
if (_marks.empty())
{
return;
}
auto& curr{ _marks.back() };
curr.commandEnd = pos;
}
void TextBuffer::SetCurrentOutputEnd(const til::point pos, ::MarkCategory category)
{
if (_marks.empty())
{
return;
}
auto& curr{ _marks.back() };
curr.outputEnd = pos;
curr.category = category;
}
48 changes: 48 additions & 0 deletions src/buffer/out/textBuffer.hpp
Expand Up @@ -64,6 +64,41 @@ namespace Microsoft::Console::Render
class Renderer;
}

enum class MarkCategory
{
Prompt = 0,
Error = 1,
Warning = 2,
Success = 3,
Info = 4
};
struct ScrollMark
{
std::optional<til::color> color;
til::point start;
til::point end; // exclusive
std::optional<til::point> commandEnd;
std::optional<til::point> outputEnd;

MarkCategory category{ MarkCategory::Info };
// Other things we may want to think about in the future are listed in
// GH#11000

bool HasCommand() const noexcept
{
return commandEnd.has_value() && *commandEnd != end;
}
bool HasOutput() const noexcept
{
return outputEnd.has_value() && *outputEnd != *commandEnd;
}
std::pair<til::point, til::point> GetExtent() const
{
til::point realEnd{ til::coalesce_value(outputEnd, commandEnd, end) };
return std::make_pair(til::point{ start }, realEnd);
}
};

class TextBuffer final
{
public:
Expand Down Expand Up @@ -228,6 +263,16 @@ class TextBuffer final
void CopyPatterns(const TextBuffer& OtherBuffer);
interval_tree::IntervalTree<til::point, size_t> GetPatterns(const til::CoordType firstRow, const til::CoordType lastRow) const;

const std::vector<ScrollMark>& GetMarks() const noexcept;
void ClearMarksInRange(const til::point start, const til::point end);
void ClearAllMarks() noexcept;
void ScrollMarks(const int delta);
void StartPromptMark(const ScrollMark& m);
void AddMark(const ScrollMark& m);
void SetCurrentPromptEnd(const til::point pos) noexcept;
void SetCurrentCommandEnd(const til::point pos);
void SetCurrentOutputEnd(const til::point pos, ::MarkCategory category);

private:
void _reserve(til::size screenBufferSize, const TextAttribute& defaultAttributes);
void _commit(const std::byte* row);
Expand All @@ -251,6 +296,7 @@ class TextBuffer final
til::point _GetWordEndForAccessibility(const til::point target, const std::wstring_view wordDelimiters, const til::point limit) const;
til::point _GetWordEndForSelection(const til::point target, const std::wstring_view wordDelimiters) const;
void _PruneHyperlinks();
void _trimMarksOutsideBuffer();

static void _AppendRTFText(std::ostringstream& contentBuilder, const std::wstring_view& text);

Expand Down Expand Up @@ -327,6 +373,8 @@ class TextBuffer final

bool _isActiveBuffer = false;

std::vector<ScrollMark> _marks;

#ifdef UNIT_TESTING
friend class TextBufferTests;
friend class UiaTextRangeTests;
Expand Down
26 changes: 13 additions & 13 deletions src/cascadia/TerminalControl/ControlCore.cpp
Expand Up @@ -2111,7 +2111,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation

void ControlCore::AddMark(const Control::ScrollMark& mark)
{
::Microsoft::Console::VirtualTerminal::DispatchTypes::ScrollMark m{};
::ScrollMark m{};

if (mark.Color.HasValue)
{
Expand Down Expand Up @@ -2140,7 +2140,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
const auto currentOffset = ScrollOffset();
const auto& marks{ _terminal->GetScrollMarks() };

std::optional<DispatchTypes::ScrollMark> tgt;
std::optional<::ScrollMark> tgt;

switch (direction)
{
Expand Down Expand Up @@ -2243,7 +2243,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
const til::point start = HasSelection() ? (goUp ? _terminal->GetSelectionAnchor() : _terminal->GetSelectionEnd()) :
_terminal->GetTextBuffer().GetCursor().GetPosition();

std::optional<DispatchTypes::ScrollMark> nearest{ std::nullopt };
std::optional<::ScrollMark> nearest{ std::nullopt };
const auto& marks{ _terminal->GetScrollMarks() };

// Early return so we don't have to check for the validity of `nearest` below after the loop exits.
Expand Down Expand Up @@ -2283,7 +2283,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
const til::point start = HasSelection() ? (goUp ? _terminal->GetSelectionAnchor() : _terminal->GetSelectionEnd()) :
_terminal->GetTextBuffer().GetCursor().GetPosition();

std::optional<DispatchTypes::ScrollMark> nearest{ std::nullopt };
std::optional<::ScrollMark> nearest{ std::nullopt };
const auto& marks{ _terminal->GetScrollMarks() };

static constexpr til::point worst{ til::CoordTypeMax, til::CoordTypeMax };
Expand Down Expand Up @@ -2357,8 +2357,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation

void ControlCore::_contextMenuSelectMark(
const til::point& pos,
bool (*filter)(const DispatchTypes::ScrollMark&),
til::point_span (*getSpan)(const DispatchTypes::ScrollMark&))
bool (*filter)(const ::ScrollMark&),
til::point_span (*getSpan)(const ::ScrollMark&))
{
// Do nothing if the caller didn't give us a way to get the span to select for this mark.
if (!getSpan)
Expand Down Expand Up @@ -2391,20 +2391,20 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
_contextMenuSelectMark(
_contextMenuBufferPosition,
[](const DispatchTypes::ScrollMark& m) -> bool { return !m.HasCommand(); },
[](const DispatchTypes::ScrollMark& m) { return til::point_span{ m.end, *m.commandEnd }; });
[](const ::ScrollMark& m) -> bool { return !m.HasCommand(); },
[](const ::ScrollMark& m) { return til::point_span{ m.end, *m.commandEnd }; });
}
void ControlCore::ContextMenuSelectOutput()
{
_contextMenuSelectMark(
_contextMenuBufferPosition,
[](const DispatchTypes::ScrollMark& m) -> bool { return !m.HasOutput(); },
[](const DispatchTypes::ScrollMark& m) { return til::point_span{ *m.commandEnd, *m.outputEnd }; });
[](const ::ScrollMark& m) -> bool { return !m.HasOutput(); },
[](const ::ScrollMark& m) { return til::point_span{ *m.commandEnd, *m.outputEnd }; });
Copy link
Member

Choose a reason for hiding this comment

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

You could use auto here.

}

bool ControlCore::_clickedOnMark(
const til::point& pos,
bool (*filter)(const DispatchTypes::ScrollMark&))
bool (*filter)(const ::ScrollMark&))
{
// Don't show this if the click was on the selection
if (_terminal->IsSelectionActive() &&
Expand Down Expand Up @@ -2442,7 +2442,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
// Relies on the anchor set in AnchorContextMenu
return _clickedOnMark(_contextMenuBufferPosition,
[](const DispatchTypes::ScrollMark& m) -> bool { return !m.HasCommand(); });
[](const ::ScrollMark& m) -> bool { return !m.HasCommand(); });
}

// Method Description:
Expand All @@ -2451,6 +2451,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
// Relies on the anchor set in AnchorContextMenu
return _clickedOnMark(_contextMenuBufferPosition,
[](const DispatchTypes::ScrollMark& m) -> bool { return !m.HasOutput(); });
[](const ::ScrollMark& m) -> bool { return !m.HasOutput(); });
}
}
6 changes: 3 additions & 3 deletions src/cascadia/TerminalControl/ControlCore.h
Expand Up @@ -372,10 +372,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation

void _contextMenuSelectMark(
const til::point& pos,
bool (*filter)(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::ScrollMark&),
til::point_span (*getSpan)(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::ScrollMark&));
bool (*filter)(const ::ScrollMark&),
til::point_span (*getSpan)(const ::ScrollMark&));

bool _clickedOnMark(const til::point& pos, bool (*filter)(const ::Microsoft::Console::VirtualTerminal::DispatchTypes::ScrollMark&));
bool _clickedOnMark(const til::point& pos, bool (*filter)(const ::ScrollMark&));

inline bool _IsClosing() const noexcept
{
Expand Down
43 changes: 14 additions & 29 deletions src/cascadia/TerminalCore/Terminal.cpp
Expand Up @@ -1344,7 +1344,7 @@ void Terminal::_updateUrlDetection()
}

// NOTE: This is the version of AddMark that comes from the UI. The VT api call into this too.
void Terminal::AddMark(const Microsoft::Console::VirtualTerminal::DispatchTypes::ScrollMark& mark,
void Terminal::AddMark(const ScrollMark& mark,
const til::point& start,
const til::point& end,
const bool fromUi)
Expand All @@ -1354,18 +1354,12 @@ void Terminal::AddMark(const Microsoft::Console::VirtualTerminal::DispatchTypes:
return;
}

DispatchTypes::ScrollMark m = mark;
ScrollMark m = mark;
m.start = start;
m.end = end;

if (fromUi)
{
_scrollMarks.insert(_scrollMarks.begin(), m);
}
else
{
_scrollMarks.push_back(m);
}
// If the mark came from the user adding a mark via the UI, don't make it the active prompt mark.
fromUi ? _activeBuffer().AddMark(m) : _activeBuffer().StartPromptMark(m);
Copy link
Member

Choose a reason for hiding this comment

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

It is very rare that using a ternary like this is the right thing to do.

Copy link
Member Author

Choose a reason for hiding this comment

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

yea that felt dirty to write


// Tell the control that the scrollbar has somehow changed. Used as a
// workaround to force the control to redraw any scrollbar marks
Expand All @@ -1389,39 +1383,30 @@ void Terminal::ClearMark()
start = til::point{ GetSelectionAnchor() };
end = til::point{ GetSelectionEnd() };
}
auto inSelection = [&start, &end](const DispatchTypes::ScrollMark& m) {
return (m.start >= start && m.start <= end) ||
(m.end >= start && m.end <= end);
};

_scrollMarks.erase(std::remove_if(_scrollMarks.begin(),
_scrollMarks.end(),
inSelection),
_scrollMarks.end());
_activeBuffer().ClearMarksInRange(start, end);

// Tell the control that the scrollbar has somehow changed. Used as a
// workaround to force the control to redraw any scrollbar marks
_NotifyScrollEvent();
}
void Terminal::ClearAllMarks() noexcept
{
_scrollMarks.clear();
_activeBuffer().ClearAllMarks();
// Tell the control that the scrollbar has somehow changed. Used as a
// workaround to force the control to redraw any scrollbar marks
_NotifyScrollEvent();
}

const std::vector<DispatchTypes::ScrollMark>& Terminal::GetScrollMarks() const noexcept
const std::vector<ScrollMark>& Terminal::GetScrollMarks() const noexcept
{
// TODO: GH#11000 - when the marks are stored per-buffer, get rid of this.
// We want to return _no_ marks when we're in the alt buffer, to effectively
// hide them. We need to return a reference, so we can't just ctor an empty
// list here just for when we're in the alt buffer.
static const std::vector<DispatchTypes::ScrollMark> _altBufferMarks{};
return _inAltBuffer() ? _altBufferMarks : _scrollMarks;
return _activeBuffer().GetMarks();
}

til::color Terminal::GetColorForMark(const Microsoft::Console::VirtualTerminal::DispatchTypes::ScrollMark& mark) const
til::color Terminal::GetColorForMark(const ScrollMark& mark) const
{
if (mark.color.has_value())
{
Expand All @@ -1430,24 +1415,24 @@ til::color Terminal::GetColorForMark(const Microsoft::Console::VirtualTerminal::

switch (mark.category)
{
case Microsoft::Console::VirtualTerminal::DispatchTypes::MarkCategory::Prompt:
case MarkCategory::Prompt:
{
return _renderSettings.GetColorAlias(ColorAlias::DefaultForeground);
}
case Microsoft::Console::VirtualTerminal::DispatchTypes::MarkCategory::Error:
case MarkCategory::Error:
{
return _renderSettings.GetColorTableEntry(TextColor::BRIGHT_RED);
}
case Microsoft::Console::VirtualTerminal::DispatchTypes::MarkCategory::Warning:
case MarkCategory::Warning:
{
return _renderSettings.GetColorTableEntry(TextColor::BRIGHT_YELLOW);
}
case Microsoft::Console::VirtualTerminal::DispatchTypes::MarkCategory::Success:
case MarkCategory::Success:
{
return _renderSettings.GetColorTableEntry(TextColor::BRIGHT_GREEN);
}
default:
case Microsoft::Console::VirtualTerminal::DispatchTypes::MarkCategory::Info:
case MarkCategory::Info:
{
return _renderSettings.GetColorAlias(ColorAlias::DefaultForeground);
}
Expand Down