Skip to content

Commit 2ee0cd8

Browse files
carlos-zamoracarlos-zamora
committed
fix/add tests; remove invalid attributes
1 parent 224614c commit 2ee0cd8

14 files changed

Lines changed: 346 additions & 325 deletions

File tree

‎src/buffer/out/textBuffer.cpp‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -149,53 +149,55 @@ TextBufferTextIterator TextBuffer::GetTextLineDataAt(const COORD at) const
149149
// - Read-only iterator of cell data.
150150
TextBufferCellIterator TextBuffer::GetCellLineDataAt(const COORD at) const
151151
{
152-
SMALL_RECT limit;
153-
limit.Top = at.Y;
154-
limit.Bottom = at.Y;
155-
limit.Left = 0;
156-
limit.Right = GetSize().RightInclusive();
152+
SMALL_RECT bounds;
153+
bounds.Top = at.Y;
154+
bounds.Bottom = at.Y;
155+
bounds.Left = 0;
156+
bounds.Right = GetSize().RightInclusive();
157157

158-
return TextBufferCellIterator(*this, at, Viewport::FromInclusive(limit));
158+
return TextBufferCellIterator(*this, at, Viewport::FromInclusive(bounds));
159159
}
160160

161161
// Routine Description:
162162
// - Retrieves read-only text iterator at the given buffer location
163163
// but restricted to operate only inside the given viewport.
164164
// Arguments:
165165
// - at - X,Y position in buffer for iterator start position
166-
// - limit - boundaries for the iterator to operate within
166+
// - bounds - boundaries for the iterator to operate within
167167
// Return Value:
168168
// - Read-only iterator of text data only.
169-
TextBufferTextIterator TextBuffer::GetTextDataAt(const COORD at, const Viewport limit) const
169+
TextBufferTextIterator TextBuffer::GetTextDataAt(const COORD at, const Viewport bounds) const
170170
{
171-
return TextBufferTextIterator(GetCellDataAt(at, limit));
171+
return TextBufferTextIterator(GetCellDataAt(at, bounds));
172172
}
173173

174174
// Routine Description:
175175
// - Retrieves read-only cell iterator at the given buffer location
176176
// but restricted to operate only inside the given viewport.
177177
// Arguments:
178178
// - at - X,Y position in buffer for iterator start position
179-
// - limit - boundaries for the iterator to operate within
179+
// - bounds - boundaries for the iterator to operate within
180180
// Return Value:
181181
// - Read-only iterator of cell data.
182-
TextBufferCellIterator TextBuffer::GetCellDataAt(const COORD at, const Viewport limit) const
182+
TextBufferCellIterator TextBuffer::GetCellDataAt(const COORD at, const Viewport bounds) const
183183
{
184-
return TextBufferCellIterator(*this, at, limit);
184+
return TextBufferCellIterator(*this, at, bounds);
185185
}
186186

187187
// Routine Description:
188188
// - Retrieves read-only cell iterator at the given buffer location
189189
// but restricted to operate only inside the given viewport.
190190
// Arguments:
191191
// - at - X,Y position in buffer for iterator start position
192-
// - limit - boundaries for the iterator to operate within
193-
// - until - X,Y position in buffer for last position for the iterator to read (inclusive)
192+
// - bounds - viewport boundaries for the iterator to operate within.
193+
// Allows for us to iterate over a sub-grid of the buffer.
194+
// - limit - X,Y position in buffer for the iterator end position (inclusive).
195+
// Allows for us to iterate through "bounds" until we hit the end of "bounds" or the limit.
194196
// Return Value:
195197
// - Read-only iterator of cell data.
196-
TextBufferCellIterator TextBuffer::GetCellDataAt(const COORD at, const Viewport limit, const COORD until) const
198+
TextBufferCellIterator TextBuffer::GetCellDataAt(const COORD at, const Viewport bounds, const COORD limit) const
197199
{
198-
return TextBufferCellIterator(*this, at, limit, until);
200+
return TextBufferCellIterator(*this, at, bounds, limit);
199201
}
200202

201203
//Routine Description:

‎src/buffer/out/textBuffer.hpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,11 +80,11 @@ class TextBuffer final
8080

