fix/add tests; remove invalid attributes

src/buffer/out/textBuffer.cpp

@@ -149,53 +149,55 @@ TextBufferTextIterator TextBuffer::GetTextLineDataAt(const COORD at) const
 // - Read-only iterator of cell data.
 TextBufferCellIterator TextBuffer::GetCellLineDataAt(const COORD at) const
 {
-    SMALL_RECT limit;
-    limit.Top = at.Y;
-    limit.Bottom = at.Y;
-    limit.Left = 0;
-    limit.Right = GetSize().RightInclusive();
+    SMALL_RECT bounds;
+    bounds.Top = at.Y;
+    bounds.Bottom = at.Y;
+    bounds.Left = 0;
+    bounds.Right = GetSize().RightInclusive();
 
-    return TextBufferCellIterator(*this, at, Viewport::FromInclusive(limit));
+    return TextBufferCellIterator(*this, at, Viewport::FromInclusive(bounds));
 }
 
 // Routine Description:
 // - Retrieves read-only text iterator at the given buffer location
 //   but restricted to operate only inside the given viewport.
 // Arguments:
 // - at - X,Y position in buffer for iterator start position
-// - limit - boundaries for the iterator to operate within
+// - bounds - boundaries for the iterator to operate within
 // Return Value:
 // - Read-only iterator of text data only.
-TextBufferTextIterator TextBuffer::GetTextDataAt(const COORD at, const Viewport limit) const
+TextBufferTextIterator TextBuffer::GetTextDataAt(const COORD at, const Viewport bounds) const
 {
-    return TextBufferTextIterator(GetCellDataAt(at, limit));
+    return TextBufferTextIterator(GetCellDataAt(at, bounds));
 }
 
 // Routine Description:
 // - Retrieves read-only cell iterator at the given buffer location
 //   but restricted to operate only inside the given viewport.
 // Arguments:
 // - at - X,Y position in buffer for iterator start position
-// - limit - boundaries for the iterator to operate within
+// - bounds - boundaries for the iterator to operate within
 // Return Value:
 // - Read-only iterator of cell data.
-TextBufferCellIterator TextBuffer::GetCellDataAt(const COORD at, const Viewport limit) const
+TextBufferCellIterator TextBuffer::GetCellDataAt(const COORD at, const Viewport bounds) const
 {
-    return TextBufferCellIterator(*this, at, limit);
+    return TextBufferCellIterator(*this, at, bounds);
 }
 
 // Routine Description:
 // - Retrieves read-only cell iterator at the given buffer location
 //   but restricted to operate only inside the given viewport.
 // Arguments:
 // - at - X,Y position in buffer for iterator start position
-// - limit - boundaries for the iterator to operate within
-// - until - X,Y position in buffer for last position for the iterator to read (inclusive)
+// - bounds - viewport boundaries for the iterator to operate within.
+//            Allows for us to iterate over a sub-grid of the buffer.
+// - limit - X,Y position in buffer for the iterator end position (inclusive).
+//           Allows for us to iterate through "bounds" until we hit the end of "bounds" or the limit.
 // Return Value:
 // - Read-only iterator of cell data.
-TextBufferCellIterator TextBuffer::GetCellDataAt(const COORD at, const Viewport limit, const COORD until) const
+TextBufferCellIterator TextBuffer::GetCellDataAt(const COORD at, const Viewport bounds, const COORD limit) const
 {
-    return TextBufferCellIterator(*this, at, limit, until);
+    return TextBufferCellIterator(*this, at, bounds, limit);
 }
 
 //Routine Description:

src/buffer/out/textBuffer.hpp

@@ -80,11 +80,11 @@ class TextBuffer final
 
     TextBufferCellIterator GetCellDataAt(const COORD at) const;
     TextBufferCellIterator GetCellLineDataAt(const COORD at) const;
