From 80a0ffbff88e7c0105fe0b81cd345a6e4c826603 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 22 Mar 2024 12:21:38 -0500 Subject: [PATCH 01/30] Starting from just the bottom, remove ScrollMarks from TextBuffer, and replace with a text attribute --- src/buffer/out/Row.cpp | 20 ++ src/buffer/out/Row.hpp | 15 ++ src/buffer/out/TextAttribute.cpp | 2 +- src/buffer/out/TextAttribute.hpp | 31 ++- src/buffer/out/textBuffer.cpp | 314 +++++++++++++++++-------- src/buffer/out/textBuffer.hpp | 27 ++- src/terminal/adapter/adaptDispatch.cpp | 40 +++- 7 files changed, 323 insertions(+), 126 deletions(-) diff --git a/src/buffer/out/Row.cpp b/src/buffer/out/Row.cpp index 4722dc0dfc4..7f1f3e4e9f6 100644 --- a/src/buffer/out/Row.cpp +++ b/src/buffer/out/Row.cpp @@ -1181,3 +1181,23 @@ CharToColumnMapper ROW::_createCharToColumnMapper(ptrdiff_t offset) const noexce const auto guessedColumn = gsl::narrow_cast(clamp(offset, 0, _columnCount)); return CharToColumnMapper{ _chars.data(), _charOffsets.data(), lastChar, guessedColumn }; } + +const std::optional& ROW::GetPromptData() const noexcept +{ + return _promptData; +} + +void ROW::StartPrompt() noexcept +{ + if (!_promptData.has_value()) + { + _promptData = {}; + } +} +void ROW::EndCommand(uint32_t exitCode) noexcept +{ + if (_promptData.has_value()) + { + _promptData->exitCode = exitCode; + } +} diff --git a/src/buffer/out/Row.hpp b/src/buffer/out/Row.hpp index 197343df6d8..b36ff6b6264 100644 --- a/src/buffer/out/Row.hpp +++ b/src/buffer/out/Row.hpp @@ -87,6 +87,15 @@ struct CharToColumnMapper til::CoordType _currentColumn; }; +struct MarkData +{ + // Scrollbar marks may have been given a color, or not. + std::optional color; + // Prompts without an exit code haven't had a matching FTCS CommandEnd + // called yet. Any value other than 0 is an error. + std::optional exitCode; +}; + class ROW final { public: @@ -167,6 +176,10 @@ class ROW final auto AttrBegin() const noexcept { return _attr.begin(); } auto AttrEnd() const noexcept { return _attr.end(); } + const std::optional& GetPromptData() const noexcept; + void StartPrompt() noexcept; + void EndCommand(uint32_t exitCode) noexcept; + #ifdef UNIT_TESTING friend constexpr bool operator==(const ROW& a, const ROW& b) noexcept; friend class RowTests; @@ -299,6 +312,8 @@ class ROW final bool _wrapForced = false; // Occurs when the user runs out of text to support a double byte character and we're forced to the next line bool _doubleBytePadded = false; + + std::optional _promptData = std::nullopt; }; #ifdef UNIT_TESTING diff --git a/src/buffer/out/TextAttribute.cpp b/src/buffer/out/TextAttribute.cpp index d5b0e5e4a5a..e154accf98c 100644 --- a/src/buffer/out/TextAttribute.cpp +++ b/src/buffer/out/TextAttribute.cpp @@ -7,7 +7,7 @@ // Keeping TextColor compact helps us keeping TextAttribute compact, // which in turn ensures that our buffer memory usage is low. -static_assert(sizeof(TextAttribute) == 16); +static_assert(sizeof(TextAttribute) == 18); static_assert(alignof(TextAttribute) == 2); // Ensure that we can memcpy() and memmove() the struct for performance. static_assert(std::is_trivially_copyable_v); diff --git a/src/buffer/out/TextAttribute.hpp b/src/buffer/out/TextAttribute.hpp index 4fe8c9e637e..9b4a75d95a1 100644 --- a/src/buffer/out/TextAttribute.hpp +++ b/src/buffer/out/TextAttribute.hpp @@ -38,6 +38,15 @@ enum class UnderlineStyle Max = DashedUnderlined }; +// We only need a few bits, but uint8_t is two bytes anyways, and apparently +// doesn't satisfy std::has_unique_object_representations_v +enum class MarkKind : uint16_t +{ + Output = 0, + Prompt = 1, + Command = 2 +}; + class TextAttribute final { public: @@ -46,7 +55,8 @@ class TextAttribute final _foreground{}, _background{}, _hyperlinkId{ 0 }, - _underlineColor{} + _underlineColor{}, + _markKind{ MarkKind::Output } { } @@ -55,7 +65,8 @@ class TextAttribute final _foreground{ gsl::at(s_legacyForegroundColorMap, wLegacyAttr & FG_ATTRS) }, _background{ gsl::at(s_legacyBackgroundColorMap, (wLegacyAttr & BG_ATTRS) >> 4) }, _hyperlinkId{ 0 }, - _underlineColor{} + _underlineColor{}, + _markKind{ MarkKind::Output } { } @@ -66,7 +77,8 @@ class TextAttribute final _foreground{ rgbForeground }, _background{ rgbBackground }, _hyperlinkId{ 0 }, - _underlineColor{ rgbUnderline } + _underlineColor{ rgbUnderline }, + _markKind{ MarkKind::Output } { } @@ -75,7 +87,8 @@ class TextAttribute final _foreground{ foreground }, _background{ background }, _hyperlinkId{ hyperlinkId }, - _underlineColor{ underlineColor } + _underlineColor{ underlineColor }, + _markKind{ MarkKind::Output } { } @@ -135,6 +148,15 @@ class TextAttribute final return _attrs; } + constexpr void SetMarkAttributes(const MarkKind attrs) noexcept + { + _markKind = attrs; + } + constexpr MarkKind GetMarkAttributes() const noexcept + { + return _markKind; + } + bool IsHyperlink() const noexcept; TextColor GetForeground() const noexcept; @@ -202,6 +224,7 @@ class TextAttribute final TextColor _foreground; // sizeof: 4, alignof: 1 TextColor _background; // sizeof: 4, alignof: 1 TextColor _underlineColor; // sizeof: 4, alignof: 1 + MarkKind _markKind; // sizeof: 2, cause I guess a uint8_t is 2B? #ifdef UNIT_TESTING friend class TextBufferTests; diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 5c71dd1d398..9c704609e07 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -1179,7 +1179,7 @@ void TextBuffer::ClearScrollback(const til::CoordType start, const til::CoordTyp GetMutableRowByOffset(y).Reset(_initialAttributes); } - ScrollMarks(-start); + // ScrollMarks(-start); ClearMarksInRange(til::point{ 0, height }, til::point{ _width, _height }); } @@ -2737,8 +2737,8 @@ void TextBuffer::Reflow(TextBuffer& oldBuffer, TextBuffer& newBuffer, const View newCursor.SetSize(oldCursor.GetSize()); newCursor.SetPosition(newCursorPos); - newBuffer._marks = oldBuffer._marks; - newBuffer._trimMarksOutsideBuffer(); + // newBuffer._marks = oldBuffer._marks; + // newBuffer._trimMarksOutsideBuffer(); } // Method Description: @@ -2888,125 +2888,241 @@ std::vector TextBuffer::SearchText(const std::wstring_view& nee return results; } -const std::vector& TextBuffer::GetMarks() const noexcept -{ - return _marks; -} +// const std::vector& 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); - }; + start; + 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()); + // _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()) + // _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(); +// } + +// // Method Description: +// // - Add a mark to our list of marks, and treat it as the active "prompt". For +// // the sake of shell integration, we need to know which mark represents the +// // current prompt/command/output. Internally, we'll always treat the _last_ +// // mark in the list as the current prompt. +// // Arguments: +// // - m: the mark to add. +// void TextBuffer::StartPromptMark(const ScrollMark& m) +// { +// _marks.push_back(m); +// } +// // Method Description: +// // - Add a mark to our list of marks. Don't treat this as the active prompt. +// // This should be used for marks created by the UI or from other user input. +// // By inserting at the start of the list, we can separate out marks that were +// // generated by client programs vs ones created by the user. +// // Arguments: +// // - m: the mark to add. +// void TextBuffer::AddMark(const ScrollMark& m) +// { +// _marks.insert(_marks.begin(), m); +// } + +// void TextBuffer::_trimMarksOutsideBuffer() +// { +// const til::CoordType height = _height; +// std::erase_if(_marks, [height](const auto& m) { +// return (m.start.y < 0) || (m.start.y >= height); +// }); +// } + +std::wstring TextBuffer::_commandForRow(const til::CoordType rowOffset, const til::CoordType bottomInclusive) const +{ + std::wstring commandBuilder; + MarkKind lastMarkKind = MarkKind::Prompt; + for (auto y = rowOffset; y <= bottomInclusive; y++) + { + // Now we need to iterate over text attributes. We need to find a + // segment of Prompt attributes, we'll skip those. Then there should be + // Command attributes. Collect up all of those, till we get to the next + // Output attribute. + + const auto& row = GetRowByOffset(y); + const auto runs = row.Attributes().runs(); + auto x = 0; + for (const auto& [attr, length] : runs) { - (*mark.outputEnd).y += delta; + const auto nextX = gsl::narrow_cast(x + length); + const auto markKind{ attr.GetMarkAttributes() }; + if (markKind != lastMarkKind) + { + if (lastMarkKind == MarkKind::Command) + { + // We've changed away from being in a command. We're done. + // Return what we've gotten so far. + return commandBuilder; + } + // Otherwise, we've changed from any state -> any state, and it doesn't really matter. + lastMarkKind = markKind; + } + + if (markKind == MarkKind::Command) + { + row.GetText(x, nextX); + } + // advance to next run of text + x = nextX; } + // we went over all the runs in this row, but we're not done yet. Keep iterating on the next row. } - _trimMarksOutsideBuffer(); + // Okay, we're at the bottom of the buffer? Yea, just return what we found. + return commandBuilder; } -// Method Description: -// - Add a mark to our list of marks, and treat it as the active "prompt". For -// the sake of shell integration, we need to know which mark represents the -// current prompt/command/output. Internally, we'll always treat the _last_ -// mark in the list as the current prompt. -// Arguments: -// - m: the mark to add. -void TextBuffer::StartPromptMark(const ScrollMark& m) +std::wstring TextBuffer::CurrentCommand() const { - _marks.push_back(m); -} -// Method Description: -// - Add a mark to our list of marks. Don't treat this as the active prompt. -// This should be used for marks created by the UI or from other user input. -// By inserting at the start of the list, we can separate out marks that were -// generated by client programs vs ones created by the user. -// Arguments: -// - m: the mark to add. -void TextBuffer::AddMark(const ScrollMark& m) -{ - _marks.insert(_marks.begin(), m); -} + // if (_marks.size() == 0) + // { + // return L""; + // } -void TextBuffer::_trimMarksOutsideBuffer() -{ - const til::CoordType height = _height; - std::erase_if(_marks, [height](const auto& m) { - return (m.start.y < 0) || (m.start.y >= height); - }); -} + // const auto& curr{ _marks.back() }; + // const auto& start{ curr.end }; + // const auto& end{ GetCursor().GetPosition() }; -std::wstring_view TextBuffer::CurrentCommand() const -{ - if (_marks.size() == 0) - { - return L""; - } + // const auto line = start.y; + // const auto& row = GetRowByOffset(line); + // return row.GetText(start.x, end.x); - const auto& curr{ _marks.back() }; - const auto& start{ curr.end }; - const auto& end{ GetCursor().GetPosition() }; - - const auto line = start.y; - const auto& row = GetRowByOffset(line); - return row.GetText(start.x, end.x); -} - -void TextBuffer::SetCurrentPromptEnd(const til::point pos) noexcept -{ - if (_marks.empty()) + auto promptY = GetCursor().GetPosition().y; + for (; promptY >= 0; promptY--) { - return; - } - auto& curr{ _marks.back() }; - curr.end = pos; -} -void TextBuffer::SetCurrentCommandEnd(const til::point pos) noexcept -{ - if (_marks.empty()) - { - return; + const auto& currRow = GetRowByOffset(promptY); + auto& rowPromptData = currRow.GetPromptData(); + if (!rowPromptData.has_value()) + { + // This row didn't start a prompt, don't even look here. + continue; + } + + // This row did start a prompt! Find the prompt that starts here. + // Presumably, no rows below us will have prompts, so pass in the last + // row with text as the bottom + return _commandForRow(promptY, _estimateOffsetOfLastCommittedRow()); } - auto& curr{ _marks.back() }; - curr.commandEnd = pos; + return L""; } -void TextBuffer::SetCurrentOutputEnd(const til::point pos, ::MarkCategory category) noexcept + +std::vector TextBuffer::Commands() const { - if (_marks.empty()) + std::vector commands{}; + const auto bottom = _estimateOffsetOfLastCommittedRow(); + auto lastPromptY = bottom; + for (auto promptY = bottom; promptY >= 0; promptY--) { - return; + const auto& currRow = GetRowByOffset(promptY); + auto& rowPromptData = currRow.GetPromptData(); + if (!rowPromptData.has_value()) + { + // This row didn't start a prompt, don't even look here. + continue; + } + + // This row did start a prompt! Find the prompt that starts here. + // Presumably, no rows below us will have prompts, so pass in the last + // row with text as the bottom + auto foundCommand = _commandForRow(promptY, lastPromptY); + if (!foundCommand.empty()) + { + commands.emplace_back(std::move(foundCommand)); + } + lastPromptY = promptY; + } + return std::move(commands); +} + +// 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) noexcept +// { +// if (_marks.empty()) +// { +// return; +// } +// auto& curr{ _marks.back() }; +// curr.commandEnd = pos; +// } +// void TextBuffer::SetCurrentOutputEnd(const til::point pos, ::MarkCategory category) noexcept +// { +// if (_marks.empty()) +// { +// return; +// } +// auto& curr{ _marks.back() }; +// curr.outputEnd = pos; +// curr.category = category; +// } + +void TextBuffer::StartPrompt() +{ + const auto currentRowOffset = GetCursor().GetPosition().y; + auto& currentRow = GetMutableRowByOffset(currentRowOffset); + currentRow.StartPrompt(); + // auto& rowPromptData = currRow.GetPromptData(); + // if (!rowPromptData.has_value()) + // { + // rowPromptData = MarkData{}; + // } +} +void TextBuffer::EndCommand(std::optional error) +{ + for (auto y = GetCursor().GetPosition().y; y >= 0; y--) + { + auto& currRow = GetMutableRowByOffset(y); + auto& rowPromptData = currRow.GetPromptData(); + if (rowPromptData.has_value()) + { + currRow.EndCommand(error.value_or(0u)); + return; + } } - auto& curr{ _marks.back() }; - curr.outputEnd = pos; - curr.category = category; } diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index 419e01471e5..c25677fabb6 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -66,7 +66,7 @@ namespace Microsoft::Console::Render class Renderer; } -enum class MarkCategory +enum class MarkCategory : uint8_t { Prompt = 0, Error = 1, @@ -330,16 +330,21 @@ class TextBuffer final std::vector SearchText(const std::wstring_view& needle, bool caseInsensitive) const; std::vector SearchText(const std::wstring_view& needle, bool caseInsensitive, til::CoordType rowBeg, til::CoordType rowEnd) const; - const std::vector& GetMarks() const noexcept; + // const std::vector& 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) noexcept; - void SetCurrentOutputEnd(const til::point pos, ::MarkCategory category) noexcept; - std::wstring_view CurrentCommand() const; + // 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) noexcept; + // void SetCurrentOutputEnd(const til::point pos, ::MarkCategory category) noexcept; + std::wstring CurrentCommand() const; + std::wstring _commandForRow(const til::CoordType rowOffset, const til::CoordType bottomInclusive) const; + std::vector Commands() const; + + void StartPrompt(); + void EndCommand(std::optional error); private: void _reserve(til::size screenBufferSize, const TextAttribute& defaultAttributes); @@ -364,7 +369,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(); + // void _trimMarksOutsideBuffer(); std::tuple _RowCopyHelper(const CopyRequest& req, const til::CoordType iRow, const ROW& row) const; static void _AppendRTFText(std::string& contentBuilder, const std::wstring_view& text); @@ -437,7 +442,7 @@ class TextBuffer final uint64_t _lastMutationId = 0; Cursor _cursor; - std::vector _marks; + // std::vector _marks; bool _isActiveBuffer = false; #ifdef UNIT_TESTING diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index dfe540936d0..4b4794f36a6 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -3695,7 +3695,7 @@ bool AdaptDispatch::DoITerm2Action(const std::wstring_view string) { // Flush the frame manually, to make sure marks end up on the right line, like the alt buffer sequence. _renderer.TriggerFlush(false); - return false; + // return false; } if constexpr (!Feature_ScrollbarMarks::IsEnabled()) @@ -3714,9 +3714,15 @@ bool AdaptDispatch::DoITerm2Action(const std::wstring_view string) if (action == L"SetMark") { - ScrollMark mark; - mark.category = MarkCategory::Prompt; - _api.MarkPrompt(mark); + // ScrollMark mark; + // mark.category = MarkCategory::Prompt; + // _api.MarkPrompt(mark); + + auto attr = _api.GetTextBuffer().GetCurrentAttributes(); + attr.SetMarkAttributes(MarkKind::Prompt); + _api.SetTextAttributes(attr); + _api.GetTextBuffer().StartPrompt(); + return true; } return false; @@ -3739,7 +3745,7 @@ bool AdaptDispatch::DoFinalTermAction(const std::wstring_view string) { // Flush the frame manually, to make sure marks end up on the right line, like the alt buffer sequence. _renderer.TriggerFlush(false); - return false; + // return false; } if constexpr (!Feature_ScrollbarMarks::IsEnabled()) @@ -3762,19 +3768,26 @@ bool AdaptDispatch::DoFinalTermAction(const std::wstring_view string) case L'A': // FTCS_PROMPT { // Simply just mark this line as a prompt line. - ScrollMark mark; - mark.category = MarkCategory::Prompt; - _api.MarkPrompt(mark); + auto attr = _api.GetTextBuffer().GetCurrentAttributes(); + attr.SetMarkAttributes(MarkKind::Prompt); + _api.SetTextAttributes(attr); + _api.GetTextBuffer().StartPrompt(); return true; } case L'B': // FTCS_COMMAND_START { - _api.MarkCommandStart(); + auto attr = _api.GetTextBuffer().GetCurrentAttributes(); + attr.SetMarkAttributes(MarkKind::Command); + _api.SetTextAttributes(attr); + _api.GetTextBuffer().StartPrompt(); return true; } case L'C': // FTCS_COMMAND_EXECUTED { - _api.MarkOutputStart(); + auto attr = _api.GetTextBuffer().GetCurrentAttributes(); + attr.SetMarkAttributes(MarkKind::Output); + _api.SetTextAttributes(attr); + _api.GetTextBuffer().StartPrompt(); return true; } case L'D': // FTCS_COMMAND_FINISHED @@ -3793,7 +3806,12 @@ bool AdaptDispatch::DoFinalTermAction(const std::wstring_view string) error = Utils::StringToUint(errorString, parsedError) ? parsedError : UINT_MAX; } - _api.MarkCommandFinish(error); + + auto attr = _api.GetTextBuffer().GetCurrentAttributes(); + attr.SetMarkAttributes(MarkKind::Output); + _api.SetTextAttributes(attr); + _api.GetTextBuffer().EndCommand(error); + // _api.MarkCommandFinish(error); return true; } default: From f88ee30a78c175c2a21ce9975ea219de247f6d17 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 22 Mar 2024 13:48:33 -0500 Subject: [PATCH 02/30] tests are starting to feel good --- src/buffer/out/Row.cpp | 2 +- src/buffer/out/textBuffer.cpp | 2 +- src/host/ut_host/ScreenBufferTests.cpp | 38 ++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/buffer/out/Row.cpp b/src/buffer/out/Row.cpp index 7f1f3e4e9f6..344d130d2f9 100644 --- a/src/buffer/out/Row.cpp +++ b/src/buffer/out/Row.cpp @@ -1191,7 +1191,7 @@ void ROW::StartPrompt() noexcept { if (!_promptData.has_value()) { - _promptData = {}; + _promptData = MarkData{}; } } void ROW::EndCommand(uint32_t exitCode) noexcept diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 9c704609e07..6466e9ce4e0 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -3000,7 +3000,7 @@ std::wstring TextBuffer::_commandForRow(const til::CoordType rowOffset, const ti if (markKind == MarkKind::Command) { - row.GetText(x, nextX); + commandBuilder += row.GetText(x, nextX); } // advance to next run of text x = nextX; diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index 98a6f09b2e5..f6d219ae7f5 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -264,6 +264,8 @@ class ScreenBufferTests TEST_METHOD(DelayedWrapReset); TEST_METHOD(EraseColorMode); + + TEST_METHOD(SimpleMarkCommand); }; void ScreenBufferTests::SingleAlternateBufferCreationTest() @@ -8404,3 +8406,39 @@ void ScreenBufferTests::EraseColorMode() VERIFY_ARE_EQUAL(expectedEraseAttr, cellData->TextAttr()); VERIFY_ARE_EQUAL(L" ", cellData->Chars()); } + +void ScreenBufferTests::SimpleMarkCommand() +{ + auto& g = ServiceLocator::LocateGlobals(); + auto& gci = g.getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& tbi = si.GetTextBuffer(); + auto& stateMachine = si.GetStateMachine(); + + // Process the opening osc 8 sequence with no custom id + stateMachine.ProcessString(L"Zero\n"); + + { + const auto currentRowOffset = tbi.GetCursor().GetPosition().y; + auto& currentRow = tbi.GetRowByOffset(currentRowOffset); + + stateMachine.ProcessString(L"\x1b]133;A\x1b\\A Prompt\x1b]133;B\x1b\\my_command\x1b]133;C\x1b\\\n"); + + VERIFY_IS_TRUE(currentRow.GetPromptData().has_value()); + } + + stateMachine.ProcessString(L"Two\n"); + // DebugBreak(); + VERIFY_ARE_EQUAL(tbi.CurrentCommand(), L"my_command"); + + stateMachine.ProcessString(L"\x1b]133;D\x1b\\\x1b]133;A\x1b\\B Prompt\x1b]133;B\x1b\\"); + + VERIFY_ARE_EQUAL(tbi.CurrentCommand(), L""); + + stateMachine.ProcessString(L"some of a command"); + VERIFY_ARE_EQUAL(tbi.CurrentCommand(), L"some of a command"); + stateMachine.ProcessString(L"\x1b[31m"); + stateMachine.ProcessString(L" & more of a command"); + + VERIFY_ARE_EQUAL(tbi.CurrentCommand(), L"some of a command & more of a command"); +} From 0bfe6032798a387a2ddf7d75c6438a6d0fb71e37 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 22 Mar 2024 14:09:00 -0500 Subject: [PATCH 03/30] this is what you call test driven development --- src/buffer/out/textBuffer.cpp | 1 + src/host/ut_host/ScreenBufferTests.cpp | 73 ++++++++++++++++++++++++-- src/terminal/adapter/adaptDispatch.cpp | 4 +- 3 files changed, 71 insertions(+), 7 deletions(-) diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 6466e9ce4e0..6ccbdee50a3 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -3070,6 +3070,7 @@ std::vector TextBuffer::Commands() const } lastPromptY = promptY; } + std::reverse(commands.begin(), commands.end()); return std::move(commands); } diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index f6d219ae7f5..b6cd179314e 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -266,6 +266,7 @@ class ScreenBufferTests TEST_METHOD(EraseColorMode); TEST_METHOD(SimpleMarkCommand); + TEST_METHOD(SimpleWrappedCommand); }; void ScreenBufferTests::SingleAlternateBufferCreationTest() @@ -8407,6 +8408,11 @@ void ScreenBufferTests::EraseColorMode() VERIFY_ARE_EQUAL(L" ", cellData->Chars()); } +#define FTCS_A L"\x1b]133;A\x1b\\" +#define FTCS_B L"\x1b]133;B\x1b\\" +#define FTCS_C L"\x1b]133;C\x1b\\" +#define FTCS_D L"\x1b]133;D\x1b\\" + void ScreenBufferTests::SimpleMarkCommand() { auto& g = ServiceLocator::LocateGlobals(); @@ -8422,23 +8428,80 @@ void ScreenBufferTests::SimpleMarkCommand() const auto currentRowOffset = tbi.GetCursor().GetPosition().y; auto& currentRow = tbi.GetRowByOffset(currentRowOffset); - stateMachine.ProcessString(L"\x1b]133;A\x1b\\A Prompt\x1b]133;B\x1b\\my_command\x1b]133;C\x1b\\\n"); + stateMachine.ProcessString(FTCS_A L"A Prompt" FTCS_B L"my_command" FTCS_C L"\n"); VERIFY_IS_TRUE(currentRow.GetPromptData().has_value()); } stateMachine.ProcessString(L"Two\n"); - // DebugBreak(); - VERIFY_ARE_EQUAL(tbi.CurrentCommand(), L"my_command"); + VERIFY_ARE_EQUAL(L"my_command", tbi.CurrentCommand()); - stateMachine.ProcessString(L"\x1b]133;D\x1b\\\x1b]133;A\x1b\\B Prompt\x1b]133;B\x1b\\"); + stateMachine.ProcessString(FTCS_D FTCS_A L"B Prompt" FTCS_B); VERIFY_ARE_EQUAL(tbi.CurrentCommand(), L""); stateMachine.ProcessString(L"some of a command"); VERIFY_ARE_EQUAL(tbi.CurrentCommand(), L"some of a command"); + // Now add some color in the middle of the command: stateMachine.ProcessString(L"\x1b[31m"); stateMachine.ProcessString(L" & more of a command"); - VERIFY_ARE_EQUAL(tbi.CurrentCommand(), L"some of a command & more of a command"); + VERIFY_ARE_EQUAL(L"some of a command & more of a command", tbi.CurrentCommand()); + + std::vector expectedCommands{ L"my_command", + L"some of a command & more of a command" }; + VERIFY_ARE_EQUAL(expectedCommands, tbi.Commands()); +} + +void ScreenBufferTests::SimpleWrappedCommand() +{ + auto& g = ServiceLocator::LocateGlobals(); + auto& gci = g.getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& tbi = si.GetTextBuffer(); + auto& stateMachine = si.GetStateMachine(); + + // Process the opening osc 8 sequence with no custom id + stateMachine.ProcessString(L"Zero\n"); + + const auto oneHundredZeros = std::wstring(100, L'0'); + { + const auto originalRowOffset = tbi.GetCursor().GetPosition().y; + auto& originalRow = tbi.GetRowByOffset(originalRowOffset); + stateMachine.ProcessString(FTCS_A L"A Prompt" FTCS_B); + + // This command is literally 100 '0' characters, so that we _know_ we wrapped. + stateMachine.ProcessString(oneHundredZeros); + + const auto secondRowOffset = tbi.GetCursor().GetPosition().y; + VERIFY_ARE_NOT_EQUAL(originalRowOffset, secondRowOffset); + auto& secondRow = tbi.GetRowByOffset(secondRowOffset); + + VERIFY_IS_TRUE(originalRow.GetPromptData().has_value()); + VERIFY_IS_FALSE(secondRow.GetPromptData().has_value()); + + stateMachine.ProcessString(FTCS_C L"\n"); + + VERIFY_IS_TRUE(originalRow.GetPromptData().has_value()); + VERIFY_IS_FALSE(secondRow.GetPromptData().has_value()); + } + + stateMachine.ProcessString(L"Two\n"); + VERIFY_ARE_EQUAL(oneHundredZeros, tbi.CurrentCommand()); + + stateMachine.ProcessString(FTCS_D FTCS_A L"B Prompt" FTCS_B); + + VERIFY_ARE_EQUAL(tbi.CurrentCommand(), L""); + + stateMachine.ProcessString(L"some of a command"); + VERIFY_ARE_EQUAL(tbi.CurrentCommand(), L"some of a command"); + // Now add some color in the middle of the command: + stateMachine.ProcessString(L"\x1b[31m"); + stateMachine.ProcessString(L" & more of a command"); + + VERIFY_ARE_EQUAL(L"some of a command & more of a command", tbi.CurrentCommand()); + + std::vector expectedCommands{ oneHundredZeros, + L"some of a command & more of a command" }; + VERIFY_ARE_EQUAL(expectedCommands, tbi.Commands()); } diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index 4b4794f36a6..d9315c0d934 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -3779,7 +3779,7 @@ bool AdaptDispatch::DoFinalTermAction(const std::wstring_view string) auto attr = _api.GetTextBuffer().GetCurrentAttributes(); attr.SetMarkAttributes(MarkKind::Command); _api.SetTextAttributes(attr); - _api.GetTextBuffer().StartPrompt(); + return true; } case L'C': // FTCS_COMMAND_EXECUTED @@ -3787,7 +3787,7 @@ bool AdaptDispatch::DoFinalTermAction(const std::wstring_view string) auto attr = _api.GetTextBuffer().GetCurrentAttributes(); attr.SetMarkAttributes(MarkKind::Output); _api.SetTextAttributes(attr); - _api.GetTextBuffer().StartPrompt(); + return true; } case L'D': // FTCS_COMMAND_FINISHED From ed79c947dfef62f5c9b40fd6e99c00785b42ebae Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 22 Mar 2024 14:25:13 -0500 Subject: [PATCH 04/30] well, Terminal.cpp compiles. That's something --- src/buffer/out/Row.cpp | 15 ++ src/buffer/out/Row.hpp | 14 ++ src/buffer/out/textBuffer.hpp | 8 - src/cascadia/TerminalCore/Terminal.cpp | 149 ++++++------- src/cascadia/TerminalCore/Terminal.hpp | 11 +- src/cascadia/TerminalCore/TerminalApi.cpp | 252 +++++++++++----------- 6 files changed, 233 insertions(+), 216 deletions(-) diff --git a/src/buffer/out/Row.cpp b/src/buffer/out/Row.cpp index 344d130d2f9..57ed9f33c44 100644 --- a/src/buffer/out/Row.cpp +++ b/src/buffer/out/Row.cpp @@ -1189,6 +1189,21 @@ const std::optional& ROW::GetPromptData() const noexcept void ROW::StartPrompt() noexcept { + // TODO! resurrect somehow + + // static bool logged = false; + // if (!logged) + // { + // TraceLoggingWrite( + // g_hCTerminalCoreProvider, + // "ShellIntegrationMarkAdded", + // TraceLoggingDescription("A mark was added via VT at least once"), + // TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), + // TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); + + // logged = true; + // } + if (!_promptData.has_value()) { _promptData = MarkData{}; diff --git a/src/buffer/out/Row.hpp b/src/buffer/out/Row.hpp index b36ff6b6264..bf8edf92623 100644 --- a/src/buffer/out/Row.hpp +++ b/src/buffer/out/Row.hpp @@ -87,6 +87,14 @@ struct CharToColumnMapper til::CoordType _currentColumn; }; +enum class MarkCategory : uint8_t +{ + Prompt = 0, + Error = 1, + Warning = 2, + Success = 3, + Info = 4 +}; struct MarkData { // Scrollbar marks may have been given a color, or not. @@ -94,6 +102,12 @@ struct MarkData // Prompts without an exit code haven't had a matching FTCS CommandEnd // called yet. Any value other than 0 is an error. std::optional exitCode; + + // TODO! MarkCategory used to be an element of a mark. Maybe we want to keep + // that? + + // Future consideration: stick the literal command as a string on here, if + // we were given it with the 633;E sequence. }; class ROW final diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index c25677fabb6..6012c0b09e4 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -66,14 +66,6 @@ namespace Microsoft::Console::Render class Renderer; } -enum class MarkCategory : uint8_t -{ - Prompt = 0, - Error = 1, - Warning = 2, - Success = 3, - Info = 4 -}; struct ScrollMark { std::optional color; diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 6eabcdab072..f02fcaa1c4f 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -734,6 +734,8 @@ TerminalInput::OutputType Terminal::SendCharEvent(const wchar_t ch, const WORD s // // Fortunately, MarkOutputStart will do all this logic for us! MarkOutputStart(); + + // TODO! ^^^^^^^^^^ that's not on Terminal anymore. That's a text attr now. } const auto keyDown = SynthesizeKeyEvent(true, 1, vkey, scanCode, ch, states.Value()); @@ -1435,36 +1437,39 @@ PointTree Terminal::_getPatterns(til::CoordType beg, til::CoordType end) const } // NOTE: This is the version of AddMark that comes from the UI. The VT api call into this too. -void Terminal::AddMark(const ScrollMark& mark, - const til::point& start, - const til::point& end, - const bool fromUi) +void Terminal::AddMark(const ScrollMark& /*mark*/, + const til::point& /*start*/, + const til::point& /*end*/, + const bool /*fromUi*/) { - if (_inAltBuffer()) - { - return; - } + // TODO! + // Basically, entirely kill this? - ScrollMark m = mark; - m.start = start; - m.end = end; + // if (_inAltBuffer()) + // { + // return; + // } - // If the mark came from the user adding a mark via the UI, don't make it the active prompt mark. - if (fromUi) - { - _activeBuffer().AddMark(m); - } - else - { - _activeBuffer().StartPromptMark(m); - } + // ScrollMark m = mark; + // m.start = start; + // m.end = end; - // Tell the control that the scrollbar has somehow changed. Used as a - // workaround to force the control to redraw any scrollbar marks - _NotifyScrollEvent(); + // // If the mark came from the user adding a mark via the UI, don't make it the active prompt mark. + // if (fromUi) + // { + // _activeBuffer().AddMark(m); + // } + // else + // { + // _activeBuffer().StartPromptMark(m); + // } - // DON'T set _currentPrompt. The VT impl will do that for you. We don't want - // UI-driven marks to set that. + // // Tell the control that the scrollbar has somehow changed. Used as a + // // workaround to force the control to redraw any scrollbar marks + // _NotifyScrollEvent(); + + // // DON'T set _currentPrompt. The VT impl will do that for you. We don't want + // // UI-driven marks to set that. } void Terminal::ClearMark() @@ -1495,56 +1500,56 @@ void Terminal::ClearAllMarks() _NotifyScrollEvent(); } -const std::vector& 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. - return _activeBuffer().GetMarks(); -} - -til::color Terminal::GetColorForMark(const ScrollMark& mark) const -{ - if (mark.color.has_value()) - { - return *mark.color; - } - - const auto& renderSettings = GetRenderSettings(); - - switch (mark.category) - { - case MarkCategory::Prompt: - { - return renderSettings.GetColorAlias(ColorAlias::DefaultForeground); - } - case MarkCategory::Error: - { - return renderSettings.GetColorTableEntry(TextColor::BRIGHT_RED); - } - case MarkCategory::Warning: - { - return renderSettings.GetColorTableEntry(TextColor::BRIGHT_YELLOW); - } - case MarkCategory::Success: - { - return renderSettings.GetColorTableEntry(TextColor::BRIGHT_GREEN); - } - default: - case MarkCategory::Info: - { - return renderSettings.GetColorAlias(ColorAlias::DefaultForeground); - } - } -} +// const std::vector& 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. +// return _activeBuffer().GetMarks(); +// } + +// til::color Terminal::GetColorForMark(const ScrollMark& mark) const +// { +// if (mark.color.has_value()) +// { +// return *mark.color; +// } + +// const auto& renderSettings = GetRenderSettings(); + +// switch (mark.category) +// { +// case MarkCategory::Prompt: +// { +// return renderSettings.GetColorAlias(ColorAlias::DefaultForeground); +// } +// case MarkCategory::Error: +// { +// return renderSettings.GetColorTableEntry(TextColor::BRIGHT_RED); +// } +// case MarkCategory::Warning: +// { +// return renderSettings.GetColorTableEntry(TextColor::BRIGHT_YELLOW); +// } +// case MarkCategory::Success: +// { +// return renderSettings.GetColorTableEntry(TextColor::BRIGHT_GREEN); +// } +// default: +// case MarkCategory::Info: +// { +// return renderSettings.GetColorAlias(ColorAlias::DefaultForeground); +// } +// } +// } std::wstring_view Terminal::CurrentCommand() const { - if (_currentPromptState != PromptState::Command) - { - return L""; - } + // if (_currentPromptState != PromptState::Command) + // { + // return L""; + // } return _activeBuffer().CurrentCommand(); } diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index 80c5e3cc905..cf4927fa342 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -167,7 +167,7 @@ class Microsoft::Terminal::Core::Terminal final : void ClearMark(); void ClearAllMarks(); - til::color GetColorForMark(const ScrollMark& mark) const; + // til::color GetColorForMark(const ScrollMark& mark) const; #pragma region ITerminalInput // These methods are defined in Terminal.cpp @@ -434,15 +434,6 @@ class Microsoft::Terminal::Core::Terminal final : }; std::optional _lastKeyEventCodes; - enum class PromptState : uint32_t - { - None = 0, - Prompt, - Command, - Output, - }; - PromptState _currentPromptState{ PromptState::None }; - static WORD _ScanCodeFromVirtualKey(const WORD vkey) noexcept; static WORD _VirtualKeyFromScanCode(const WORD scanCode) noexcept; static WORD _VirtualKeyFromCharacter(const wchar_t ch) noexcept; diff --git a/src/cascadia/TerminalCore/TerminalApi.cpp b/src/cascadia/TerminalCore/TerminalApi.cpp index 9a5ea2221ac..277ade7394b 100644 --- a/src/cascadia/TerminalCore/TerminalApi.cpp +++ b/src/cascadia/TerminalCore/TerminalApi.cpp @@ -301,137 +301,137 @@ void Terminal::UseMainScreenBuffer() } // NOTE: This is the version of AddMark that comes from VT -void Terminal::MarkPrompt(const ScrollMark& mark) +void Terminal::MarkPrompt(const ScrollMark& /*mark*/) { - _assertLocked(); - - static bool logged = false; - if (!logged) - { - TraceLoggingWrite( - g_hCTerminalCoreProvider, - "ShellIntegrationMarkAdded", - TraceLoggingDescription("A mark was added via VT at least once"), - TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), - TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); - - logged = true; - } - - const til::point cursorPos{ _activeBuffer().GetCursor().GetPosition() }; - AddMark(mark, cursorPos, cursorPos, false); - - if (mark.category == MarkCategory::Prompt) - { - _currentPromptState = PromptState::Prompt; - } + // _assertLocked(); + + // static bool logged = false; + // if (!logged) + // { + // TraceLoggingWrite( + // g_hCTerminalCoreProvider, + // "ShellIntegrationMarkAdded", + // TraceLoggingDescription("A mark was added via VT at least once"), + // TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), + // TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); + + // logged = true; + // } + + // const til::point cursorPos{ _activeBuffer().GetCursor().GetPosition() }; + // AddMark(mark, cursorPos, cursorPos, false); + + // if (mark.category == MarkCategory::Prompt) + // { + // _currentPromptState = PromptState::Prompt; + // } } void Terminal::MarkCommandStart() { - _assertLocked(); - - const til::point cursorPos{ _activeBuffer().GetCursor().GetPosition() }; - - if ((_currentPromptState == PromptState::Prompt) && - (_activeBuffer().GetMarks().size() > 0)) - { - // We were in the right state, and there's a previous mark to work - // with. - // - //We can just do the work below safely. - } - else if (_currentPromptState == PromptState::Command) - { - // We're already in the command state. We don't want to end the current - // mark. We don't want to make a new one. We want to just leave the - // current command going. - return; - } - else - { - // If there was no last mark, or we're in a weird state, - // then abandon the current state, and just insert a new Prompt mark that - // start's & ends here, and got to State::Command. - - ScrollMark mark; - mark.category = MarkCategory::Prompt; - AddMark(mark, cursorPos, cursorPos, false); - } - _activeBuffer().SetCurrentPromptEnd(cursorPos); - _currentPromptState = PromptState::Command; + // _assertLocked(); + + // const til::point cursorPos{ _activeBuffer().GetCursor().GetPosition() }; + + // if ((_currentPromptState == PromptState::Prompt) && + // (_activeBuffer().GetMarks().size() > 0)) + // { + // // We were in the right state, and there's a previous mark to work + // // with. + // // + // //We can just do the work below safely. + // } + // else if (_currentPromptState == PromptState::Command) + // { + // // We're already in the command state. We don't want to end the current + // // mark. We don't want to make a new one. We want to just leave the + // // current command going. + // return; + // } + // else + // { + // // If there was no last mark, or we're in a weird state, + // // then abandon the current state, and just insert a new Prompt mark that + // // start's & ends here, and got to State::Command. + + // ScrollMark mark; + // mark.category = MarkCategory::Prompt; + // AddMark(mark, cursorPos, cursorPos, false); + // } + // _activeBuffer().SetCurrentPromptEnd(cursorPos); + // _currentPromptState = PromptState::Command; } void Terminal::MarkOutputStart() { - _assertLocked(); - - const til::point cursorPos{ _activeBuffer().GetCursor().GetPosition() }; - - if ((_currentPromptState == PromptState::Command) && - (_activeBuffer().GetMarks().size() > 0)) - { - // We were in the right state, and there's a previous mark to work - // with. - // - //We can just do the work below safely. - } - else if (_currentPromptState == PromptState::Output) - { - // We're already in the output state. We don't want to end the current - // mark. We don't want to make a new one. We want to just leave the - // current output going. - return; - } - else - { - // If there was no last mark, or we're in a weird state, - // then abandon the current state, and just insert a new Prompt mark that - // start's & ends here, and the command ends here, and go to State::Output. - - ScrollMark mark; - mark.category = MarkCategory::Prompt; - AddMark(mark, cursorPos, cursorPos, false); - } - _activeBuffer().SetCurrentCommandEnd(cursorPos); - _currentPromptState = PromptState::Output; + // _assertLocked(); + + // const til::point cursorPos{ _activeBuffer().GetCursor().GetPosition() }; + + // if ((_currentPromptState == PromptState::Command) && + // (_activeBuffer().GetMarks().size() > 0)) + // { + // // We were in the right state, and there's a previous mark to work + // // with. + // // + // //We can just do the work below safely. + // } + // else if (_currentPromptState == PromptState::Output) + // { + // // We're already in the output state. We don't want to end the current + // // mark. We don't want to make a new one. We want to just leave the + // // current output going. + // return; + // } + // else + // { + // // If there was no last mark, or we're in a weird state, + // // then abandon the current state, and just insert a new Prompt mark that + // // start's & ends here, and the command ends here, and go to State::Output. + + // ScrollMark mark; + // mark.category = MarkCategory::Prompt; + // AddMark(mark, cursorPos, cursorPos, false); + // } + // _activeBuffer().SetCurrentCommandEnd(cursorPos); + // _currentPromptState = PromptState::Output; } -void Terminal::MarkCommandFinish(std::optional error) +void Terminal::MarkCommandFinish(std::optional /*error*/) { - _assertLocked(); - - const til::point cursorPos{ _activeBuffer().GetCursor().GetPosition() }; - auto category = MarkCategory::Prompt; - if (error.has_value()) - { - category = *error == 0u ? - MarkCategory::Success : - MarkCategory::Error; - } - - if ((_currentPromptState == PromptState::Output) && - (_activeBuffer().GetMarks().size() > 0)) - { - // We were in the right state, and there's a previous mark to work - // with. - // - //We can just do the work below safely. - } - else - { - // If there was no last mark, or we're in a weird state, then abandon - // the current state, and just insert a new Prompt mark that start's & - // ends here, and the command ends here, AND the output ends here. and - // go to State::Output. - - ScrollMark mark; - mark.category = MarkCategory::Prompt; - AddMark(mark, cursorPos, cursorPos, false); - _activeBuffer().SetCurrentCommandEnd(cursorPos); - } - _activeBuffer().SetCurrentOutputEnd(cursorPos, category); - _currentPromptState = PromptState::None; + // _assertLocked(); + + // const til::point cursorPos{ _activeBuffer().GetCursor().GetPosition() }; + // auto category = MarkCategory::Prompt; + // if (error.has_value()) + // { + // category = *error == 0u ? + // MarkCategory::Success : + // MarkCategory::Error; + // } + + // if ((_currentPromptState == PromptState::Output) && + // (_activeBuffer().GetMarks().size() > 0)) + // { + // // We were in the right state, and there's a previous mark to work + // // with. + // // + // //We can just do the work below safely. + // } + // else + // { + // // If there was no last mark, or we're in a weird state, then abandon + // // the current state, and just insert a new Prompt mark that start's & + // // ends here, and the command ends here, AND the output ends here. and + // // go to State::Output. + + // ScrollMark mark; + // mark.category = MarkCategory::Prompt; + // AddMark(mark, cursorPos, cursorPos, false); + // _activeBuffer().SetCurrentCommandEnd(cursorPos); + // } + // _activeBuffer().SetCurrentOutputEnd(cursorPos, category); + // _currentPromptState = PromptState::None; } // Method Description: @@ -497,16 +497,16 @@ void Terminal::NotifyBufferRotation(const int delta) // manually erase our pattern intervals since the locations have changed now _patternIntervalTree = {}; - auto& marks{ _activeBuffer().GetMarks() }; - const auto hasScrollMarks = marks.size() > 0; - if (hasScrollMarks) - { - _activeBuffer().ScrollMarks(-delta); - } + // auto& marks{ _activeBuffer().GetMarks() }; + // const auto hasScrollMarks = marks.size() > 0; + // if (hasScrollMarks) + // { + // _activeBuffer().ScrollMarks(-delta); + // } const auto oldScrollOffset = _scrollOffset; _PreserveUserScrollOffset(delta); - if (_scrollOffset != oldScrollOffset || hasScrollMarks || AlwaysNotifyOnBufferRotation()) + if (_scrollOffset != oldScrollOffset || AlwaysNotifyOnBufferRotation()) { _NotifyScrollEvent(); } From ec1eb90dd9e61a75704935e98c2ecf40fe9c3b82 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 22 Mar 2024 15:26:06 -0500 Subject: [PATCH 05/30] This sorta compiles, but with huge gaps in features. I'm totally just going to hand spans back out of the text buffer. genius. --- src/buffer/out/Row.cpp | 7 +- src/buffer/out/Row.hpp | 8 +- src/buffer/out/textBuffer.cpp | 15 + src/buffer/out/textBuffer.hpp | 54 +-- src/cascadia/TerminalControl/ControlCore.cpp | 372 ++++++++++--------- src/cascadia/TerminalControl/ICoreState.idl | 7 +- src/cascadia/TerminalControl/TermControl.cpp | 2 +- src/cascadia/TerminalCore/Terminal.cpp | 81 ++-- src/cascadia/TerminalCore/Terminal.hpp | 4 +- 9 files changed, 284 insertions(+), 266 deletions(-) diff --git a/src/buffer/out/Row.cpp b/src/buffer/out/Row.cpp index 57ed9f33c44..f32a60573ed 100644 --- a/src/buffer/out/Row.cpp +++ b/src/buffer/out/Row.cpp @@ -1206,7 +1206,11 @@ void ROW::StartPrompt() noexcept if (!_promptData.has_value()) { - _promptData = MarkData{}; + _promptData = MarkData{ + .color = std::nullopt, + .exitCode = std::nullopt, + .category = MarkCategory::Prompt + }; } } void ROW::EndCommand(uint32_t exitCode) noexcept @@ -1214,5 +1218,6 @@ void ROW::EndCommand(uint32_t exitCode) noexcept if (_promptData.has_value()) { _promptData->exitCode = exitCode; + _promptData->category = exitCode == 0 ? MarkCategory::Success : MarkCategory::Error; } } diff --git a/src/buffer/out/Row.hpp b/src/buffer/out/Row.hpp index bf8edf92623..f5978761d12 100644 --- a/src/buffer/out/Row.hpp +++ b/src/buffer/out/Row.hpp @@ -89,11 +89,11 @@ struct CharToColumnMapper enum class MarkCategory : uint8_t { - Prompt = 0, + Default = 0, Error = 1, Warning = 2, Success = 3, - Info = 4 + Prompt = 4 }; struct MarkData { @@ -102,9 +102,7 @@ struct MarkData // Prompts without an exit code haven't had a matching FTCS CommandEnd // called yet. Any value other than 0 is an error. std::optional exitCode; - - // TODO! MarkCategory used to be an element of a mark. Maybe we want to keep - // that? + MarkCategory category{ MarkCategory::Default }; // Future consideration: stick the literal command as a string on here, if // we were given it with the 633;E sequence. diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 6ccbdee50a3..a1327c16e3e 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -2887,6 +2887,21 @@ std::vector TextBuffer::SearchText(const std::wstring_view& nee return results; } +std::vector TextBuffer::GetMarks() const noexcept +{ + std::vector marks; + const auto bottom = _estimateOffsetOfLastCommittedRow(); + for (auto y = 0; y <= bottom; y++) + { + const auto& row = GetRowByOffset(y); + const auto& data{ row.GetPromptData() }; + if (data.has_value()) + { + marks.emplace_back(y, *data); + } + } + return std::move(marks); +} // const std::vector& TextBuffer::GetMarks() const noexcept // { diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index 6012c0b09e4..d8ea6bde428 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -66,31 +66,37 @@ namespace Microsoft::Console::Render class Renderer; } +// struct ScrollMark +// { +// std::optional color; +// til::point start; +// til::point end; // exclusive +// std::optional commandEnd; +// std::optional 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 GetExtent() const +// { +// til::point realEnd{ til::coalesce_value(outputEnd, commandEnd, end) }; +// return std::make_pair(til::point{ start }, realEnd); +// } +// }; + struct ScrollMark { - std::optional color; - til::point start; - til::point end; // exclusive - std::optional commandEnd; - std::optional 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 GetExtent() const - { - til::point realEnd{ til::coalesce_value(outputEnd, commandEnd, end) }; - return std::make_pair(til::point{ start }, realEnd); - } + til::CoordType row{ 0 }; + MarkData data; }; class TextBuffer final @@ -322,7 +328,7 @@ class TextBuffer final std::vector SearchText(const std::wstring_view& needle, bool caseInsensitive) const; std::vector SearchText(const std::wstring_view& needle, bool caseInsensitive, til::CoordType rowBeg, til::CoordType rowEnd) const; - // const std::vector& GetMarks() const noexcept; + std::vector GetMarks() const noexcept; void ClearMarksInRange(const til::point start, const til::point end); void ClearAllMarks() noexcept; // void ScrollMarks(const int delta); diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 4027e914d8f..98f1905d4a0 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -1884,68 +1884,70 @@ namespace winrt::Microsoft::Terminal::Control::implementation // approach, but it's good enough to bring value to 90% of use cases. const auto cursorPos{ _terminal->GetCursorPosition() }; - // Does the current buffer line have a mark on it? - const auto& marks{ _terminal->GetScrollMarks() }; - if (!marks.empty()) - { - const auto& last{ marks.back() }; - const auto [start, end] = last.GetExtent(); - const auto lastNonSpace = _terminal->GetTextBuffer().GetLastNonSpaceCharacter(); - - // If the user clicked off to the right side of the prompt, we - // want to send keystrokes to the last character in the prompt +1. - // - // We don't want to send too many here. In CMD, if the user's - // last command is longer than what they've currently typed, and - // they press right arrow at the end of the prompt, COOKED_READ - // will fill in characters from the previous command. - // - // By only sending keypresses to the end of the command + 1, we - // should leave the cursor at the very end of the prompt, - // without adding any characters from a previous command. - auto clampedClick = terminalPosition; - if (terminalPosition > lastNonSpace) - { - clampedClick = lastNonSpace + til::point{ 1, 0 }; - _terminal->GetTextBuffer().GetSize().Clamp(clampedClick); - } - - if (clampedClick >= end) - { - // Get the distance between the cursor and the click, in cells. - const auto bufferSize = _terminal->GetTextBuffer().GetSize(); - - // First, make sure to iterate from the first point to the - // second. The user may have clicked _earlier_ in the - // buffer! - auto goRight = clampedClick > cursorPos; - const auto startPoint = goRight ? cursorPos : clampedClick; - const auto endPoint = goRight ? clampedClick : cursorPos; - - const auto delta = _terminal->GetTextBuffer().GetCellDistance(startPoint, endPoint); - const WORD key = goRight ? VK_RIGHT : VK_LEFT; - - std::wstring buffer; - const auto append = [&](TerminalInput::OutputType&& out) { - if (out) - { - buffer.append(std::move(*out)); - } - }; - - // Send an up and a down once per cell. This won't - // accurately handle wide characters, or continuation - // prompts, or cases where a single escape character in the - // command (e.g. ^[) takes up two cells. - for (size_t i = 0u; i < delta; i++) - { - append(_terminal->SendKeyEvent(key, 0, {}, true)); - append(_terminal->SendKeyEvent(key, 0, {}, false)); - } - - _sendInputToConnection(buffer); - } - } + // TODO! yea this all needs re-writing vvvvv + + // // Does the current buffer line have a mark on it? + // const auto& marks{ _terminal->GetScrollMarks() }; + // if (!marks.empty()) + // { + // const auto& last{ marks.back() }; + // const auto [start, end] = last.GetExtent(); + // const auto lastNonSpace = _terminal->GetTextBuffer().GetLastNonSpaceCharacter(); + + // // If the user clicked off to the right side of the prompt, we + // // want to send keystrokes to the last character in the prompt +1. + // // + // // We don't want to send too many here. In CMD, if the user's + // // last command is longer than what they've currently typed, and + // // they press right arrow at the end of the prompt, COOKED_READ + // // will fill in characters from the previous command. + // // + // // By only sending keypresses to the end of the command + 1, we + // // should leave the cursor at the very end of the prompt, + // // without adding any characters from a previous command. + // auto clampedClick = terminalPosition; + // if (terminalPosition > lastNonSpace) + // { + // clampedClick = lastNonSpace + til::point{ 1, 0 }; + // _terminal->GetTextBuffer().GetSize().Clamp(clampedClick); + // } + + // if (clampedClick >= end) + // { + // // Get the distance between the cursor and the click, in cells. + // const auto bufferSize = _terminal->GetTextBuffer().GetSize(); + + // // First, make sure to iterate from the first point to the + // // second. The user may have clicked _earlier_ in the + // // buffer! + // auto goRight = clampedClick > cursorPos; + // const auto startPoint = goRight ? cursorPos : clampedClick; + // const auto endPoint = goRight ? clampedClick : cursorPos; + + // const auto delta = _terminal->GetTextBuffer().GetCellDistance(startPoint, endPoint); + // const WORD key = goRight ? VK_RIGHT : VK_LEFT; + + // std::wstring buffer; + // const auto append = [&](TerminalInput::OutputType&& out) { + // if (out) + // { + // buffer.append(std::move(*out)); + // } + // }; + + // // Send an up and a down once per cell. This won't + // // accurately handle wide characters, or continuation + // // prompts, or cases where a single escape character in the + // // command (e.g. ^[) takes up two cells. + // for (size_t i = 0u; i < delta; i++) + // { + // append(_terminal->SendKeyEvent(key, 0, {}, true)); + // append(_terminal->SendKeyEvent(key, 0, {}, false)); + // } + + // _sendInputToConnection(buffer); + // } + // } } _updateSelectionUI(); } @@ -2090,32 +2092,18 @@ namespace winrt::Microsoft::Terminal::Control::implementation const auto& textBuffer = _terminal->GetTextBuffer(); std::vector commands; - - for (const auto& mark : _terminal->GetScrollMarks()) + const auto bufferCommands{ textBuffer.Commands() }; + for (const auto& commandInBuffer : bufferCommands) { - // The command text is between the `end` (which denotes the end of - // the prompt) and the `commandEnd`. - bool markHasCommand = mark.commandEnd.has_value() && - mark.commandEnd != mark.end; - if (!markHasCommand) - { - continue; - } - - // Get the text of the command - const auto line = mark.end.y; - const auto& row = textBuffer.GetRowByOffset(line); - const auto commandText = row.GetText(mark.end.x, mark.commandEnd->x); - - // Trim off trailing spaces. - const auto strEnd = commandText.find_last_not_of(UNICODE_SPACE); + const auto strEnd = commandInBuffer.find_last_not_of(UNICODE_SPACE); if (strEnd != std::string::npos) { - const auto trimmed = commandText.substr(0, strEnd + 1); + const auto trimmed = commandInBuffer.substr(0, strEnd + 1); commands.push_back(winrt::hstring{ trimmed }); } } + auto context = winrt::make_self(std::move(commands)); context->CurrentCommandline(winrt::hstring{ _terminal->CurrentCommand() }); @@ -2338,37 +2326,38 @@ namespace winrt::Microsoft::Terminal::Control::implementation for (const auto& mark : internalMarks) { v.emplace_back( - mark.start.to_core_point(), - mark.end.to_core_point(), + mark.row, OptionalFromColor(_terminal->GetColorForMark(mark))); } return winrt::single_threaded_vector(std::move(v)); } - void ControlCore::AddMark(const Control::ScrollMark& mark) + void ControlCore::AddMark(const Control::ScrollMark& /*mark*/) { - const auto lock = _terminal->LockForReading(); - ::ScrollMark m{}; + // TODO! Re-implement adding a mark in the UI - if (mark.Color.HasValue) - { - m.color = til::color{ mark.Color.Color }; - } + // const auto lock = _terminal->LockForReading(); + // ::ScrollMark m{}; - if (_terminal->IsSelectionActive()) - { - m.start = til::point{ _terminal->GetSelectionAnchor() }; - m.end = til::point{ _terminal->GetSelectionEnd() }; - } - else - { - m.start = m.end = til::point{ _terminal->GetTextBuffer().GetCursor().GetPosition() }; - } + // if (mark.Color.HasValue) + // { + // m.color = til::color{ mark.Color.Color }; + // } + + // if (_terminal->IsSelectionActive()) + // { + // m.start = til::point{ _terminal->GetSelectionAnchor() }; + // m.end = til::point{ _terminal->GetSelectionEnd() }; + // } + // else + // { + // m.start = m.end = til::point{ _terminal->GetTextBuffer().GetCursor().GetPosition() }; + // } - // The version of this that only accepts a ScrollMark will automatically - // set the start & end to the cursor position. - _terminal->AddMark(m, m.start, m.end, true); + // // The version of this that only accepts a ScrollMark will automatically + // // set the start & end to the cursor position. + // _terminal->AddMark(m, m.start, m.end, true); } void ControlCore::ClearMark() @@ -2398,7 +2387,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation int highest = currentOffset; for (const auto& mark : marks) { - const auto newY = mark.start.y; + const auto newY = mark.row; if (newY > highest) { tgt = mark; @@ -2412,7 +2401,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation int lowest = currentOffset; for (const auto& mark : marks) { - const auto newY = mark.start.y; + const auto newY = mark.row; if (newY < lowest) { tgt = mark; @@ -2426,7 +2415,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation int minDistance = INT_MAX; for (const auto& mark : marks) { - const auto delta = mark.start.y - currentOffset; + const auto delta = mark.row - currentOffset; if (delta > 0 && delta < minDistance) { tgt = mark; @@ -2441,7 +2430,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation int minDistance = INT_MAX; for (const auto& mark : marks) { - const auto delta = currentOffset - mark.start.y; + const auto delta = currentOffset - mark.row; if (delta > 0 && delta < minDistance) { tgt = mark; @@ -2459,8 +2448,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation // then raise a _terminalScrollPositionChanged to inform the control to update the scrollbar. if (tgt.has_value()) { - UserScrollViewport(tgt->start.y); - _terminalScrollPositionChanged(tgt->start.y, viewHeight, bufferSize); + UserScrollViewport(tgt->row); + _terminalScrollPositionChanged(tgt->row, viewHeight, bufferSize); } else { @@ -2512,30 +2501,32 @@ namespace winrt::Microsoft::Terminal::Control::implementation return; } - static constexpr til::point worst{ til::CoordTypeMax, til::CoordTypeMax }; - til::point bestDistance{ worst }; + // TODO!!!! - for (const auto& m : marks) - { - if (!m.HasCommand()) - { - continue; - } + // static constexpr til::point worst{ til::CoordTypeMax, til::CoordTypeMax }; + // til::point bestDistance{ worst }; - const auto distance = goUp ? start - m.end : m.end - start; - if ((distance > til::point{ 0, 0 }) && distance < bestDistance) - { - nearest = m; - bestDistance = distance; - } - } + // for (const auto& m : marks) + // { + // if (!m.HasCommand()) + // { + // continue; + // } - if (nearest.has_value()) - { - const auto start = nearest->end; - auto end = *nearest->commandEnd; - _selectSpan(til::point_span{ start, end }); - } + // const auto distance = goUp ? start - m.end : m.end - start; + // if ((distance > til::point{ 0, 0 }) && distance < bestDistance) + // { + // nearest = m; + // bestDistance = distance; + // } + // } + + // if (nearest.has_value()) + // { + // const auto start = nearest->end; + // auto end = *nearest->commandEnd; + // _selectSpan(til::point_span{ start, end }); + // } } void ControlCore::SelectOutput(const bool goUp) @@ -2547,31 +2538,33 @@ namespace winrt::Microsoft::Terminal::Control::implementation std::optional<::ScrollMark> nearest{ std::nullopt }; const auto& marks{ _terminal->GetScrollMarks() }; - - static constexpr til::point worst{ til::CoordTypeMax, til::CoordTypeMax }; - til::point bestDistance{ worst }; - - for (const auto& m : marks) - { - if (!m.HasOutput()) - { - continue; - } - - const auto distance = goUp ? start - *m.commandEnd : *m.commandEnd - start; - if ((distance > til::point{ 0, 0 }) && distance < bestDistance) - { - nearest = m; - bestDistance = distance; - } - } - - if (nearest.has_value()) - { - const auto start = *nearest->commandEnd; - auto end = *nearest->outputEnd; - _selectSpan(til::point_span{ start, end }); - } + marks; + // TODO!!!! + + // static constexpr til::point worst{ til::CoordTypeMax, til::CoordTypeMax }; + // til::point bestDistance{ worst }; + + // for (const auto& m : marks) + // { + // if (!m.HasOutput()) + // { + // continue; + // } + + // const auto distance = goUp ? start - *m.commandEnd : *m.commandEnd - start; + // if ((distance > til::point{ 0, 0 }) && distance < bestDistance) + // { + // nearest = m; + // bestDistance = distance; + // } + // } + + // if (nearest.has_value()) + // { + // const auto start = *nearest->commandEnd; + // auto end = *nearest->outputEnd; + // _selectSpan(til::point_span{ start, end }); + // } } void ControlCore::ColorSelection(const Control::SelectionColor& fg, const Control::SelectionColor& bg, Core::MatchMode matchMode) @@ -2621,7 +2614,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation } void ControlCore::_contextMenuSelectMark( - const til::point& pos, + const til::point& /*pos*/, bool (*filter)(const ::ScrollMark&), til::point_span (*getSpan)(const ::ScrollMark&)) { @@ -2633,6 +2626,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation return; } const auto& marks{ _terminal->GetScrollMarks() }; + + // TODO! this'll all need to be redone. + for (auto&& m : marks) { // If the caller gave us a way to filter marks, check that now. @@ -2641,32 +2637,32 @@ namespace winrt::Microsoft::Terminal::Control::implementation { continue; } - // If they clicked _anywhere_ in the mark... - const auto [markStart, markEnd] = m.GetExtent(); - if (markStart <= pos && - markEnd >= pos) - { - // ... select the part of the mark the caller told us about. - _selectSpan(getSpan(m)); - // And quick bail - return; - } + // // If they clicked _anywhere_ in the mark... + // const auto [markStart, markEnd] = m.GetExtent(); + // if (markStart <= pos && + // markEnd >= pos) + // { + // // ... select the part of the mark the caller told us about. + // _selectSpan(getSpan(m)); + // // And quick bail + // return; + // } } } void ControlCore::ContextMenuSelectCommand() { - _contextMenuSelectMark( - _contextMenuBufferPosition, - [](const ::ScrollMark& m) -> bool { return !m.HasCommand(); }, - [](const ::ScrollMark& m) { return til::point_span{ m.end, *m.commandEnd }; }); + // _contextMenuSelectMark( + // _contextMenuBufferPosition, + // [](const ::ScrollMark& m) -> bool { return !m.HasCommand(); }, + // [](const ::ScrollMark& m) { return til::point_span{ m.end, *m.commandEnd }; }); } void ControlCore::ContextMenuSelectOutput() { - _contextMenuSelectMark( - _contextMenuBufferPosition, - [](const ::ScrollMark& m) -> bool { return !m.HasOutput(); }, - [](const ::ScrollMark& m) { return til::point_span{ *m.commandEnd, *m.outputEnd }; }); + // _contextMenuSelectMark( + // _contextMenuBufferPosition, + // [](const ::ScrollMark& m) -> bool { return !m.HasOutput(); }, + // [](const ::ScrollMark& m) { return til::point_span{ *m.commandEnd, *m.outputEnd }; }); } bool ControlCore::_clickedOnMark( @@ -2691,12 +2687,14 @@ namespace winrt::Microsoft::Terminal::Control::implementation { continue; } - const auto [start, end] = m.GetExtent(); - if (start <= pos && - end >= pos) - { - return true; - } + // TODO! redo all of me + + // const auto [start, end] = m.GetExtent(); + // if (start <= pos && + // end >= pos) + // { + // return true; + // } } // Didn't click on a mark with a command - don't show. @@ -2709,17 +2707,21 @@ namespace winrt::Microsoft::Terminal::Control::implementation // * Otherwise yea, show it. bool ControlCore::ShouldShowSelectCommand() { - // Relies on the anchor set in AnchorContextMenu - return _clickedOnMark(_contextMenuBufferPosition, - [](const ::ScrollMark& m) -> bool { return !m.HasCommand(); }); + return false; // TODO! + + // // Relies on the anchor set in AnchorContextMenu + // return _clickedOnMark(_contextMenuBufferPosition, + // [](const ::ScrollMark& m) -> bool { return !m.HasCommand(); }); } // Method Description: // * Same as ShouldShowSelectCommand, but with the mark needing output bool ControlCore::ShouldShowSelectOutput() { - // Relies on the anchor set in AnchorContextMenu - return _clickedOnMark(_contextMenuBufferPosition, - [](const ::ScrollMark& m) -> bool { return !m.HasOutput(); }); + return false; // TODO! + + // // Relies on the anchor set in AnchorContextMenu + // return _clickedOnMark(_contextMenuBufferPosition, + // [](const ::ScrollMark& m) -> bool { return !m.HasOutput(); }); } } diff --git a/src/cascadia/TerminalControl/ICoreState.idl b/src/cascadia/TerminalControl/ICoreState.idl index 0073b70788a..a3333473c28 100644 --- a/src/cascadia/TerminalControl/ICoreState.idl +++ b/src/cascadia/TerminalControl/ICoreState.idl @@ -13,11 +13,8 @@ namespace Microsoft.Terminal.Control struct ScrollMark { - // There are other members of DispatchTypes::ScrollMark, but these are - // all we need to expose up and set downwards currently. Additional - // members can be bubbled as necessary. - Microsoft.Terminal.Core.Point Start; - Microsoft.Terminal.Core.Point End; // exclusive + // Additional members can be bubbled as necessary. + Int32 Row; Microsoft.Terminal.Core.OptionalColor Color; }; diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index de8ec920d3e..681b5bf391d 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -368,7 +368,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { for (const auto& m : marks) { - const auto row = m.Start.Y; + const auto row = m.Row; const til::color color{ m.Color.Color }; const auto base = dataAt(row); drawPip(base, color); diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index f02fcaa1c4f..0d3888e6bce 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -1500,49 +1500,44 @@ void Terminal::ClearAllMarks() _NotifyScrollEvent(); } -// const std::vector& 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. -// return _activeBuffer().GetMarks(); -// } - -// til::color Terminal::GetColorForMark(const ScrollMark& mark) const -// { -// if (mark.color.has_value()) -// { -// return *mark.color; -// } - -// const auto& renderSettings = GetRenderSettings(); - -// switch (mark.category) -// { -// case MarkCategory::Prompt: -// { -// return renderSettings.GetColorAlias(ColorAlias::DefaultForeground); -// } -// case MarkCategory::Error: -// { -// return renderSettings.GetColorTableEntry(TextColor::BRIGHT_RED); -// } -// case MarkCategory::Warning: -// { -// return renderSettings.GetColorTableEntry(TextColor::BRIGHT_YELLOW); -// } -// case MarkCategory::Success: -// { -// return renderSettings.GetColorTableEntry(TextColor::BRIGHT_GREEN); -// } -// default: -// case MarkCategory::Info: -// { -// return renderSettings.GetColorAlias(ColorAlias::DefaultForeground); -// } -// } -// } +std::vector Terminal::GetScrollMarks() const noexcept +{ + // We want to return _no_ marks when we're in the alt buffer, to effectively + // hide them. + return _inAltBuffer() ? std::vector{} : std::move(_activeBuffer().GetMarks()); +} + +til::color Terminal::GetColorForMark(const ScrollMark& mark) const +{ + if (mark.data.color.has_value()) + { + return *mark.data.color; + } + + const auto& renderSettings = GetRenderSettings(); + + switch (mark.data.category) + { + case MarkCategory::Error: + { + return renderSettings.GetColorTableEntry(TextColor::BRIGHT_RED); + } + case MarkCategory::Warning: + { + return renderSettings.GetColorTableEntry(TextColor::BRIGHT_YELLOW); + } + case MarkCategory::Success: + { + return renderSettings.GetColorTableEntry(TextColor::BRIGHT_GREEN); + } + case MarkCategory::Prompt: + case MarkCategory::Default: + default: + { + return renderSettings.GetColorAlias(ColorAlias::DefaultForeground); + } + } +} std::wstring_view Terminal::CurrentCommand() const { diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index cf4927fa342..37d20114991 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -117,7 +117,7 @@ class Microsoft::Terminal::Core::Terminal final : RenderSettings& GetRenderSettings() noexcept; const RenderSettings& GetRenderSettings() const noexcept; - const std::vector& GetScrollMarks() const noexcept; + std::vector GetScrollMarks() const noexcept; void AddMark(const ScrollMark& mark, const til::point& start, const til::point& end, @@ -167,7 +167,7 @@ class Microsoft::Terminal::Core::Terminal final : void ClearMark(); void ClearAllMarks(); - // til::color GetColorForMark(const ScrollMark& mark) const; + til::color GetColorForMark(const ScrollMark& mark) const; #pragma region ITerminalInput // These methods are defined in Terminal.cpp From ef86ff0e3c9584bd26f335e1bd4de1806f055445 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 22 Mar 2024 17:00:16 -0500 Subject: [PATCH 06/30] HOLY SHIT THE TESTS ALL PASS --- src/buffer/out/textBuffer.cpp | 134 +++++++++++++- src/buffer/out/textBuffer.hpp | 59 ++++--- src/cascadia/TerminalControl/ControlCore.cpp | 173 +++++++++---------- src/host/ut_host/ScreenBufferTests.cpp | 77 +++++++++ 4 files changed, 317 insertions(+), 126 deletions(-) diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index a1327c16e3e..5aecd0392cc 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -2889,17 +2889,43 @@ std::vector TextBuffer::SearchText(const std::wstring_view& nee } std::vector TextBuffer::GetMarks() const noexcept { - std::vector marks; + // std::vector marks; + // const auto bottom = _estimateOffsetOfLastCommittedRow(); + // for (auto y = 0; y <= bottom; y++) + // { + // const auto& row = GetRowByOffset(y); + // const auto& data{ row.GetPromptData() }; + // if (data.has_value()) + // { + // marks.emplace_back(y, *data); + // } + // } + // return std::move(marks); + + std::vector marks{}; const auto bottom = _estimateOffsetOfLastCommittedRow(); - for (auto y = 0; y <= bottom; y++) + auto lastPromptY = bottom; + for (auto promptY = bottom; promptY >= 0; promptY--) { - const auto& row = GetRowByOffset(y); - const auto& data{ row.GetPromptData() }; - if (data.has_value()) + const auto& currRow = GetRowByOffset(promptY); + auto& rowPromptData = currRow.GetPromptData(); + if (!rowPromptData.has_value()) { - marks.emplace_back(y, *data); + // This row didn't start a prompt, don't even look here. + continue; } + + // This row did start a prompt! Find the prompt that starts here. + // Presumably, no rows below us will have prompts, so pass in the last + // row with text as the bottom + marks.push_back(_scrollMarkForRow(promptY, lastPromptY)); + // if (!foundCommand.empty()) + // { + // commands.emplace_back(std::move(foundCommand)); + // } + lastPromptY = promptY; } + std::reverse(marks.begin(), marks.end()); return std::move(marks); } @@ -2982,6 +3008,102 @@ void TextBuffer::ClearAllMarks() noexcept // return (m.start.y < 0) || (m.start.y >= height); // }); // } +ScrollMark TextBuffer::_scrollMarkForRow(const til::CoordType rowOffset, const til::CoordType bottomInclusive) const +{ + const auto& startRow = GetRowByOffset(rowOffset); + const auto& rowPromptData = startRow.GetPromptData(); + assert(rowPromptData.has_value()); + + ScrollMark mark{ + .data = *rowPromptData, + }; + + bool startedPrompt = false; + // bool foundEnd = false; + bool startedCommand = false; + bool startedOutput = false; + MarkKind lastMarkKind = MarkKind::Output; + + const auto endThisMark = [&](auto x, auto y) { + if (!startedCommand) + { + mark.end = til::point{ x, y }; + } + else if (!startedOutput) // startedCommand = true + { + mark.commandEnd = til::point{ x, y }; + } + else if (startedOutput) + { + mark.outputEnd = til::point{ x, y }; + } + }; + auto x = 0; + auto y = rowOffset; + + for (; y <= bottomInclusive; y++) + { + // Now we need to iterate over text attributes. We need to find a + // segment of Prompt attributes, we'll skip those. Then there should be + // Command attributes. Collect up all of those, till we get to the next + // Output attribute. + + const auto& row = GetRowByOffset(y); + const auto runs = row.Attributes().runs(); + x = 0; + for (const auto& [attr, length] : runs) + { + const auto nextX = gsl::narrow_cast(x + length); + const auto markKind{ attr.GetMarkAttributes() }; + if (markKind != lastMarkKind) + { + if (markKind == MarkKind::Prompt) + { + if (startedPrompt) + { + // we got a _new_ prompt. bail out. + endThisMark(x, y); + break; + } + else + { + // We entered the first prompt here + startedPrompt = true; + mark.start = til::point{ x, y }; + } + } + else if (markKind == MarkKind::Command && startedPrompt) + { + // mark.end = til::point{x, y}; + // foundEnd = true; + endThisMark(x, y); + startedCommand = true; + } + else if (markKind == MarkKind::Output && startedPrompt) + { + endThisMark(x, y); + startedOutput = true; + // if (!foundEnd) + // { + // mark.end = til::point{x, y}; + // foundEnd = true; + // } + // mark.commandEnd = til::point{x, y}; + } + // Otherwise, we've changed from any state -> any state, and it doesn't really matter. + lastMarkKind = markKind; + } + + // advance to next run of text + x = nextX; + } + // we went over all the runs in this row, but we're not done yet. Keep iterating on the next row. + } + + // Okay, we're at the bottom of the buffer? Yea, just return what we found. + // endThisMark(x, y); + return mark; +} std::wstring TextBuffer::_commandForRow(const til::CoordType rowOffset, const til::CoordType bottomInclusive) const { diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index d8ea6bde428..b22cb34db6e 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -66,39 +66,41 @@ namespace Microsoft::Console::Render class Renderer; } -// struct ScrollMark -// { -// std::optional color; -// til::point start; -// til::point end; // exclusive -// std::optional commandEnd; -// std::optional 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 GetExtent() const -// { -// til::point realEnd{ til::coalesce_value(outputEnd, commandEnd, end) }; -// return std::make_pair(til::point{ start }, realEnd); -// } -// }; - struct ScrollMark { - til::CoordType row{ 0 }; MarkData data; + + // std::optional color; + til::point start; + til::point end; // exclusive + std::optional commandEnd; + std::optional 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 GetExtent() const + { + til::point realEnd{ til::coalesce_value(outputEnd, commandEnd, end) }; + return std::make_pair(til::point{ start }, realEnd); + } }; +// struct ScrollMark +// { +// til::CoordType row{ 0 }; +// MarkData data; +// }; + class TextBuffer final { public: @@ -339,6 +341,7 @@ class TextBuffer final // void SetCurrentOutputEnd(const til::point pos, ::MarkCategory category) noexcept; std::wstring CurrentCommand() const; std::wstring _commandForRow(const til::CoordType rowOffset, const til::CoordType bottomInclusive) const; + ScrollMark _scrollMarkForRow(const til::CoordType rowOffset, const til::CoordType bottomInclusive) const; std::vector Commands() const; void StartPrompt(); diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 98f1905d4a0..ae8a0d2c83a 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -2326,7 +2326,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation for (const auto& mark : internalMarks) { v.emplace_back( - mark.row, + mark.start.y, OptionalFromColor(_terminal->GetColorForMark(mark))); } @@ -2387,7 +2387,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation int highest = currentOffset; for (const auto& mark : marks) { - const auto newY = mark.row; + const auto newY = mark.start.y; if (newY > highest) { tgt = mark; @@ -2401,7 +2401,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation int lowest = currentOffset; for (const auto& mark : marks) { - const auto newY = mark.row; + const auto newY = mark.start.y; if (newY < lowest) { tgt = mark; @@ -2415,7 +2415,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation int minDistance = INT_MAX; for (const auto& mark : marks) { - const auto delta = mark.row - currentOffset; + const auto delta = mark.start.y - currentOffset; if (delta > 0 && delta < minDistance) { tgt = mark; @@ -2430,7 +2430,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation int minDistance = INT_MAX; for (const auto& mark : marks) { - const auto delta = currentOffset - mark.row; + const auto delta = currentOffset - mark.start.y; if (delta > 0 && delta < minDistance) { tgt = mark; @@ -2448,8 +2448,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation // then raise a _terminalScrollPositionChanged to inform the control to update the scrollbar. if (tgt.has_value()) { - UserScrollViewport(tgt->row); - _terminalScrollPositionChanged(tgt->row, viewHeight, bufferSize); + UserScrollViewport(tgt->start.y); + _terminalScrollPositionChanged(tgt->start.y, viewHeight, bufferSize); } else { @@ -2501,32 +2501,30 @@ namespace winrt::Microsoft::Terminal::Control::implementation return; } - // TODO!!!! + static constexpr til::point worst{ til::CoordTypeMax, til::CoordTypeMax }; + til::point bestDistance{ worst }; - // static constexpr til::point worst{ til::CoordTypeMax, til::CoordTypeMax }; - // til::point bestDistance{ worst }; + for (const auto& m : marks) + { + if (!m.HasCommand()) + { + continue; + } - // for (const auto& m : marks) - // { - // if (!m.HasCommand()) - // { - // continue; - // } - - // const auto distance = goUp ? start - m.end : m.end - start; - // if ((distance > til::point{ 0, 0 }) && distance < bestDistance) - // { - // nearest = m; - // bestDistance = distance; - // } - // } + const auto distance = goUp ? start - m.end : m.end - start; + if ((distance > til::point{ 0, 0 }) && distance < bestDistance) + { + nearest = m; + bestDistance = distance; + } + } - // if (nearest.has_value()) - // { - // const auto start = nearest->end; - // auto end = *nearest->commandEnd; - // _selectSpan(til::point_span{ start, end }); - // } + if (nearest.has_value()) + { + const auto start = nearest->end; + auto end = *nearest->commandEnd; + _selectSpan(til::point_span{ start, end }); + } } void ControlCore::SelectOutput(const bool goUp) @@ -2538,33 +2536,31 @@ namespace winrt::Microsoft::Terminal::Control::implementation std::optional<::ScrollMark> nearest{ std::nullopt }; const auto& marks{ _terminal->GetScrollMarks() }; - marks; - // TODO!!!! - // static constexpr til::point worst{ til::CoordTypeMax, til::CoordTypeMax }; - // til::point bestDistance{ worst }; + static constexpr til::point worst{ til::CoordTypeMax, til::CoordTypeMax }; + til::point bestDistance{ worst }; - // for (const auto& m : marks) - // { - // if (!m.HasOutput()) - // { - // continue; - // } - - // const auto distance = goUp ? start - *m.commandEnd : *m.commandEnd - start; - // if ((distance > til::point{ 0, 0 }) && distance < bestDistance) - // { - // nearest = m; - // bestDistance = distance; - // } - // } + for (const auto& m : marks) + { + if (!m.HasOutput()) + { + continue; + } - // if (nearest.has_value()) - // { - // const auto start = *nearest->commandEnd; - // auto end = *nearest->outputEnd; - // _selectSpan(til::point_span{ start, end }); - // } + const auto distance = goUp ? start - *m.commandEnd : *m.commandEnd - start; + if ((distance > til::point{ 0, 0 }) && distance < bestDistance) + { + nearest = m; + bestDistance = distance; + } + } + + if (nearest.has_value()) + { + const auto start = *nearest->commandEnd; + auto end = *nearest->outputEnd; + _selectSpan(til::point_span{ start, end }); + } } void ControlCore::ColorSelection(const Control::SelectionColor& fg, const Control::SelectionColor& bg, Core::MatchMode matchMode) @@ -2614,7 +2610,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation } void ControlCore::_contextMenuSelectMark( - const til::point& /*pos*/, + const til::point& pos, bool (*filter)(const ::ScrollMark&), til::point_span (*getSpan)(const ::ScrollMark&)) { @@ -2627,8 +2623,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation } const auto& marks{ _terminal->GetScrollMarks() }; - // TODO! this'll all need to be redone. - for (auto&& m : marks) { // If the caller gave us a way to filter marks, check that now. @@ -2637,32 +2631,32 @@ namespace winrt::Microsoft::Terminal::Control::implementation { continue; } - // // If they clicked _anywhere_ in the mark... - // const auto [markStart, markEnd] = m.GetExtent(); - // if (markStart <= pos && - // markEnd >= pos) - // { - // // ... select the part of the mark the caller told us about. - // _selectSpan(getSpan(m)); - // // And quick bail - // return; - // } + // If they clicked _anywhere_ in the mark... + const auto [markStart, markEnd] = m.GetExtent(); + if (markStart <= pos && + markEnd >= pos) + { + // ... select the part of the mark the caller told us about. + _selectSpan(getSpan(m)); + // And quick bail + return; + } } } void ControlCore::ContextMenuSelectCommand() { - // _contextMenuSelectMark( - // _contextMenuBufferPosition, - // [](const ::ScrollMark& m) -> bool { return !m.HasCommand(); }, - // [](const ::ScrollMark& m) { return til::point_span{ m.end, *m.commandEnd }; }); + _contextMenuSelectMark( + _contextMenuBufferPosition, + [](const ::ScrollMark& m) -> bool { return !m.HasCommand(); }, + [](const ::ScrollMark& m) { return til::point_span{ m.end, *m.commandEnd }; }); } void ControlCore::ContextMenuSelectOutput() { - // _contextMenuSelectMark( - // _contextMenuBufferPosition, - // [](const ::ScrollMark& m) -> bool { return !m.HasOutput(); }, - // [](const ::ScrollMark& m) { return til::point_span{ *m.commandEnd, *m.outputEnd }; }); + _contextMenuSelectMark( + _contextMenuBufferPosition, + [](const ::ScrollMark& m) -> bool { return !m.HasOutput(); }, + [](const ::ScrollMark& m) { return til::point_span{ *m.commandEnd, *m.outputEnd }; }); } bool ControlCore::_clickedOnMark( @@ -2687,14 +2681,13 @@ namespace winrt::Microsoft::Terminal::Control::implementation { continue; } - // TODO! redo all of me - // const auto [start, end] = m.GetExtent(); - // if (start <= pos && - // end >= pos) - // { - // return true; - // } + const auto [start, end] = m.GetExtent(); + if (start <= pos && + end >= pos) + { + return true; + } } // Didn't click on a mark with a command - don't show. @@ -2707,21 +2700,17 @@ namespace winrt::Microsoft::Terminal::Control::implementation // * Otherwise yea, show it. bool ControlCore::ShouldShowSelectCommand() { - return false; // TODO! - - // // Relies on the anchor set in AnchorContextMenu - // return _clickedOnMark(_contextMenuBufferPosition, - // [](const ::ScrollMark& m) -> bool { return !m.HasCommand(); }); + // Relies on the anchor set in AnchorContextMenu + return _clickedOnMark(_contextMenuBufferPosition, + [](const ::ScrollMark& m) -> bool { return !m.HasCommand(); }); } // Method Description: // * Same as ShouldShowSelectCommand, but with the mark needing output bool ControlCore::ShouldShowSelectOutput() { - return false; // TODO! - - // // Relies on the anchor set in AnchorContextMenu - // return _clickedOnMark(_contextMenuBufferPosition, - // [](const ::ScrollMark& m) -> bool { return !m.HasOutput(); }); + // Relies on the anchor set in AnchorContextMenu + return _clickedOnMark(_contextMenuBufferPosition, + [](const ::ScrollMark& m) -> bool { return !m.HasOutput(); }); } } diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index b6cd179314e..71308bb283b 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -267,6 +267,7 @@ class ScreenBufferTests TEST_METHOD(SimpleMarkCommand); TEST_METHOD(SimpleWrappedCommand); + TEST_METHOD(ComplicatedPromptRegions); }; void ScreenBufferTests::SingleAlternateBufferCreationTest() @@ -8505,3 +8506,79 @@ void ScreenBufferTests::SimpleWrappedCommand() L"some of a command & more of a command" }; VERIFY_ARE_EQUAL(expectedCommands, tbi.Commands()); } + +void _writePrompt(StateMachine& stateMachine, const auto& path) +{ + stateMachine.ProcessString(L"\x1b]133;D\x7"); + stateMachine.ProcessString(L"\x1b]133;A\x7"); + stateMachine.ProcessString(L"\x1b]9;9;"); + stateMachine.ProcessString(path); + stateMachine.ProcessString(L"\x7"); + stateMachine.ProcessString(L"PWSH "); + stateMachine.ProcessString(path); + stateMachine.ProcessString(L"> "); + stateMachine.ProcessString(L"\x1b]133;B\x7"); +} + +void ScreenBufferTests::ComplicatedPromptRegions() +{ + auto& g = ServiceLocator::LocateGlobals(); + auto& gci = g.getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& tbi = si.GetTextBuffer(); + auto& stateMachine = si.GetStateMachine(); + + // A prompt looks like: + // `PWSH C:\Windows> ` + // + // which is 17 characters for C:\Windows + + _writePrompt(stateMachine, L"C:\\Windows"); + stateMachine.ProcessString(L"Foo-bar"); + stateMachine.ProcessString(L"\x1b]133;C\x7"); + stateMachine.ProcessString(L"\r\n"); + stateMachine.ProcessString(L"This is some text \r\n"); // y=1 + stateMachine.ProcessString(L"with varying amounts \r\n"); // y=2 + stateMachine.ProcessString(L"of whitespace \r\n"); // y=3 + + _writePrompt(stateMachine, L"C:\\Windows"); // y=4 + + Log::Comment(L"Check the buffer contents"); + const auto& cursor = tbi.GetCursor(); + + { + const til::point expectedCursor{ 17, 4 }; + VERIFY_ARE_EQUAL(expectedCursor, cursor.GetPosition()); + } + const WEX::TestExecution::DisableVerifyExceptions disableExceptionsScope; + + const auto& row0 = tbi.GetRowByOffset(0); + const auto& row4 = tbi.GetRowByOffset(4); + VERIFY_IS_TRUE(row0.GetPromptData().has_value()); + VERIFY_IS_TRUE(row4.GetPromptData().has_value()); + + const auto marks = tbi.GetMarks(); + VERIFY_ARE_EQUAL(2u, marks.size()); + + { + auto& mark = marks[0]; + const til::point expectedStart{ 0, 0 }; + const til::point expectedEnd{ 17, 0 }; + const til::point expectedOutputStart{ 24, 0 }; // `Foo-Bar` is 7 characters + const til::point expectedOutputEnd{ 0, 4 }; + VERIFY_ARE_EQUAL(expectedStart, mark.start); + VERIFY_ARE_EQUAL(expectedEnd, mark.end); + + VERIFY_ARE_EQUAL(expectedOutputStart, *mark.commandEnd); + VERIFY_ARE_EQUAL(expectedOutputEnd, *mark.outputEnd); + } + { + auto& mark = marks[1]; + const til::point expectedStart{ 0, 4 }; + const til::point expectedEnd{ 17, 4 }; + VERIFY_ARE_EQUAL(expectedStart, mark.start); + VERIFY_ARE_EQUAL(expectedEnd, mark.end); + VERIFY_IS_FALSE(mark.commandEnd.has_value()); + VERIFY_IS_FALSE(mark.outputEnd.has_value()); + } +} From 65f7b345402ba487fb99c9b1928cf641009fb660 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 25 Mar 2024 05:59:10 -0500 Subject: [PATCH 07/30] roundtrip tests, but these still don't actually fix the issues in Terminal --- src/buffer/out/TextAttribute.hpp | 13 +-- src/buffer/out/textBuffer.cpp | 7 +- .../ConptyRoundtripTests.cpp | 101 ++++++++++++++++++ src/terminal/adapter/adaptDispatch.cpp | 36 ++++--- 4 files changed, 134 insertions(+), 23 deletions(-) diff --git a/src/buffer/out/TextAttribute.hpp b/src/buffer/out/TextAttribute.hpp index 9b4a75d95a1..26d70729e42 100644 --- a/src/buffer/out/TextAttribute.hpp +++ b/src/buffer/out/TextAttribute.hpp @@ -42,9 +42,10 @@ enum class UnderlineStyle // doesn't satisfy std::has_unique_object_representations_v enum class MarkKind : uint16_t { - Output = 0, + None = 0, Prompt = 1, - Command = 2 + Command = 2, + Output = 3, }; class TextAttribute final @@ -56,7 +57,7 @@ class TextAttribute final _background{}, _hyperlinkId{ 0 }, _underlineColor{}, - _markKind{ MarkKind::Output } + _markKind{ MarkKind::None } { } @@ -66,7 +67,7 @@ class TextAttribute final _background{ gsl::at(s_legacyBackgroundColorMap, (wLegacyAttr & BG_ATTRS) >> 4) }, _hyperlinkId{ 0 }, _underlineColor{}, - _markKind{ MarkKind::Output } + _markKind{ MarkKind::None } { } @@ -78,7 +79,7 @@ class TextAttribute final _background{ rgbBackground }, _hyperlinkId{ 0 }, _underlineColor{ rgbUnderline }, - _markKind{ MarkKind::Output } + _markKind{ MarkKind::None } { } @@ -88,7 +89,7 @@ class TextAttribute final _background{ background }, _hyperlinkId{ hyperlinkId }, _underlineColor{ underlineColor }, - _markKind{ MarkKind::Output } + _markKind{ MarkKind::None } { } diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 5aecd0392cc..298dbda4871 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -3029,7 +3029,7 @@ ScrollMark TextBuffer::_scrollMarkForRow(const til::CoordType rowOffset, const t { mark.end = til::point{ x, y }; } - else if (!startedOutput) // startedCommand = true + else if (!startedOutput /*|| !mark.commandEnd.has_value()*/) // startedCommand = true { mark.commandEnd = til::point{ x, y }; } @@ -3055,7 +3055,8 @@ ScrollMark TextBuffer::_scrollMarkForRow(const til::CoordType rowOffset, const t { const auto nextX = gsl::narrow_cast(x + length); const auto markKind{ attr.GetMarkAttributes() }; - if (markKind != lastMarkKind) + if (/*markKind != MarkKind::None &&*/ + markKind != lastMarkKind) { if (markKind == MarkKind::Prompt) { @@ -3079,7 +3080,7 @@ ScrollMark TextBuffer::_scrollMarkForRow(const til::CoordType rowOffset, const t endThisMark(x, y); startedCommand = true; } - else if (markKind == MarkKind::Output && startedPrompt) + else if ((markKind == MarkKind::Output || markKind == MarkKind::None) && startedPrompt) { endThisMark(x, y); startedOutput = true; diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index f7948e5c59d..e42853c18eb 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -233,6 +233,8 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final TEST_METHOD(TestNoExtendedAttrsOptimization); TEST_METHOD(TestNoBackgroundAttrsOptimization); + TEST_METHOD(ComplicatedPromptRegions); + private: bool _writeCallback(const char* const pch, const size_t cch); void _flushFirstFrame(); @@ -4315,3 +4317,102 @@ void ConptyRoundtripTests::TestNoBackgroundAttrsOptimization() Log::Comment(L"========== Check terminal buffer =========="); verifyBuffer(*termTb); } + +void _writePrompt(StateMachine& stateMachine, const auto& path) +{ + stateMachine.ProcessString(L"\x1b]133;D\x7"); + stateMachine.ProcessString(L"\x1b]133;A\x7"); + stateMachine.ProcessString(L"\x1b]9;9;"); + stateMachine.ProcessString(path); + stateMachine.ProcessString(L"\x7"); + stateMachine.ProcessString(L"PWSH "); + stateMachine.ProcessString(path); + stateMachine.ProcessString(L"> "); + stateMachine.ProcessString(L"\x1b]133;B\x7"); +} + +void ConptyRoundtripTests::ComplicatedPromptRegions() +{ + Log::Comment(L"Same as the ScreenBufferTests::ComplicatedPromptRegions, but in conpty"); + + auto& g = ServiceLocator::LocateGlobals(); + auto& renderer = *g.pRender; + auto& gci = g.getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& sm = si.GetStateMachine(); + + auto* hostTb = &si.GetTextBuffer(); + auto* termTb = term->_mainBuffer.get(); + + gci.LockConsole(); // Lock must be taken to manipulate alt/main buffer state. + auto unlock = wil::scope_exit([&] { gci.UnlockConsole(); }); + + _flushFirstFrame(); + + _checkConptyOutput = false; + + auto verifyBuffer = [&](const TextBuffer& tb) { + const auto& cursor = tb.GetCursor(); + { + const til::point expectedCursor{ 17, 4 }; + VERIFY_ARE_EQUAL(expectedCursor, cursor.GetPosition()); + } + const WEX::TestExecution::DisableVerifyExceptions disableExceptionsScope; + + const auto& row0 = tb.GetRowByOffset(0); + const auto& row4 = tb.GetRowByOffset(4); + VERIFY_IS_TRUE(row0.GetPromptData().has_value()); + VERIFY_IS_TRUE(row4.GetPromptData().has_value()); + + const auto marks = tb.GetMarks(); + VERIFY_ARE_EQUAL(2u, marks.size()); + + { + auto& mark = marks[0]; + const til::point expectedStart{ 0, 0 }; + const til::point expectedEnd{ 17, 0 }; + const til::point expectedOutputStart{ 24, 0 }; // `Foo-Bar` is 7 characters + const til::point expectedOutputEnd{ 0, 4 }; + VERIFY_ARE_EQUAL(expectedStart, mark.start); + VERIFY_ARE_EQUAL(expectedEnd, mark.end); + + VERIFY_ARE_EQUAL(expectedOutputStart, *mark.commandEnd); + VERIFY_ARE_EQUAL(expectedOutputEnd, *mark.outputEnd); + } + { + auto& mark = marks[1]; + const til::point expectedStart{ 0, 4 }; + const til::point expectedEnd{ 17, 4 }; + VERIFY_ARE_EQUAL(expectedStart, mark.start); + VERIFY_ARE_EQUAL(expectedEnd, mark.end); + VERIFY_IS_FALSE(mark.commandEnd.has_value()); + VERIFY_IS_FALSE(mark.outputEnd.has_value()); + } + }; + + Log::Comment(L"========== Fill test content =========="); + + // A prompt looks like: + // `PWSH C:\Windows> ` + // + // which is 17 characters for C:\Windows + + _writePrompt(sm, L"C:\\Windows"); + sm.ProcessString(L"Foo-bar"); + sm.ProcessString(L"\x1b]133;C\x7"); + sm.ProcessString(L"\r\n"); + sm.ProcessString(L"This is some text \r\n"); // y=1 + sm.ProcessString(L"with varying amounts \r\n"); // y=2 + sm.ProcessString(L"of whitespace \r\n"); // y=3 + + _writePrompt(sm, L"C:\\Windows"); // y=4 + + Log::Comment(L"========== Check host buffer =========="); + verifyBuffer(*hostTb); + // DebugBreak(); + Log::Comment(L"Painting the frame"); + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + Log::Comment(L"========== Check terminal buffer =========="); + verifyBuffer(*termTb); +} diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index d9315c0d934..6db57932272 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -3695,7 +3695,7 @@ bool AdaptDispatch::DoITerm2Action(const std::wstring_view string) { // Flush the frame manually, to make sure marks end up on the right line, like the alt buffer sequence. _renderer.TriggerFlush(false); - // return false; + return false; } if constexpr (!Feature_ScrollbarMarks::IsEnabled()) @@ -3741,12 +3741,6 @@ bool AdaptDispatch::DoITerm2Action(const std::wstring_view string) bool AdaptDispatch::DoFinalTermAction(const std::wstring_view string) { // This is not implemented in conhost. - if (_api.IsConsolePty()) - { - // Flush the frame manually, to make sure marks end up on the right line, like the alt buffer sequence. - _renderer.TriggerFlush(false); - // return false; - } if constexpr (!Feature_ScrollbarMarks::IsEnabled()) { @@ -3759,7 +3753,7 @@ bool AdaptDispatch::DoFinalTermAction(const std::wstring_view string) { return false; } - + bool handled = false; const auto action = til::at(parts, 0); if (action.size() == 1) { @@ -3772,7 +3766,9 @@ bool AdaptDispatch::DoFinalTermAction(const std::wstring_view string) attr.SetMarkAttributes(MarkKind::Prompt); _api.SetTextAttributes(attr); _api.GetTextBuffer().StartPrompt(); - return true; + + handled = true; + break; } case L'B': // FTCS_COMMAND_START { @@ -3780,7 +3776,8 @@ bool AdaptDispatch::DoFinalTermAction(const std::wstring_view string) attr.SetMarkAttributes(MarkKind::Command); _api.SetTextAttributes(attr); - return true; + handled = true; + break; } case L'C': // FTCS_COMMAND_EXECUTED { @@ -3788,7 +3785,8 @@ bool AdaptDispatch::DoFinalTermAction(const std::wstring_view string) attr.SetMarkAttributes(MarkKind::Output); _api.SetTextAttributes(attr); - return true; + handled = true; + break; } case L'D': // FTCS_COMMAND_FINISHED { @@ -3812,20 +3810,30 @@ bool AdaptDispatch::DoFinalTermAction(const std::wstring_view string) _api.SetTextAttributes(attr); _api.GetTextBuffer().EndCommand(error); // _api.MarkCommandFinish(error); - return true; + + handled = true; + break; } default: { - return false; + handled = false; } } } + if (_api.IsConsolePty()) + { + // DebugBreak(); + // Flush the frame manually, to make sure marks end up on the right line, like the alt buffer sequence. + _renderer.TriggerFlush(false); + return false; + } + // When we add the rest of the FTCS sequences (GH#11000), we should add a // simple state machine here to track the most recently emitted mark from // this set of sequences, and which sequence was emitted last, so we can // modify the state of that mark as we go. - return false; + return handled; } // Method Description: // - Performs a VsCode action From d0aaf82dff95e17442014f500113ff88407371e3 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 25 Mar 2024 08:53:37 -0500 Subject: [PATCH 08/30] okay the tests aren't self-consistent but I think I'm getting closer --- src/buffer/out/textBuffer.cpp | 21 +- .../ConptyRoundtripTests.cpp | 200 ++++++++++++++++-- src/host/ut_host/ScreenBufferTests.cpp | 4 +- src/terminal/adapter/adaptDispatch.cpp | 3 + 4 files changed, 201 insertions(+), 27 deletions(-) diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 298dbda4871..c9856353f07 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -3040,7 +3040,7 @@ ScrollMark TextBuffer::_scrollMarkForRow(const til::CoordType rowOffset, const t }; auto x = 0; auto y = rowOffset; - + til::point lastMarkedText{ x, y }; for (; y <= bottomInclusive; y++) { // Now we need to iterate over text attributes. We need to find a @@ -3055,7 +3055,7 @@ ScrollMark TextBuffer::_scrollMarkForRow(const til::CoordType rowOffset, const t { const auto nextX = gsl::narrow_cast(x + length); const auto markKind{ attr.GetMarkAttributes() }; - if (/*markKind != MarkKind::None &&*/ + if (markKind != MarkKind::None && markKind != lastMarkKind) { if (markKind == MarkKind::Prompt) @@ -3080,7 +3080,7 @@ ScrollMark TextBuffer::_scrollMarkForRow(const til::CoordType rowOffset, const t endThisMark(x, y); startedCommand = true; } - else if ((markKind == MarkKind::Output || markKind == MarkKind::None) && startedPrompt) + else if ((markKind == MarkKind::Output /*|| markKind == MarkKind::None*/) && startedPrompt) { endThisMark(x, y); startedOutput = true; @@ -3092,9 +3092,15 @@ ScrollMark TextBuffer::_scrollMarkForRow(const til::CoordType rowOffset, const t // mark.commandEnd = til::point{x, y}; } // Otherwise, we've changed from any state -> any state, and it doesn't really matter. - lastMarkKind = markKind; + if (markKind != MarkKind::None) + { + lastMarkKind = markKind; + } + } + if (markKind != MarkKind::None) + { + lastMarkedText = { nextX, y }; } - // advance to next run of text x = nextX; } @@ -3102,7 +3108,10 @@ ScrollMark TextBuffer::_scrollMarkForRow(const til::CoordType rowOffset, const t } // Okay, we're at the bottom of the buffer? Yea, just return what we found. - // endThisMark(x, y); + if (!startedCommand) + { + endThisMark(lastMarkedText.x, lastMarkedText.y); + } return mark; } diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index e42853c18eb..4028ff88811 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -233,7 +233,8 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final TEST_METHOD(TestNoExtendedAttrsOptimization); TEST_METHOD(TestNoBackgroundAttrsOptimization); - TEST_METHOD(ComplicatedPromptRegions); + TEST_METHOD(SimplePromptRegions); + TEST_METHOD(MultilinePromptRegions); private: bool _writeCallback(const char* const pch, const size_t cch); @@ -4318,20 +4319,7 @@ void ConptyRoundtripTests::TestNoBackgroundAttrsOptimization() verifyBuffer(*termTb); } -void _writePrompt(StateMachine& stateMachine, const auto& path) -{ - stateMachine.ProcessString(L"\x1b]133;D\x7"); - stateMachine.ProcessString(L"\x1b]133;A\x7"); - stateMachine.ProcessString(L"\x1b]9;9;"); - stateMachine.ProcessString(path); - stateMachine.ProcessString(L"\x7"); - stateMachine.ProcessString(L"PWSH "); - stateMachine.ProcessString(path); - stateMachine.ProcessString(L"> "); - stateMachine.ProcessString(L"\x1b]133;B\x7"); -} - -void ConptyRoundtripTests::ComplicatedPromptRegions() +void ConptyRoundtripTests::SimplePromptRegions() { Log::Comment(L"Same as the ScreenBufferTests::ComplicatedPromptRegions, but in conpty"); @@ -4392,10 +4380,21 @@ void ConptyRoundtripTests::ComplicatedPromptRegions() Log::Comment(L"========== Fill test content =========="); - // A prompt looks like: - // `PWSH C:\Windows> ` - // - // which is 17 characters for C:\Windows + auto _writePrompt = [](StateMachine& stateMachine, const auto& path) { + // A prompt looks like: + // `PWSH C:\Windows> ` + // + // which is 17 characters for C:\Windows + stateMachine.ProcessString(L"\x1b]133;D\x7"); + stateMachine.ProcessString(L"\x1b]133;A\x7"); + stateMachine.ProcessString(L"\x1b]9;9;"); + stateMachine.ProcessString(path); + stateMachine.ProcessString(L"\x7"); + stateMachine.ProcessString(L"PWSH "); + stateMachine.ProcessString(path); + stateMachine.ProcessString(L"> "); + stateMachine.ProcessString(L"\x1b]133;B\x7"); + }; _writePrompt(sm, L"C:\\Windows"); sm.ProcessString(L"Foo-bar"); @@ -4416,3 +4415,166 @@ void ConptyRoundtripTests::ComplicatedPromptRegions() Log::Comment(L"========== Check terminal buffer =========="); verifyBuffer(*termTb); } + +void ConptyRoundtripTests::MultilinePromptRegions() +{ + auto& g = ServiceLocator::LocateGlobals(); + auto& renderer = *g.pRender; + auto& gci = g.getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& sm = si.GetStateMachine(); + + auto* hostTb = &si.GetTextBuffer(); + auto* termTb = term->_mainBuffer.get(); + + gci.LockConsole(); // Lock must be taken to manipulate alt/main buffer state. + auto unlock = wil::scope_exit([&] { gci.UnlockConsole(); }); + + _flushFirstFrame(); + + _checkConptyOutput = false; + + auto bufferWidth = term->GetViewport().Width(); + + auto verifyBuffer = [&](const TextBuffer& tb) { + const auto& cursor = tb.GetCursor(); + { + const til::point expectedCursor{ 2, 6 }; + VERIFY_ARE_EQUAL(expectedCursor, cursor.GetPosition()); + } + const WEX::TestExecution::DisableVerifyExceptions disableExceptionsScope; + + const auto& row0 = tb.GetRowByOffset(0); + const auto& row5 = tb.GetRowByOffset(5); + VERIFY_IS_TRUE(row0.GetPromptData().has_value()); + VERIFY_IS_TRUE(row5.GetPromptData().has_value()); + + const auto marks = tb.GetMarks(); + VERIFY_ARE_EQUAL(2u, marks.size()); + + { + Log::Comment(L"Row 0"); + const auto& row = tb.GetRowByOffset(0); + const auto& attrs = row.Attributes(); + const auto& runs = attrs.runs(); + VERIFY_ARE_EQUAL(2, runs.size()); + auto run0 = runs[0]; + auto run1 = runs[1]; + VERIFY_ARE_EQUAL(17, run0.length); + VERIFY_ARE_EQUAL(MarkKind::Prompt, run0.value.GetMarkAttributes()); + + VERIFY_ARE_EQUAL(bufferWidth - 17, run1.length); + VERIFY_ARE_EQUAL(MarkKind::None, run1.value.GetMarkAttributes()); + } + { + Log::Comment(L"Row 1"); + const auto& row = tb.GetRowByOffset(1); + const auto& attrs = row.Attributes(); + const auto& runs = attrs.runs(); + VERIFY_ARE_EQUAL(3, runs.size()); + auto run0 = runs[0]; + auto run1 = runs[1]; + auto run2 = runs[2]; + VERIFY_ARE_EQUAL(2, run0.length); + VERIFY_ARE_EQUAL(MarkKind::Prompt, run0.value.GetMarkAttributes()); + + VERIFY_ARE_EQUAL(7, run1.length); + VERIFY_ARE_EQUAL(MarkKind::Command, run1.value.GetMarkAttributes()); + + VERIFY_ARE_EQUAL(bufferWidth - 9, run2.length); + VERIFY_ARE_EQUAL(MarkKind::None, run2.value.GetMarkAttributes()); + } + { + Log::Comment(L"Row 2"); + const auto& row = tb.GetRowByOffset(2); + const auto& attrs = row.Attributes(); + const auto& runs = attrs.runs(); + VERIFY_ARE_EQUAL(2, runs.size()); + auto run0 = runs[0]; + auto run1 = runs[1]; + VERIFY_ARE_EQUAL(22, run0.length); + VERIFY_ARE_EQUAL(MarkKind::Output, run0.value.GetMarkAttributes()); + + VERIFY_ARE_EQUAL(bufferWidth - 22, run1.length); + VERIFY_ARE_EQUAL(MarkKind::None, run1.value.GetMarkAttributes()); + } + { + Log::Comment(L"Row 3"); + const auto& row = tb.GetRowByOffset(3); + const auto& attrs = row.Attributes(); + const auto& runs = attrs.runs(); + VERIFY_ARE_EQUAL(2, runs.size()); + auto run0 = runs[0]; + auto run1 = runs[1]; + VERIFY_ARE_EQUAL(22, run0.length); + VERIFY_ARE_EQUAL(MarkKind::Output, run0.value.GetMarkAttributes()); + + VERIFY_ARE_EQUAL(bufferWidth - 22, run1.length); + VERIFY_ARE_EQUAL(MarkKind::None, run1.value.GetMarkAttributes()); + } + + { + auto& mark = marks[0]; + const til::point expectedStart{ 0, 0 }; + const til::point expectedEnd{ 2, 1 }; + // const til::point expectedOutputStart{ 9, 1 }; // `Foo-Bar` is 7 characters + // The command technically ends at {9,1} (the end of the Foo-Bar string). + // However, the first character in the output is at {0,2}. + const til::point expectedOutputStart{ 0, 2 }; // `Foo-Bar` is 7 characters + const til::point expectedOutputEnd{ 0, 5 }; + VERIFY_ARE_EQUAL(expectedStart, mark.start); + VERIFY_ARE_EQUAL(expectedEnd, mark.end); + + VERIFY_ARE_EQUAL(expectedOutputStart, *mark.commandEnd); + VERIFY_ARE_EQUAL(expectedOutputEnd, *mark.outputEnd); + } + { + auto& mark = marks[1]; + const til::point expectedStart{ 0, 5 }; + const til::point expectedEnd{ 2, 6 }; + VERIFY_ARE_EQUAL(expectedStart, mark.start); + VERIFY_ARE_EQUAL(expectedEnd, mark.end); + VERIFY_IS_FALSE(mark.commandEnd.has_value()); + VERIFY_IS_FALSE(mark.outputEnd.has_value()); + } + }; + + Log::Comment(L"========== Fill test content =========="); + + auto _writePrompt = [](StateMachine& stateMachine, const auto& path) { + // A prompt looks like: + // `PWSH C:\Windows >` + // `> ` + // + // which two rows. The first is 17 characters for C:\Windows + stateMachine.ProcessString(L"\x1b]133;D\x7"); + stateMachine.ProcessString(L"\x1b]133;A\x7"); + stateMachine.ProcessString(L"\x1b]9;9;"); + stateMachine.ProcessString(path); + stateMachine.ProcessString(L"\x7"); + stateMachine.ProcessString(L"PWSH "); + stateMachine.ProcessString(path); + stateMachine.ProcessString(L" >\r\n"); + stateMachine.ProcessString(L"> "); + stateMachine.ProcessString(L"\x1b]133;B\x7"); + }; + + _writePrompt(sm, L"C:\\Windows"); // y=0,1 + sm.ProcessString(L"Foo-bar"); + sm.ProcessString(L"\x1b]133;C\x7"); + sm.ProcessString(L"\r\n"); + sm.ProcessString(L"This is some text \r\n"); // y=2 + sm.ProcessString(L"with varying amounts \r\n"); // y=3 + sm.ProcessString(L"of whitespace \r\n"); // y=4 + + _writePrompt(sm, L"C:\\Windows"); // y=5, 6 + + Log::Comment(L"========== Check host buffer =========="); + verifyBuffer(*hostTb); + // DebugBreak(); + Log::Comment(L"Painting the frame"); + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + Log::Comment(L"========== Check terminal buffer =========="); + verifyBuffer(*termTb); +} diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index 71308bb283b..e156c0ee33c 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -267,7 +267,7 @@ class ScreenBufferTests TEST_METHOD(SimpleMarkCommand); TEST_METHOD(SimpleWrappedCommand); - TEST_METHOD(ComplicatedPromptRegions); + TEST_METHOD(SimplePromptRegions); }; void ScreenBufferTests::SingleAlternateBufferCreationTest() @@ -8520,7 +8520,7 @@ void _writePrompt(StateMachine& stateMachine, const auto& path) stateMachine.ProcessString(L"\x1b]133;B\x7"); } -void ScreenBufferTests::ComplicatedPromptRegions() +void ScreenBufferTests::SimplePromptRegions() { auto& g = ServiceLocator::LocateGlobals(); auto& gci = g.getConsoleInformation(); diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index 6db57932272..8905529975d 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -3766,6 +3766,9 @@ bool AdaptDispatch::DoFinalTermAction(const std::wstring_view string) attr.SetMarkAttributes(MarkKind::Prompt); _api.SetTextAttributes(attr); _api.GetTextBuffer().StartPrompt(); + // auto& tb = _api.GetTextBuffer(); + // auto& cursor= tb.GetCursor(); + // auto& row = tb.GetRowByOffset(cursor.GetPosition().y); handled = true; break; From 0b39a616193163e71d7405dbbab66a5bf9910634 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 25 Mar 2024 09:09:42 -0500 Subject: [PATCH 09/30] closer --- src/buffer/out/TextAttribute.cpp | 1 + src/cascadia/TerminalCore/Terminal.cpp | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/buffer/out/TextAttribute.cpp b/src/buffer/out/TextAttribute.cpp index e154accf98c..65799d1d6f5 100644 --- a/src/buffer/out/TextAttribute.cpp +++ b/src/buffer/out/TextAttribute.cpp @@ -434,4 +434,5 @@ void TextAttribute::SetStandardErase() noexcept { _attrs = CharacterAttributes::Normal; _hyperlinkId = 0; + _markKind = MarkKind::None; } diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 0d3888e6bce..7bdb576b5b5 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -733,9 +733,12 @@ TerminalInput::OutputType Terminal::SendCharEvent(const wchar_t ch, const WORD s // command_executed now. // // Fortunately, MarkOutputStart will do all this logic for us! - MarkOutputStart(); + // MarkOutputStart(); // TODO! ^^^^^^^^^^ that's not on Terminal anymore. That's a text attr now. + auto attr = _activeBuffer().GetCurrentAttributes(); + attr.SetMarkAttributes(MarkKind::Output); + SetTextAttributes(attr); } const auto keyDown = SynthesizeKeyEvent(true, 1, vkey, scanCode, ch, states.Value()); From 3aa16d9925b4f2d9351b44200db8276e7d2ff914 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 25 Mar 2024 10:22:24 -0500 Subject: [PATCH 10/30] did this really work? --- src/buffer/out/textBuffer.cpp | 28 ++- .../ConptyRoundtripTests.cpp | 204 ++++++++++++++++++ 2 files changed, 221 insertions(+), 11 deletions(-) diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index c9856353f07..d57b5db296c 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -3055,35 +3055,45 @@ ScrollMark TextBuffer::_scrollMarkForRow(const til::CoordType rowOffset, const t { const auto nextX = gsl::narrow_cast(x + length); const auto markKind{ attr.GetMarkAttributes() }; - if (markKind != MarkKind::None && - markKind != lastMarkKind) + + if (markKind != MarkKind::None) { + lastMarkedText = { nextX, y }; + // } + + // if (markKind != MarkKind::None && + // markKind != lastMarkKind) + // { if (markKind == MarkKind::Prompt) { - if (startedPrompt) + if (startedCommand || startedOutput) { // we got a _new_ prompt. bail out. - endThisMark(x, y); + // endThisMark(x, y); + // endThisMark(lastMarkedText.x, lastMarkedText.y); break; } - else + if (!startedPrompt) { // We entered the first prompt here startedPrompt = true; mark.start = til::point{ x, y }; } + endThisMark(lastMarkedText.x, lastMarkedText.y); } else if (markKind == MarkKind::Command && startedPrompt) { // mark.end = til::point{x, y}; // foundEnd = true; - endThisMark(x, y); + // endThisMark(x, y); startedCommand = true; + endThisMark(lastMarkedText.x, lastMarkedText.y); } else if ((markKind == MarkKind::Output /*|| markKind == MarkKind::None*/) && startedPrompt) { - endThisMark(x, y); + // endThisMark(x, y); startedOutput = true; + endThisMark(lastMarkedText.x, lastMarkedText.y); // if (!foundEnd) // { // mark.end = til::point{x, y}; @@ -3097,10 +3107,6 @@ ScrollMark TextBuffer::_scrollMarkForRow(const til::CoordType rowOffset, const t lastMarkKind = markKind; } } - if (markKind != MarkKind::None) - { - lastMarkedText = { nextX, y }; - } // advance to next run of text x = nextX; } diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index 4028ff88811..57bd9d2d8f9 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -235,6 +235,7 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final TEST_METHOD(SimplePromptRegions); TEST_METHOD(MultilinePromptRegions); + TEST_METHOD(ManyMultilinePromptsWithTrailingSpaces); private: bool _writeCallback(const char* const pch, const size_t cch); @@ -4578,3 +4579,206 @@ void ConptyRoundtripTests::MultilinePromptRegions() Log::Comment(L"========== Check terminal buffer =========="); verifyBuffer(*termTb); } + +void ConptyRoundtripTests::ManyMultilinePromptsWithTrailingSpaces() +{ + auto& g = ServiceLocator::LocateGlobals(); + auto& renderer = *g.pRender; + auto& gci = g.getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& sm = si.GetStateMachine(); + + auto* hostTb = &si.GetTextBuffer(); + auto* termTb = term->_mainBuffer.get(); + + gci.LockConsole(); // Lock must be taken to manipulate alt/main buffer state. + auto unlock = wil::scope_exit([&] { gci.UnlockConsole(); }); + + _flushFirstFrame(); + + _checkConptyOutput = false; + + auto bufferWidth = term->GetViewport().Width(); + + auto verifyFirstRowOfPrompt = [&](const ROW& row) { + const auto& attrs = row.Attributes(); + const auto& runs = attrs.runs(); + VERIFY_ARE_EQUAL(2, runs.size()); + auto run0 = runs[0]; + auto run1 = runs[1]; + VERIFY_ARE_EQUAL(17, run0.length); + VERIFY_ARE_EQUAL(MarkKind::Prompt, run0.value.GetMarkAttributes()); + + VERIFY_ARE_EQUAL(bufferWidth - 17, run1.length); + VERIFY_ARE_EQUAL(MarkKind::None, run1.value.GetMarkAttributes()); + }; + auto verifySecondRowOfPrompt = [&](const ROW& row, const auto expectedCommandLength) { + const auto& attrs = row.Attributes(); + const auto& runs = attrs.runs(); + VERIFY_ARE_EQUAL(3, runs.size()); + auto run0 = runs[0]; + auto run1 = runs[1]; + auto run2 = runs[2]; + VERIFY_ARE_EQUAL(2, run0.length); + VERIFY_ARE_EQUAL(MarkKind::Prompt, run0.value.GetMarkAttributes()); + + VERIFY_ARE_EQUAL(expectedCommandLength, run1.length); + VERIFY_ARE_EQUAL(MarkKind::Command, run1.value.GetMarkAttributes()); + + VERIFY_ARE_EQUAL(bufferWidth - (2 + expectedCommandLength), run2.length); + VERIFY_ARE_EQUAL(MarkKind::None, run2.value.GetMarkAttributes()); + }; + + auto verifyBuffer = [&](const TextBuffer& tb) { + const auto& cursor = tb.GetCursor(); + { + const til::point expectedCursor{ 0, 11 }; + VERIFY_ARE_EQUAL(expectedCursor, cursor.GetPosition()); + } + const WEX::TestExecution::DisableVerifyExceptions disableExceptionsScope; + + const auto marks = tb.GetMarks(); + VERIFY_ARE_EQUAL(3u, marks.size()); + + Log::Comment(L"Row 0"); + verifyFirstRowOfPrompt(tb.GetRowByOffset(0)); + + Log::Comment(L"Row 1"); + verifySecondRowOfPrompt(tb.GetRowByOffset(1), 7); + + { + Log::Comment(L"Row 2"); + const auto& row = tb.GetRowByOffset(2); + const auto& attrs = row.Attributes(); + const auto& runs = attrs.runs(); + VERIFY_ARE_EQUAL(2, runs.size()); + auto run0 = runs[0]; + auto run1 = runs[1]; + VERIFY_ARE_EQUAL(22, run0.length); + VERIFY_ARE_EQUAL(MarkKind::Output, run0.value.GetMarkAttributes()); + + VERIFY_ARE_EQUAL(bufferWidth - 22, run1.length); + VERIFY_ARE_EQUAL(MarkKind::None, run1.value.GetMarkAttributes()); + } + { + Log::Comment(L"Row 3"); + const auto& row = tb.GetRowByOffset(3); + const auto& attrs = row.Attributes(); + const auto& runs = attrs.runs(); + VERIFY_ARE_EQUAL(2, runs.size()); + auto run0 = runs[0]; + auto run1 = runs[1]; + VERIFY_ARE_EQUAL(22, run0.length); + VERIFY_ARE_EQUAL(MarkKind::Output, run0.value.GetMarkAttributes()); + + VERIFY_ARE_EQUAL(bufferWidth - 22, run1.length); + VERIFY_ARE_EQUAL(MarkKind::None, run1.value.GetMarkAttributes()); + } + + Log::Comment(L"Row 5"); + verifyFirstRowOfPrompt(tb.GetRowByOffset(5)); + + Log::Comment(L"Row 6"); + verifySecondRowOfPrompt(tb.GetRowByOffset(6), 7); + + Log::Comment(L"Row 8"); + verifyFirstRowOfPrompt(tb.GetRowByOffset(8)); + + Log::Comment(L"Row 9"); + verifySecondRowOfPrompt(tb.GetRowByOffset(9), 6); + + { + Log::Comment(L"Foo-bar mark on rows 0 & 1"); + + auto& mark = marks[0]; + const til::point expectedStart{ 0, 0 }; + const til::point expectedEnd{ 2, 1 }; + + // The command ends at {9,1} (the end of the Foo-Bar string). + // However, the first character in the output is at {0,2}. + const til::point expectedOutputStart{ 9, 1 }; // `Foo-Bar` is 7 characters + const til::point expectedOutputEnd{ 13, 4 }; + VERIFY_ARE_EQUAL(expectedStart, mark.start); + VERIFY_ARE_EQUAL(expectedEnd, mark.end); + + VERIFY_ARE_EQUAL(expectedOutputStart, *mark.commandEnd); + VERIFY_ARE_EQUAL(expectedOutputEnd, *mark.outputEnd); + } + { + Log::Comment(L"Boo-far mark on rows 5 & 6"); + auto& mark = marks[1]; + const til::point expectedStart{ 0, 5 }; + const til::point expectedEnd{ 2, 6 }; + const til::point expectedOutputStart{ 9, 6 }; // `Boo-far` is 7 characters + const til::point expectedOutputEnd{ 22, 7 }; + + VERIFY_ARE_EQUAL(expectedStart, mark.start); + VERIFY_ARE_EQUAL(expectedEnd, mark.end); + + VERIFY_ARE_EQUAL(expectedOutputStart, *mark.commandEnd); + VERIFY_ARE_EQUAL(expectedOutputEnd, *mark.outputEnd); + } + { + Log::Comment(L"yikes? mark on rows 8 & 9"); + auto& mark = marks[2]; + const til::point expectedStart{ 0, 8 }; + const til::point expectedEnd{ 2, 9 }; + const til::point expectedOutputStart{ 8, 9 }; // `yikes?` is 6 characters + const til::point expectedOutputEnd{ 22, 10 }; + + VERIFY_ARE_EQUAL(expectedStart, mark.start); + VERIFY_ARE_EQUAL(expectedEnd, mark.end); + + VERIFY_ARE_EQUAL(expectedOutputStart, *mark.commandEnd); + VERIFY_ARE_EQUAL(expectedOutputEnd, *mark.outputEnd); + } + }; + + Log::Comment(L"========== Fill test content =========="); + + auto writePrompt = [](StateMachine& stateMachine, const auto& path) { + // A prompt looks like: + // `PWSH C:\Windows >` + // `> ` + // + // which two rows. The first is 17 characters for C:\Windows + stateMachine.ProcessString(L"\x1b]133;D\x7"); + stateMachine.ProcessString(L"\x1b]133;A\x7"); + stateMachine.ProcessString(L"\x1b]9;9;"); + stateMachine.ProcessString(path); + stateMachine.ProcessString(L"\x7"); + stateMachine.ProcessString(L"PWSH "); + stateMachine.ProcessString(path); + stateMachine.ProcessString(L" >\r\n"); + stateMachine.ProcessString(L"> "); + stateMachine.ProcessString(L"\x1b]133;B\x7"); + }; + auto writeCommand = [](StateMachine& stateMachine, const auto& cmd) { + stateMachine.ProcessString(cmd); + stateMachine.ProcessString(L"\x1b]133;C\x7"); + stateMachine.ProcessString(L"\r\n"); + }; + + writePrompt(sm, L"C:\\Windows"); // y=0,1 + writeCommand(sm, L"Foo-bar"); + sm.ProcessString(L"This is some text \r\n"); // y=2 + sm.ProcessString(L"with varying amounts \r\n"); // y=3 + sm.ProcessString(L"of whitespace\r\n"); // y=4 + + writePrompt(sm, L"C:\\Windows"); // y=5, 6 + writeCommand(sm, L"Boo-far"); // y=6 + sm.ProcessString(L"This is more text \r\n"); // y=7 + + writePrompt(sm, L"C:\\Windows"); // y=8,9 + writeCommand(sm, L"yikes?"); // y=9 + sm.ProcessString(L"This is even more \r\n"); // y=10 + + Log::Comment(L"========== Check host buffer =========="); + verifyBuffer(*hostTb); + // DebugBreak(); + Log::Comment(L"Painting the frame"); + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + Log::Comment(L"========== Check terminal buffer =========="); + verifyBuffer(*termTb); +} From c28f8c0766d758c9c3cef8ca357e16faf98b4ff4 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 25 Mar 2024 10:28:35 -0500 Subject: [PATCH 11/30] Yea I'm okay with these test changes --- src/cascadia/UnitTests_Control/ControlCoreTests.cpp | 2 +- .../UnitTests_TerminalCore/ConptyRoundtripTests.cpp | 13 +++++-------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/cascadia/UnitTests_Control/ControlCoreTests.cpp b/src/cascadia/UnitTests_Control/ControlCoreTests.cpp index c38c0221c5a..f4657e82527 100644 --- a/src/cascadia/UnitTests_Control/ControlCoreTests.cpp +++ b/src/cascadia/UnitTests_Control/ControlCoreTests.cpp @@ -502,7 +502,7 @@ namespace ControlUnitTests const auto& start = core->_terminal->GetSelectionAnchor(); const auto& end = core->_terminal->GetSelectionEnd(); const til::point expectedStart{ 24, 0 }; // The character after the prompt - const til::point expectedEnd{ 29, 3 }; // x = buffer.right + const til::point expectedEnd{ 21, 3 }; // x = the end of the text VERIFY_ARE_EQUAL(expectedStart, start); VERIFY_ARE_EQUAL(expectedEnd, end); } diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index 57bd9d2d8f9..277acee3c98 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -4361,7 +4361,7 @@ void ConptyRoundtripTests::SimplePromptRegions() const til::point expectedStart{ 0, 0 }; const til::point expectedEnd{ 17, 0 }; const til::point expectedOutputStart{ 24, 0 }; // `Foo-Bar` is 7 characters - const til::point expectedOutputEnd{ 0, 4 }; + const til::point expectedOutputEnd{ 13, 3 }; VERIFY_ARE_EQUAL(expectedStart, mark.start); VERIFY_ARE_EQUAL(expectedEnd, mark.end); @@ -4403,7 +4403,7 @@ void ConptyRoundtripTests::SimplePromptRegions() sm.ProcessString(L"\r\n"); sm.ProcessString(L"This is some text \r\n"); // y=1 sm.ProcessString(L"with varying amounts \r\n"); // y=2 - sm.ProcessString(L"of whitespace \r\n"); // y=3 + sm.ProcessString(L"of whitespace\r\n"); // y=3 _writePrompt(sm, L"C:\\Windows"); // y=4 @@ -4518,11 +4518,8 @@ void ConptyRoundtripTests::MultilinePromptRegions() auto& mark = marks[0]; const til::point expectedStart{ 0, 0 }; const til::point expectedEnd{ 2, 1 }; - // const til::point expectedOutputStart{ 9, 1 }; // `Foo-Bar` is 7 characters - // The command technically ends at {9,1} (the end of the Foo-Bar string). - // However, the first character in the output is at {0,2}. - const til::point expectedOutputStart{ 0, 2 }; // `Foo-Bar` is 7 characters - const til::point expectedOutputEnd{ 0, 5 }; + const til::point expectedOutputStart{ 9, 1 }; // `Foo-Bar` is 7 characters + const til::point expectedOutputEnd{ 13, 4 }; VERIFY_ARE_EQUAL(expectedStart, mark.start); VERIFY_ARE_EQUAL(expectedEnd, mark.end); @@ -4566,7 +4563,7 @@ void ConptyRoundtripTests::MultilinePromptRegions() sm.ProcessString(L"\r\n"); sm.ProcessString(L"This is some text \r\n"); // y=2 sm.ProcessString(L"with varying amounts \r\n"); // y=3 - sm.ProcessString(L"of whitespace \r\n"); // y=4 + sm.ProcessString(L"of whitespace\r\n"); // y=4 _writePrompt(sm, L"C:\\Windows"); // y=5, 6 From c6e5934c3a4cd92ec0d46754f2b6e3917a55bd4c Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 25 Mar 2024 10:50:44 -0500 Subject: [PATCH 12/30] rename, so we can have simple marks for optimized scrollbars --- src/buffer/out/textBuffer.cpp | 43 +++-- src/buffer/out/textBuffer.hpp | 17 +- src/cascadia/TerminalControl/ControlCore.cpp | 174 +++++++++--------- src/cascadia/TerminalControl/ControlCore.h | 6 +- src/cascadia/TerminalCore/Terminal.cpp | 23 +-- src/cascadia/TerminalCore/Terminal.hpp | 5 +- .../ConptyRoundtripTests.cpp | 6 +- src/host/ut_host/ScreenBufferTests.cpp | 2 +- 8 files changed, 140 insertions(+), 136 deletions(-) diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index d57b5db296c..b430726662f 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -2887,22 +2887,24 @@ std::vector TextBuffer::SearchText(const std::wstring_view& nee return results; } -std::vector TextBuffer::GetMarks() const noexcept +std::vector TextBuffer::GetMarkRows() const noexcept { - // std::vector marks; - // const auto bottom = _estimateOffsetOfLastCommittedRow(); - // for (auto y = 0; y <= bottom; y++) - // { - // const auto& row = GetRowByOffset(y); - // const auto& data{ row.GetPromptData() }; - // if (data.has_value()) - // { - // marks.emplace_back(y, *data); - // } - // } - // return std::move(marks); - - std::vector marks{}; + std::vector marks; + const auto bottom = _estimateOffsetOfLastCommittedRow(); + for (auto y = 0; y <= bottom; y++) + { + const auto& row = GetRowByOffset(y); + const auto& data{ row.GetPromptData() }; + if (data.has_value()) + { + marks.emplace_back(y, *data); + } + } + return std::move(marks); +} +std::vector TextBuffer::GetMarkExtents() const noexcept +{ + std::vector marks{}; const auto bottom = _estimateOffsetOfLastCommittedRow(); auto lastPromptY = bottom; for (auto promptY = bottom; promptY >= 0; promptY--) @@ -2918,11 +2920,8 @@ std::vector TextBuffer::GetMarks() const noexcept // This row did start a prompt! Find the prompt that starts here. // Presumably, no rows below us will have prompts, so pass in the last // row with text as the bottom - marks.push_back(_scrollMarkForRow(promptY, lastPromptY)); - // if (!foundCommand.empty()) - // { - // commands.emplace_back(std::move(foundCommand)); - // } + marks.push_back(_scrollMarkExtentForRow(promptY, lastPromptY)); + lastPromptY = promptY; } std::reverse(marks.begin(), marks.end()); @@ -3008,13 +3007,13 @@ void TextBuffer::ClearAllMarks() noexcept // return (m.start.y < 0) || (m.start.y >= height); // }); // } -ScrollMark TextBuffer::_scrollMarkForRow(const til::CoordType rowOffset, const til::CoordType bottomInclusive) const +MarkExtents TextBuffer::_scrollMarkExtentForRow(const til::CoordType rowOffset, const til::CoordType bottomInclusive) const { const auto& startRow = GetRowByOffset(rowOffset); const auto& rowPromptData = startRow.GetPromptData(); assert(rowPromptData.has_value()); - ScrollMark mark{ + MarkExtents mark{ .data = *rowPromptData, }; diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index b22cb34db6e..ac0d2462398 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -66,7 +66,7 @@ namespace Microsoft::Console::Render class Renderer; } -struct ScrollMark +struct MarkExtents { MarkData data; @@ -95,11 +95,11 @@ struct ScrollMark } }; -// struct ScrollMark -// { -// til::CoordType row{ 0 }; -// MarkData data; -// }; +struct ScrollMark +{ + til::CoordType row{ 0 }; + MarkData data; +}; class TextBuffer final { @@ -330,7 +330,8 @@ class TextBuffer final std::vector SearchText(const std::wstring_view& needle, bool caseInsensitive) const; std::vector SearchText(const std::wstring_view& needle, bool caseInsensitive, til::CoordType rowBeg, til::CoordType rowEnd) const; - std::vector GetMarks() const noexcept; + std::vector GetMarkRows() const noexcept; + std::vector GetMarkExtents() const noexcept; void ClearMarksInRange(const til::point start, const til::point end); void ClearAllMarks() noexcept; // void ScrollMarks(const int delta); @@ -341,7 +342,7 @@ class TextBuffer final // void SetCurrentOutputEnd(const til::point pos, ::MarkCategory category) noexcept; std::wstring CurrentCommand() const; std::wstring _commandForRow(const til::CoordType rowOffset, const til::CoordType bottomInclusive) const; - ScrollMark _scrollMarkForRow(const til::CoordType rowOffset, const til::CoordType bottomInclusive) const; + MarkExtents _scrollMarkExtentForRow(const til::CoordType rowOffset, const til::CoordType bottomInclusive) const; std::vector Commands() const; void StartPrompt(); diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index ae8a0d2c83a..b525aebcbce 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -1884,70 +1884,68 @@ namespace winrt::Microsoft::Terminal::Control::implementation // approach, but it's good enough to bring value to 90% of use cases. const auto cursorPos{ _terminal->GetCursorPosition() }; - // TODO! yea this all needs re-writing vvvvv - - // // Does the current buffer line have a mark on it? - // const auto& marks{ _terminal->GetScrollMarks() }; - // if (!marks.empty()) - // { - // const auto& last{ marks.back() }; - // const auto [start, end] = last.GetExtent(); - // const auto lastNonSpace = _terminal->GetTextBuffer().GetLastNonSpaceCharacter(); - - // // If the user clicked off to the right side of the prompt, we - // // want to send keystrokes to the last character in the prompt +1. - // // - // // We don't want to send too many here. In CMD, if the user's - // // last command is longer than what they've currently typed, and - // // they press right arrow at the end of the prompt, COOKED_READ - // // will fill in characters from the previous command. - // // - // // By only sending keypresses to the end of the command + 1, we - // // should leave the cursor at the very end of the prompt, - // // without adding any characters from a previous command. - // auto clampedClick = terminalPosition; - // if (terminalPosition > lastNonSpace) - // { - // clampedClick = lastNonSpace + til::point{ 1, 0 }; - // _terminal->GetTextBuffer().GetSize().Clamp(clampedClick); - // } - - // if (clampedClick >= end) - // { - // // Get the distance between the cursor and the click, in cells. - // const auto bufferSize = _terminal->GetTextBuffer().GetSize(); - - // // First, make sure to iterate from the first point to the - // // second. The user may have clicked _earlier_ in the - // // buffer! - // auto goRight = clampedClick > cursorPos; - // const auto startPoint = goRight ? cursorPos : clampedClick; - // const auto endPoint = goRight ? clampedClick : cursorPos; - - // const auto delta = _terminal->GetTextBuffer().GetCellDistance(startPoint, endPoint); - // const WORD key = goRight ? VK_RIGHT : VK_LEFT; - - // std::wstring buffer; - // const auto append = [&](TerminalInput::OutputType&& out) { - // if (out) - // { - // buffer.append(std::move(*out)); - // } - // }; - - // // Send an up and a down once per cell. This won't - // // accurately handle wide characters, or continuation - // // prompts, or cases where a single escape character in the - // // command (e.g. ^[) takes up two cells. - // for (size_t i = 0u; i < delta; i++) - // { - // append(_terminal->SendKeyEvent(key, 0, {}, true)); - // append(_terminal->SendKeyEvent(key, 0, {}, false)); - // } - - // _sendInputToConnection(buffer); - // } - // } + // Does the current buffer line have a mark on it? + const auto& marks{ _terminal->GetMarkExtents() }; + if (!marks.empty()) + { + const auto& last{ marks.back() }; + const auto [start, end] = last.GetExtent(); + const auto lastNonSpace = _terminal->GetTextBuffer().GetLastNonSpaceCharacter(); + + // If the user clicked off to the right side of the prompt, we + // want to send keystrokes to the last character in the prompt +1. + // + // We don't want to send too many here. In CMD, if the user's + // last command is longer than what they've currently typed, and + // they press right arrow at the end of the prompt, COOKED_READ + // will fill in characters from the previous command. + // + // By only sending keypresses to the end of the command + 1, we + // should leave the cursor at the very end of the prompt, + // without adding any characters from a previous command. + auto clampedClick = terminalPosition; + if (terminalPosition > lastNonSpace) + { + clampedClick = lastNonSpace + til::point{ 1, 0 }; + _terminal->GetTextBuffer().GetSize().Clamp(clampedClick); + } + + if (clampedClick >= end) + { + // Get the distance between the cursor and the click, in cells. + const auto bufferSize = _terminal->GetTextBuffer().GetSize(); + + // First, make sure to iterate from the first point to the + // second. The user may have clicked _earlier_ in the + // buffer! + auto goRight = clampedClick > cursorPos; + const auto startPoint = goRight ? cursorPos : clampedClick; + const auto endPoint = goRight ? clampedClick : cursorPos; + + const auto delta = _terminal->GetTextBuffer().GetCellDistance(startPoint, endPoint); + const WORD key = goRight ? VK_RIGHT : VK_LEFT; + + std::wstring buffer; + const auto append = [&](TerminalInput::OutputType&& out) { + if (out) + { + buffer.append(std::move(*out)); + } + }; + + // Send an up and a down once per cell. This won't + // accurately handle wide characters, or continuation + // prompts, or cases where a single escape character in the + // command (e.g. ^[) takes up two cells. + for (size_t i = 0u; i < delta; i++) + { + append(_terminal->SendKeyEvent(key, 0, {}, true)); + append(_terminal->SendKeyEvent(key, 0, {}, false)); + } + + _sendInputToConnection(buffer); + } + } } _updateSelectionUI(); } @@ -2315,19 +2313,23 @@ namespace winrt::Microsoft::Terminal::Control::implementation _owningHwnd = owner; } + // This one is fairly hot! it gets called every time we redraw the scrollbar + // marks, which is frequently. Fortunately, we don't need to bother with + // collecting up the actual extents of the marks in here - we just need the + // rows they start on. Windows::Foundation::Collections::IVector ControlCore::ScrollMarks() const { const auto lock = _terminal->LockForReading(); - const auto& internalMarks = _terminal->GetScrollMarks(); + const auto& markRows = _terminal->GetMarkRows(); std::vector v; - v.reserve(internalMarks.size()); + v.reserve(markRows.size()); - for (const auto& mark : internalMarks) + for (const auto& mark : markRows) { v.emplace_back( - mark.start.y, - OptionalFromColor(_terminal->GetColorForMark(mark))); + mark.row, + OptionalFromColor(_terminal->GetColorForMark(mark.data))); } return winrt::single_threaded_vector(std::move(v)); @@ -2376,9 +2378,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation { const auto lock = _terminal->LockForWriting(); const auto currentOffset = ScrollOffset(); - const auto& marks{ _terminal->GetScrollMarks() }; + const auto& marks{ _terminal->GetMarkExtents() }; - std::optional<::ScrollMark> tgt; + std::optional<::MarkExtents> tgt; switch (direction) { @@ -2492,8 +2494,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation const til::point start = _terminal->IsSelectionActive() ? (goUp ? _terminal->GetSelectionAnchor() : _terminal->GetSelectionEnd()) : _terminal->GetTextBuffer().GetCursor().GetPosition(); - std::optional<::ScrollMark> nearest{ std::nullopt }; - const auto& marks{ _terminal->GetScrollMarks() }; + std::optional<::MarkExtents> nearest{ std::nullopt }; + const auto& marks{ _terminal->GetMarkExtents() }; // Early return so we don't have to check for the validity of `nearest` below after the loop exits. if (marks.empty()) @@ -2534,8 +2536,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation const til::point start = _terminal->IsSelectionActive() ? (goUp ? _terminal->GetSelectionAnchor() : _terminal->GetSelectionEnd()) : _terminal->GetTextBuffer().GetCursor().GetPosition(); - std::optional<::ScrollMark> nearest{ std::nullopt }; - const auto& marks{ _terminal->GetScrollMarks() }; + std::optional<::MarkExtents> nearest{ std::nullopt }; + const auto& marks{ _terminal->GetMarkExtents() }; static constexpr til::point worst{ til::CoordTypeMax, til::CoordTypeMax }; til::point bestDistance{ worst }; @@ -2611,8 +2613,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation void ControlCore::_contextMenuSelectMark( const til::point& pos, - bool (*filter)(const ::ScrollMark&), - til::point_span (*getSpan)(const ::ScrollMark&)) + bool (*filter)(const ::MarkExtents&), + til::point_span (*getSpan)(const ::MarkExtents&)) { const auto lock = _terminal->LockForWriting(); @@ -2621,7 +2623,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { return; } - const auto& marks{ _terminal->GetScrollMarks() }; + const auto& marks{ _terminal->GetMarkExtents() }; for (auto&& m : marks) { @@ -2648,20 +2650,20 @@ namespace winrt::Microsoft::Terminal::Control::implementation { _contextMenuSelectMark( _contextMenuBufferPosition, - [](const ::ScrollMark& m) -> bool { return !m.HasCommand(); }, - [](const ::ScrollMark& m) { return til::point_span{ m.end, *m.commandEnd }; }); + [](const ::MarkExtents& m) -> bool { return !m.HasCommand(); }, + [](const ::MarkExtents& m) { return til::point_span{ m.end, *m.commandEnd }; }); } void ControlCore::ContextMenuSelectOutput() { _contextMenuSelectMark( _contextMenuBufferPosition, - [](const ::ScrollMark& m) -> bool { return !m.HasOutput(); }, - [](const ::ScrollMark& m) { return til::point_span{ *m.commandEnd, *m.outputEnd }; }); + [](const ::MarkExtents& m) -> bool { return !m.HasOutput(); }, + [](const ::MarkExtents& m) { return til::point_span{ *m.commandEnd, *m.outputEnd }; }); } bool ControlCore::_clickedOnMark( const til::point& pos, - bool (*filter)(const ::ScrollMark&)) + bool (*filter)(const ::MarkExtents&)) { const auto lock = _terminal->LockForWriting(); @@ -2674,7 +2676,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation } // DO show this if the click was on a mark with a command - const auto& marks{ _terminal->GetScrollMarks() }; + const auto& marks{ _terminal->GetMarkExtents() }; for (auto&& m : marks) { if (filter && filter(m)) @@ -2702,7 +2704,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { // Relies on the anchor set in AnchorContextMenu return _clickedOnMark(_contextMenuBufferPosition, - [](const ::ScrollMark& m) -> bool { return !m.HasCommand(); }); + [](const ::MarkExtents& m) -> bool { return !m.HasCommand(); }); } // Method Description: @@ -2711,6 +2713,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation { // Relies on the anchor set in AnchorContextMenu return _clickedOnMark(_contextMenuBufferPosition, - [](const ::ScrollMark& m) -> bool { return !m.HasOutput(); }); + [](const ::MarkExtents& m) -> bool { return !m.HasOutput(); }); } } diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h index d79df07d1bc..0ce19ff46c6 100644 --- a/src/cascadia/TerminalControl/ControlCore.h +++ b/src/cascadia/TerminalControl/ControlCore.h @@ -399,10 +399,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation void _contextMenuSelectMark( const til::point& pos, - bool (*filter)(const ::ScrollMark&), - til::point_span (*getSpan)(const ::ScrollMark&)); + bool (*filter)(const ::MarkExtents&), + til::point_span (*getSpan)(const ::MarkExtents&)); - bool _clickedOnMark(const til::point& pos, bool (*filter)(const ::ScrollMark&)); + bool _clickedOnMark(const til::point& pos, bool (*filter)(const ::MarkExtents&)); inline bool _IsClosing() const noexcept { diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 7bdb576b5b5..412101a6d0a 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -1503,23 +1503,29 @@ void Terminal::ClearAllMarks() _NotifyScrollEvent(); } -std::vector Terminal::GetScrollMarks() const noexcept +std::vector Terminal::GetMarkRows() const noexcept { // We want to return _no_ marks when we're in the alt buffer, to effectively // hide them. - return _inAltBuffer() ? std::vector{} : std::move(_activeBuffer().GetMarks()); + return _inAltBuffer() ? std::vector{} : std::move(_activeBuffer().GetMarkRows()); +} +std::vector Terminal::GetMarkExtents() const noexcept +{ + // We want to return _no_ marks when we're in the alt buffer, to effectively + // hide them. + return _inAltBuffer() ? std::vector{} : std::move(_activeBuffer().GetMarkExtents()); } -til::color Terminal::GetColorForMark(const ScrollMark& mark) const +til::color Terminal::GetColorForMark(const MarkData& markData) const { - if (mark.data.color.has_value()) + if (markData.color.has_value()) { - return *mark.data.color; + return *markData.color; } const auto& renderSettings = GetRenderSettings(); - switch (mark.data.category) + switch (markData.category) { case MarkCategory::Error: { @@ -1544,11 +1550,6 @@ til::color Terminal::GetColorForMark(const ScrollMark& mark) const std::wstring_view Terminal::CurrentCommand() const { - // if (_currentPromptState != PromptState::Command) - // { - // return L""; - // } - return _activeBuffer().CurrentCommand(); } diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index 37d20114991..7da1575e3cf 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -117,7 +117,8 @@ class Microsoft::Terminal::Core::Terminal final : RenderSettings& GetRenderSettings() noexcept; const RenderSettings& GetRenderSettings() const noexcept; - std::vector GetScrollMarks() const noexcept; + std::vector GetMarkRows() const noexcept; + std::vector GetMarkExtents() const noexcept; void AddMark(const ScrollMark& mark, const til::point& start, const til::point& end, @@ -167,7 +168,7 @@ class Microsoft::Terminal::Core::Terminal final : void ClearMark(); void ClearAllMarks(); - til::color GetColorForMark(const ScrollMark& mark) const; + til::color GetColorForMark(const MarkData& markData) const; #pragma region ITerminalInput // These methods are defined in Terminal.cpp diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index 277acee3c98..9dc9c25af32 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -4353,7 +4353,7 @@ void ConptyRoundtripTests::SimplePromptRegions() VERIFY_IS_TRUE(row0.GetPromptData().has_value()); VERIFY_IS_TRUE(row4.GetPromptData().has_value()); - const auto marks = tb.GetMarks(); + const auto marks = tb.GetMarkExtents(); VERIFY_ARE_EQUAL(2u, marks.size()); { @@ -4450,7 +4450,7 @@ void ConptyRoundtripTests::MultilinePromptRegions() VERIFY_IS_TRUE(row0.GetPromptData().has_value()); VERIFY_IS_TRUE(row5.GetPromptData().has_value()); - const auto marks = tb.GetMarks(); + const auto marks = tb.GetMarkExtents(); VERIFY_ARE_EQUAL(2u, marks.size()); { @@ -4634,7 +4634,7 @@ void ConptyRoundtripTests::ManyMultilinePromptsWithTrailingSpaces() } const WEX::TestExecution::DisableVerifyExceptions disableExceptionsScope; - const auto marks = tb.GetMarks(); + const auto marks = tb.GetMarkExtents(); VERIFY_ARE_EQUAL(3u, marks.size()); Log::Comment(L"Row 0"); diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index e156c0ee33c..a402d4c36fd 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -8557,7 +8557,7 @@ void ScreenBufferTests::SimplePromptRegions() VERIFY_IS_TRUE(row0.GetPromptData().has_value()); VERIFY_IS_TRUE(row4.GetPromptData().has_value()); - const auto marks = tbi.GetMarks(); + const auto marks = tbi.GetMarkExtents(); VERIFY_ARE_EQUAL(2u, marks.size()); { From e8aa067d99f5f5d33c97b8826f7380144b66233a Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 25 Mar 2024 11:58:07 -0500 Subject: [PATCH 13/30] hey it worked for non-shell-integration CMD --- src/buffer/out/textBuffer.cpp | 33 ++++++++++--- src/buffer/out/textBuffer.hpp | 2 +- src/cascadia/TerminalCore/Terminal.cpp | 65 ++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 7 deletions(-) diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index b430726662f..7d8d289aaf3 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -2902,8 +2902,15 @@ std::vector TextBuffer::GetMarkRows() const noexcept } return std::move(marks); } -std::vector TextBuffer::GetMarkExtents() const noexcept + +std::vector TextBuffer::GetMarkExtents(std::optional limit) const noexcept { + if (limit.has_value() && + *limit == 0) + { + return {}; + } + std::vector marks{}; const auto bottom = _estimateOffsetOfLastCommittedRow(); auto lastPromptY = bottom; @@ -2922,8 +2929,15 @@ std::vector TextBuffer::GetMarkExtents() const noexcept // row with text as the bottom marks.push_back(_scrollMarkExtentForRow(promptY, lastPromptY)); + if (limit.has_value() && + marks.size() >= *limit) + { + break; + } + lastPromptY = promptY; } + std::reverse(marks.begin(), marks.end()); return std::move(marks); } @@ -3024,17 +3038,17 @@ MarkExtents TextBuffer::_scrollMarkExtentForRow(const til::CoordType rowOffset, MarkKind lastMarkKind = MarkKind::Output; const auto endThisMark = [&](auto x, auto y) { - if (!startedCommand) + if (startedOutput) { - mark.end = til::point{ x, y }; + mark.outputEnd = til::point{ x, y }; } - else if (!startedOutput /*|| !mark.commandEnd.has_value()*/) // startedCommand = true + /*else */ if (!startedOutput && startedCommand /*|| !mark.commandEnd.has_value()*/) // startedCommand = true { mark.commandEnd = til::point{ x, y }; } - else if (startedOutput) + /*else */ if (!startedCommand) { - mark.outputEnd = til::point{ x, y }; + mark.end = til::point{ x, y }; } }; auto x = 0; @@ -3092,6 +3106,13 @@ MarkExtents TextBuffer::_scrollMarkExtentForRow(const til::CoordType rowOffset, { // endThisMark(x, y); startedOutput = true; + if (!mark.commandEnd.has_value()) + { + // immediately just end the command at the start here, so we can treat this whole run as output + mark.commandEnd = mark.end; // til::point{ x, y }; + startedCommand = true; + } + endThisMark(lastMarkedText.x, lastMarkedText.y); // if (!foundEnd) // { diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index ac0d2462398..ac1f570bf15 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -331,7 +331,7 @@ class TextBuffer final std::vector SearchText(const std::wstring_view& needle, bool caseInsensitive, til::CoordType rowBeg, til::CoordType rowEnd) const; std::vector GetMarkRows() const noexcept; - std::vector GetMarkExtents() const noexcept; + std::vector GetMarkExtents(std::optional limit = std::nullopt) const noexcept; void ClearMarksInRange(const til::point start, const til::point end); void ClearAllMarks() noexcept; // void ScrollMarks(const int delta); diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 412101a6d0a..533ae1653e9 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -736,6 +736,71 @@ TerminalInput::OutputType Terminal::SendCharEvent(const wchar_t ch, const WORD s // MarkOutputStart(); // TODO! ^^^^^^^^^^ that's not on Terminal anymore. That's a text attr now. + + // We need to be a little tricky here, to try and support folks that are + // auto-marking prompts, but don't necessarily have the rest of shell + // integration enabled. + // + // We'll set the current attributes to Output, so that the output after + // here is marked as the command output. But we also need to make sure + // that a mark was started. + // + // We can't just check if the current row has a mark - there could be a + // multiline prompt. + // + // What we're gonna do is get the most recent mark above the cursor. If it's got an outputEnd, then + + // Only get the most recent mark. + const auto mostRecentMarks = _activeBuffer().GetMarkExtents(1u); + if (!mostRecentMarks.empty()) + { + const auto& mostRecentMark = mostRecentMarks[0]; + if (mostRecentMark.HasOutput()) + { + // The most recent command mark had output. That suggests that either: + // * shell integration wasn't enabled (but the user would still + // like lines with enters to be marked as prompts) + // * or we're in the middle of a command that's ongoing. + + // If the mark doesn't have any command - then we know we're + // playing silly games with just marking whole lines as prompts, + // then immediately going to output. + // --> add a new mark to this row, set all the attrs in this + // row to be Prompt, and set the current attrs to Output. + // + // If it does have a command, then we're still in the output of + // that command. + // --> the current attrs should already be set to Output. + if (!mostRecentMark.HasCommand()) + { + auto& row = _activeBuffer().GetMutableRowByOffset(_activeBuffer().GetCursor().GetPosition().y); + row.StartPrompt(); + for (auto& [attr, len] : row.Attributes().runs()) + { + attr.SetMarkAttributes(MarkKind::Prompt); + } + } + } + else + { + // The most recent command mark _didn't_ have output yet. Great! + // we'll leave it alone, and just start treating text as Output. + } + } + else + { + // There were no marks at all! + // --> add a new mark to this row, set all the attrs in this row + // to be Prompt, and set the current attrs to Output. + + auto& row = _activeBuffer().GetMutableRowByOffset(_activeBuffer().GetCursor().GetPosition().y); + row.StartPrompt(); + for (auto& [attr, len] : row.Attributes().runs()) + { + attr.SetMarkAttributes(MarkKind::Prompt); + } + } + auto attr = _activeBuffer().GetCurrentAttributes(); attr.SetMarkAttributes(MarkKind::Output); SetTextAttributes(attr); From 73cd1b3a993e2ace1ded9c79787adefa2499019d Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 25 Mar 2024 13:59:42 -0500 Subject: [PATCH 14/30] no error code is different than 0 --- src/buffer/out/Row.cpp | 9 ++++++--- src/buffer/out/Row.hpp | 2 +- src/buffer/out/textBuffer.cpp | 20 +++----------------- src/cascadia/TerminalCore/Terminal.cpp | 21 ++++----------------- src/cascadia/TerminalCore/TerminalApi.cpp | 7 ------- 5 files changed, 14 insertions(+), 45 deletions(-) diff --git a/src/buffer/out/Row.cpp b/src/buffer/out/Row.cpp index f32a60573ed..622b4afa090 100644 --- a/src/buffer/out/Row.cpp +++ b/src/buffer/out/Row.cpp @@ -1213,11 +1213,14 @@ void ROW::StartPrompt() noexcept }; } } -void ROW::EndCommand(uint32_t exitCode) noexcept +void ROW::EndCommand(std::optional error) noexcept { if (_promptData.has_value()) { - _promptData->exitCode = exitCode; - _promptData->category = exitCode == 0 ? MarkCategory::Success : MarkCategory::Error; + _promptData->exitCode = error; + if (error.has_value()) + { + _promptData->category = *error == 0 ? MarkCategory::Success : MarkCategory::Error; + } } } diff --git a/src/buffer/out/Row.hpp b/src/buffer/out/Row.hpp index f5978761d12..5c51546f3de 100644 --- a/src/buffer/out/Row.hpp +++ b/src/buffer/out/Row.hpp @@ -190,7 +190,7 @@ class ROW final const std::optional& GetPromptData() const noexcept; void StartPrompt() noexcept; - void EndCommand(uint32_t exitCode) noexcept; + void EndCommand(std::optional error) noexcept; #ifdef UNIT_TESTING friend constexpr bool operator==(const ROW& a, const ROW& b) noexcept; diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 7d8d289aaf3..9432943bc88 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -3072,18 +3072,12 @@ MarkExtents TextBuffer::_scrollMarkExtentForRow(const til::CoordType rowOffset, if (markKind != MarkKind::None) { lastMarkedText = { nextX, y }; - // } - // if (markKind != MarkKind::None && - // markKind != lastMarkKind) - // { if (markKind == MarkKind::Prompt) { if (startedCommand || startedOutput) { // we got a _new_ prompt. bail out. - // endThisMark(x, y); - // endThisMark(lastMarkedText.x, lastMarkedText.y); break; } if (!startedPrompt) @@ -3096,15 +3090,11 @@ MarkExtents TextBuffer::_scrollMarkExtentForRow(const til::CoordType rowOffset, } else if (markKind == MarkKind::Command && startedPrompt) { - // mark.end = til::point{x, y}; - // foundEnd = true; - // endThisMark(x, y); startedCommand = true; endThisMark(lastMarkedText.x, lastMarkedText.y); } - else if ((markKind == MarkKind::Output /*|| markKind == MarkKind::None*/) && startedPrompt) + else if ((markKind == MarkKind::Output) && startedPrompt) { - // endThisMark(x, y); startedOutput = true; if (!mark.commandEnd.has_value()) { @@ -3114,12 +3104,6 @@ MarkExtents TextBuffer::_scrollMarkExtentForRow(const til::CoordType rowOffset, } endThisMark(lastMarkedText.x, lastMarkedText.y); - // if (!foundEnd) - // { - // mark.end = til::point{x, y}; - // foundEnd = true; - // } - // mark.commandEnd = til::point{x, y}; } // Otherwise, we've changed from any state -> any state, and it doesn't really matter. if (markKind != MarkKind::None) @@ -3136,6 +3120,8 @@ MarkExtents TextBuffer::_scrollMarkExtentForRow(const til::CoordType rowOffset, // Okay, we're at the bottom of the buffer? Yea, just return what we found. if (!startedCommand) { + // If we never got to a Command or Output run, then we never set .end. + // Set it here to the last run we saw. endThisMark(lastMarkedText.x, lastMarkedText.y); } return mark; diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 533ae1653e9..386f8f36aa5 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -720,23 +720,6 @@ TerminalInput::OutputType Terminal::SendCharEvent(const wchar_t ch, const WORD s // Then treat this line like it's a prompt mark. if (_autoMarkPrompts && vkey == VK_RETURN && !_inAltBuffer()) { - // * If we have a current prompt: - // - Then we did know that the prompt started, (we may have also - // already gotten a MarkCommandStart sequence). The user has pressed - // enter, and we're treating that like the prompt has now ended. - // - Perform a FTCS_COMMAND_EXECUTED, so that we start marking this - // as output. - // - This enables CMD to have full FTCS support, even though there's - // no point in CMD to insert a "pre exec" hook - // * Else: We don't have a prompt. We don't know anything else, but we - // can set the whole line as the prompt, no command, and start the - // command_executed now. - // - // Fortunately, MarkOutputStart will do all this logic for us! - // MarkOutputStart(); - - // TODO! ^^^^^^^^^^ that's not on Terminal anymore. That's a text attr now. - // We need to be a little tricky here, to try and support folks that are // auto-marking prompts, but don't necessarily have the rest of shell // integration enabled. @@ -779,6 +762,8 @@ TerminalInput::OutputType Terminal::SendCharEvent(const wchar_t ch, const WORD s { attr.SetMarkAttributes(MarkKind::Prompt); } + // This changed the scrollbar marks - raise a notification to update them + _NotifyScrollEvent(); } } else @@ -799,6 +784,8 @@ TerminalInput::OutputType Terminal::SendCharEvent(const wchar_t ch, const WORD s { attr.SetMarkAttributes(MarkKind::Prompt); } + // This changed the scrollbar marks - raise a notification to update them + _NotifyScrollEvent(); } auto attr = _activeBuffer().GetCurrentAttributes(); diff --git a/src/cascadia/TerminalCore/TerminalApi.cpp b/src/cascadia/TerminalCore/TerminalApi.cpp index 277ade7394b..31ae5788256 100644 --- a/src/cascadia/TerminalCore/TerminalApi.cpp +++ b/src/cascadia/TerminalCore/TerminalApi.cpp @@ -497,13 +497,6 @@ void Terminal::NotifyBufferRotation(const int delta) // manually erase our pattern intervals since the locations have changed now _patternIntervalTree = {}; - // auto& marks{ _activeBuffer().GetMarks() }; - // const auto hasScrollMarks = marks.size() > 0; - // if (hasScrollMarks) - // { - // _activeBuffer().ScrollMarks(-delta); - // } - const auto oldScrollOffset = _scrollOffset; _PreserveUserScrollOffset(delta); if (_scrollOffset != oldScrollOffset || AlwaysNotifyOnBufferRotation()) From 68f8994223777f708cbe9c2008f11153e3a5cce7 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 25 Mar 2024 16:28:12 -0500 Subject: [PATCH 15/30] start killing lots of old code --- src/buffer/out/Row.cpp | 18 +- src/buffer/out/Row.hpp | 2 +- src/buffer/out/textBuffer.cpp | 181 +++++++++++++----- src/buffer/out/textBuffer.hpp | 10 +- src/cascadia/TerminalCore/Terminal.cpp | 106 +++++----- src/cascadia/TerminalCore/Terminal.hpp | 5 - src/cascadia/TerminalCore/TerminalApi.cpp | 134 ------------- src/host/outputStream.cpp | 19 -- src/host/outputStream.hpp | 5 - src/terminal/adapter/ITerminalApi.hpp | 5 - src/terminal/adapter/adaptDispatch.cpp | 33 ++-- .../adapter/ut_adapter/adapterTest.cpp | 16 -- 12 files changed, 214 insertions(+), 320 deletions(-) diff --git a/src/buffer/out/Row.cpp b/src/buffer/out/Row.cpp index 622b4afa090..1111734b85b 100644 --- a/src/buffer/out/Row.cpp +++ b/src/buffer/out/Row.cpp @@ -1189,21 +1189,6 @@ const std::optional& ROW::GetPromptData() const noexcept void ROW::StartPrompt() noexcept { - // TODO! resurrect somehow - - // static bool logged = false; - // if (!logged) - // { - // TraceLoggingWrite( - // g_hCTerminalCoreProvider, - // "ShellIntegrationMarkAdded", - // TraceLoggingDescription("A mark was added via VT at least once"), - // TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), - // TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); - - // logged = true; - // } - if (!_promptData.has_value()) { _promptData = MarkData{ @@ -1213,7 +1198,8 @@ void ROW::StartPrompt() noexcept }; } } -void ROW::EndCommand(std::optional error) noexcept + +void ROW::EndOutput(std::optional error) noexcept { if (_promptData.has_value()) { diff --git a/src/buffer/out/Row.hpp b/src/buffer/out/Row.hpp index 5c51546f3de..c716dca7ef9 100644 --- a/src/buffer/out/Row.hpp +++ b/src/buffer/out/Row.hpp @@ -190,7 +190,7 @@ class ROW final const std::optional& GetPromptData() const noexcept; void StartPrompt() noexcept; - void EndCommand(std::optional error) noexcept; + void EndOutput(std::optional error) noexcept; #ifdef UNIT_TESTING friend constexpr bool operator==(const ROW& a, const ROW& b) noexcept; diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 9432943bc88..2950a2d2ded 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -3032,7 +3032,6 @@ MarkExtents TextBuffer::_scrollMarkExtentForRow(const til::CoordType rowOffset, }; bool startedPrompt = false; - // bool foundEnd = false; bool startedCommand = false; bool startedOutput = false; MarkKind lastMarkKind = MarkKind::Output; @@ -3042,11 +3041,11 @@ MarkExtents TextBuffer::_scrollMarkExtentForRow(const til::CoordType rowOffset, { mark.outputEnd = til::point{ x, y }; } - /*else */ if (!startedOutput && startedCommand /*|| !mark.commandEnd.has_value()*/) // startedCommand = true + if (!startedOutput && startedCommand) { mark.commandEnd = til::point{ x, y }; } - /*else */ if (!startedCommand) + if (!startedCommand) { mark.end = til::point{ x, y }; } @@ -3172,19 +3171,6 @@ std::wstring TextBuffer::_commandForRow(const til::CoordType rowOffset, const ti std::wstring TextBuffer::CurrentCommand() const { - // if (_marks.size() == 0) - // { - // return L""; - // } - - // const auto& curr{ _marks.back() }; - // const auto& start{ curr.end }; - // const auto& end{ GetCursor().GetPosition() }; - - // const auto line = start.y; - // const auto& row = GetRowByOffset(line); - // return row.GetText(start.x, end.x); - auto promptY = GetCursor().GetPosition().y; for (; promptY >= 0; promptY--) { @@ -3233,47 +3219,138 @@ std::vector TextBuffer::Commands() const return std::move(commands); } -// 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) noexcept -// { -// if (_marks.empty()) -// { -// return; -// } -// auto& curr{ _marks.back() }; -// curr.commandEnd = pos; -// } -// void TextBuffer::SetCurrentOutputEnd(const til::point pos, ::MarkCategory category) noexcept -// { -// if (_marks.empty()) -// { -// return; -// } -// auto& curr{ _marks.back() }; -// curr.outputEnd = pos; -// curr.category = category; -// } - void TextBuffer::StartPrompt() { const auto currentRowOffset = GetCursor().GetPosition().y; auto& currentRow = GetMutableRowByOffset(currentRowOffset); currentRow.StartPrompt(); - // auto& rowPromptData = currRow.GetPromptData(); - // if (!rowPromptData.has_value()) - // { - // rowPromptData = MarkData{}; - // } + + auto attr = GetCurrentAttributes(); + attr.SetMarkAttributes(MarkKind::Prompt); + SetCurrentAttributes(attr); +} + +void TextBuffer::StartCommand() +{ + const auto mostRecentMarks = GetMarkExtents(1u); + if (!mostRecentMarks.empty()) + { + const auto& mostRecentMark = mostRecentMarks[0]; + if (mostRecentMark.HasOutput()) + { + // The most recent command mark had output. That suggests that either: + // * shell integration wasn't enabled (but the user would still + // like lines with enters to be marked as prompts) + // * or we're in the middle of a command that's ongoing. + + // If the mark doesn't have any command - then we know we're + // playing silly games with just marking whole lines as prompts, + // then immediately going to output. + // --> add a new mark to this row, set all the attrs in this + // row to be Prompt, and set the current attrs to Output. + // + // If it does have a command, then we're still in the output of + // that command. + // --> the current attrs should already be set to Output. + if (!mostRecentMark.HasCommand()) + { + auto& row = GetMutableRowByOffset(GetCursor().GetPosition().y); + row.StartPrompt(); + // for (auto& [attr, len] : row.Attributes().runs()) + // { + // attr.SetMarkAttributes(MarkKind::Prompt); + // } + // // This changed the scrollbar marks - raise a notification to update them + // _NotifyScrollEvent(); + } + } + else + { + // The most recent command mark _didn't_ have output yet. Great! + // we'll leave it alone, and just start treating text as Command. + } + } + else + { + // There were no marks at all! + // --> add a new mark to this row, set all the attrs in this row + // to be Prompt, and set the current attrs to Output. + + auto& row = GetMutableRowByOffset(GetCursor().GetPosition().y); + row.StartPrompt(); + // for (auto& [attr, len] : row.Attributes().runs()) + // { + // attr.SetMarkAttributes(MarkKind::Prompt); + // } + // // This changed the scrollbar marks - raise a notification to update them + // _NotifyScrollEvent(); + } + + auto attr = GetCurrentAttributes(); + attr.SetMarkAttributes(MarkKind::Command); + SetCurrentAttributes(attr); } -void TextBuffer::EndCommand(std::optional error) +void TextBuffer::StartOutput() +{ + const auto mostRecentMarks = GetMarkExtents(1u); + if (!mostRecentMarks.empty()) + { + const auto& mostRecentMark = mostRecentMarks[0]; + if (mostRecentMark.HasOutput()) + { + // The most recent command mark had output. That suggests that either: + // * shell integration wasn't enabled (but the user would still + // like lines with enters to be marked as prompts) + // * or we're in the middle of a command that's ongoing. + + // If the mark doesn't have any command - then we know we're + // playing silly games with just marking whole lines as prompts, + // then immediately going to output. + // --> add a new mark to this row, set all the attrs in this + // row to be Prompt, and set the current attrs to Output. + // + // If it does have a command, then we're still in the output of + // that command. + // --> the current attrs should already be set to Output. + if (!mostRecentMark.HasCommand()) + { + auto& row = GetMutableRowByOffset(GetCursor().GetPosition().y); + row.StartPrompt(); + // for (auto& [attr, len] : row.Attributes().runs()) + // { + // attr.SetMarkAttributes(MarkKind::Prompt); + // } + // // This changed the scrollbar marks - raise a notification to update them + // _NotifyScrollEvent(); + } + } + else + { + // The most recent command mark _didn't_ have output yet. Great! + // we'll leave it alone, and just start treating text as Output. + } + } + else + { + // There were no marks at all! + // --> add a new mark to this row, set all the attrs in this row + // to be Prompt, and set the current attrs to Output. + + auto& row = GetMutableRowByOffset(GetCursor().GetPosition().y); + row.StartPrompt(); + // for (auto& [attr, len] : row.Attributes().runs()) + // { + // attr.SetMarkAttributes(MarkKind::Prompt); + // } + // // This changed the scrollbar marks - raise a notification to update them + // _NotifyScrollEvent(); + } + auto attr = GetCurrentAttributes(); + attr.SetMarkAttributes(MarkKind::Output); + SetCurrentAttributes(attr); +} + +void TextBuffer::EndOutput(std::optional error) { for (auto y = GetCursor().GetPosition().y; y >= 0; y--) { @@ -3281,7 +3358,7 @@ void TextBuffer::EndCommand(std::optional error) auto& rowPromptData = currRow.GetPromptData(); if (rowPromptData.has_value()) { - currRow.EndCommand(error.value_or(0u)); + currRow.EndOutput(error); return; } } diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index ac1f570bf15..08a7ba41e4c 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -334,19 +334,15 @@ class TextBuffer final std::vector GetMarkExtents(std::optional limit = std::nullopt) 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) noexcept; - // void SetCurrentOutputEnd(const til::point pos, ::MarkCategory category) noexcept; std::wstring CurrentCommand() const; std::wstring _commandForRow(const til::CoordType rowOffset, const til::CoordType bottomInclusive) const; MarkExtents _scrollMarkExtentForRow(const til::CoordType rowOffset, const til::CoordType bottomInclusive) const; std::vector Commands() const; void StartPrompt(); - void EndCommand(std::optional error); + void StartCommand(); + void StartOutput(); + void EndOutput(std::optional error); private: void _reserve(til::size screenBufferSize, const TextAttribute& defaultAttributes); diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 386f8f36aa5..30bf08296d6 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -733,53 +733,67 @@ TerminalInput::OutputType Terminal::SendCharEvent(const wchar_t ch, const WORD s // // What we're gonna do is get the most recent mark above the cursor. If it's got an outputEnd, then - // Only get the most recent mark. - const auto mostRecentMarks = _activeBuffer().GetMarkExtents(1u); - if (!mostRecentMarks.empty()) + // // Only get the most recent mark. + // const auto mostRecentMarks = _activeBuffer().GetMarkExtents(1u); + // if (!mostRecentMarks.empty()) + // { + // const auto& mostRecentMark = mostRecentMarks[0]; + // if (mostRecentMark.HasOutput()) + // { + // // The most recent command mark had output. That suggests that either: + // // * shell integration wasn't enabled (but the user would still + // // like lines with enters to be marked as prompts) + // // * or we're in the middle of a command that's ongoing. + + // // If the mark doesn't have any command - then we know we're + // // playing silly games with just marking whole lines as prompts, + // // then immediately going to output. + // // --> add a new mark to this row, set all the attrs in this + // // row to be Prompt, and set the current attrs to Output. + // // + // // If it does have a command, then we're still in the output of + // // that command. + // // --> the current attrs should already be set to Output. + // if (!mostRecentMark.HasCommand()) + // { + // auto& row = _activeBuffer().GetMutableRowByOffset(_activeBuffer().GetCursor().GetPosition().y); + // row.StartPrompt(); + // for (auto& [attr, len] : row.Attributes().runs()) + // { + // attr.SetMarkAttributes(MarkKind::Prompt); + // } + // // This changed the scrollbar marks - raise a notification to update them + // _NotifyScrollEvent(); + // } + // } + // else + // { + // // The most recent command mark _didn't_ have output yet. Great! + // // we'll leave it alone, and just start treating text as Output. + // } + // } + // else + // { + // // There were no marks at all! + // // --> add a new mark to this row, set all the attrs in this row + // // to be Prompt, and set the current attrs to Output. + + // auto& row = _activeBuffer().GetMutableRowByOffset(_activeBuffer().GetCursor().GetPosition().y); + // row.StartPrompt(); + // for (auto& [attr, len] : row.Attributes().runs()) + // { + // attr.SetMarkAttributes(MarkKind::Prompt); + // } + // // This changed the scrollbar marks - raise a notification to update them + // _NotifyScrollEvent(); + // } + + auto& row = _activeBuffer().GetMutableRowByOffset(_activeBuffer().GetCursor().GetPosition().y); + const bool hadMarkBefore = row.GetPromptData().has_value(); + _activeBuffer().StartOutput(); + const bool hadMarkAfter = row.GetPromptData().has_value(); + if (!hadMarkBefore && hadMarkAfter) { - const auto& mostRecentMark = mostRecentMarks[0]; - if (mostRecentMark.HasOutput()) - { - // The most recent command mark had output. That suggests that either: - // * shell integration wasn't enabled (but the user would still - // like lines with enters to be marked as prompts) - // * or we're in the middle of a command that's ongoing. - - // If the mark doesn't have any command - then we know we're - // playing silly games with just marking whole lines as prompts, - // then immediately going to output. - // --> add a new mark to this row, set all the attrs in this - // row to be Prompt, and set the current attrs to Output. - // - // If it does have a command, then we're still in the output of - // that command. - // --> the current attrs should already be set to Output. - if (!mostRecentMark.HasCommand()) - { - auto& row = _activeBuffer().GetMutableRowByOffset(_activeBuffer().GetCursor().GetPosition().y); - row.StartPrompt(); - for (auto& [attr, len] : row.Attributes().runs()) - { - attr.SetMarkAttributes(MarkKind::Prompt); - } - // This changed the scrollbar marks - raise a notification to update them - _NotifyScrollEvent(); - } - } - else - { - // The most recent command mark _didn't_ have output yet. Great! - // we'll leave it alone, and just start treating text as Output. - } - } - else - { - // There were no marks at all! - // --> add a new mark to this row, set all the attrs in this row - // to be Prompt, and set the current attrs to Output. - - auto& row = _activeBuffer().GetMutableRowByOffset(_activeBuffer().GetCursor().GetPosition().y); - row.StartPrompt(); for (auto& [attr, len] : row.Attributes().runs()) { attr.SetMarkAttributes(MarkKind::Prompt); diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index 7da1575e3cf..2349e580d4d 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -152,11 +152,6 @@ class Microsoft::Terminal::Core::Terminal final : void UseAlternateScreenBuffer(const TextAttribute& attrs) override; void UseMainScreenBuffer() override; - void MarkPrompt(const ScrollMark& mark) override; - void MarkCommandStart() override; - void MarkOutputStart() override; - void MarkCommandFinish(std::optional error) override; - bool IsConsolePty() const noexcept override; bool IsVtInputEnabled() const noexcept override; void NotifyAccessibilityChange(const til::rect& changedRect) noexcept override; diff --git a/src/cascadia/TerminalCore/TerminalApi.cpp b/src/cascadia/TerminalCore/TerminalApi.cpp index 31ae5788256..121910cd697 100644 --- a/src/cascadia/TerminalCore/TerminalApi.cpp +++ b/src/cascadia/TerminalCore/TerminalApi.cpp @@ -300,140 +300,6 @@ void Terminal::UseMainScreenBuffer() _activeBuffer().TriggerRedrawAll(); } -// NOTE: This is the version of AddMark that comes from VT -void Terminal::MarkPrompt(const ScrollMark& /*mark*/) -{ - // _assertLocked(); - - // static bool logged = false; - // if (!logged) - // { - // TraceLoggingWrite( - // g_hCTerminalCoreProvider, - // "ShellIntegrationMarkAdded", - // TraceLoggingDescription("A mark was added via VT at least once"), - // TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), - // TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); - - // logged = true; - // } - - // const til::point cursorPos{ _activeBuffer().GetCursor().GetPosition() }; - // AddMark(mark, cursorPos, cursorPos, false); - - // if (mark.category == MarkCategory::Prompt) - // { - // _currentPromptState = PromptState::Prompt; - // } -} - -void Terminal::MarkCommandStart() -{ - // _assertLocked(); - - // const til::point cursorPos{ _activeBuffer().GetCursor().GetPosition() }; - - // if ((_currentPromptState == PromptState::Prompt) && - // (_activeBuffer().GetMarks().size() > 0)) - // { - // // We were in the right state, and there's a previous mark to work - // // with. - // // - // //We can just do the work below safely. - // } - // else if (_currentPromptState == PromptState::Command) - // { - // // We're already in the command state. We don't want to end the current - // // mark. We don't want to make a new one. We want to just leave the - // // current command going. - // return; - // } - // else - // { - // // If there was no last mark, or we're in a weird state, - // // then abandon the current state, and just insert a new Prompt mark that - // // start's & ends here, and got to State::Command. - - // ScrollMark mark; - // mark.category = MarkCategory::Prompt; - // AddMark(mark, cursorPos, cursorPos, false); - // } - // _activeBuffer().SetCurrentPromptEnd(cursorPos); - // _currentPromptState = PromptState::Command; -} - -void Terminal::MarkOutputStart() -{ - // _assertLocked(); - - // const til::point cursorPos{ _activeBuffer().GetCursor().GetPosition() }; - - // if ((_currentPromptState == PromptState::Command) && - // (_activeBuffer().GetMarks().size() > 0)) - // { - // // We were in the right state, and there's a previous mark to work - // // with. - // // - // //We can just do the work below safely. - // } - // else if (_currentPromptState == PromptState::Output) - // { - // // We're already in the output state. We don't want to end the current - // // mark. We don't want to make a new one. We want to just leave the - // // current output going. - // return; - // } - // else - // { - // // If there was no last mark, or we're in a weird state, - // // then abandon the current state, and just insert a new Prompt mark that - // // start's & ends here, and the command ends here, and go to State::Output. - - // ScrollMark mark; - // mark.category = MarkCategory::Prompt; - // AddMark(mark, cursorPos, cursorPos, false); - // } - // _activeBuffer().SetCurrentCommandEnd(cursorPos); - // _currentPromptState = PromptState::Output; -} - -void Terminal::MarkCommandFinish(std::optional /*error*/) -{ - // _assertLocked(); - - // const til::point cursorPos{ _activeBuffer().GetCursor().GetPosition() }; - // auto category = MarkCategory::Prompt; - // if (error.has_value()) - // { - // category = *error == 0u ? - // MarkCategory::Success : - // MarkCategory::Error; - // } - - // if ((_currentPromptState == PromptState::Output) && - // (_activeBuffer().GetMarks().size() > 0)) - // { - // // We were in the right state, and there's a previous mark to work - // // with. - // // - // //We can just do the work below safely. - // } - // else - // { - // // If there was no last mark, or we're in a weird state, then abandon - // // the current state, and just insert a new Prompt mark that start's & - // // ends here, and the command ends here, AND the output ends here. and - // // go to State::Output. - - // ScrollMark mark; - // mark.category = MarkCategory::Prompt; - // AddMark(mark, cursorPos, cursorPos, false); - // _activeBuffer().SetCurrentCommandEnd(cursorPos); - // } - // _activeBuffer().SetCurrentOutputEnd(cursorPos, category); - // _currentPromptState = PromptState::None; -} - // Method Description: // - Reacts to a client asking us to show or hide the window. // Arguments: diff --git a/src/host/outputStream.cpp b/src/host/outputStream.cpp index 70b270e3cd8..044889544c7 100644 --- a/src/host/outputStream.cpp +++ b/src/host/outputStream.cpp @@ -424,25 +424,6 @@ void ConhostInternalGetSet::NotifyBufferRotation(const int delta) } } -void ConhostInternalGetSet::MarkPrompt(const ::ScrollMark& /*mark*/) -{ - // Not implemented for conhost. -} - -void ConhostInternalGetSet::MarkCommandStart() -{ - // Not implemented for conhost. -} - -void ConhostInternalGetSet::MarkOutputStart() -{ - // Not implemented for conhost. -} - -void ConhostInternalGetSet::MarkCommandFinish(std::optional /*error*/) -{ - // Not implemented for conhost. -} void ConhostInternalGetSet::InvokeCompletions(std::wstring_view /*menuJson*/, unsigned int /*replaceLength*/) { // Not implemented for conhost. diff --git a/src/host/outputStream.hpp b/src/host/outputStream.hpp index 45ea6052e21..850efe9892a 100644 --- a/src/host/outputStream.hpp +++ b/src/host/outputStream.hpp @@ -69,11 +69,6 @@ class ConhostInternalGetSet final : public Microsoft::Console::VirtualTerminal:: void NotifyAccessibilityChange(const til::rect& changedRect) override; void NotifyBufferRotation(const int delta) override; - void MarkPrompt(const ScrollMark& mark) override; - void MarkCommandStart() override; - void MarkOutputStart() override; - void MarkCommandFinish(std::optional error) override; - void InvokeCompletions(std::wstring_view menuJson, unsigned int replaceLength) override; private: diff --git a/src/terminal/adapter/ITerminalApi.hpp b/src/terminal/adapter/ITerminalApi.hpp index 3375a968b04..24125780f4b 100644 --- a/src/terminal/adapter/ITerminalApi.hpp +++ b/src/terminal/adapter/ITerminalApi.hpp @@ -81,11 +81,6 @@ namespace Microsoft::Console::VirtualTerminal virtual void NotifyAccessibilityChange(const til::rect& changedRect) = 0; virtual void NotifyBufferRotation(const int delta) = 0; - virtual void MarkPrompt(const ScrollMark& mark) = 0; - virtual void MarkCommandStart() = 0; - virtual void MarkOutputStart() = 0; - virtual void MarkCommandFinish(std::optional error) = 0; - virtual void InvokeCompletions(std::wstring_view menuJson, unsigned int replaceLength) = 0; }; } diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index 8905529975d..a67bc2ddd8b 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -3671,7 +3671,8 @@ bool AdaptDispatch::DoConEmuAction(const std::wstring_view string) // This seems like basically the same as 133;B - the end of the prompt, the start of the commandline. else if (subParam == 12) { - _api.MarkCommandStart(); + // _api.MarkCommandStart(); + _api.GetTextBuffer().StartCommand(); return true; } @@ -3762,9 +3763,9 @@ bool AdaptDispatch::DoFinalTermAction(const std::wstring_view string) case L'A': // FTCS_PROMPT { // Simply just mark this line as a prompt line. - auto attr = _api.GetTextBuffer().GetCurrentAttributes(); - attr.SetMarkAttributes(MarkKind::Prompt); - _api.SetTextAttributes(attr); + // auto attr = _api.GetTextBuffer().GetCurrentAttributes(); + // attr.SetMarkAttributes(MarkKind::Prompt); + // _api.SetTextAttributes(attr); _api.GetTextBuffer().StartPrompt(); // auto& tb = _api.GetTextBuffer(); // auto& cursor= tb.GetCursor(); @@ -3775,18 +3776,22 @@ bool AdaptDispatch::DoFinalTermAction(const std::wstring_view string) } case L'B': // FTCS_COMMAND_START { - auto attr = _api.GetTextBuffer().GetCurrentAttributes(); - attr.SetMarkAttributes(MarkKind::Command); - _api.SetTextAttributes(attr); + // auto attr = _api.GetTextBuffer().GetCurrentAttributes(); + // attr.SetMarkAttributes(MarkKind::Command); + // _api.SetTextAttributes(attr); + + _api.GetTextBuffer().StartCommand(); handled = true; break; } case L'C': // FTCS_COMMAND_EXECUTED { - auto attr = _api.GetTextBuffer().GetCurrentAttributes(); - attr.SetMarkAttributes(MarkKind::Output); - _api.SetTextAttributes(attr); + // auto attr = _api.GetTextBuffer().GetCurrentAttributes(); + // attr.SetMarkAttributes(MarkKind::Output); + // _api.SetTextAttributes(attr); + + _api.GetTextBuffer().StartOutput(); handled = true; break; @@ -3808,10 +3813,10 @@ bool AdaptDispatch::DoFinalTermAction(const std::wstring_view string) UINT_MAX; } - auto attr = _api.GetTextBuffer().GetCurrentAttributes(); - attr.SetMarkAttributes(MarkKind::Output); - _api.SetTextAttributes(attr); - _api.GetTextBuffer().EndCommand(error); + // auto attr = _api.GetTextBuffer().GetCurrentAttributes(); + // attr.SetMarkAttributes(MarkKind::Output); + // _api.SetTextAttributes(attr); + _api.GetTextBuffer().EndOutput(error); // _api.MarkCommandFinish(error); handled = true; diff --git a/src/terminal/adapter/ut_adapter/adapterTest.cpp b/src/terminal/adapter/ut_adapter/adapterTest.cpp index da8b7a35d3a..c4012933dc1 100644 --- a/src/terminal/adapter/ut_adapter/adapterTest.cpp +++ b/src/terminal/adapter/ut_adapter/adapterTest.cpp @@ -215,22 +215,6 @@ class TestGetSet final : public ITerminalApi Log::Comment(L"NotifyBufferRotation MOCK called..."); } - void MarkPrompt(const ScrollMark& /*mark*/) override - { - Log::Comment(L"MarkPrompt MOCK called..."); - } - void MarkCommandStart() override - { - Log::Comment(L"MarkCommandStart MOCK called..."); - } - void MarkOutputStart() override - { - Log::Comment(L"MarkOutputStart MOCK called..."); - } - void MarkCommandFinish(std::optional /*error*/) override - { - Log::Comment(L"MarkCommandFinish MOCK called..."); - } void InvokeCompletions(std::wstring_view menuJson, unsigned int replaceLength) override { Log::Comment(L"InvokeCompletions MOCK called..."); From f1ff25844a166e8eb63f7aa8a4c0c4b832f5b656 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 25 Mar 2024 16:50:07 -0500 Subject: [PATCH 16/30] some code cleanup --- src/buffer/out/textBuffer.cpp | 86 ++++++-------------------- src/buffer/out/textBuffer.hpp | 5 +- src/cascadia/TerminalCore/Terminal.cpp | 72 ++------------------- 3 files changed, 26 insertions(+), 137 deletions(-) diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 2950a2d2ded..8a34bbe84a6 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -3230,8 +3230,13 @@ void TextBuffer::StartPrompt() SetCurrentAttributes(attr); } -void TextBuffer::StartCommand() +bool TextBuffer::_createPromptMarkIfNeeded() { + // We might get here out-of-order, without seeing a StartPrompt (FTCS A) + // first. Since StartPrompt actually sets up the prompt mark on the ROW, we + // need to do a bit of extra work here to start a new mark (if the last one + // wasn't in an appropriate state). + const auto mostRecentMarks = GetMarkExtents(1u); if (!mostRecentMarks.empty()) { @@ -3256,18 +3261,13 @@ void TextBuffer::StartCommand() { auto& row = GetMutableRowByOffset(GetCursor().GetPosition().y); row.StartPrompt(); - // for (auto& [attr, len] : row.Attributes().runs()) - // { - // attr.SetMarkAttributes(MarkKind::Prompt); - // } - // // This changed the scrollbar marks - raise a notification to update them - // _NotifyScrollEvent(); + return true; } } else { // The most recent command mark _didn't_ have output yet. Great! - // we'll leave it alone, and just start treating text as Command. + // we'll leave it alone, and just start treating text as Command or Output. } } else @@ -3278,76 +3278,26 @@ void TextBuffer::StartCommand() auto& row = GetMutableRowByOffset(GetCursor().GetPosition().y); row.StartPrompt(); - // for (auto& [attr, len] : row.Attributes().runs()) - // { - // attr.SetMarkAttributes(MarkKind::Prompt); - // } - // // This changed the scrollbar marks - raise a notification to update them - // _NotifyScrollEvent(); + return true; } + return false; +} +bool TextBuffer::StartCommand() +{ + const auto createdMark = _createPromptMarkIfNeeded(); auto attr = GetCurrentAttributes(); attr.SetMarkAttributes(MarkKind::Command); SetCurrentAttributes(attr); + return createdMark; } -void TextBuffer::StartOutput() +bool TextBuffer::StartOutput() { - const auto mostRecentMarks = GetMarkExtents(1u); - if (!mostRecentMarks.empty()) - { - const auto& mostRecentMark = mostRecentMarks[0]; - if (mostRecentMark.HasOutput()) - { - // The most recent command mark had output. That suggests that either: - // * shell integration wasn't enabled (but the user would still - // like lines with enters to be marked as prompts) - // * or we're in the middle of a command that's ongoing. - - // If the mark doesn't have any command - then we know we're - // playing silly games with just marking whole lines as prompts, - // then immediately going to output. - // --> add a new mark to this row, set all the attrs in this - // row to be Prompt, and set the current attrs to Output. - // - // If it does have a command, then we're still in the output of - // that command. - // --> the current attrs should already be set to Output. - if (!mostRecentMark.HasCommand()) - { - auto& row = GetMutableRowByOffset(GetCursor().GetPosition().y); - row.StartPrompt(); - // for (auto& [attr, len] : row.Attributes().runs()) - // { - // attr.SetMarkAttributes(MarkKind::Prompt); - // } - // // This changed the scrollbar marks - raise a notification to update them - // _NotifyScrollEvent(); - } - } - else - { - // The most recent command mark _didn't_ have output yet. Great! - // we'll leave it alone, and just start treating text as Output. - } - } - else - { - // There were no marks at all! - // --> add a new mark to this row, set all the attrs in this row - // to be Prompt, and set the current attrs to Output. - - auto& row = GetMutableRowByOffset(GetCursor().GetPosition().y); - row.StartPrompt(); - // for (auto& [attr, len] : row.Attributes().runs()) - // { - // attr.SetMarkAttributes(MarkKind::Prompt); - // } - // // This changed the scrollbar marks - raise a notification to update them - // _NotifyScrollEvent(); - } + const auto createdMark = _createPromptMarkIfNeeded(); auto attr = GetCurrentAttributes(); attr.SetMarkAttributes(MarkKind::Output); SetCurrentAttributes(attr); + return createdMark; } void TextBuffer::EndOutput(std::optional error) diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index 08a7ba41e4c..d2bf340524f 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -337,11 +337,12 @@ class TextBuffer final std::wstring CurrentCommand() const; std::wstring _commandForRow(const til::CoordType rowOffset, const til::CoordType bottomInclusive) const; MarkExtents _scrollMarkExtentForRow(const til::CoordType rowOffset, const til::CoordType bottomInclusive) const; + bool _createPromptMarkIfNeeded(); std::vector Commands() const; void StartPrompt(); - void StartCommand(); - void StartOutput(); + bool StartCommand(); + bool StartOutput(); void EndOutput(std::optional error); private: diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 30bf08296d6..3c393e84422 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -727,73 +727,15 @@ TerminalInput::OutputType Terminal::SendCharEvent(const wchar_t ch, const WORD s // We'll set the current attributes to Output, so that the output after // here is marked as the command output. But we also need to make sure // that a mark was started. - // // We can't just check if the current row has a mark - there could be a // multiline prompt. // - // What we're gonna do is get the most recent mark above the cursor. If it's got an outputEnd, then - - // // Only get the most recent mark. - // const auto mostRecentMarks = _activeBuffer().GetMarkExtents(1u); - // if (!mostRecentMarks.empty()) - // { - // const auto& mostRecentMark = mostRecentMarks[0]; - // if (mostRecentMark.HasOutput()) - // { - // // The most recent command mark had output. That suggests that either: - // // * shell integration wasn't enabled (but the user would still - // // like lines with enters to be marked as prompts) - // // * or we're in the middle of a command that's ongoing. - - // // If the mark doesn't have any command - then we know we're - // // playing silly games with just marking whole lines as prompts, - // // then immediately going to output. - // // --> add a new mark to this row, set all the attrs in this - // // row to be Prompt, and set the current attrs to Output. - // // - // // If it does have a command, then we're still in the output of - // // that command. - // // --> the current attrs should already be set to Output. - // if (!mostRecentMark.HasCommand()) - // { - // auto& row = _activeBuffer().GetMutableRowByOffset(_activeBuffer().GetCursor().GetPosition().y); - // row.StartPrompt(); - // for (auto& [attr, len] : row.Attributes().runs()) - // { - // attr.SetMarkAttributes(MarkKind::Prompt); - // } - // // This changed the scrollbar marks - raise a notification to update them - // _NotifyScrollEvent(); - // } - // } - // else - // { - // // The most recent command mark _didn't_ have output yet. Great! - // // we'll leave it alone, and just start treating text as Output. - // } - // } - // else - // { - // // There were no marks at all! - // // --> add a new mark to this row, set all the attrs in this row - // // to be Prompt, and set the current attrs to Output. - - // auto& row = _activeBuffer().GetMutableRowByOffset(_activeBuffer().GetCursor().GetPosition().y); - // row.StartPrompt(); - // for (auto& [attr, len] : row.Attributes().runs()) - // { - // attr.SetMarkAttributes(MarkKind::Prompt); - // } - // // This changed the scrollbar marks - raise a notification to update them - // _NotifyScrollEvent(); - // } - - auto& row = _activeBuffer().GetMutableRowByOffset(_activeBuffer().GetCursor().GetPosition().y); - const bool hadMarkBefore = row.GetPromptData().has_value(); - _activeBuffer().StartOutput(); - const bool hadMarkAfter = row.GetPromptData().has_value(); - if (!hadMarkBefore && hadMarkAfter) + // (TextBuffer::_createPromptMarkIfNeeded does that work for us) + + const bool createdMark = _activeBuffer().StartOutput(); + if (createdMark) { + auto& row = _activeBuffer().GetMutableRowByOffset(_activeBuffer().GetCursor().GetPosition().y); for (auto& [attr, len] : row.Attributes().runs()) { attr.SetMarkAttributes(MarkKind::Prompt); @@ -801,10 +743,6 @@ TerminalInput::OutputType Terminal::SendCharEvent(const wchar_t ch, const WORD s // This changed the scrollbar marks - raise a notification to update them _NotifyScrollEvent(); } - - auto attr = _activeBuffer().GetCurrentAttributes(); - attr.SetMarkAttributes(MarkKind::Output); - SetTextAttributes(attr); } const auto keyDown = SynthesizeKeyEvent(true, 1, vkey, scanCode, ch, states.Value()); From bc5a638d2440a0e47ff7b75c690e63bb8c5b9b86 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 26 Mar 2024 06:12:08 -0500 Subject: [PATCH 17/30] re-add marks via the UI --- src/buffer/out/Row.cpp | 4 ++ src/buffer/out/Row.hpp | 1 + src/buffer/out/textBuffer.cpp | 13 ++++++ src/cascadia/TerminalControl/ControlCore.cpp | 39 ++++++---------- src/cascadia/TerminalCore/Terminal.cpp | 47 ++++++-------------- src/cascadia/TerminalCore/Terminal.hpp | 5 +-- 6 files changed, 47 insertions(+), 62 deletions(-) diff --git a/src/buffer/out/Row.cpp b/src/buffer/out/Row.cpp index 1111734b85b..2a76429c1d8 100644 --- a/src/buffer/out/Row.cpp +++ b/src/buffer/out/Row.cpp @@ -1186,6 +1186,10 @@ const std::optional& ROW::GetPromptData() const noexcept { return _promptData; } +void ROW::SetPromptData(std::optional data) noexcept +{ + _promptData = data; +} void ROW::StartPrompt() noexcept { diff --git a/src/buffer/out/Row.hpp b/src/buffer/out/Row.hpp index c716dca7ef9..b587476cf74 100644 --- a/src/buffer/out/Row.hpp +++ b/src/buffer/out/Row.hpp @@ -189,6 +189,7 @@ class ROW final auto AttrEnd() const noexcept { return _attr.end(); } const std::optional& GetPromptData() const noexcept; + void SetPromptData(std::optional data) noexcept; void StartPrompt() noexcept; void EndOutput(std::optional error) noexcept; diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 8a34bbe84a6..2eb1ce7b7f2 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -2924,6 +2924,19 @@ std::vector TextBuffer::GetMarkExtents(std::optional limit) continue; } + // Future thought! In #11000 & #14792, we considered the possibility of + // scrolling to only an error mark, or something like that. Perhaps in + // the future, add a customizable filter that's a set of types of mark + // to include? + // + // For now, skip any "Default" marks, since those came from the UI. We + // just want the ones that correspond to shell integration. + + if (rowPromptData->category == MarkCategory::Default) + { + continue; + } + // This row did start a prompt! Find the prompt that starts here. // Presumably, no rows below us will have prompts, so pass in the last // row with text as the bottom diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index b525aebcbce..cac1edb1331 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -2335,31 +2335,20 @@ namespace winrt::Microsoft::Terminal::Control::implementation return winrt::single_threaded_vector(std::move(v)); } - void ControlCore::AddMark(const Control::ScrollMark& /*mark*/) - { - // TODO! Re-implement adding a mark in the UI - - // const auto lock = _terminal->LockForReading(); - // ::ScrollMark m{}; - - // if (mark.Color.HasValue) - // { - // m.color = til::color{ mark.Color.Color }; - // } - - // if (_terminal->IsSelectionActive()) - // { - // m.start = til::point{ _terminal->GetSelectionAnchor() }; - // m.end = til::point{ _terminal->GetSelectionEnd() }; - // } - // else - // { - // m.start = m.end = til::point{ _terminal->GetTextBuffer().GetCursor().GetPosition() }; - // } - - // // The version of this that only accepts a ScrollMark will automatically - // // set the start & end to the cursor position. - // _terminal->AddMark(m, m.start, m.end, true); + void ControlCore::AddMark(const Control::ScrollMark& mark) + { + const auto lock = _terminal->LockForReading(); + ::MarkData m{}; + + if (mark.Color.HasValue) + { + m.color = til::color{ mark.Color.Color }; + } + const auto row = (_terminal->IsSelectionActive()) ? + _terminal->GetSelectionAnchor().y : + _terminal->GetTextBuffer().GetCursor().GetPosition().y; + + _terminal->AddMarkFromUI(m, row); } void ControlCore::ClearMark() diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 3c393e84422..d55c0feb584 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -1444,39 +1444,20 @@ PointTree Terminal::_getPatterns(til::CoordType beg, til::CoordType end) const } // NOTE: This is the version of AddMark that comes from the UI. The VT api call into this too. -void Terminal::AddMark(const ScrollMark& /*mark*/, - const til::point& /*start*/, - const til::point& /*end*/, - const bool /*fromUi*/) -{ - // TODO! - // Basically, entirely kill this? - - // if (_inAltBuffer()) - // { - // return; - // } - - // ScrollMark m = mark; - // m.start = start; - // m.end = end; - - // // If the mark came from the user adding a mark via the UI, don't make it the active prompt mark. - // if (fromUi) - // { - // _activeBuffer().AddMark(m); - // } - // else - // { - // _activeBuffer().StartPromptMark(m); - // } - - // // Tell the control that the scrollbar has somehow changed. Used as a - // // workaround to force the control to redraw any scrollbar marks - // _NotifyScrollEvent(); - - // // DON'T set _currentPrompt. The VT impl will do that for you. We don't want - // // UI-driven marks to set that. +void Terminal::AddMarkFromUI(MarkData mark, + const til::CoordType& y) +{ + if (_inAltBuffer()) + { + return; + } + + auto& row = _activeBuffer().GetMutableRowByOffset(y); + row.SetPromptData(mark); + + // 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::ClearMark() diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index 2349e580d4d..068071abd8a 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -119,10 +119,7 @@ class Microsoft::Terminal::Core::Terminal final : std::vector GetMarkRows() const noexcept; std::vector GetMarkExtents() const noexcept; - void AddMark(const ScrollMark& mark, - const til::point& start, - const til::point& end, - const bool fromUi); + void AddMarkFromUI(MarkData mark, const til::CoordType& y); til::property AlwaysNotifyOnBufferRotation; From 5f0933e67d916c20f4f76f52286f563032619e7d Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 26 Mar 2024 06:26:00 -0500 Subject: [PATCH 18/30] is this really everything? --- src/buffer/out/textBuffer.cpp | 85 +++++++---------------------------- src/buffer/out/textBuffer.hpp | 11 ++--- 2 files changed, 21 insertions(+), 75 deletions(-) diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 2eb1ce7b7f2..b12f6611026 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -2955,85 +2955,30 @@ std::vector TextBuffer::GetMarkExtents(std::optional limit) return std::move(marks); } -// const std::vector& TextBuffer::GetMarks() const noexcept -// { -// return _marks; -// } - // Remove all marks between `start` & `end`, inclusive. void TextBuffer::ClearMarksInRange( const til::point start, const til::point end) { - start; - end; - // auto inRange = [&start, &end](const ScrollMark& m) { - // return (m.start >= start && m.start <= end) || - // (m.end >= start && m.end <= end); - // }; + auto top = std::clamp(std::min(start.y, end.y), 0, _height - 1); + auto bottom = std::clamp(std::max(start.y, end.y), 0, _height - 1); - // _marks.erase(std::remove_if(_marks.begin(), - // _marks.end(), - // inRange), - // _marks.end()); + for (auto y = top; y <= bottom; y++) + { + auto& row = GetMutableRowByOffset(y); + auto& runs = row.Attributes().runs(); + row.SetPromptData(std::nullopt); + for (auto& [attr, length] : runs) + { + attr.SetMarkAttributes(MarkKind::None); + } + } } 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(); -// } - -// // Method Description: -// // - Add a mark to our list of marks, and treat it as the active "prompt". For -// // the sake of shell integration, we need to know which mark represents the -// // current prompt/command/output. Internally, we'll always treat the _last_ -// // mark in the list as the current prompt. -// // Arguments: -// // - m: the mark to add. -// void TextBuffer::StartPromptMark(const ScrollMark& m) -// { -// _marks.push_back(m); -// } -// // Method Description: -// // - Add a mark to our list of marks. Don't treat this as the active prompt. -// // This should be used for marks created by the UI or from other user input. -// // By inserting at the start of the list, we can separate out marks that were -// // generated by client programs vs ones created by the user. -// // Arguments: -// // - m: the mark to add. -// void TextBuffer::AddMark(const ScrollMark& m) -// { -// _marks.insert(_marks.begin(), m); -// } - -// void TextBuffer::_trimMarksOutsideBuffer() -// { -// const til::CoordType height = _height; -// std::erase_if(_marks, [height](const auto& m) { -// return (m.start.y < 0) || (m.start.y >= height); -// }); -// } + ClearMarksInRange({ 0, 0 }, { _width - 1, _height - 1 }); +} + MarkExtents TextBuffer::_scrollMarkExtentForRow(const til::CoordType rowOffset, const til::CoordType bottomInclusive) const { const auto& startRow = GetRowByOffset(rowOffset); diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index d2bf340524f..468803f16b8 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -330,16 +330,13 @@ class TextBuffer final std::vector SearchText(const std::wstring_view& needle, bool caseInsensitive) const; std::vector SearchText(const std::wstring_view& needle, bool caseInsensitive, til::CoordType rowBeg, til::CoordType rowEnd) const; + // Mark handling std::vector GetMarkRows() const noexcept; std::vector GetMarkExtents(std::optional limit = std::nullopt) const noexcept; void ClearMarksInRange(const til::point start, const til::point end); void ClearAllMarks() noexcept; std::wstring CurrentCommand() const; - std::wstring _commandForRow(const til::CoordType rowOffset, const til::CoordType bottomInclusive) const; - MarkExtents _scrollMarkExtentForRow(const til::CoordType rowOffset, const til::CoordType bottomInclusive) const; - bool _createPromptMarkIfNeeded(); std::vector Commands() const; - void StartPrompt(); bool StartCommand(); bool StartOutput(); @@ -368,7 +365,11 @@ 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(); + + std::wstring _commandForRow(const til::CoordType rowOffset, const til::CoordType bottomInclusive) const; + MarkExtents _scrollMarkExtentForRow(const til::CoordType rowOffset, const til::CoordType bottomInclusive) const; + bool _createPromptMarkIfNeeded(); + std::tuple _RowCopyHelper(const CopyRequest& req, const til::CoordType iRow, const ROW& row) const; static void _AppendRTFText(std::string& contentBuilder, const std::wstring_view& text); From f78dec3c7c05900b1100fe28e83d86dbc51b5b43 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 26 Mar 2024 06:46:41 -0500 Subject: [PATCH 19/30] oh, yea, we should reflow them. --- src/buffer/out/textBuffer.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index b12f6611026..b32a196da46 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -2617,6 +2617,14 @@ void TextBuffer::Reflow(TextBuffer& oldBuffer, TextBuffer& newBuffer, const View oldRowLimit = std::max(oldRowLimit, oldCursorPos.x + 1); } + // Immediately copy this mark over to our new row. The positions of the + // marks themselves will be preserved, since they're just text + // attributes. But the "bookmar" + if (oldRow.GetPromptData().has_value()) + { + newBuffer.GetMutableRowByOffset(newY).SetPromptData(oldRow.GetPromptData()); + } + til::CoordType oldX = 0; // Copy oldRow into newBuffer until oldRow has been fully consumed. From 6afd08fbe18cf8af8c31a2b167739438a39dbe28 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 26 Mar 2024 08:29:59 -0500 Subject: [PATCH 20/30] last bits of cleanup --- src/buffer/out/Marks.hpp | 91 +++++++++++++++++++ src/buffer/out/Row.cpp | 8 +- src/buffer/out/Row.hpp | 28 +----- src/buffer/out/textBuffer.cpp | 36 +++++--- src/buffer/out/textBuffer.hpp | 36 -------- src/cascadia/TerminalControl/ControlCore.cpp | 2 +- src/cascadia/TerminalCore/Terminal.cpp | 6 +- src/cascadia/TerminalCore/Terminal.hpp | 4 +- .../ConptyRoundtripTests.cpp | 8 +- src/host/ut_host/ScreenBufferTests.cpp | 14 +-- src/terminal/adapter/adaptDispatch.cpp | 63 ++++--------- 11 files changed, 157 insertions(+), 139 deletions(-) create mode 100644 src/buffer/out/Marks.hpp diff --git a/src/buffer/out/Marks.hpp b/src/buffer/out/Marks.hpp new file mode 100644 index 00000000000..be1519b1f8d --- /dev/null +++ b/src/buffer/out/Marks.hpp @@ -0,0 +1,91 @@ +/*++ +Copyright (c) Microsoft Corporation +Licensed under the MIT license. + +Module Name: +- marks.hpp + +Abstract: +- Definitions for types that are used for "scroll marks" and shell integration + in the buffer. +- Scroll marks are identified by the existence of "ScrollbarData" on a ROW in the buffer. +- Shell integration will then also markup the buffer with special + TextAttributes, to identify regions of text as the Prompt, the Command, the + Output, etc. +- MarkExtents are used to abstract away those regions of text, so a caller + doesn't need to iterate over the buffer themselves. +--*/ + +#pragma once + +enum class MarkCategory : uint8_t +{ + Default = 0, + Error = 1, + Warning = 2, + Success = 3, + Prompt = 4 +}; + +// This is the data that's stored on each ROW, to suggest that there's something +// interesting on this row to show in the scrollbar. Also used in conjunction +// with shell integration - when a prompt is added through shell integration, +// we'll also add a scrollbar mark as a quick "bookmark" to the start of that +// command. +struct ScrollbarData +{ + MarkCategory category{ MarkCategory::Default }; + + // Scrollbar marks may have been given a color, or not. + std::optional color; + + // Prompts without an exit code haven't had a matching FTCS CommandEnd + // called yet. Any value other than 0 is an error. + std::optional exitCode; + // Future consideration: stick the literal command as a string on here, if + // we were given it with the 633;E sequence. +}; + +// Helper struct for describing the bounds of a command and it's output, +// * The Prompt is between the start & end +// * The Command is between the end & commandEnd +// * The Output is between the commandEnd & outputEnd +// +// These are not actually stored in the buffer. The buffer can produce them for +// callers, to make resoning about regions of the buffer easier. +struct MarkExtents +{ + // Data from the row + ScrollbarData data; + + til::point start; + til::point end; // exclusive + std::optional commandEnd; + std::optional 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 GetExtent() const + { + til::point realEnd{ til::coalesce_value(outputEnd, commandEnd, end) }; + return std::make_pair(til::point{ start }, realEnd); + } +}; + +// Another helper, for when callers would like to know just about the data of +// the scrollbar, but don't actually need all the extents of prompts. +struct ScrollMark +{ + til::CoordType row{ 0 }; + ScrollbarData data; +}; diff --git a/src/buffer/out/Row.cpp b/src/buffer/out/Row.cpp index 2a76429c1d8..1c648227089 100644 --- a/src/buffer/out/Row.cpp +++ b/src/buffer/out/Row.cpp @@ -1182,11 +1182,11 @@ CharToColumnMapper ROW::_createCharToColumnMapper(ptrdiff_t offset) const noexce return CharToColumnMapper{ _chars.data(), _charOffsets.data(), lastChar, guessedColumn }; } -const std::optional& ROW::GetPromptData() const noexcept +const std::optional& ROW::GetScrollbarData() const noexcept { return _promptData; } -void ROW::SetPromptData(std::optional data) noexcept +void ROW::SetScrollbarData(std::optional data) noexcept { _promptData = data; } @@ -1195,10 +1195,10 @@ void ROW::StartPrompt() noexcept { if (!_promptData.has_value()) { - _promptData = MarkData{ + _promptData = ScrollbarData{ + .category = MarkCategory::Prompt, .color = std::nullopt, .exitCode = std::nullopt, - .category = MarkCategory::Prompt }; } } diff --git a/src/buffer/out/Row.hpp b/src/buffer/out/Row.hpp index b587476cf74..d26fc87e7ed 100644 --- a/src/buffer/out/Row.hpp +++ b/src/buffer/out/Row.hpp @@ -8,6 +8,7 @@ #include "LineRendition.hpp" #include "OutputCell.hpp" #include "OutputCellIterator.hpp" +#include "Marks.hpp" class ROW; class TextBuffer; @@ -87,27 +88,6 @@ struct CharToColumnMapper til::CoordType _currentColumn; }; -enum class MarkCategory : uint8_t -{ - Default = 0, - Error = 1, - Warning = 2, - Success = 3, - Prompt = 4 -}; -struct MarkData -{ - // Scrollbar marks may have been given a color, or not. - std::optional color; - // Prompts without an exit code haven't had a matching FTCS CommandEnd - // called yet. Any value other than 0 is an error. - std::optional exitCode; - MarkCategory category{ MarkCategory::Default }; - - // Future consideration: stick the literal command as a string on here, if - // we were given it with the 633;E sequence. -}; - class ROW final { public: @@ -188,8 +168,8 @@ class ROW final auto AttrBegin() const noexcept { return _attr.begin(); } auto AttrEnd() const noexcept { return _attr.end(); } - const std::optional& GetPromptData() const noexcept; - void SetPromptData(std::optional data) noexcept; + const std::optional& GetScrollbarData() const noexcept; + void SetScrollbarData(std::optional data) noexcept; void StartPrompt() noexcept; void EndOutput(std::optional error) noexcept; @@ -326,7 +306,7 @@ class ROW final // Occurs when the user runs out of text to support a double byte character and we're forced to the next line bool _doubleBytePadded = false; - std::optional _promptData = std::nullopt; + std::optional _promptData = std::nullopt; }; #ifdef UNIT_TESTING diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index b32a196da46..b6596c36b46 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -2620,9 +2620,9 @@ void TextBuffer::Reflow(TextBuffer& oldBuffer, TextBuffer& newBuffer, const View // Immediately copy this mark over to our new row. The positions of the // marks themselves will be preserved, since they're just text // attributes. But the "bookmar" - if (oldRow.GetPromptData().has_value()) + if (oldRow.GetScrollbarData().has_value()) { - newBuffer.GetMutableRowByOffset(newY).SetPromptData(oldRow.GetPromptData()); + newBuffer.GetMutableRowByOffset(newY).SetScrollbarData(oldRow.GetScrollbarData()); } til::CoordType oldX = 0; @@ -2895,6 +2895,9 @@ std::vector TextBuffer::SearchText(const std::wstring_view& nee return results; } + +// Collect up all the rows that were marked, and the data marked on that row. +// This is what should be used for hot paths, like updating the scrollbar. std::vector TextBuffer::GetMarkRows() const noexcept { std::vector marks; @@ -2902,7 +2905,7 @@ std::vector TextBuffer::GetMarkRows() const noexcept for (auto y = 0; y <= bottom; y++) { const auto& row = GetRowByOffset(y); - const auto& data{ row.GetPromptData() }; + const auto& data{ row.GetScrollbarData() }; if (data.has_value()) { marks.emplace_back(y, *data); @@ -2911,6 +2914,14 @@ std::vector TextBuffer::GetMarkRows() const noexcept return std::move(marks); } +// Get all the regions for all the shell integration marks in the buffer. +// Marks will be returned in top-down order. +// +// This possibly iterates over every run in the buffer, so don't do this on a +// hot path. Just do this once pwe user input, if at all possible. +// +// Use `limit` to control how many you get, _starting from the bottom_. (e.g. +// limit=1 will just give you the "most recent mark"). std::vector TextBuffer::GetMarkExtents(std::optional limit) const noexcept { if (limit.has_value() && @@ -2925,7 +2936,7 @@ std::vector TextBuffer::GetMarkExtents(std::optional limit) for (auto promptY = bottom; promptY >= 0; promptY--) { const auto& currRow = GetRowByOffset(promptY); - auto& rowPromptData = currRow.GetPromptData(); + auto& rowPromptData = currRow.GetScrollbarData(); if (!rowPromptData.has_value()) { // This row didn't start a prompt, don't even look here. @@ -2975,7 +2986,7 @@ void TextBuffer::ClearMarksInRange( { auto& row = GetMutableRowByOffset(y); auto& runs = row.Attributes().runs(); - row.SetPromptData(std::nullopt); + row.SetScrollbarData(std::nullopt); for (auto& [attr, length] : runs) { attr.SetMarkAttributes(MarkKind::None); @@ -2987,10 +2998,13 @@ void TextBuffer::ClearAllMarks() noexcept ClearMarksInRange({ 0, 0 }, { _width - 1, _height - 1 }); } -MarkExtents TextBuffer::_scrollMarkExtentForRow(const til::CoordType rowOffset, const til::CoordType bottomInclusive) const +// Collect up the extent of the prompt and possibly command and output for the +// mark that starts on this row. +MarkExtents TextBuffer::_scrollMarkExtentForRow(const til::CoordType rowOffset, + const til::CoordType bottomInclusive) const { const auto& startRow = GetRowByOffset(rowOffset); - const auto& rowPromptData = startRow.GetPromptData(); + const auto& rowPromptData = startRow.GetScrollbarData(); assert(rowPromptData.has_value()); MarkExtents mark{ @@ -3064,7 +3078,7 @@ MarkExtents TextBuffer::_scrollMarkExtentForRow(const til::CoordType rowOffset, if (!mark.commandEnd.has_value()) { // immediately just end the command at the start here, so we can treat this whole run as output - mark.commandEnd = mark.end; // til::point{ x, y }; + mark.commandEnd = mark.end; startedCommand = true; } @@ -3141,7 +3155,7 @@ std::wstring TextBuffer::CurrentCommand() const for (; promptY >= 0; promptY--) { const auto& currRow = GetRowByOffset(promptY); - auto& rowPromptData = currRow.GetPromptData(); + auto& rowPromptData = currRow.GetScrollbarData(); if (!rowPromptData.has_value()) { // This row didn't start a prompt, don't even look here. @@ -3164,7 +3178,7 @@ std::vector TextBuffer::Commands() const for (auto promptY = bottom; promptY >= 0; promptY--) { const auto& currRow = GetRowByOffset(promptY); - auto& rowPromptData = currRow.GetPromptData(); + auto& rowPromptData = currRow.GetScrollbarData(); if (!rowPromptData.has_value()) { // This row didn't start a prompt, don't even look here. @@ -3271,7 +3285,7 @@ void TextBuffer::EndOutput(std::optional error) for (auto y = GetCursor().GetPosition().y; y >= 0; y--) { auto& currRow = GetMutableRowByOffset(y); - auto& rowPromptData = currRow.GetPromptData(); + auto& rowPromptData = currRow.GetScrollbarData(); if (rowPromptData.has_value()) { currRow.EndOutput(error); diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index 468803f16b8..660c155cfea 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -66,41 +66,6 @@ namespace Microsoft::Console::Render class Renderer; } -struct MarkExtents -{ - MarkData data; - - // std::optional color; - til::point start; - til::point end; // exclusive - std::optional commandEnd; - std::optional 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 GetExtent() const - { - til::point realEnd{ til::coalesce_value(outputEnd, commandEnd, end) }; - return std::make_pair(til::point{ start }, realEnd); - } -}; - -struct ScrollMark -{ - til::CoordType row{ 0 }; - MarkData data; -}; - class TextBuffer final { public: @@ -442,7 +407,6 @@ class TextBuffer final uint64_t _lastMutationId = 0; Cursor _cursor; - // std::vector _marks; bool _isActiveBuffer = false; #ifdef UNIT_TESTING diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index cac1edb1331..3cd2235997a 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -2338,7 +2338,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation void ControlCore::AddMark(const Control::ScrollMark& mark) { const auto lock = _terminal->LockForReading(); - ::MarkData m{}; + ::ScrollbarData m{}; if (mark.Color.HasValue) { diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index d55c0feb584..42e42a3af15 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -1444,7 +1444,7 @@ PointTree Terminal::_getPatterns(til::CoordType beg, til::CoordType end) const } // NOTE: This is the version of AddMark that comes from the UI. The VT api call into this too. -void Terminal::AddMarkFromUI(MarkData mark, +void Terminal::AddMarkFromUI(ScrollbarData mark, const til::CoordType& y) { if (_inAltBuffer()) @@ -1453,7 +1453,7 @@ void Terminal::AddMarkFromUI(MarkData mark, } auto& row = _activeBuffer().GetMutableRowByOffset(y); - row.SetPromptData(mark); + row.SetScrollbarData(mark); // Tell the control that the scrollbar has somehow changed. Used as a // workaround to force the control to redraw any scrollbar marks @@ -1501,7 +1501,7 @@ std::vector Terminal::GetMarkExtents() const noexcept return _inAltBuffer() ? std::vector{} : std::move(_activeBuffer().GetMarkExtents()); } -til::color Terminal::GetColorForMark(const MarkData& markData) const +til::color Terminal::GetColorForMark(const ScrollbarData& markData) const { if (markData.color.has_value()) { diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index 068071abd8a..a25df00afca 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -119,7 +119,7 @@ class Microsoft::Terminal::Core::Terminal final : std::vector GetMarkRows() const noexcept; std::vector GetMarkExtents() const noexcept; - void AddMarkFromUI(MarkData mark, const til::CoordType& y); + void AddMarkFromUI(ScrollbarData mark, const til::CoordType& y); til::property AlwaysNotifyOnBufferRotation; @@ -160,7 +160,7 @@ class Microsoft::Terminal::Core::Terminal final : void ClearMark(); void ClearAllMarks(); - til::color GetColorForMark(const MarkData& markData) const; + til::color GetColorForMark(const ScrollbarData& markData) const; #pragma region ITerminalInput // These methods are defined in Terminal.cpp diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index 9dc9c25af32..f5f8cc9a028 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -4350,8 +4350,8 @@ void ConptyRoundtripTests::SimplePromptRegions() const auto& row0 = tb.GetRowByOffset(0); const auto& row4 = tb.GetRowByOffset(4); - VERIFY_IS_TRUE(row0.GetPromptData().has_value()); - VERIFY_IS_TRUE(row4.GetPromptData().has_value()); + VERIFY_IS_TRUE(row0.GetScrollbarData().has_value()); + VERIFY_IS_TRUE(row4.GetScrollbarData().has_value()); const auto marks = tb.GetMarkExtents(); VERIFY_ARE_EQUAL(2u, marks.size()); @@ -4447,8 +4447,8 @@ void ConptyRoundtripTests::MultilinePromptRegions() const auto& row0 = tb.GetRowByOffset(0); const auto& row5 = tb.GetRowByOffset(5); - VERIFY_IS_TRUE(row0.GetPromptData().has_value()); - VERIFY_IS_TRUE(row5.GetPromptData().has_value()); + VERIFY_IS_TRUE(row0.GetScrollbarData().has_value()); + VERIFY_IS_TRUE(row5.GetScrollbarData().has_value()); const auto marks = tb.GetMarkExtents(); VERIFY_ARE_EQUAL(2u, marks.size()); diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index a402d4c36fd..562f7e88b90 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -8431,7 +8431,7 @@ void ScreenBufferTests::SimpleMarkCommand() stateMachine.ProcessString(FTCS_A L"A Prompt" FTCS_B L"my_command" FTCS_C L"\n"); - VERIFY_IS_TRUE(currentRow.GetPromptData().has_value()); + VERIFY_IS_TRUE(currentRow.GetScrollbarData().has_value()); } stateMachine.ProcessString(L"Two\n"); @@ -8478,13 +8478,13 @@ void ScreenBufferTests::SimpleWrappedCommand() VERIFY_ARE_NOT_EQUAL(originalRowOffset, secondRowOffset); auto& secondRow = tbi.GetRowByOffset(secondRowOffset); - VERIFY_IS_TRUE(originalRow.GetPromptData().has_value()); - VERIFY_IS_FALSE(secondRow.GetPromptData().has_value()); + VERIFY_IS_TRUE(originalRow.GetScrollbarData().has_value()); + VERIFY_IS_FALSE(secondRow.GetScrollbarData().has_value()); stateMachine.ProcessString(FTCS_C L"\n"); - VERIFY_IS_TRUE(originalRow.GetPromptData().has_value()); - VERIFY_IS_FALSE(secondRow.GetPromptData().has_value()); + VERIFY_IS_TRUE(originalRow.GetScrollbarData().has_value()); + VERIFY_IS_FALSE(secondRow.GetScrollbarData().has_value()); } stateMachine.ProcessString(L"Two\n"); @@ -8554,8 +8554,8 @@ void ScreenBufferTests::SimplePromptRegions() const auto& row0 = tbi.GetRowByOffset(0); const auto& row4 = tbi.GetRowByOffset(4); - VERIFY_IS_TRUE(row0.GetPromptData().has_value()); - VERIFY_IS_TRUE(row4.GetPromptData().has_value()); + VERIFY_IS_TRUE(row0.GetScrollbarData().has_value()); + VERIFY_IS_TRUE(row4.GetScrollbarData().has_value()); const auto marks = tbi.GetMarkExtents(); VERIFY_ARE_EQUAL(2u, marks.size()); diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index a67bc2ddd8b..9af916eaf10 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -3671,7 +3671,6 @@ bool AdaptDispatch::DoConEmuAction(const std::wstring_view string) // This seems like basically the same as 133;B - the end of the prompt, the start of the commandline. else if (subParam == 12) { - // _api.MarkCommandStart(); _api.GetTextBuffer().StartCommand(); return true; } @@ -3691,12 +3690,12 @@ bool AdaptDispatch::DoConEmuAction(const std::wstring_view string) // - false in conhost, true for the SetMark action, otherwise false. bool AdaptDispatch::DoITerm2Action(const std::wstring_view string) { - // This is not implemented in conhost. - if (_api.IsConsolePty()) + const auto isConPty = _api.IsConsolePty(); + if (isConPty) { - // Flush the frame manually, to make sure marks end up on the right line, like the alt buffer sequence. + // Flush the frame manually, to make sure marks end up on the right + // line, like the alt buffer sequence. _renderer.TriggerFlush(false); - return false; } if constexpr (!Feature_ScrollbarMarks::IsEnabled()) @@ -3713,20 +3712,14 @@ bool AdaptDispatch::DoITerm2Action(const std::wstring_view string) const auto action = til::at(parts, 0); + bool handled = false; if (action == L"SetMark") { - // ScrollMark mark; - // mark.category = MarkCategory::Prompt; - // _api.MarkPrompt(mark); - - auto attr = _api.GetTextBuffer().GetCurrentAttributes(); - attr.SetMarkAttributes(MarkKind::Prompt); - _api.SetTextAttributes(attr); _api.GetTextBuffer().StartPrompt(); - - return true; + handled = true; } - return false; + + return handled && !isConPty; } // Method Description: @@ -3741,7 +3734,13 @@ bool AdaptDispatch::DoITerm2Action(const std::wstring_view string) // - false in conhost, true for the SetMark action, otherwise false. bool AdaptDispatch::DoFinalTermAction(const std::wstring_view string) { - // This is not implemented in conhost. + const auto isConPty = _api.IsConsolePty(); + if (isConPty) + { + // Flush the frame manually, to make sure marks end up on the right + // line, like the alt buffer sequence. + _renderer.TriggerFlush(false); + } if constexpr (!Feature_ScrollbarMarks::IsEnabled()) { @@ -3762,37 +3761,19 @@ bool AdaptDispatch::DoFinalTermAction(const std::wstring_view string) { case L'A': // FTCS_PROMPT { - // Simply just mark this line as a prompt line. - // auto attr = _api.GetTextBuffer().GetCurrentAttributes(); - // attr.SetMarkAttributes(MarkKind::Prompt); - // _api.SetTextAttributes(attr); _api.GetTextBuffer().StartPrompt(); - // auto& tb = _api.GetTextBuffer(); - // auto& cursor= tb.GetCursor(); - // auto& row = tb.GetRowByOffset(cursor.GetPosition().y); - handled = true; break; } case L'B': // FTCS_COMMAND_START { - // auto attr = _api.GetTextBuffer().GetCurrentAttributes(); - // attr.SetMarkAttributes(MarkKind::Command); - // _api.SetTextAttributes(attr); - _api.GetTextBuffer().StartCommand(); - handled = true; break; } case L'C': // FTCS_COMMAND_EXECUTED { - // auto attr = _api.GetTextBuffer().GetCurrentAttributes(); - // attr.SetMarkAttributes(MarkKind::Output); - // _api.SetTextAttributes(attr); - _api.GetTextBuffer().StartOutput(); - handled = true; break; } @@ -3813,11 +3794,7 @@ bool AdaptDispatch::DoFinalTermAction(const std::wstring_view string) UINT_MAX; } - // auto attr = _api.GetTextBuffer().GetCurrentAttributes(); - // attr.SetMarkAttributes(MarkKind::Output); - // _api.SetTextAttributes(attr); _api.GetTextBuffer().EndOutput(error); - // _api.MarkCommandFinish(error); handled = true; break; @@ -3829,19 +3806,11 @@ bool AdaptDispatch::DoFinalTermAction(const std::wstring_view string) } } - if (_api.IsConsolePty()) - { - // DebugBreak(); - // Flush the frame manually, to make sure marks end up on the right line, like the alt buffer sequence. - _renderer.TriggerFlush(false); - return false; - } - // When we add the rest of the FTCS sequences (GH#11000), we should add a // simple state machine here to track the most recently emitted mark from // this set of sequences, and which sequence was emitted last, so we can // modify the state of that mark as we go. - return handled; + return handled && !isConPty; } // Method Description: // - Performs a VsCode action From ca50d96e810eff24a5f71e928b81f1e06ab89bf3 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 26 Mar 2024 09:16:08 -0500 Subject: [PATCH 21/30] spel --- src/buffer/out/Marks.hpp | 2 +- src/buffer/out/textBuffer.cpp | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/buffer/out/Marks.hpp b/src/buffer/out/Marks.hpp index be1519b1f8d..07e24a0372f 100644 --- a/src/buffer/out/Marks.hpp +++ b/src/buffer/out/Marks.hpp @@ -52,7 +52,7 @@ struct ScrollbarData // * The Output is between the commandEnd & outputEnd // // These are not actually stored in the buffer. The buffer can produce them for -// callers, to make resoning about regions of the buffer easier. +// callers, to make reasoning about regions of the buffer easier. struct MarkExtents { // Data from the row diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index b6596c36b46..aa890260d60 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -2619,7 +2619,11 @@ void TextBuffer::Reflow(TextBuffer& oldBuffer, TextBuffer& newBuffer, const View // Immediately copy this mark over to our new row. The positions of the // marks themselves will be preserved, since they're just text - // attributes. But the "bookmar" + // attributes. But the "bookmark" needs to get moved to the new row too. + // * If a row wraps as it reflows, that's fine - we want to leave the + // mark on the row it started on. + // * If the second row of a wrapped row had a mark, and it de-flows onto a + // single row, that's fine! The mark was on that logical row. if (oldRow.GetScrollbarData().has_value()) { newBuffer.GetMutableRowByOffset(newY).SetScrollbarData(oldRow.GetScrollbarData()); @@ -2918,7 +2922,7 @@ std::vector TextBuffer::GetMarkRows() const noexcept // Marks will be returned in top-down order. // // This possibly iterates over every run in the buffer, so don't do this on a -// hot path. Just do this once pwe user input, if at all possible. +// hot path. Just do this once per user input, if at all possible. // // Use `limit` to control how many you get, _starting from the bottom_. (e.g. // limit=1 will just give you the "most recent mark"). From ad3e599d8053bd77e0e97c49effe1a8ab6509a00 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 26 Mar 2024 09:28:43 -0500 Subject: [PATCH 22/30] audit, and I forgot to update a test --- src/buffer/out/textBuffer.cpp | 14 +++++++------- src/buffer/out/textBuffer.hpp | 6 +++--- src/host/ut_host/ScreenBufferTests.cpp | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index aa890260d60..3865c22867c 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -2902,7 +2902,7 @@ std::vector TextBuffer::SearchText(const std::wstring_view& nee // Collect up all the rows that were marked, and the data marked on that row. // This is what should be used for hot paths, like updating the scrollbar. -std::vector TextBuffer::GetMarkRows() const noexcept +std::vector TextBuffer::GetMarkRows() const { std::vector marks; const auto bottom = _estimateOffsetOfLastCommittedRow(); @@ -2915,7 +2915,7 @@ std::vector TextBuffer::GetMarkRows() const noexcept marks.emplace_back(y, *data); } } - return std::move(marks); + return marks; } // Get all the regions for all the shell integration marks in the buffer. @@ -2926,7 +2926,7 @@ std::vector TextBuffer::GetMarkRows() const noexcept // // Use `limit` to control how many you get, _starting from the bottom_. (e.g. // limit=1 will just give you the "most recent mark"). -std::vector TextBuffer::GetMarkExtents(std::optional limit) const noexcept +std::vector TextBuffer::GetMarkExtents(std::optional limit) const { if (limit.has_value() && *limit == 0) @@ -2975,7 +2975,7 @@ std::vector TextBuffer::GetMarkExtents(std::optional limit) } std::reverse(marks.begin(), marks.end()); - return std::move(marks); + return marks; } // Remove all marks between `start` & `end`, inclusive. @@ -2997,7 +2997,7 @@ void TextBuffer::ClearMarksInRange( } } } -void TextBuffer::ClearAllMarks() noexcept +void TextBuffer::ClearAllMarks() { ClearMarksInRange({ 0, 0 }, { _width - 1, _height - 1 }); } @@ -3200,7 +3200,7 @@ std::vector TextBuffer::Commands() const lastPromptY = promptY; } std::reverse(commands.begin(), commands.end()); - return std::move(commands); + return commands; } void TextBuffer::StartPrompt() @@ -3224,7 +3224,7 @@ bool TextBuffer::_createPromptMarkIfNeeded() const auto mostRecentMarks = GetMarkExtents(1u); if (!mostRecentMarks.empty()) { - const auto& mostRecentMark = mostRecentMarks[0]; + const auto& mostRecentMark = til::at(mostRecentMarks, 0); if (mostRecentMark.HasOutput()) { // The most recent command mark had output. That suggests that either: diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index 660c155cfea..4fed95e1d0a 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -296,10 +296,10 @@ class TextBuffer final std::vector SearchText(const std::wstring_view& needle, bool caseInsensitive, til::CoordType rowBeg, til::CoordType rowEnd) const; // Mark handling - std::vector GetMarkRows() const noexcept; - std::vector GetMarkExtents(std::optional limit = std::nullopt) const noexcept; + std::vector GetMarkRows() const; + std::vector GetMarkExtents(std::optional limit = std::nullopt) const; void ClearMarksInRange(const til::point start, const til::point end); - void ClearAllMarks() noexcept; + void ClearAllMarks(); std::wstring CurrentCommand() const; std::vector Commands() const; void StartPrompt(); diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index 562f7e88b90..db596e47066 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -8565,7 +8565,7 @@ void ScreenBufferTests::SimplePromptRegions() const til::point expectedStart{ 0, 0 }; const til::point expectedEnd{ 17, 0 }; const til::point expectedOutputStart{ 24, 0 }; // `Foo-Bar` is 7 characters - const til::point expectedOutputEnd{ 0, 4 }; + const til::point expectedOutputEnd{ 22, 3 }; VERIFY_ARE_EQUAL(expectedStart, mark.start); VERIFY_ARE_EQUAL(expectedEnd, mark.end); From 43b64f42796dd915f0b874ab51f71f8509f68a5f Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 27 Mar 2024 06:40:21 -0500 Subject: [PATCH 23/30] actually fix build --- src/cascadia/TerminalCore/Terminal.cpp | 6 +++--- src/cascadia/TerminalCore/Terminal.hpp | 6 +++--- .../ConptyRoundtripTests.cpp | 16 ++++++++-------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 42e42a3af15..083bd38aa7f 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -1488,13 +1488,13 @@ void Terminal::ClearAllMarks() _NotifyScrollEvent(); } -std::vector Terminal::GetMarkRows() const noexcept +std::vector Terminal::GetMarkRows() const { // We want to return _no_ marks when we're in the alt buffer, to effectively // hide them. return _inAltBuffer() ? std::vector{} : std::move(_activeBuffer().GetMarkRows()); } -std::vector Terminal::GetMarkExtents() const noexcept +std::vector Terminal::GetMarkExtents() const { // We want to return _no_ marks when we're in the alt buffer, to effectively // hide them. @@ -1533,7 +1533,7 @@ til::color Terminal::GetColorForMark(const ScrollbarData& markData) const } } -std::wstring_view Terminal::CurrentCommand() const +std::wstring Terminal::CurrentCommand() const { return _activeBuffer().CurrentCommand(); } diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index a25df00afca..28519868b3c 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -117,13 +117,13 @@ class Microsoft::Terminal::Core::Terminal final : RenderSettings& GetRenderSettings() noexcept; const RenderSettings& GetRenderSettings() const noexcept; - std::vector GetMarkRows() const noexcept; - std::vector GetMarkExtents() const noexcept; + std::vector GetMarkRows() const; + std::vector GetMarkExtents() const; void AddMarkFromUI(ScrollbarData mark, const til::CoordType& y); til::property AlwaysNotifyOnBufferRotation; - std::wstring_view CurrentCommand() const; + std::wstring CurrentCommand() const; #pragma region ITerminalApi // These methods are defined in TerminalApi.cpp diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index f5f8cc9a028..bae2ebfbbc6 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -4458,7 +4458,7 @@ void ConptyRoundtripTests::MultilinePromptRegions() const auto& row = tb.GetRowByOffset(0); const auto& attrs = row.Attributes(); const auto& runs = attrs.runs(); - VERIFY_ARE_EQUAL(2, runs.size()); + VERIFY_ARE_EQUAL(2u, runs.size()); auto run0 = runs[0]; auto run1 = runs[1]; VERIFY_ARE_EQUAL(17, run0.length); @@ -4472,7 +4472,7 @@ void ConptyRoundtripTests::MultilinePromptRegions() const auto& row = tb.GetRowByOffset(1); const auto& attrs = row.Attributes(); const auto& runs = attrs.runs(); - VERIFY_ARE_EQUAL(3, runs.size()); + VERIFY_ARE_EQUAL(3u, runs.size()); auto run0 = runs[0]; auto run1 = runs[1]; auto run2 = runs[2]; @@ -4490,7 +4490,7 @@ void ConptyRoundtripTests::MultilinePromptRegions() const auto& row = tb.GetRowByOffset(2); const auto& attrs = row.Attributes(); const auto& runs = attrs.runs(); - VERIFY_ARE_EQUAL(2, runs.size()); + VERIFY_ARE_EQUAL(2u, runs.size()); auto run0 = runs[0]; auto run1 = runs[1]; VERIFY_ARE_EQUAL(22, run0.length); @@ -4504,7 +4504,7 @@ void ConptyRoundtripTests::MultilinePromptRegions() const auto& row = tb.GetRowByOffset(3); const auto& attrs = row.Attributes(); const auto& runs = attrs.runs(); - VERIFY_ARE_EQUAL(2, runs.size()); + VERIFY_ARE_EQUAL(2u, runs.size()); auto run0 = runs[0]; auto run1 = runs[1]; VERIFY_ARE_EQUAL(22, run0.length); @@ -4600,7 +4600,7 @@ void ConptyRoundtripTests::ManyMultilinePromptsWithTrailingSpaces() auto verifyFirstRowOfPrompt = [&](const ROW& row) { const auto& attrs = row.Attributes(); const auto& runs = attrs.runs(); - VERIFY_ARE_EQUAL(2, runs.size()); + VERIFY_ARE_EQUAL(2u, runs.size()); auto run0 = runs[0]; auto run1 = runs[1]; VERIFY_ARE_EQUAL(17, run0.length); @@ -4612,7 +4612,7 @@ void ConptyRoundtripTests::ManyMultilinePromptsWithTrailingSpaces() auto verifySecondRowOfPrompt = [&](const ROW& row, const auto expectedCommandLength) { const auto& attrs = row.Attributes(); const auto& runs = attrs.runs(); - VERIFY_ARE_EQUAL(3, runs.size()); + VERIFY_ARE_EQUAL(3u, runs.size()); auto run0 = runs[0]; auto run1 = runs[1]; auto run2 = runs[2]; @@ -4648,7 +4648,7 @@ void ConptyRoundtripTests::ManyMultilinePromptsWithTrailingSpaces() const auto& row = tb.GetRowByOffset(2); const auto& attrs = row.Attributes(); const auto& runs = attrs.runs(); - VERIFY_ARE_EQUAL(2, runs.size()); + VERIFY_ARE_EQUAL(2u, runs.size()); auto run0 = runs[0]; auto run1 = runs[1]; VERIFY_ARE_EQUAL(22, run0.length); @@ -4662,7 +4662,7 @@ void ConptyRoundtripTests::ManyMultilinePromptsWithTrailingSpaces() const auto& row = tb.GetRowByOffset(3); const auto& attrs = row.Attributes(); const auto& runs = attrs.runs(); - VERIFY_ARE_EQUAL(2, runs.size()); + VERIFY_ARE_EQUAL(2u, runs.size()); auto run0 = runs[0]; auto run1 = runs[1]; VERIFY_ARE_EQUAL(22, run0.length); From e573896d5b1586ae191515f794e4f576b4433dc9 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 4 Apr 2024 11:21:01 -0500 Subject: [PATCH 24/30] I think we all learned something here about storing state in two places --- src/buffer/out/Marks.hpp | 2 +- src/buffer/out/TextAttribute.hpp | 5 +- src/buffer/out/textBuffer.cpp | 79 ++++--- src/buffer/out/textBuffer.hpp | 2 +- .../ConptyRoundtripTests.cpp | 199 ++++++++++++++++-- src/host/VtIo.cpp | 3 +- src/host/VtIo.hpp | 2 +- src/host/getset.cpp | 4 +- src/host/globals.cpp | 4 +- src/host/globals.h | 2 +- src/host/ut_host/ScreenBufferTests.cpp | 12 +- src/terminal/adapter/adaptDispatch.cpp | 2 +- 12 files changed, 237 insertions(+), 79 deletions(-) diff --git a/src/buffer/out/Marks.hpp b/src/buffer/out/Marks.hpp index 07e24a0372f..655416f8354 100644 --- a/src/buffer/out/Marks.hpp +++ b/src/buffer/out/Marks.hpp @@ -78,7 +78,7 @@ struct MarkExtents std::pair GetExtent() const { til::point realEnd{ til::coalesce_value(outputEnd, commandEnd, end) }; - return std::make_pair(til::point{ start }, realEnd); + return std::make_pair(start, realEnd); } }; diff --git a/src/buffer/out/TextAttribute.hpp b/src/buffer/out/TextAttribute.hpp index 26d70729e42..44d8563176a 100644 --- a/src/buffer/out/TextAttribute.hpp +++ b/src/buffer/out/TextAttribute.hpp @@ -38,8 +38,7 @@ enum class UnderlineStyle Max = DashedUnderlined }; -// We only need a few bits, but uint8_t is two bytes anyways, and apparently -// doesn't satisfy std::has_unique_object_representations_v +// We only need a few bits, but uint8_t apparently doesn't satisfy std::has_unique_object_representations_v enum class MarkKind : uint16_t { None = 0, @@ -225,7 +224,7 @@ class TextAttribute final TextColor _foreground; // sizeof: 4, alignof: 1 TextColor _background; // sizeof: 4, alignof: 1 TextColor _underlineColor; // sizeof: 4, alignof: 1 - MarkKind _markKind; // sizeof: 2, cause I guess a uint8_t is 2B? + MarkKind _markKind; // sizeof: 2, alignof: 1 #ifdef UNIT_TESTING friend class TextBufferTests; diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index faada830ebf..ab334c725c1 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -1178,7 +1178,6 @@ void TextBuffer::ClearScrollback(const til::CoordType start, const til::CoordTyp GetMutableRowByOffset(y).Reset(_initialAttributes); } - // ScrollMarks(-start); ClearMarksInRange(til::point{ 0, height }, til::point{ _width, _height }); } @@ -3003,9 +3002,6 @@ void TextBuffer::Reflow(TextBuffer& oldBuffer, TextBuffer& newBuffer, const View assert(newCursorPos.y >= 0 && newCursorPos.y < newHeight); newCursor.SetSize(oldCursor.GetSize()); newCursor.SetPosition(newCursorPos); - - // newBuffer._marks = oldBuffer._marks; - // newBuffer._trimMarksOutsideBuffer(); } // Method Description: @@ -3183,8 +3179,7 @@ std::vector TextBuffer::GetMarkRows() const // limit=1 will just give you the "most recent mark"). std::vector TextBuffer::GetMarkExtents(std::optional limit) const { - if (limit.has_value() && - *limit == 0) + if (limit == 0) { return {}; } @@ -3220,6 +3215,8 @@ std::vector TextBuffer::GetMarkExtents(std::optional limit) // row with text as the bottom marks.push_back(_scrollMarkExtentForRow(promptY, lastPromptY)); + // operator>=(T, optional) will return true if the optional is + // nullopt, unfortunately. if (limit.has_value() && marks.size() >= *limit) { @@ -3239,7 +3236,7 @@ void TextBuffer::ClearMarksInRange( const til::point end) { auto top = std::clamp(std::min(start.y, end.y), 0, _height - 1); - auto bottom = std::clamp(std::max(start.y, end.y), 0, _height - 1); + auto bottom = std::clamp(std::max(start.y, end.y), 0, _estimateOffsetOfLastCommittedRow()); for (auto y = top; y <= bottom; y++) { @@ -3480,46 +3477,38 @@ bool TextBuffer::_createPromptMarkIfNeeded() if (!mostRecentMarks.empty()) { const auto& mostRecentMark = til::at(mostRecentMarks, 0); - if (mostRecentMark.HasOutput()) - { - // The most recent command mark had output. That suggests that either: - // * shell integration wasn't enabled (but the user would still - // like lines with enters to be marked as prompts) - // * or we're in the middle of a command that's ongoing. - - // If the mark doesn't have any command - then we know we're - // playing silly games with just marking whole lines as prompts, - // then immediately going to output. - // --> add a new mark to this row, set all the attrs in this - // row to be Prompt, and set the current attrs to Output. - // - // If it does have a command, then we're still in the output of - // that command. - // --> the current attrs should already be set to Output. - if (!mostRecentMark.HasCommand()) - { - auto& row = GetMutableRowByOffset(GetCursor().GetPosition().y); - row.StartPrompt(); - return true; - } - } - else + if (!mostRecentMark.HasOutput()) { // The most recent command mark _didn't_ have output yet. Great! // we'll leave it alone, and just start treating text as Command or Output. + return false; } - } - else - { - // There were no marks at all! - // --> add a new mark to this row, set all the attrs in this row - // to be Prompt, and set the current attrs to Output. - auto& row = GetMutableRowByOffset(GetCursor().GetPosition().y); - row.StartPrompt(); - return true; + // The most recent command mark had output. That suggests that either: + // * shell integration wasn't enabled (but the user would still + // like lines with enters to be marked as prompts) + // * or we're in the middle of a command that's ongoing. + + // If it does have a command, then we're still in the output of + // that command. + // --> the current attrs should already be set to Output. + if (mostRecentMark.HasCommand()) + { + return false; + } + // If the mark doesn't have any command - then we know we're + // playing silly games with just marking whole lines as prompts, + // then immediately going to output. + // --> Below, we'll add a new mark to this row. } - return false; + + // There were no marks at all! + // --> add a new mark to this row, set all the attrs in this row + // to be Prompt, and set the current attrs to Output. + + auto& row = GetMutableRowByOffset(GetCursor().GetPosition().y); + row.StartPrompt(); + return true; } bool TextBuffer::StartCommand() @@ -3539,8 +3528,14 @@ bool TextBuffer::StartOutput() return createdMark; } -void TextBuffer::EndOutput(std::optional error) +// Find the row above the cursor where this most recent prompt started, and set +// the exit code on that row's scroll mark. +void TextBuffer::EndCurrentCommand(std::optional error) { + // auto attr = GetCurrentAttributes(); + // attr.SetMarkAttributes(MarkKind::None); + // SetCurrentAttributes(attr); + for (auto y = GetCursor().GetPosition().y; y >= 0; y--) { auto& currRow = GetMutableRowByOffset(y); diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index 5593cbdbd8e..a0c8a9868a6 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -307,7 +307,7 @@ class TextBuffer final void StartPrompt(); bool StartCommand(); bool StartOutput(); - void EndOutput(std::optional error); + void EndCurrentCommand(std::optional error); private: void _reserve(til::size screenBufferSize, const TextAttribute& defaultAttributes); diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index bae2ebfbbc6..584cf07378f 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -131,7 +131,9 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final // Manually set the console into conpty mode. We're not actually going // to set up the pipes for conpty, but we want the console to behave // like it would in conpty mode. - g.EnableConptyModeForTests(std::move(vtRenderEngine)); + // + // Also, make sure to backdoor neable the resize quirk here too. + g.EnableConptyModeForTests(std::move(vtRenderEngine), true); expectedOutput.clear(); _checkConptyOutput = true; @@ -236,6 +238,7 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final TEST_METHOD(SimplePromptRegions); TEST_METHOD(MultilinePromptRegions); TEST_METHOD(ManyMultilinePromptsWithTrailingSpaces); + TEST_METHOD(ReflowPromptRegions); private: bool _writeCallback(const char* const pch, const size_t cch); @@ -337,8 +340,9 @@ void ConptyRoundtripTests::_clearConpty() _resizeConpty(newSize.width, newSize.height); // After we resize, make sure to get the new textBuffers - return { &ServiceLocator::LocateGlobals().getConsoleInformation().GetActiveOutputBuffer().GetTextBuffer(), - term->_mainBuffer.get() }; + TextBuffer* hostTb = &ServiceLocator::LocateGlobals().getConsoleInformation().GetActiveOutputBuffer().GetTextBuffer(); + TextBuffer* termTb = term->_mainBuffer.get(); + return { hostTb, termTb }; } void ConptyRoundtripTests::ConptyOutputTestCanary() @@ -4320,6 +4324,11 @@ void ConptyRoundtripTests::TestNoBackgroundAttrsOptimization() verifyBuffer(*termTb); } +#define FTCS_A L"\x1b]133;A\x1b\\" +#define FTCS_B L"\x1b]133;B\x1b\\" +#define FTCS_C L"\x1b]133;C\x1b\\" +#define FTCS_D L"\x1b]133;D\x1b\\" + void ConptyRoundtripTests::SimplePromptRegions() { Log::Comment(L"Same as the ScreenBufferTests::ComplicatedPromptRegions, but in conpty"); @@ -4386,20 +4395,20 @@ void ConptyRoundtripTests::SimplePromptRegions() // `PWSH C:\Windows> ` // // which is 17 characters for C:\Windows - stateMachine.ProcessString(L"\x1b]133;D\x7"); - stateMachine.ProcessString(L"\x1b]133;A\x7"); + stateMachine.ProcessString(FTCS_D); + stateMachine.ProcessString(FTCS_A); stateMachine.ProcessString(L"\x1b]9;9;"); stateMachine.ProcessString(path); stateMachine.ProcessString(L"\x7"); stateMachine.ProcessString(L"PWSH "); stateMachine.ProcessString(path); stateMachine.ProcessString(L"> "); - stateMachine.ProcessString(L"\x1b]133;B\x7"); + stateMachine.ProcessString(FTCS_B); }; _writePrompt(sm, L"C:\\Windows"); sm.ProcessString(L"Foo-bar"); - sm.ProcessString(L"\x1b]133;C\x7"); + sm.ProcessString(FTCS_C); sm.ProcessString(L"\r\n"); sm.ProcessString(L"This is some text \r\n"); // y=1 sm.ProcessString(L"with varying amounts \r\n"); // y=2 @@ -4409,7 +4418,7 @@ void ConptyRoundtripTests::SimplePromptRegions() Log::Comment(L"========== Check host buffer =========="); verifyBuffer(*hostTb); - // DebugBreak(); + Log::Comment(L"Painting the frame"); VERIFY_SUCCEEDED(renderer.PaintFrame()); @@ -4545,8 +4554,8 @@ void ConptyRoundtripTests::MultilinePromptRegions() // `> ` // // which two rows. The first is 17 characters for C:\Windows - stateMachine.ProcessString(L"\x1b]133;D\x7"); - stateMachine.ProcessString(L"\x1b]133;A\x7"); + stateMachine.ProcessString(FTCS_D); + stateMachine.ProcessString(FTCS_A); stateMachine.ProcessString(L"\x1b]9;9;"); stateMachine.ProcessString(path); stateMachine.ProcessString(L"\x7"); @@ -4554,12 +4563,12 @@ void ConptyRoundtripTests::MultilinePromptRegions() stateMachine.ProcessString(path); stateMachine.ProcessString(L" >\r\n"); stateMachine.ProcessString(L"> "); - stateMachine.ProcessString(L"\x1b]133;B\x7"); + stateMachine.ProcessString(FTCS_B); }; _writePrompt(sm, L"C:\\Windows"); // y=0,1 sm.ProcessString(L"Foo-bar"); - sm.ProcessString(L"\x1b]133;C\x7"); + sm.ProcessString(FTCS_C); sm.ProcessString(L"\r\n"); sm.ProcessString(L"This is some text \r\n"); // y=2 sm.ProcessString(L"with varying amounts \r\n"); // y=3 @@ -4569,7 +4578,7 @@ void ConptyRoundtripTests::MultilinePromptRegions() Log::Comment(L"========== Check host buffer =========="); verifyBuffer(*hostTb); - // DebugBreak(); + Log::Comment(L"Painting the frame"); VERIFY_SUCCEEDED(renderer.PaintFrame()); @@ -4739,8 +4748,8 @@ void ConptyRoundtripTests::ManyMultilinePromptsWithTrailingSpaces() // `> ` // // which two rows. The first is 17 characters for C:\Windows - stateMachine.ProcessString(L"\x1b]133;D\x7"); - stateMachine.ProcessString(L"\x1b]133;A\x7"); + stateMachine.ProcessString(FTCS_D); + stateMachine.ProcessString(FTCS_A); stateMachine.ProcessString(L"\x1b]9;9;"); stateMachine.ProcessString(path); stateMachine.ProcessString(L"\x7"); @@ -4748,11 +4757,11 @@ void ConptyRoundtripTests::ManyMultilinePromptsWithTrailingSpaces() stateMachine.ProcessString(path); stateMachine.ProcessString(L" >\r\n"); stateMachine.ProcessString(L"> "); - stateMachine.ProcessString(L"\x1b]133;B\x7"); + stateMachine.ProcessString(FTCS_B); }; auto writeCommand = [](StateMachine& stateMachine, const auto& cmd) { stateMachine.ProcessString(cmd); - stateMachine.ProcessString(L"\x1b]133;C\x7"); + stateMachine.ProcessString(FTCS_C); stateMachine.ProcessString(L"\r\n"); }; @@ -4772,10 +4781,164 @@ void ConptyRoundtripTests::ManyMultilinePromptsWithTrailingSpaces() Log::Comment(L"========== Check host buffer =========="); verifyBuffer(*hostTb); - // DebugBreak(); + Log::Comment(L"Painting the frame"); VERIFY_SUCCEEDED(renderer.PaintFrame()); Log::Comment(L"========== Check terminal buffer =========="); verifyBuffer(*termTb); } + +void ConptyRoundtripTests::ReflowPromptRegions() +{ + BEGIN_TEST_METHOD_PROPERTIES() + TEST_METHOD_PROPERTY(L"IsolationLevel", L"Method") // always isolate things that resize the buffer + TEST_METHOD_PROPERTY(L"Data:dx", L"{-15, -1, 0, 1, 15}") + END_TEST_METHOD_PROPERTIES() + + INIT_TEST_PROPERTY(int, dx, L"The change in width of the buffer"); + + auto& g = ServiceLocator::LocateGlobals(); + auto& renderer = *g.pRender; + auto& gci = g.getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer(); + auto& sm = si.GetStateMachine(); + + auto* hostTb = &si.GetTextBuffer(); + auto* termTb = term->_mainBuffer.get(); + + gci.LockConsole(); // Lock must be taken to manipulate alt/main buffer state. + auto unlock = wil::scope_exit([&] { gci.UnlockConsole(); }); + + _flushFirstFrame(); + + _checkConptyOutput = false; + + auto originalWidth = term->GetViewport().Width(); + + auto verifyBuffer = [&](const TextBuffer& tb, const til::rect& /*viewport*/, const bool /*isTerminal*/, const bool afterResize) { + const WEX::TestExecution::DisableVerifyExceptions disableExceptionsScope; + + // Just the dx=+1 case doesn't unwrap the line onto one line, but the dx=+15 case does. + const bool unwrapped = afterResize && dx > 1; + const int unwrapAdjust = unwrapped ? -1 : 0; + const auto marks = tb.GetMarkExtents(); + VERIFY_ARE_EQUAL(3u, marks.size()); + { + Log::Comment(L"Mark 0"); + + auto& mark = marks[0]; + const til::point expectedStart{ 0, 0 }; + const til::point expectedEnd{ 10, 0 }; + const til::point expectedOutputStart{ 17, 0 }; // `Foo-Bar` is 7 characters + const til::point expectedOutputEnd{ 13, 3 }; + VERIFY_ARE_EQUAL(expectedStart, mark.start); + VERIFY_ARE_EQUAL(expectedEnd, mark.end); + + VERIFY_ARE_EQUAL(expectedOutputStart, *mark.commandEnd); + VERIFY_ARE_EQUAL(expectedOutputEnd, *mark.outputEnd); + } + { + Log::Comment(L"Mark 1"); + + auto& mark = marks[1]; + const til::point expectedStart{ 0, 4 }; + const til::point expectedEnd{ 10, 4 }; + // {originalWidth} characters of 'F', maybe wrapped. + const til::point originalPos = til::point{ 10, 5 }; + til::point afterPos = originalPos; + // walk that original pos dx times into the actual real place in the buffer. + auto bufferViewport = tb.GetSize(); + const auto walkDir = Viewport::WalkDir{ dx < 0 ? Viewport::XWalk::LeftToRight : Viewport::XWalk::RightToLeft, + dx < 0 ? Viewport::YWalk::TopToBottom : Viewport::YWalk::BottomToTop }; + for (auto i = 0; i < std::abs(dx); i++) + { + bufferViewport.WalkInBounds(afterPos, + walkDir); + } + const auto expectedOutputStart = !afterResize ? + originalPos : // printed exactly a row, so we're exactly below the prompt + afterPos; + const til::point expectedOutputEnd{ 22, 6 + unwrapAdjust }; + VERIFY_ARE_EQUAL(expectedStart, mark.start); + VERIFY_ARE_EQUAL(expectedEnd, mark.end); + + VERIFY_ARE_EQUAL(expectedOutputStart, *mark.commandEnd); + VERIFY_ARE_EQUAL(expectedOutputEnd, *mark.outputEnd); + } + { + Log::Comment(L"Mark 2"); + + auto& mark = marks[2]; + const til::point expectedStart{ 0, 7 + unwrapAdjust }; + const til::point expectedEnd{ 10, 7 + unwrapAdjust }; + VERIFY_ARE_EQUAL(expectedStart, mark.start); + VERIFY_ARE_EQUAL(expectedEnd, mark.end); + VERIFY_IS_TRUE(mark.commandEnd.has_value()); + VERIFY_IS_FALSE(mark.outputEnd.has_value()); + } + }; + + Log::Comment(L"========== Fill test content =========="); + + auto writePrompt = [](StateMachine& stateMachine, const auto& path) { + // A prompt looks like: + // `PWSH C:\> ` + // + // which is 10 characters for "C:\" + stateMachine.ProcessString(FTCS_D); + stateMachine.ProcessString(FTCS_A); + stateMachine.ProcessString(L"\x1b]9;9;"); + stateMachine.ProcessString(path); + stateMachine.ProcessString(L"\x7"); + stateMachine.ProcessString(L"PWSH "); + stateMachine.ProcessString(path); + stateMachine.ProcessString(L"> "); + stateMachine.ProcessString(FTCS_B); + }; + auto writeCommand = [](StateMachine& stateMachine, const auto& cmd) { + stateMachine.ProcessString(cmd); + stateMachine.ProcessString(FTCS_C); + stateMachine.ProcessString(L"\r\n"); + }; + + // This first prompt didn't reflow at all + writePrompt(sm, L"C:\\"); // y=0 + writeCommand(sm, L"Foo-bar"); // y=0 + sm.ProcessString(L"This is some text \r\n"); // y=1 + sm.ProcessString(L"with varying amounts \r\n"); // y=2 + sm.ProcessString(L"of whitespace\r\n"); // y=3 + + // This second one, the command does. It stretches across lines + writePrompt(sm, L"C:\\"); // y=4 + writeCommand(sm, std::wstring(originalWidth, L'F')); // y=4,5 + sm.ProcessString(L"This is more text \r\n"); // y=6 + + writePrompt(sm, L"C:\\"); // y=7 + writeCommand(sm, L"yikes?"); // y=7 + + Log::Comment(L"========== Checking the host buffer state (before) =========="); + verifyBuffer(*hostTb, si.GetViewport().ToExclusive(), false, false); + + Log::Comment(L"Painting the frame"); + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + Log::Comment(L"========== Checking the terminal buffer state (before) =========="); + verifyBuffer(*termTb, term->_mutableViewport.ToExclusive(), true, false); + + // After we resize, make sure to get the new textBuffers + std::tie(hostTb, termTb) = _performResize({ TerminalViewWidth + dx, + TerminalViewHeight }); + + Log::Comment(L"Painting the frame"); + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + Log::Comment(L"========== Checking the host buffer state (after) =========="); + verifyBuffer(*hostTb, si.GetViewport().ToExclusive(), false, true); + + Log::Comment(L"Painting the frame"); + VERIFY_SUCCEEDED(renderer.PaintFrame()); + + Log::Comment(L"========== Checking the terminal buffer state (after) =========="); + verifyBuffer(*termTb, term->_mutableViewport.ToExclusive(), true, true); +} diff --git a/src/host/VtIo.cpp b/src/host/VtIo.cpp index 3d6412add00..23500c2f6a8 100644 --- a/src/host/VtIo.cpp +++ b/src/host/VtIo.cpp @@ -446,9 +446,10 @@ void VtIo::CorkRenderer(bool corked) const noexcept // - vtRenderEngine: a VT renderer that our VtIo should use as the vt engine during these tests // Return Value: // - -void VtIo::EnableConptyModeForTests(std::unique_ptr vtRenderEngine) +void VtIo::EnableConptyModeForTests(std::unique_ptr vtRenderEngine, const bool resizeQuirk) { _initialized = true; + _resizeQuirk = resizeQuirk; _pVtRenderEngine = std::move(vtRenderEngine); } #endif diff --git a/src/host/VtIo.hpp b/src/host/VtIo.hpp index 7cd12036f5d..eccdb06aaac 100644 --- a/src/host/VtIo.hpp +++ b/src/host/VtIo.hpp @@ -43,7 +43,7 @@ namespace Microsoft::Console::VirtualTerminal void CorkRenderer(bool corked) const noexcept; #ifdef UNIT_TESTING - void EnableConptyModeForTests(std::unique_ptr vtRenderEngine); + void EnableConptyModeForTests(std::unique_ptr vtRenderEngine, const bool resizeQuirk = false); #endif bool IsResizeQuirkEnabled() const; diff --git a/src/host/getset.cpp b/src/host/getset.cpp index 3d371d4b24f..732c3295b57 100644 --- a/src/host/getset.cpp +++ b/src/host/getset.cpp @@ -859,7 +859,9 @@ void ApiRoutines::GetLargestConsoleWindowSizeImpl(const SCREEN_INFORMATION& cont // GH#3490 - If we're in conpty mode, don't invalidate the entire // viewport. In conpty mode, the VtEngine will later decide what // part of the buffer actually needs to be re-sent to the terminal. - if (!(g.getConsoleInformation().IsInVtIoMode() && g.getConsoleInformation().GetVtIo()->IsResizeQuirkEnabled())) + const auto inConpty{ g.getConsoleInformation().IsInVtIoMode() }; + const auto resizeQuirk{ g.getConsoleInformation().GetVtIo()->IsResizeQuirkEnabled() }; + if (!(inConpty && resizeQuirk)) { WriteToScreen(context, context.GetViewport()); } diff --git a/src/host/globals.cpp b/src/host/globals.cpp index f36fc3686ab..d1bbb4f297b 100644 --- a/src/host/globals.cpp +++ b/src/host/globals.cpp @@ -35,9 +35,9 @@ bool Globals::IsHeadless() const // - vtRenderEngine: a VT renderer that our VtIo should use as the vt engine during these tests // Return Value: // - -void Globals::EnableConptyModeForTests(std::unique_ptr vtRenderEngine) +void Globals::EnableConptyModeForTests(std::unique_ptr vtRenderEngine, const bool resizeQuirk) { launchArgs.EnableConptyModeForTests(); - getConsoleInformation().GetVtIo()->EnableConptyModeForTests(std::move(vtRenderEngine)); + getConsoleInformation().GetVtIo()->EnableConptyModeForTests(std::move(vtRenderEngine), resizeQuirk); } #endif diff --git a/src/host/globals.h b/src/host/globals.h index 589cf166480..9cebbc0808a 100644 --- a/src/host/globals.h +++ b/src/host/globals.h @@ -77,7 +77,7 @@ class Globals bool defaultTerminalMarkerCheckRequired = false; #ifdef UNIT_TESTING - void EnableConptyModeForTests(std::unique_ptr vtRenderEngine); + void EnableConptyModeForTests(std::unique_ptr vtRenderEngine, const bool resizeQuirk = false); #endif private: diff --git a/src/host/ut_host/ScreenBufferTests.cpp b/src/host/ut_host/ScreenBufferTests.cpp index db596e47066..7e34d2f31e6 100644 --- a/src/host/ut_host/ScreenBufferTests.cpp +++ b/src/host/ut_host/ScreenBufferTests.cpp @@ -8422,7 +8422,6 @@ void ScreenBufferTests::SimpleMarkCommand() auto& tbi = si.GetTextBuffer(); auto& stateMachine = si.GetStateMachine(); - // Process the opening osc 8 sequence with no custom id stateMachine.ProcessString(L"Zero\n"); { @@ -8462,7 +8461,6 @@ void ScreenBufferTests::SimpleWrappedCommand() auto& tbi = si.GetTextBuffer(); auto& stateMachine = si.GetStateMachine(); - // Process the opening osc 8 sequence with no custom id stateMachine.ProcessString(L"Zero\n"); const auto oneHundredZeros = std::wstring(100, L'0'); @@ -8507,17 +8505,17 @@ void ScreenBufferTests::SimpleWrappedCommand() VERIFY_ARE_EQUAL(expectedCommands, tbi.Commands()); } -void _writePrompt(StateMachine& stateMachine, const auto& path) +static void _writePrompt(StateMachine& stateMachine, const auto& path) { - stateMachine.ProcessString(L"\x1b]133;D\x7"); - stateMachine.ProcessString(L"\x1b]133;A\x7"); + stateMachine.ProcessString(FTCS_D); + stateMachine.ProcessString(FTCS_A); stateMachine.ProcessString(L"\x1b]9;9;"); stateMachine.ProcessString(path); stateMachine.ProcessString(L"\x7"); stateMachine.ProcessString(L"PWSH "); stateMachine.ProcessString(path); stateMachine.ProcessString(L"> "); - stateMachine.ProcessString(L"\x1b]133;B\x7"); + stateMachine.ProcessString(FTCS_B); } void ScreenBufferTests::SimplePromptRegions() @@ -8535,7 +8533,7 @@ void ScreenBufferTests::SimplePromptRegions() _writePrompt(stateMachine, L"C:\\Windows"); stateMachine.ProcessString(L"Foo-bar"); - stateMachine.ProcessString(L"\x1b]133;C\x7"); + stateMachine.ProcessString(FTCS_C); stateMachine.ProcessString(L"\r\n"); stateMachine.ProcessString(L"This is some text \r\n"); // y=1 stateMachine.ProcessString(L"with varying amounts \r\n"); // y=2 diff --git a/src/terminal/adapter/adaptDispatch.cpp b/src/terminal/adapter/adaptDispatch.cpp index 9af916eaf10..cdf50148ef3 100644 --- a/src/terminal/adapter/adaptDispatch.cpp +++ b/src/terminal/adapter/adaptDispatch.cpp @@ -3794,7 +3794,7 @@ bool AdaptDispatch::DoFinalTermAction(const std::wstring_view string) UINT_MAX; } - _api.GetTextBuffer().EndOutput(error); + _api.GetTextBuffer().EndCurrentCommand(error); handled = true; break; From 11d82a2a791e9ce345c3f521fedf9d1ada5f04b6 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 4 Apr 2024 16:14:33 -0500 Subject: [PATCH 25/30] a TextBufferTest for @dhowett --- .../ConptyRoundtripTests.cpp | 2 +- src/host/ut_host/TextBufferTests.cpp | 134 ++++++++++++++++++ 2 files changed, 135 insertions(+), 1 deletion(-) diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index 584cf07378f..d5457688a50 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -132,7 +132,7 @@ class TerminalCoreUnitTests::ConptyRoundtripTests final // to set up the pipes for conpty, but we want the console to behave // like it would in conpty mode. // - // Also, make sure to backdoor neable the resize quirk here too. + // Also, make sure to backdoor enable the resize quirk here too. g.EnableConptyModeForTests(std::move(vtRenderEngine), true); expectedOutput.clear(); diff --git a/src/host/ut_host/TextBufferTests.cpp b/src/host/ut_host/TextBufferTests.cpp index 648b054778b..33660d876a8 100644 --- a/src/host/ut_host/TextBufferTests.cpp +++ b/src/host/ut_host/TextBufferTests.cpp @@ -161,6 +161,8 @@ class TextBufferTests TEST_METHOD(HyperlinkTrim); TEST_METHOD(NoHyperlinkTrim); + + TEST_METHOD(ReflowPromptRegions); }; void TextBufferTests::TestBufferCreate() @@ -2852,3 +2854,135 @@ void TextBufferTests::NoHyperlinkTrim() VERIFY_ARE_EQUAL(_buffer->GetHyperlinkUriFromId(id), url); VERIFY_ARE_EQUAL(_buffer->_hyperlinkCustomIdMap[finalCustomId], id); } + +#define FTCS_A L"\x1b]133;A\x1b\\" +#define FTCS_B L"\x1b]133;B\x1b\\" +#define FTCS_C L"\x1b]133;C\x1b\\" +#define FTCS_D L"\x1b]133;D\x1b\\" +void TextBufferTests::ReflowPromptRegions() +{ + BEGIN_TEST_METHOD_PROPERTIES() + TEST_METHOD_PROPERTY(L"IsolationLevel", L"Method") // always isolate things that resize the buffer + TEST_METHOD_PROPERTY(L"Data:dx", L"{-15, -1, 0, 1, 15}") + END_TEST_METHOD_PROPERTIES() + + INIT_TEST_PROPERTY(int, dx, L"The change in width of the buffer"); + + auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); + auto& si = gci.GetActiveOutputBuffer().GetActiveBuffer(); + auto* tbi = &si.GetTextBuffer(); + auto& sm = si.GetStateMachine(); + const auto oldSize{ tbi->GetSize() }; + + auto verifyBuffer = [&](const TextBuffer& tb, const til::rect& /*viewport*/, const bool /*isTerminal*/, const bool afterResize) { + const WEX::TestExecution::DisableVerifyExceptions disableExceptionsScope; + + // Just the dx=+1 case doesn't unwrap the line onto one line, but the dx=+15 case does. + const bool unwrapped = afterResize && dx > 1; + const int unwrapAdjust = unwrapped ? -1 : 0; + const auto marks = tb.GetMarkExtents(); + VERIFY_ARE_EQUAL(3u, marks.size()); + { + Log::Comment(L"Mark 0"); + + auto& mark = marks[0]; + const til::point expectedStart{ 0, 0 }; + const til::point expectedEnd{ 10, 0 }; + const til::point expectedOutputStart{ 17, 0 }; // `Foo-Bar` is 7 characters + const til::point expectedOutputEnd{ 13, 3 }; + VERIFY_ARE_EQUAL(expectedStart, mark.start); + VERIFY_ARE_EQUAL(expectedEnd, mark.end); + + VERIFY_ARE_EQUAL(expectedOutputStart, *mark.commandEnd); + VERIFY_ARE_EQUAL(expectedOutputEnd, *mark.outputEnd); + } + { + Log::Comment(L"Mark 1"); + + auto& mark = marks[1]; + const til::point expectedStart{ 0, 4 }; + const til::point expectedEnd{ 10, 4 }; + // {originalWidth} characters of 'F', maybe wrapped. + const til::point originalPos = til::point{ 10, 5 }; + til::point afterPos = originalPos; + // walk that original pos dx times into the actual real place in the buffer. + auto bufferViewport = tb.GetSize(); + const auto walkDir = Viewport::WalkDir{ dx < 0 ? Viewport::XWalk::LeftToRight : Viewport::XWalk::RightToLeft, + dx < 0 ? Viewport::YWalk::TopToBottom : Viewport::YWalk::BottomToTop }; + for (auto i = 0; i < std::abs(dx); i++) + { + bufferViewport.WalkInBounds(afterPos, + walkDir); + } + const auto expectedOutputStart = !afterResize ? + originalPos : // printed exactly a row, so we're exactly below the prompt + afterPos; + const til::point expectedOutputEnd{ 22, 6 + unwrapAdjust }; + VERIFY_ARE_EQUAL(expectedStart, mark.start); + VERIFY_ARE_EQUAL(expectedEnd, mark.end); + + VERIFY_ARE_EQUAL(expectedOutputStart, *mark.commandEnd); + VERIFY_ARE_EQUAL(expectedOutputEnd, *mark.outputEnd); + } + { + Log::Comment(L"Mark 2"); + + auto& mark = marks[2]; + const til::point expectedStart{ 0, 7 + unwrapAdjust }; + const til::point expectedEnd{ 10, 7 + unwrapAdjust }; + VERIFY_ARE_EQUAL(expectedStart, mark.start); + VERIFY_ARE_EQUAL(expectedEnd, mark.end); + VERIFY_IS_TRUE(mark.commandEnd.has_value()); + VERIFY_IS_FALSE(mark.outputEnd.has_value()); + } + }; + + Log::Comment(L"========== Fill test content =========="); + + auto writePrompt = [](StateMachine& stateMachine, const auto& path) { + // A prompt looks like: + // `PWSH C:\> ` + // + // which is 10 characters for "C:\" + stateMachine.ProcessString(FTCS_D); + stateMachine.ProcessString(FTCS_A); + stateMachine.ProcessString(L"\x1b]9;9;"); + stateMachine.ProcessString(path); + stateMachine.ProcessString(L"\x7"); + stateMachine.ProcessString(L"PWSH "); + stateMachine.ProcessString(path); + stateMachine.ProcessString(L"> "); + stateMachine.ProcessString(FTCS_B); + }; + auto writeCommand = [](StateMachine& stateMachine, const auto& cmd) { + stateMachine.ProcessString(cmd); + stateMachine.ProcessString(FTCS_C); + stateMachine.ProcessString(L"\r\n"); + }; + + // This first prompt didn't reflow at all + writePrompt(sm, L"C:\\"); // y=0 + writeCommand(sm, L"Foo-bar"); // y=0 + sm.ProcessString(L"This is some text \r\n"); // y=1 + sm.ProcessString(L"with varying amounts \r\n"); // y=2 + sm.ProcessString(L"of whitespace\r\n"); // y=3 + + // This second one, the command does. It stretches across lines + writePrompt(sm, L"C:\\"); // y=4 + writeCommand(sm, std::wstring(oldSize.Width(), L'F')); // y=4,5 + sm.ProcessString(L"This is more text \r\n"); // y=6 + + writePrompt(sm, L"C:\\"); // y=7 + writeCommand(sm, L"yikes?"); // y=7 + + Log::Comment(L"========== Checking the buffer state (before) =========="); + verifyBuffer(*tbi, si.GetViewport().ToExclusive(), false, false); + + // After we resize, make sure to get the new textBuffers + til::size newSize{ oldSize.Width() + dx, oldSize.Height() }; + auto newBuffer = std::make_unique(newSize, TextAttribute{ 0x7 }, 0, false, _renderer); + TextBuffer::Reflow(*tbi, *newBuffer); + + Log::Comment(L"========== Checking the host buffer state (after) =========="); + verifyBuffer(*newBuffer, si.GetViewport().ToExclusive(), false, true); +} From 3cf6a3cca53352eccd3b82bf3bd1e50d3e0ad7a5 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 4 Apr 2024 16:35:44 -0500 Subject: [PATCH 26/30] I agree with all this strongly --- src/buffer/out/textBuffer.cpp | 27 +++++++++++++++++++------- src/buffer/out/textBuffer.hpp | 4 +++- src/cascadia/TerminalCore/Terminal.cpp | 16 ++++++--------- src/cascadia/TerminalCore/Terminal.hpp | 2 +- 4 files changed, 30 insertions(+), 19 deletions(-) diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index ab334c725c1..a617362a60b 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -3177,9 +3177,9 @@ std::vector TextBuffer::GetMarkRows() const // // Use `limit` to control how many you get, _starting from the bottom_. (e.g. // limit=1 will just give you the "most recent mark"). -std::vector TextBuffer::GetMarkExtents(std::optional limit) const +std::vector TextBuffer::GetMarkExtents(size_t limit) const { - if (limit == 0) + if (limit == 0u) { return {}; } @@ -3217,8 +3217,7 @@ std::vector TextBuffer::GetMarkExtents(std::optional limit) // operator>=(T, optional) will return true if the optional is // nullopt, unfortunately. - if (limit.has_value() && - marks.size() >= *limit) + if (marks.size() >= limit) { break; } @@ -3532,9 +3531,9 @@ bool TextBuffer::StartOutput() // the exit code on that row's scroll mark. void TextBuffer::EndCurrentCommand(std::optional error) { - // auto attr = GetCurrentAttributes(); - // attr.SetMarkAttributes(MarkKind::None); - // SetCurrentAttributes(attr); + auto attr = GetCurrentAttributes(); + attr.SetMarkAttributes(MarkKind::None); + SetCurrentAttributes(attr); for (auto y = GetCursor().GetPosition().y; y >= 0; y--) { @@ -3547,3 +3546,17 @@ void TextBuffer::EndCurrentCommand(std::optional error) } } } + +void TextBuffer::SetScrollbarData(ScrollbarData mark, til::CoordType y) +{ + auto& row = GetMutableRowByOffset(y); + row.SetScrollbarData(mark); +} +void TextBuffer::ManuallyMarkRowAsPrompt(til::CoordType y) +{ + auto& row = GetMutableRowByOffset(y); + for (auto& [attr, len] : row.Attributes().runs()) + { + attr.SetMarkAttributes(MarkKind::Prompt); + } +} diff --git a/src/buffer/out/textBuffer.hpp b/src/buffer/out/textBuffer.hpp index a0c8a9868a6..79872cea0c4 100644 --- a/src/buffer/out/textBuffer.hpp +++ b/src/buffer/out/textBuffer.hpp @@ -299,7 +299,7 @@ class TextBuffer final // Mark handling std::vector GetMarkRows() const; - std::vector GetMarkExtents(std::optional limit = std::nullopt) const; + std::vector GetMarkExtents(size_t limit = SIZE_T_MAX) const; void ClearMarksInRange(const til::point start, const til::point end); void ClearAllMarks(); std::wstring CurrentCommand() const; @@ -308,6 +308,8 @@ class TextBuffer final bool StartCommand(); bool StartOutput(); void EndCurrentCommand(std::optional error); + void SetScrollbarData(ScrollbarData mark, til::CoordType y); + void ManuallyMarkRowAsPrompt(til::CoordType y); private: void _reserve(til::size screenBufferSize, const TextAttribute& defaultAttributes); diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 72c48d9dd27..074247ec934 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -735,11 +735,8 @@ TerminalInput::OutputType Terminal::SendCharEvent(const wchar_t ch, const WORD s const bool createdMark = _activeBuffer().StartOutput(); if (createdMark) { - auto& row = _activeBuffer().GetMutableRowByOffset(_activeBuffer().GetCursor().GetPosition().y); - for (auto& [attr, len] : row.Attributes().runs()) - { - attr.SetMarkAttributes(MarkKind::Prompt); - } + _activeBuffer().ManuallyMarkRowAsPrompt(_activeBuffer().GetCursor().GetPosition().y); + // This changed the scrollbar marks - raise a notification to update them _NotifyScrollEvent(); } @@ -1445,15 +1442,14 @@ PointTree Terminal::_getPatterns(til::CoordType beg, til::CoordType end) const // NOTE: This is the version of AddMark that comes from the UI. The VT api call into this too. void Terminal::AddMarkFromUI(ScrollbarData mark, - const til::CoordType& y) + til::CoordType y) { if (_inAltBuffer()) { return; } - auto& row = _activeBuffer().GetMutableRowByOffset(y); - row.SetScrollbarData(mark); + _activeBuffer().SetScrollbarData(mark, y); // Tell the control that the scrollbar has somehow changed. Used as a // workaround to force the control to redraw any scrollbar marks @@ -1492,13 +1488,13 @@ std::vector Terminal::GetMarkRows() const { // We want to return _no_ marks when we're in the alt buffer, to effectively // hide them. - return _inAltBuffer() ? std::vector{} : std::move(_activeBuffer().GetMarkRows()); + return _inAltBuffer() ? std::vector{} : _activeBuffer().GetMarkRows(); } std::vector Terminal::GetMarkExtents() const { // We want to return _no_ marks when we're in the alt buffer, to effectively // hide them. - return _inAltBuffer() ? std::vector{} : std::move(_activeBuffer().GetMarkExtents()); + return _inAltBuffer() ? std::vector{} : _activeBuffer().GetMarkExtents(); } til::color Terminal::GetColorForMark(const ScrollbarData& markData) const diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index cb00b7a8287..e792ee8d21c 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -119,7 +119,7 @@ class Microsoft::Terminal::Core::Terminal final : std::vector GetMarkRows() const; std::vector GetMarkExtents() const; - void AddMarkFromUI(ScrollbarData mark, const til::CoordType& y); + void AddMarkFromUI(ScrollbarData mark, til::CoordType y); til::property AlwaysNotifyOnBufferRotation; From e0fa1c3076e9d542eaecb77e44b39d59cf0663bb Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 5 Apr 2024 13:32:51 -0500 Subject: [PATCH 27/30] a very weird reflow bug? --- src/cascadia/TerminalCore/Terminal.cpp | 7 +++++++ .../UnitTests_TerminalCore/ConptyRoundtripTests.cpp | 5 ++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 074247ec934..83837518993 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -411,6 +411,13 @@ try // before, and shouldn't be now either. _scrollOffset = originalOffsetWasZero ? 0 : static_cast(::base::ClampSub(_mutableViewport.Top(), newVisibleTop)); + // Make sure that the new cursor position is still in the mutable viewport + const auto currentNewPos = _mainBuffer->GetCursor().GetPosition(); + if (currentNewPos.y < proposedTop) + { + _mainBuffer->GetCursor().SetPosition(til::point{ currentNewPos.x, proposedTop }); + } + _mainBuffer->TriggerRedrawAll(); _NotifyScrollEvent(); return S_OK; diff --git a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp index d5457688a50..4c582d0f44a 100644 --- a/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp +++ b/src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp @@ -753,6 +753,9 @@ void ConptyRoundtripTests::MoveCursorAtEOL() VERIFY_SUCCEEDED(renderer.PaintFrame()); verifyData1(termTb); + + // This test is sometimes flaky in cleanup. + expectedOutput.clear(); } void ConptyRoundtripTests::TestResizeHeight() @@ -2646,6 +2649,7 @@ void ConptyRoundtripTests::ResizeRepaintVimExeBuffer() auto verifyBuffer = [&](const TextBuffer& tb, const til::rect& viewport) { const auto firstRow = viewport.top; const auto width = viewport.width(); + const WEX::TestExecution::DisableVerifyExceptions disableExceptionsScope; // First row VERIFY_IS_FALSE(tb.GetRowByOffset(firstRow).WasWrapForced()); @@ -2702,7 +2706,6 @@ void ConptyRoundtripTests::ResizeRepaintVimExeBuffer() Log::Comment(L"========== Checking the host buffer state (after) =========="); verifyBuffer(*hostTb, si.GetViewport().ToExclusive()); - Log::Comment(L"Painting the frame"); VERIFY_SUCCEEDED(renderer.PaintFrame()); From 4da4d1f8bd376c0c098cb023d96bb9fefd740639 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 5 Apr 2024 13:34:24 -0500 Subject: [PATCH 28/30] last nits --- src/buffer/out/Row.cpp | 20 +++++++++++++++----- src/buffer/out/textBuffer.cpp | 16 ++++------------ src/host/getset.cpp | 5 ++--- 3 files changed, 21 insertions(+), 20 deletions(-) diff --git a/src/buffer/out/Row.cpp b/src/buffer/out/Row.cpp index 1c648227089..8e318da41ef 100644 --- a/src/buffer/out/Row.cpp +++ b/src/buffer/out/Row.cpp @@ -230,6 +230,7 @@ void ROW::Reset(const TextAttribute& attr) noexcept _lineRendition = LineRendition::SingleWidth; _wrapForced = false; _doubleBytePadded = false; + _promptData = std::nullopt; _init(); } @@ -1195,11 +1196,20 @@ void ROW::StartPrompt() noexcept { if (!_promptData.has_value()) { - _promptData = ScrollbarData{ - .category = MarkCategory::Prompt, - .color = std::nullopt, - .exitCode = std::nullopt, - }; + // You'd be tempted to write: + // + // _promptData = ScrollbarData{ + // .category = MarkCategory::Prompt, + // .color = std::nullopt, + // .exitCode = std::nullopt, + // }; + // + // But that's not very optimal! Read this thread for a breakdown of how + // weird std::optional can be some times: + // + // https://github.com/microsoft/terminal/pull/16937#discussion_r1553660833 + + _promptData.emplace(MarkCategory::Prompt); } } diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index a617362a60b..6daeb6fa25c 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -3460,9 +3460,7 @@ void TextBuffer::StartPrompt() auto& currentRow = GetMutableRowByOffset(currentRowOffset); currentRow.StartPrompt(); - auto attr = GetCurrentAttributes(); - attr.SetMarkAttributes(MarkKind::Prompt); - SetCurrentAttributes(attr); + _currentAttributes.SetMarkAttributes(MarkKind::Prompt); } bool TextBuffer::_createPromptMarkIfNeeded() @@ -3513,17 +3511,13 @@ bool TextBuffer::_createPromptMarkIfNeeded() bool TextBuffer::StartCommand() { const auto createdMark = _createPromptMarkIfNeeded(); - auto attr = GetCurrentAttributes(); - attr.SetMarkAttributes(MarkKind::Command); - SetCurrentAttributes(attr); + _currentAttributes.SetMarkAttributes(MarkKind::Command); return createdMark; } bool TextBuffer::StartOutput() { const auto createdMark = _createPromptMarkIfNeeded(); - auto attr = GetCurrentAttributes(); - attr.SetMarkAttributes(MarkKind::Output); - SetCurrentAttributes(attr); + _currentAttributes.SetMarkAttributes(MarkKind::Output); return createdMark; } @@ -3531,9 +3525,7 @@ bool TextBuffer::StartOutput() // the exit code on that row's scroll mark. void TextBuffer::EndCurrentCommand(std::optional error) { - auto attr = GetCurrentAttributes(); - attr.SetMarkAttributes(MarkKind::None); - SetCurrentAttributes(attr); + _currentAttributes.SetMarkAttributes(MarkKind::None); for (auto y = GetCursor().GetPosition().y; y >= 0; y--) { diff --git a/src/host/getset.cpp b/src/host/getset.cpp index 732c3295b57..495771fdad3 100644 --- a/src/host/getset.cpp +++ b/src/host/getset.cpp @@ -859,9 +859,8 @@ void ApiRoutines::GetLargestConsoleWindowSizeImpl(const SCREEN_INFORMATION& cont // GH#3490 - If we're in conpty mode, don't invalidate the entire // viewport. In conpty mode, the VtEngine will later decide what // part of the buffer actually needs to be re-sent to the terminal. - const auto inConpty{ g.getConsoleInformation().IsInVtIoMode() }; - const auto resizeQuirk{ g.getConsoleInformation().GetVtIo()->IsResizeQuirkEnabled() }; - if (!(inConpty && resizeQuirk)) + if (!(g.getConsoleInformation().IsInVtIoMode() && + g.getConsoleInformation().GetVtIo()->IsResizeQuirkEnabled())) { WriteToScreen(context, context.GetViewport()); } From 351208b6b9f339bf3188b0923f383171f71a6dbe Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 5 Apr 2024 14:28:24 -0500 Subject: [PATCH 29/30] this is a noop --- src/buffer/out/textBuffer.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/buffer/out/textBuffer.cpp b/src/buffer/out/textBuffer.cpp index 6daeb6fa25c..44e1e6b0714 100644 --- a/src/buffer/out/textBuffer.cpp +++ b/src/buffer/out/textBuffer.cpp @@ -3340,10 +3340,7 @@ MarkExtents TextBuffer::_scrollMarkExtentForRow(const til::CoordType rowOffset, endThisMark(lastMarkedText.x, lastMarkedText.y); } // Otherwise, we've changed from any state -> any state, and it doesn't really matter. - if (markKind != MarkKind::None) - { - lastMarkKind = markKind; - } + lastMarkKind = markKind; } // advance to next run of text x = nextX; From dc21f313e05156a0bb59448367adbf85fa881a5a Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 5 Apr 2024 14:35:28 -0500 Subject: [PATCH 30/30] do what leonard suggested instead --- src/cascadia/TerminalCore/Terminal.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index 83837518993..9d3dd1d1097 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -397,6 +397,9 @@ try proposedTop = ::base::ClampSub(proposedTop, ::base::ClampSub(proposedBottom, bufferSize.height)); } + // Keep the cursor in the mutable viewport + proposedTop = std::min(proposedTop, newCursorPos.y); + _mutableViewport = Viewport::FromDimensions({ 0, proposedTop }, viewportSize); _mainBuffer.swap(newTextBuffer); @@ -411,13 +414,6 @@ try // before, and shouldn't be now either. _scrollOffset = originalOffsetWasZero ? 0 : static_cast(::base::ClampSub(_mutableViewport.Top(), newVisibleTop)); - // Make sure that the new cursor position is still in the mutable viewport - const auto currentNewPos = _mainBuffer->GetCursor().GetPosition(); - if (currentNewPos.y < proposedTop) - { - _mainBuffer->GetCursor().SetPosition(til::point{ currentNewPos.x, proposedTop }); - } - _mainBuffer->TriggerRedrawAll(); _NotifyScrollEvent(); return S_OK;