8181
TextBufferCellIterator GetCellDataAt(const COORD at) const;
8282
TextBufferCellIterator GetCellLineDataAt(const COORD at) const;
83-
TextBufferCellIterator GetCellDataAt(const COORD at, const Microsoft::Console::Types::Viewport limit) const;
84-
TextBufferCellIterator GetCellDataAt(const COORD at, const Microsoft::Console::Types::Viewport limit, const COORD until) const;
83+
TextBufferCellIterator GetCellDataAt(const COORD at, const Microsoft::Console::Types::Viewport bounds) const;
84+
TextBufferCellIterator GetCellDataAt(const COORD at, const Microsoft::Console::Types::Viewport bounds, const COORD limit) const;
8585
TextBufferTextIterator GetTextDataAt(const COORD at) const;
8686
TextBufferTextIterator GetTextLineDataAt(const COORD at) const;
87-
TextBufferTextIterator GetTextDataAt(const COORD at, const Microsoft::Console::Types::Viewport limit) const;
87+
TextBufferTextIterator GetTextDataAt(const COORD at, const Microsoft::Console::Types::Viewport bounds) const;
8888

8989
// Text insertion functions
9090
OutputCellIterator Write(const OutputCellIterator givenIt);

‎src/buffer/out/textBufferCellIterator.cpp‎

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -29,21 +29,21 @@ TextBufferCellIterator::TextBufferCellIterator(const TextBuffer& buffer, COORD p
2929
// Arguments:
3030
// - buffer - Pointer to screen buffer to seek through
3131
// - pos - Starting position to retrieve text data from (within screen buffer bounds)
32-
// - limits - Viewport limits to restrict the iterator within the buffer bounds (smaller than the buffer itself)
33-
TextBufferCellIterator::TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Viewport limits) :
32+
// - bounds - Viewport boundaries to restrict the iterator within the buffer bounds (smaller than the buffer itself)
33+
TextBufferCellIterator::TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Viewport bounds) :
3434
_buffer(buffer),
3535
_pos(pos),
3636
_pRow(s_GetRow(buffer, pos)),
37-
_bounds(limits),
37+
_bounds(bounds),
3838
_exceeded(false),
3939
_view({}, {}, {}, TextAttributeBehavior::Stored),
4040
_attrIter(s_GetRow(buffer, pos)->GetAttrRow().cbegin())
4141
{
4242
// Throw if the bounds rectangle is not limited to the inside of the given buffer.
43-
THROW_HR_IF(E_INVALIDARG, !buffer.GetSize().IsInBounds(limits));
43+
THROW_HR_IF(E_INVALIDARG, !buffer.GetSize().IsInBounds(bounds));
4444

4545
// Throw if the coordinate is not limited to the inside of the given buffer.
46-
THROW_HR_IF(E_INVALIDARG, !limits.IsInBounds(pos));
46+
THROW_HR_IF(E_INVALIDARG, !bounds.IsInBounds(pos));
4747

4848
_attrIter += pos.X;
4949

@@ -55,18 +55,15 @@ TextBufferCellIterator::TextBufferCellIterator(const TextBuffer& buffer, COORD p
5555
// Arguments:
5656
// - buffer - Text buffer to seek through
5757
// - pos - Starting position to retrieve text data from (within screen buffer bounds)
58-
// - limits - Viewport limits to restrict the iterator within the buffer bounds (smaller than the buffer itself)
59-
// - endPosInclusive - last position to iterate through (inclusive)
60-
TextBufferCellIterator::TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Viewport limits, const COORD endPosInclusive) :
61-
TextBufferCellIterator(buffer, pos, limits)
58+
// - bounds - Viewport boundaries to restrict the iterator within the buffer bounds (smaller than the buffer itself)
59+
// - limit - last position to iterate through (inclusive)
60+
TextBufferCellIterator::TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Viewport bounds, const COORD limit) :
61+
TextBufferCellIterator(buffer, pos, bounds)
6262
{
6363
// Throw if the coordinate is not limited to the inside of the given buffer.
64-
THROW_HR_IF(E_INVALIDARG, !_bounds.IsInBounds(endPosInclusive));
64+
THROW_HR_IF(E_INVALIDARG, !_bounds.IsInBounds(limit));
6565

66-
// Throw if pos is past endPos
67-
THROW_HR_IF(E_INVALIDARG, _bounds.CompareInBounds(pos, endPosInclusive) > 0);
68-
69-
_endPosInclusive = endPosInclusive;
66+
_limit = limit;
7067
}
7168