-    TextBufferCellIterator GetCellDataAt(const COORD at, const Microsoft::Console::Types::Viewport limit) const;
-    TextBufferCellIterator GetCellDataAt(const COORD at, const Microsoft::Console::Types::Viewport limit, const COORD until) const;
+    TextBufferCellIterator GetCellDataAt(const COORD at, const Microsoft::Console::Types::Viewport bounds) const;
+    TextBufferCellIterator GetCellDataAt(const COORD at, const Microsoft::Console::Types::Viewport bounds, const COORD limit) const;
     TextBufferTextIterator GetTextDataAt(const COORD at) const;
     TextBufferTextIterator GetTextLineDataAt(const COORD at) const;
-    TextBufferTextIterator GetTextDataAt(const COORD at, const Microsoft::Console::Types::Viewport limit) const;
+    TextBufferTextIterator GetTextDataAt(const COORD at, const Microsoft::Console::Types::Viewport bounds) const;
 
     // Text insertion functions
     OutputCellIterator Write(const OutputCellIterator givenIt);

src/buffer/out/textBufferCellIterator.cpp

@@ -29,21 +29,21 @@ TextBufferCellIterator::TextBufferCellIterator(const TextBuffer& buffer, COORD p
 // Arguments:
 // - buffer - Pointer to screen buffer to seek through
 // - pos - Starting position to retrieve text data from (within screen buffer bounds)
-// - limits - Viewport limits to restrict the iterator within the buffer bounds (smaller than the buffer itself)
-TextBufferCellIterator::TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Viewport limits) :
+// - bounds - Viewport boundaries to restrict the iterator within the buffer bounds (smaller than the buffer itself)
+TextBufferCellIterator::TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Viewport bounds) :
     _buffer(buffer),
     _pos(pos),
     _pRow(s_GetRow(buffer, pos)),
-    _bounds(limits),
+    _bounds(bounds),
     _exceeded(false),
     _view({}, {}, {}, TextAttributeBehavior::Stored),
     _attrIter(s_GetRow(buffer, pos)->GetAttrRow().cbegin())
 {
     // Throw if the bounds rectangle is not limited to the inside of the given buffer.
-    THROW_HR_IF(E_INVALIDARG, !buffer.GetSize().IsInBounds(limits));
+    THROW_HR_IF(E_INVALIDARG, !buffer.GetSize().IsInBounds(bounds));
 
     // Throw if the coordinate is not limited to the inside of the given buffer.
-    THROW_HR_IF(E_INVALIDARG, !limits.IsInBounds(pos));
+    THROW_HR_IF(E_INVALIDARG, !bounds.IsInBounds(pos));
 
     _attrIter += pos.X;
 
@@ -55,18 +55,15 @@ TextBufferCellIterator::TextBufferCellIterator(const TextBuffer& buffer, COORD p
 // Arguments:
 // - buffer - Text buffer to seek through
 // - pos - Starting position to retrieve text data from (within screen buffer bounds)
-// - limits - Viewport limits to restrict the iterator within the buffer bounds (smaller than the buffer itself)
-// - endPosInclusive - last position to iterate through (inclusive)
-TextBufferCellIterator::TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Viewport limits, const COORD endPosInclusive) :
-    TextBufferCellIterator(buffer, pos, limits)
+// - bounds - Viewport boundaries to restrict the iterator within the buffer bounds (smaller than the buffer itself)
+// - limit - last position to iterate through (inclusive)
+TextBufferCellIterator::TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Viewport bounds, const COORD limit) :
+    TextBufferCellIterator(buffer, pos, bounds)
 {
     // Throw if the coordinate is not limited to the inside of the given buffer.
-    THROW_HR_IF(E_INVALIDARG, !_bounds.IsInBounds(endPosInclusive));
+    THROW_HR_IF(E_INVALIDARG, !_bounds.IsInBounds(limit));
 
-    // Throw if pos is past endPos
-    THROW_HR_IF(E_INVALIDARG, _bounds.CompareInBounds(pos, endPosInclusive) > 0);
-
-    _endPosInclusive = endPosInclusive;
+    _limit = limit;
 }
 
 // Routine Description:
@@ -92,7 +89,7 @@ bool TextBufferCellIterator::operator==(const TextBufferCellIterator& it) const
            _bounds == it._bounds &&
            _pRow == it._pRow &&
            _attrIter == it._attrIter &&
