Make selection an exclusive range

src/buffer/out/Row.cpp

@@ -80,19 +80,20 @@ constexpr OutIt copy_n_small(InIt first, Diff count, OutIt dest)
     return dest;
 }
 
-CharToColumnMapper::CharToColumnMapper(const wchar_t* chars, const uint16_t* charOffsets, ptrdiff_t lastCharOffset, til::CoordType currentColumn) noexcept :
+CharToColumnMapper::CharToColumnMapper(const wchar_t* chars, const uint16_t* charOffsets, ptrdiff_t charsLength, til::CoordType currentColumn, til::CoordType columnCount) noexcept :
     _chars{ chars },
     _charOffsets{ charOffsets },
-    _lastCharOffset{ lastCharOffset },
-    _currentColumn{ currentColumn }
+    _charsLength{ charsLength },
+    _currentColumn{ currentColumn },
+    _columnCount{ columnCount }
 {
 }
 
 // If given a position (`offset`) inside the ROW's text, this function will return the corresponding column.
 // This function in particular returns the glyph's first column.
 til::CoordType CharToColumnMapper::GetLeadingColumnAt(ptrdiff_t targetOffset) noexcept
 {
-    targetOffset = clamp(targetOffset, 0, _lastCharOffset);
+    targetOffset = clamp(targetOffset, 0, _charsLength);
 
     // This code needs to fulfill two conditions on top of the obvious (a forward/backward search):
     // A: We never want to stop on a column that is marked with CharOffsetsTrailer (= "GetLeadingColumn").
@@ -130,10 +131,14 @@ til::CoordType CharToColumnMapper::GetLeadingColumnAt(ptrdiff_t targetOffset) no
 til::CoordType CharToColumnMapper::GetTrailingColumnAt(ptrdiff_t offset) noexcept
 {
     auto col = GetLeadingColumnAt(offset);
-    // This loop is a little redundant with the forward search loop in GetLeadingColumnAt()
-    // but it's realistically not worth caring about this. This code is not a bottleneck.
-    for (; WI_IsFlagSet(_charOffsets[col + 1], CharOffsetsTrailer); ++col)
+
+    if (col < _columnCount)
     {
+        // This loop is a little redundant with the forward search loop in GetLeadingColumnAt()
+        // but it's realistically not worth caring about this. This code is not a bottleneck.
+        for (; WI_IsFlagSet(_charOffsets[col + 1], CharOffsetsTrailer); ++col)
+        {
+        }
     }
     return col;
 }
@@ -1222,15 +1227,15 @@ T ROW::_adjustForward(T column) const noexcept
 }
 
 // Creates a CharToColumnMapper given an offset into _chars.data().
-// In other words, for a 120 column ROW with just ASCII text, the offset should be [0,120).
+// In other words, for a 120 column ROW with just ASCII text, the offset should be [0,120].
 CharToColumnMapper ROW::_createCharToColumnMapper(ptrdiff_t offset) const noexcept
 {
     const auto charsSize = _charSize();
-    const auto lastChar = gsl::narrow_cast<ptrdiff_t>(charsSize - 1);
+    const auto lastChar = gsl::narrow_cast<ptrdiff_t>(charsSize);
     // We can sort of guess what column belongs to what offset because BMP glyphs are very common and
     // UTF-16 stores them in 1 char. In other words, usually a ROW will have N chars for N columns.
     const auto guessedColumn = gsl::narrow_cast<til::CoordType>(clamp(offset, 0, _columnCount));
-    return CharToColumnMapper{ _chars.data(), _charOffsets.data(), lastChar, guessedColumn };
+    return CharToColumnMapper{ _chars.data(), _charOffsets.data(), lastChar, guessedColumn, _columnCount };
 }
 
 const std::optional<ScrollbarData>& ROW::GetScrollbarData() const noexcept

src/buffer/out/Row.hpp

@@ -71,7 +71,7 @@ struct RowCopyTextFromState
 // into a ROW's text this class can tell you what cell that pointer belongs to.
 struct CharToColumnMapper
 {
-    CharToColumnMapper(const wchar_t* chars, const uint16_t* charOffsets, ptrdiff_t lastCharOffset, til::CoordType currentColumn) noexcept;
+    CharToColumnMapper(const wchar_t* chars, const uint16_t* charOffsets, ptrdiff_t lastCharOffset, til::CoordType currentColumn, til::CoordType columnCount) noexcept;
 
     til::CoordType GetLeadingColumnAt(ptrdiff_t targetOffset) noexcept;
     til::CoordType GetTrailingColumnAt(ptrdiff_t offset) noexcept;
@@ -85,8 +85,9 @@ struct CharToColumnMapper
 
     const wchar_t* _chars;
     const uint16_t* _charOffsets;
-    ptrdiff_t _lastCharOffset;
+    ptrdiff_t _charsLength;
     til::CoordType _currentColumn;
+    til::CoordType _columnCount;
 };
 
 class ROW final