7269
// Routine Description:
@@ -92,7 +89,7 @@ bool TextBufferCellIterator::operator==(const TextBufferCellIterator& it) const
9289
_bounds == it._bounds &&
9390
_pRow == it._pRow &&
9491
_attrIter == it._attrIter &&
95-
_endPosInclusive == _endPosInclusive;
92+
_limit == _limit;
9693
}
9794

9895
// Routine Description:
@@ -118,22 +115,22 @@ TextBufferCellIterator& TextBufferCellIterator::operator+=(const ptrdiff_t& move
118115
auto newPos = _pos;
119116
while (move > 0 && !_exceeded)
120117
{
121-
// If we have an endPos, check if we've exceeded it
122-
if (_endPosInclusive.has_value())
118+
// If we have a limit, check if we've exceeded it
119+
if (_limit.has_value())
123120
{
124-
_exceeded = _bounds.CompareInBounds(newPos, *_endPosInclusive) > 0;
121+
_exceeded |= (newPos == _limit);
125122
}
126123

127-
// If we already exceeded from endPos, we'll short-circuit and _not_ increment
124+
// If we already exceeded limit, we'll short-circuit and _not_ increment
128125
_exceeded |= !_bounds.IncrementInBounds(newPos);
129126
move--;
130127
}
131128
while (move < 0 && !_exceeded)
132129
{
133130
// If we have an endPos, check if we've exceeded it
134-
if (_endPosInclusive.has_value())
131+
if (_limit.has_value())
135132
{
136-
_exceeded = _bounds.CompareInBounds(newPos, *_endPosInclusive) < 0;
133+
_exceeded |= (newPos == _limit);
137134
}
138135

139136
// If we already exceeded from endPos, we'll short-circuit and _not_ decrement

‎src/buffer/out/textBufferCellIterator.hpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ class TextBufferCellIterator
2626
{
2727
public:
2828
TextBufferCellIterator(const TextBuffer& buffer, COORD pos);
29-
TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Microsoft::Console::Types::Viewport limits);
30-
TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Microsoft::Console::Types::Viewport limits, const COORD endPosInclusive);
29+
TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Microsoft::Console::Types::Viewport bounds);
30+
TextBufferCellIterator(const TextBuffer& buffer, COORD pos, const Microsoft::Console::Types::Viewport bounds, const COORD limit);
3131

3232
operator bool() const noexcept;
3333

@@ -63,7 +63,7 @@ class TextBufferCellIterator
6363
const Microsoft::Console::Types::Viewport _bounds;
6464
bool _exceeded;
6565
COORD _pos;
66-
std::optional<COORD> _endPosInclusive;
66+
std::optional<COORD> _limit;
6767

6868
#if UNIT_TESTING
6969
friend class TextBufferIteratorTests;

‎src/cascadia/TerminalControl/ControlCore.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -592,6 +592,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
592592
const int newDpi = static_cast<int>(static_cast<double>(USER_DEFAULT_SCREEN_DPI) *
593593
_compositionScale);
594594

595+
_terminal->SetFontInfo(_actualFont);
596+
595597
// TODO: MSFT:20895307 If the font doesn't exist, this doesn't
596598
// actually fail. We need a way to gracefully fallback.
597599
_renderer->TriggerFontChange(newDpi, _desiredFont, _actualFont);