-           _endPosInclusive == _endPosInclusive;
+           _limit == _limit;
 }
 
 // Routine Description:
@@ -118,22 +115,22 @@ TextBufferCellIterator& TextBufferCellIterator::operator+=(const ptrdiff_t& move
     auto newPos = _pos;
     while (move > 0 && !_exceeded)
     {
-        // If we have an endPos, check if we've exceeded it
-        if (_endPosInclusive.has_value())
+        // If we have a limit, check if we've exceeded it
+        if (_limit.has_value())
         {
-            _exceeded = _bounds.CompareInBounds(newPos, *_endPosInclusive) > 0;
+            _exceeded |= (newPos == _limit);
         }
 
-        // If we already exceeded from endPos, we'll short-circuit and _not_ increment
+        // If we already exceeded limit, we'll short-circuit and _not_ increment
         _exceeded |= !_bounds.IncrementInBounds(newPos);
         move--;
     }
     while (move < 0 && !_exceeded)
     {
         // If we have an endPos, check if we've exceeded it
-        if (_endPosInclusive.has_value())
+        if (_limit.has_value())
         {
-            _exceeded = _bounds.CompareInBounds(newPos, *_endPosInclusive) < 0;
+            _exceeded |= (newPos == _limit);
         }
 
         // If we already exceeded from endPos, we'll short-circuit and _not_ decrement

src/buffer/out/textBufferCellIterator.hpp

@@ -26,8 +26,8 @@ class TextBufferCellIterator
 {
 public:
     TextBufferCellIterator(const TextBuffer& buffer, COORD pos);
-    TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Microsoft::Console::Types::Viewport limits);
-    TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Microsoft::Console::Types::Viewport limits, const COORD endPosInclusive);
+    TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Microsoft::Console::Types::Viewport bounds);
+    TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Microsoft::Console::Types::Viewport bounds, const COORD limit);
 
     operator bool() const noexcept;
 
@@ -63,7 +63,7 @@ class TextBufferCellIterator
     const Microsoft::Console::Types::Viewport _bounds;
     bool _exceeded;
     COORD _pos;
-    std::optional<COORD> _endPosInclusive;
+    std::optional<COORD> _limit;
 
 #if UNIT_TESTING
     friend class TextBufferIteratorTests;

src/cascadia/TerminalControl/ControlCore.cpp

@@ -592,6 +592,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         const int newDpi = static_cast<int>(static_cast<double>(USER_DEFAULT_SCREEN_DPI) *
                                             _compositionScale);
 
+        _terminal->SetFontInfo(_actualFont);
+
         // TODO: MSFT:20895307 If the font doesn't exist, this doesn't
         //      actually fail. We need a way to gracefully fallback.
         _renderer->TriggerFontChange(newDpi, _desiredFont, _actualFont);

src/cascadia/TerminalControl/XamlUiaTextRange.cpp

