From b50ddb12bbf4f08e6af595a8064e35bed24b8fc9 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Fri, 20 Dec 2024 16:10:55 -0800 Subject: [PATCH 1/3] Add support for tabbing to embedded hyperlinks --- .../TerminalCore/TerminalSelection.cpp | 58 +++++++++++++++++-- 1 file changed, 52 insertions(+), 6 deletions(-) diff --git a/src/cascadia/TerminalCore/TerminalSelection.cpp b/src/cascadia/TerminalCore/TerminalSelection.cpp index 9501f64a85e..d69fc8f041b 100644 --- a/src/cascadia/TerminalCore/TerminalSelection.cpp +++ b/src/cascadia/TerminalCore/TerminalSelection.cpp @@ -437,7 +437,8 @@ void Terminal::SelectHyperlink(const SearchDirection dir) } // 0. Useful tools/vars - const auto bufferSize = _activeBuffer().GetSize(); + const auto& buffer = _activeBuffer(); + const auto bufferSize = buffer.GetSize(); const auto viewportHeight = _GetMutableViewport().Height(); // The patterns are stored relative to the "search area". Initially, this search area will be the viewport, @@ -547,15 +548,60 @@ void Terminal::SelectHyperlink(const SearchDirection dir) searchArea = Viewport::FromDimensions(searchStart, { searchEnd.x + 1, searchEnd.y + 1 }); } } + } - // 1.C) Nothing was found. Bail! - if (!result.has_value()) + // 2. We found a hyperlink from the pattern tree. Look for embedded hyperlinks too! + // Use the result (if one was found) to narrow down the search. + searchStart = dir == SearchDirection::Forward ? + _selection->start : + (result ? result->second : bufferSize.Origin()); + searchEnd = dir == SearchDirection::Forward ? + (result ? result->first : buffer.GetLastNonSpaceCharacter()) : + _selection->start; + auto iter = buffer.GetCellDataAt(dir == SearchDirection::Forward ? searchStart : searchEnd); + while (dir == SearchDirection::Forward ? iter.Pos() < searchEnd : iter.Pos() > searchStart) + { + // Don't let us select the same hyperlink again + if (iter.Pos() < _selection->start || iter.Pos() > _selection->end) { - return; + if (auto attr = iter->TextAttr(); attr.IsHyperlink()) + { + // Found an embedded hyperlink! + const auto hyperlinkId = attr.GetHyperlinkId(); + + // Expand the start to include the entire hyperlink + TextBufferCellIterator hyperlinkStartIter{ buffer, iter.Pos() }; + while (attr.IsHyperlink() && attr.GetHyperlinkId() == hyperlinkId) + { + hyperlinkStartIter--; + attr = hyperlinkStartIter->TextAttr(); + } + // undo a move to be inclusive + hyperlinkStartIter++; + + // Expand the end to include the entire hyperlink + // No need to undo a move! We'll decrement in the next step anyways. + TextBufferCellIterator hyperlinkEndIter{ buffer, iter.Pos() }; + attr = hyperlinkEndIter->TextAttr(); + while (attr.IsHyperlink() && attr.GetHyperlinkId() == hyperlinkId) + { + hyperlinkEndIter++; + attr = hyperlinkEndIter->TextAttr(); + } + + result = { hyperlinkStartIter.Pos(), hyperlinkEndIter.Pos() }; + break; + } } + iter += dir == SearchDirection::Forward ? 1 : -1; } - // 2. Select the hyperlink + // 3. Select the hyperlink, if one exists + if (!result.has_value()) + { + return; + } + else { auto selection{ _selection.write() }; wil::hide_name _selection; @@ -566,7 +612,7 @@ void Terminal::SelectHyperlink(const SearchDirection dir) _selectionEndpoint = SelectionEndpoint::End; } - // 3. Scroll to the selected area (if necessary) + // 4. Scroll to the selected area (if necessary) _ScrollToPoint(_selection->end); } From 51b712425f9fe137486937b2adfd4a383d7eb70e Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Tue, 4 Feb 2025 14:26:19 -0800 Subject: [PATCH 2/3] unindent --- .../TerminalCore/TerminalSelection.cpp | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/cascadia/TerminalCore/TerminalSelection.cpp b/src/cascadia/TerminalCore/TerminalSelection.cpp index d69fc8f041b..f4dbcb11cef 100644 --- a/src/cascadia/TerminalCore/TerminalSelection.cpp +++ b/src/cascadia/TerminalCore/TerminalSelection.cpp @@ -601,19 +601,16 @@ void Terminal::SelectHyperlink(const SearchDirection dir) { return; } - else - { - auto selection{ _selection.write() }; - wil::hide_name _selection; - selection->start = result->first; - selection->pivot = result->first; - selection->end = result->second; - _selectionIsTargetingUrl = true; - _selectionEndpoint = SelectionEndpoint::End; - } + auto selection{ _selection.write() }; + wil::hide_name _selection; + selection->start = result->first; + selection->pivot = result->first; + selection->end = result->second; + _selectionIsTargetingUrl = true; + _selectionEndpoint = SelectionEndpoint::End; // 4. Scroll to the selected area (if necessary) - _ScrollToPoint(_selection->end); + _ScrollToPoint(selection->end); } Terminal::UpdateSelectionParams Terminal::ConvertKeyEventToUpdateSelectionParams(const ControlKeyStates mods, const WORD vkey) const noexcept From aa612b32855e08b6cc4dfa7c199acd81c25f069b Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Wed, 19 Feb 2025 16:19:57 -0800 Subject: [PATCH 3/3] apply feedback; fix crash when selection off end --- .../TerminalCore/TerminalSelection.cpp | 52 +++++++++++++------ 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/src/cascadia/TerminalCore/TerminalSelection.cpp b/src/cascadia/TerminalCore/TerminalSelection.cpp index f4dbcb11cef..707415015a6 100644 --- a/src/cascadia/TerminalCore/TerminalSelection.cpp +++ b/src/cascadia/TerminalCore/TerminalSelection.cpp @@ -505,8 +505,18 @@ void Terminal::SelectHyperlink(const SearchDirection dir) }; // 1. Look for the hyperlink - til::point searchStart = dir == SearchDirection::Forward ? _selection->start : til::point{ bufferSize.Left(), _VisibleStartIndex() }; - til::point searchEnd = dir == SearchDirection::Forward ? til::point{ bufferSize.RightInclusive(), _VisibleEndIndex() } : _selection->start; + til::point searchStart; + til::point searchEnd; + if (dir == SearchDirection::Forward) + { + searchStart = _selection->start; + searchEnd = til::point{ bufferSize.RightInclusive(), _VisibleEndIndex() }; + } + else + { + searchStart = til::point{ bufferSize.Left(), _VisibleStartIndex() }; + searchEnd = _selection->start; + } // 1.A) Try searching the current viewport (no scrolling required) auto resultList = _patternIntervalTree.findContained(convertToSearchArea(searchStart), convertToSearchArea(searchEnd)); @@ -552,13 +562,22 @@ void Terminal::SelectHyperlink(const SearchDirection dir) // 2. We found a hyperlink from the pattern tree. Look for embedded hyperlinks too! // Use the result (if one was found) to narrow down the search. - searchStart = dir == SearchDirection::Forward ? - _selection->start : - (result ? result->second : bufferSize.Origin()); - searchEnd = dir == SearchDirection::Forward ? - (result ? result->first : buffer.GetLastNonSpaceCharacter()) : - _selection->start; - auto iter = buffer.GetCellDataAt(dir == SearchDirection::Forward ? searchStart : searchEnd); + if (dir == SearchDirection::Forward) + { + searchStart = _selection->start; + searchEnd = (result ? result->first : buffer.GetLastNonSpaceCharacter()); + } + else + { + searchStart = (result ? result->second : bufferSize.Origin()); + searchEnd = _selection->start; + } + + // Careful! Selection can point to RightExclusive(), which doesn't contain data! + // Clamp to be safe. + auto initialPos = dir == SearchDirection::Forward ? searchStart : searchEnd; + bufferSize.Clamp(initialPos); + auto iter = buffer.GetCellDataAt(initialPos); while (dir == SearchDirection::Forward ? iter.Pos() < searchEnd : iter.Pos() > searchStart) { // Don't let us select the same hyperlink again @@ -571,21 +590,24 @@ void Terminal::SelectHyperlink(const SearchDirection dir) // Expand the start to include the entire hyperlink TextBufferCellIterator hyperlinkStartIter{ buffer, iter.Pos() }; - while (attr.IsHyperlink() && attr.GetHyperlinkId() == hyperlinkId) + while (hyperlinkStartIter.Pos() > searchStart && attr.IsHyperlink() && attr.GetHyperlinkId() == hyperlinkId) { - hyperlinkStartIter--; + --hyperlinkStartIter; attr = hyperlinkStartIter->TextAttr(); } - // undo a move to be inclusive - hyperlinkStartIter++; + if (hyperlinkStartIter.Pos() != bufferSize.Origin()) + { + // undo a move to be inclusive + ++hyperlinkStartIter; + } // Expand the end to include the entire hyperlink // No need to undo a move! We'll decrement in the next step anyways. TextBufferCellIterator hyperlinkEndIter{ buffer, iter.Pos() }; attr = hyperlinkEndIter->TextAttr(); - while (attr.IsHyperlink() && attr.GetHyperlinkId() == hyperlinkId) + while (hyperlinkEndIter.Pos() < searchEnd && attr.IsHyperlink() && attr.GetHyperlinkId() == hyperlinkId) { - hyperlinkEndIter++; + ++hyperlinkEndIter; attr = hyperlinkEndIter->TextAttr(); }