‎src/cascadia/TerminalControl/XamlUiaTextRange.cpp‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation
103103
}
104104
case VT_I4:
105105
{
106-
return box_value(result.iVal);
106+
// Surprisingly, `long` is _not_ a WinRT type.
107+
// So we have to use `int32_t` to make sure this is output properly.
108+
// Otherwise, you'll get "Attribute does not exist" out the other end.
109+
return box_value<int32_t>(result.lVal);
107110
}
108111
case VT_R8:
109112
{
@@ -122,17 +125,16 @@ namespace winrt::Microsoft::Terminal::Control::implementation
122125
// are supported at this time.
123126
// So we need to figure out what was actually intended to be returned.
124127

125-
IUnknown* notSupportedVal;
126-
UiaGetReservedNotSupportedValue(&notSupportedVal);
127-
if (result.punkVal == notSupportedVal)
128-
{
129-
// See below for why we need to throw this special value.
130-
winrt::throw_hresult(XAML_E_NOT_SUPPORTED);
131-
}
128+
// use C++11 magic statics to make sure we only do this once.
129+
static const auto mixedAttributeVal = []() {
130+
IUnknown* resultRaw;
131+
com_ptr<IUnknown> result;
132+
UiaGetReservedMixedAttributeValue(&resultRaw);
133+
result.attach(resultRaw);
134+
return result;
135+
}();
132136

133-
IUnknown* mixedAttributeVal;
134-
UiaGetReservedMixedAttributeValue(&mixedAttributeVal);
135-
if (result.punkVal == mixedAttributeVal)
137+
if (result.punkVal == mixedAttributeVal.get())
136138
{
137139
return Windows::UI::Xaml::DependencyProperty::UnsetValue();
138140
}

‎src/cascadia/TerminalCore/Terminal.hpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ class Microsoft::Terminal::Core::Terminal final :
6868

6969
void UpdateSettings(winrt::Microsoft::Terminal::Core::ICoreSettings settings);
7070
void UpdateAppearance(const winrt::Microsoft::Terminal::Core::ICoreAppearance& appearance);
71+
void SetFontInfo(const FontInfo& fontInfo);
7172

7273
// Write goes through the parser
7374
void Write(std::wstring_view stringView);
@@ -160,6 +161,7 @@ class Microsoft::Terminal::Core::Terminal final :
160161
COORD GetTextBufferEndPosition() const noexcept override;
161162
const TextBuffer& GetTextBuffer() noexcept override;
162163
const FontInfo& GetFontInfo() noexcept override;
164+
std::pair<COLORREF, COLORREF> GetAttributeColors(const TextAttribute& attr) const noexcept override;
163165

164166
void LockConsole() noexcept override;
165167
void UnlockConsole() noexcept override;
@@ -168,7 +170,6 @@ class Microsoft::Terminal::Core::Terminal final :
168170
#pragma region IRenderData
169171
// These methods are defined in TerminalRenderData.cpp
170172
const TextAttribute GetDefaultBrushColors() noexcept override;
171-
std::pair<COLORREF, COLORREF> GetAttributeColors(const TextAttribute& attr) const noexcept override;
172173
COORD GetCursorPosition() const noexcept override;
173174
bool IsCursorVisible() const noexcept override;
174175
bool IsCursorOn() const noexcept override;
@@ -276,6 +277,7 @@ class Microsoft::Terminal::Core::Terminal final :
276277
size_t _hyperlinkPatternId;
277278

278279
std::wstring _workingDirectory;
280+
std::optional<FontInfo> _fontInfo;
279281
#pragma region Text Selection
280282
// a selection is represented as a range between two COORDs (start and end)
281283
// the pivot is the COORD that remains selected when you extend a selection in any direction

‎src/cascadia/TerminalCore/terminalrenderdata.cpp‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@ const TextBuffer& Terminal::GetTextBuffer() noexcept
3434
#pragma warning(disable : 26447)
3535
const FontInfo& Terminal::GetFontInfo() noexcept
3636
{
37+
if (_fontInfo)
38+
{
39+
return *_fontInfo;
40+
}
41+
3742
// TODO: This font value is only used to check if the font is a raster font.
3843
// Otherwise, the font is changed with the renderer via TriggerFontChange.
3944
// The renderer never uses any of the other members from the value returned
@@ -45,6 +50,11 @@ const FontInfo& Terminal::GetFontInfo() noexcept
4550
}
4651
#pragma warning(pop)
4752

53+
void Terminal::SetFontInfo(const FontInfo& fontInfo)
54+
{
55+
_fontInfo = fontInfo;
56+
}
57+
4858
const TextAttribute Terminal::GetDefaultBrushColors() noexcept
4959
{
5060
return TextAttribute{};

‎src/host/renderData.hpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ class RenderData final :
2727
COORD GetTextBufferEndPosition() const noexcept override;
2828
const TextBuffer& GetTextBuffer() noexcept override;
2929
const FontInfo& GetFontInfo() noexcept override;
30+
std::pair<COLORREF, COLORREF> GetAttributeColors(const TextAttribute& attr) const noexcept override;
3031

3132
std::vector<Microsoft::Console::Types::Viewport> GetSelectionRects() noexcept override;
3233

@@ -37,8 +38,6 @@ class RenderData final :
3738
#pragma region IRenderData
3839
const TextAttribute GetDefaultBrushColors() noexcept override;
3940

40-
std::pair<COLORREF, COLORREF> GetAttributeColors(const TextAttribute& attr) const noexcept override;
41-
4241
COORD GetCursorPosition() const noexcept override;
4342
bool IsCursorVisible() const noexcept override;
4443
bool IsCursorOn() const noexcept override;

0 commit comments

Comments
 (0)