@@ -103,7 +103,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         }
         case VT_I4:
         {
-            return box_value(result.iVal);
+            // Surprisingly, `long` is _not_ a WinRT type.
+            // So we have to use `int32_t` to make sure this is output properly.
+            // Otherwise, you'll get "Attribute does not exist" out the other end.
+            return box_value<int32_t>(result.lVal);
         }
         case VT_R8:
         {
@@ -122,17 +125,16 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             //  are supported at this time.
             // So we need to figure out what was actually intended to be returned.
 
-            IUnknown* notSupportedVal;
-            UiaGetReservedNotSupportedValue(&notSupportedVal);
-            if (result.punkVal == notSupportedVal)
-            {
-                // See below for why we need to throw this special value.
-                winrt::throw_hresult(XAML_E_NOT_SUPPORTED);
-            }
+            // use C++11 magic statics to make sure we only do this once.
+            static const auto mixedAttributeVal = []() {
+                IUnknown* resultRaw;
+                com_ptr<IUnknown> result;
+                UiaGetReservedMixedAttributeValue(&resultRaw);
+                result.attach(resultRaw);
+                return result;
+            }();
 
-            IUnknown* mixedAttributeVal;
-            UiaGetReservedMixedAttributeValue(&mixedAttributeVal);
-            if (result.punkVal == mixedAttributeVal)
+            if (result.punkVal == mixedAttributeVal.get())
             {
                 return Windows::UI::Xaml::DependencyProperty::UnsetValue();
             }

src/cascadia/TerminalCore/Terminal.hpp

@@ -68,6 +68,7 @@ class Microsoft::Terminal::Core::Terminal final :
 
     void UpdateSettings(winrt::Microsoft::Terminal::Core::ICoreSettings settings);
     void UpdateAppearance(const winrt::Microsoft::Terminal::Core::ICoreAppearance& appearance);
+    void SetFontInfo(const FontInfo& fontInfo);
 
     // Write goes through the parser
     void Write(std::wstring_view stringView);
@@ -160,6 +161,7 @@ class Microsoft::Terminal::Core::Terminal final :
     COORD GetTextBufferEndPosition() const noexcept override;
     const TextBuffer& GetTextBuffer() noexcept override;
     const FontInfo& GetFontInfo() noexcept override;
+    std::pair<COLORREF, COLORREF> GetAttributeColors(const TextAttribute& attr) const noexcept override;
 
     void LockConsole() noexcept override;
     void UnlockConsole() noexcept override;
@@ -168,7 +170,6 @@ class Microsoft::Terminal::Core::Terminal final :
 #pragma region IRenderData
     // These methods are defined in TerminalRenderData.cpp
     const TextAttribute GetDefaultBrushColors() noexcept override;
-    std::pair<COLORREF, COLORREF> GetAttributeColors(const TextAttribute& attr) const noexcept override;
     COORD GetCursorPosition() const noexcept override;
     bool IsCursorVisible() const noexcept override;
     bool IsCursorOn() const noexcept override;
@@ -276,6 +277,7 @@ class Microsoft::Terminal::Core::Terminal final :
     size_t _hyperlinkPatternId;
 
     std::wstring _workingDirectory;
+    std::optional<FontInfo> _fontInfo;
 #pragma region Text Selection
     // a selection is represented as a range between two COORDs (start and end)
     // the pivot is the COORD that remains selected when you extend a selection in any direction

src/cascadia/TerminalCore/terminalrenderdata.cpp

@@ -34,6 +34,11 @@ const TextBuffer& Terminal::GetTextBuffer() noexcept
 #pragma warning(disable : 26447)
 const FontInfo& Terminal::GetFontInfo() noexcept
 {
+    if (_fontInfo)
+    {
+        return *_fontInfo;
+    }
+
     // TODO: This font value is only used to check if the font is a raster font.
     // Otherwise, the font is changed with the renderer via TriggerFontChange.
     // The renderer never uses any of the other members from the value returned
@@ -45,6 +50,11 @@ const FontInfo& Terminal::GetFontInfo() noexcept
 }
 #pragma warning(pop)
 
+void Terminal::SetFontInfo(const FontInfo& fontInfo)
+{
+    _fontInfo = fontInfo;
+}
+
 const TextAttribute Terminal::GetDefaultBrushColors() noexcept
 {
     return TextAttribute{};

src/host/renderData.hpp

@@ -27,6 +27,7 @@ class RenderData final :
     COORD GetTextBufferEndPosition() const noexcept override;
     const TextBuffer& GetTextBuffer() noexcept override;
     const FontInfo& GetFontInfo() noexcept override;
+    std::pair<COLORREF, COLORREF> GetAttributeColors(const TextAttribute& attr) const noexcept override;
 
     std::vector<Microsoft::Console::Types::Viewport> GetSelectionRects() noexcept override;
 
@@ -37,8 +38,6 @@ class RenderData final :
 #pragma region IRenderData
     const TextAttribute GetDefaultBrushColors() noexcept override;
 
-    std::pair<COLORREF, COLORREF> GetAttributeColors(const TextAttribute& attr) const noexcept override;
-
     COORD GetCursorPosition() const noexcept override;
     bool IsCursorVisible() const noexcept override;
     bool IsCursorOn() const noexcept override;