src/buffer/out/UTextAdapter.cpp

@@ -436,7 +436,7 @@ til::point_span Microsoft::Console::ICU::BufferRangeFromMatch(UText* ut, URegula
     if (utextAccess(ut, nativeIndexEnd, true))
     {
         const auto y = accessCurrentRow(ut);
-        ret.end.x = textBuffer.GetRowByOffset(y).GetTrailingColumnAtCharOffset(ut->chunkOffset);
+        ret.end.x = textBuffer.GetRowByOffset(y).GetLeadingColumnAtCharOffset(ut->chunkOffset);
         ret.end.y = y;
     }
     else

src/buffer/out/textBuffer.cpp

@@ -1970,7 +1970,7 @@ const std::vector<til::inclusive_rect> TextBuffer::GetTextRects(til::point start
 // - Else if a blockSelection, returns spans corresponding to each line in the block selection
 // Arguments:
 // - start: beginning of the text region of interest (inclusive)
-// - end: the other end of the text region of interest (inclusive)
+// - end: the other end of the text region of interest (exclusive)
 // - blockSelection: when enabled, get spans for each line covered by the block
 // - bufferCoordinates: when enabled, treat the coordinates as relative to
 //                      the buffer rather than the screen.

src/cascadia/TerminalCore/Terminal.cpp

@@ -502,7 +502,7 @@ std::wstring Terminal::GetHyperlinkAtBufferPosition(const til::point bufferPos)
         if (result.has_value())
         {
             result->start = _ConvertToBufferCell(result->start, false);
-            result->stop = _ConvertToBufferCell(result->stop, false);
+            result->stop = _ConvertToBufferCell(result->stop, true);
         }
     }
     else

src/cascadia/TerminalCore/Terminal.hpp

@@ -477,7 +477,7 @@ class Microsoft::Terminal::Core::Terminal final :
     std::vector<til::point_span> _GetSelectionSpans() const noexcept;
     std::pair<til::point, til::point> _PivotSelection(const til::point targetPos, bool& targetStart) const noexcept;
     std::pair<til::point, til::point> _ExpandSelectionAnchors(std::pair<til::point, til::point> anchors) const;
-    til::point _ConvertToBufferCell(const til::point viewportPos, bool allowRightExclusive = false) const;
+    til::point _ConvertToBufferCell(const til::point viewportPos, bool allowRightExclusive) const;
     void _ScrollToPoint(const til::point pos);
     void _MoveByChar(SelectionDirection direction, til::point& pos);
     void _MoveByWord(SelectionDirection direction, til::point& pos);

src/cascadia/TerminalCore/TerminalSelection.cpp

@@ -560,7 +560,6 @@ void Terminal::SelectHyperlink(const SearchDirection dir)
         selection->start = result->first;
         selection->pivot = result->first;
         selection->end = result->second;
-        bufferSize.DecrementInBounds(selection->end);
         _selectionIsTargetingUrl = true;
         _selectionEndpoint = SelectionEndpoint::End;
     }

src/host/selection.cpp

@@ -43,11 +43,18 @@ void Selection::_RegenerateSelectionSpans() const
     endSelectionAnchor.x = (_d->coordSelectionAnchor.x == _d->srSelectionRect.left) ? _d->srSelectionRect.right : _d->srSelectionRect.left;
     endSelectionAnchor.y = (_d->coordSelectionAnchor.y == _d->srSelectionRect.top) ? _d->srSelectionRect.bottom : _d->srSelectionRect.top;
 
+    // GH #18106: Conhost uses an inclusive range for selection. GetTextSpans() needs an exclusive range.
+    // Increment the "end" point (bottom-right-most) to make it exclusive.
+    // NOTE: if start is past end, increment start instead
+    const auto& buffer = screenInfo.GetTextBuffer();
+    auto startSelectionAnchor = _d->coordSelectionAnchor;
+    buffer.GetSize().IncrementInExclusiveBounds(startSelectionAnchor <= endSelectionAnchor ? endSelectionAnchor : startSelectionAnchor);
+
     const auto blockSelection = !IsLineSelection();
-    _lastSelectionSpans = screenInfo.GetTextBuffer().GetTextSpans(_d->coordSelectionAnchor,
-                                                                  endSelectionAnchor,
-                                                                  blockSelection,
-                                                                  false);
+    _lastSelectionSpans = buffer.GetTextSpans(startSelectionAnchor,
+                                              endSelectionAnchor,
+                                              blockSelection,
+                                              false);
     _lastSelectionGeneration = _d.generation();
 }