Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove dependency on IsGlyphFullWidth for IRM/DECSWL #16903

Merged
merged 6 commits into from
Apr 10, 2024
Merged

Remove dependency on IsGlyphFullWidth for IRM/DECSWL #16903

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a3b29c1
Remove dependency on IsGlyphFullWidth for IRM/DECSWL
lhecker Mar 19, 2024
a15471f
Forgot to pick this change
lhecker Mar 19, 2024
53305b5
Address feedback by j4james
lhecker Mar 25, 2024
7b17ec6
Merge remote-tracking branch 'origin/main' into dev/lhecker/irm-IsGly…
lhecker Apr 2, 2024
c45fdf2
Add tests, Fix offsets
lhecker Apr 2, 2024
ca27ae2
Fix spelling
lhecker Apr 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions src/buffer/out/textBuffer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -589,10 +589,13 @@ void TextBuffer::Insert(til::CoordType row, const TextAttribute& attributes, Row
r.ReplaceText(restoreState);

// Restore trailing attributes as well.
auto& rowAttr = r.Attributes();
const auto& scratchAttr = scratch.Attributes();
const auto restoreAttr = scratchAttr.slice(gsl::narrow<uint16_t>(state.columnBegin), gsl::narrow<uint16_t>(state.columnLimit));
rowAttr.replace(gsl::narrow<uint16_t>(state.columnEnd), gsl::narrow<uint16_t>(state.columnEnd + restoreAttr.size()), restoreAttr);
if (const auto copyAmount = restoreState.columnEnd - restoreState.columnBegin; copyAmount > 0)
{
auto& rowAttr = r.Attributes();
const auto& scratchAttr = scratch.Attributes();
const auto restoreAttr = scratchAttr.slice(gsl::narrow<uint16_t>(state.columnBegin), gsl::narrow<uint16_t>(state.columnBegin + copyAmount));
rowAttr.replace(gsl::narrow<uint16_t>(restoreState.columnBegin), gsl::narrow<uint16_t>(restoreState.columnEnd), restoreAttr);
}

TriggerRedraw(Viewport::FromExclusive({ state.columnBeginDirty, row, restoreState.columnEndDirty, row + 1 }));
}
Expand Down
91 changes: 86 additions & 5 deletions src/host/ut_host/TextBufferTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,8 @@

TEST_METHOD(TestBurrito);
TEST_METHOD(TestOverwriteChars);
TEST_METHOD(TestRowReplaceText);
TEST_METHOD(TestReplace);
TEST_METHOD(TestInsert);

TEST_METHOD(TestAppendRTFText);

Expand Down Expand Up @@ -2003,13 +2004,12 @@
#undef complex1
}

void TextBufferTests::TestRowReplaceText()
void TextBufferTests::TestReplace()
{
static constexpr til::size bufferSize{ 10, 3 };
static constexpr UINT cursorSize = 12;
const TextAttribute attr{ 0x7f };
TextBuffer buffer{ bufferSize, attr, cursorSize, false, _renderer };
auto& row = buffer.GetMutableRowByOffset(0);

#define complex L"\U0001F41B"

Expand Down Expand Up @@ -2073,17 +2073,98 @@
.columnBegin = t.input.columnBegin,
.columnLimit = t.input.columnLimit,
};
row.ReplaceText(actual);
buffer.Replace(0, attr, actual);
VERIFY_ARE_EQUAL(t.expected.text, actual.text);
VERIFY_ARE_EQUAL(t.expected.columnEnd, actual.columnEnd);
VERIFY_ARE_EQUAL(t.expected.columnBeginDirty, actual.columnBeginDirty);
VERIFY_ARE_EQUAL(t.expected.columnEndDirty, actual.columnEndDirty);
VERIFY_ARE_EQUAL(t.expectedRow, row.GetText());
VERIFY_ARE_EQUAL(t.expectedRow, buffer.GetRowByOffset(0).GetText());
}

#undef complex
}

void TextBufferTests::TestInsert()
{
static constexpr til::size bufferSize{ 10, 3 };
static constexpr UINT cursorSize = 12;
static constexpr TextAttribute attr1{ 0x11111111, 0x00000000 };
static constexpr TextAttribute attr2{ 0x22222222, 0x00000000 };
static constexpr TextAttribute attr3{ 0x33333333, 0x00000000 };
TextBuffer buffer{ bufferSize, attr1, cursorSize, false, _renderer };

struct Test
{
const wchar_t* description;
struct
{
std::wstring_view text;
til::CoordType columnBegin = 0;
til::CoordType columnLimit = 0;
TextAttribute attr;
} input;
struct
{
std::wstring_view text;
til::CoordType columnEnd = 0;
til::CoordType columnBeginDirty = 0;
til::CoordType columnEndDirty = 0;
} expected;
std::wstring_view expectedRow;
};

static constexpr std::array tests{
Test{
L"Not enough space -> early exit",
{ L"aaa", 5, 5, attr1 },
{ L"aaa", 5, 5, 5 },
L" ",
},
Test{
L"Too much to fit",
{ L"aaaaabbb", 0, 5, attr1 },
Fixed Show fixed Hide fixed
{ L"bbb", 5, 0, 5 },
Fixed Show fixed Hide fixed
L"aaaaa ",
},
Test{
L"Wide char intersects limit",
{ L"bbbb😄", 0, 5, attr2 },
{ L"😄", 5, 0, 5 },
L"bbbb ",
},
Test{
L"Insert middle",
{ L"cc", 2, 5, attr3 },
{ L"", 4, 2, 4 },
L"bbccb ",
Fixed Show fixed Hide fixed
},
};

for (const auto& t : tests)
{
Log::Comment(t.description);
RowWriteState actual{
.text = t.input.text,
.columnBegin = t.input.columnBegin,
.columnLimit = t.input.columnLimit,
};
buffer.Insert(0, t.input.attr, actual);
VERIFY_ARE_EQUAL(t.expected.text, actual.text);
VERIFY_ARE_EQUAL(t.expected.columnEnd, actual.columnEnd);
VERIFY_ARE_EQUAL(t.expected.columnBeginDirty, actual.columnBeginDirty);
VERIFY_ARE_EQUAL(t.expected.columnEndDirty, actual.columnEndDirty);
VERIFY_ARE_EQUAL(t.expectedRow, buffer.GetRowByOffset(0).GetText());
}

auto& scratch = buffer.GetScratchpadRow();
scratch.ReplaceAttributes(0, 5, attr2);
scratch.ReplaceAttributes(2, 4, attr3);

const auto& expectedAttr = scratch.Attributes();
const auto& actualAttr = buffer.GetRowByOffset(0).Attributes();
VERIFY_ARE_EQUAL(expectedAttr, actualAttr);
}

void TextBufferTests::TestAppendRTFText()
{
{
Expand Down
12 changes: 0 additions & 12 deletions src/inc/til/rle.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1042,18 +1042,6 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
#ifdef __WEX_COMMON_H__
namespace WEX::TestExecution
{
template<typename T, typename S, typename Container>
class VerifyOutputTraits<::til::basic_rle<T, S, Container>>
{
using rle_vector = ::til::basic_rle<T, S, Container>;

public:
static WEX::Common::NoThrowString ToString(const rle_vector& object)
{
return WEX::Common::NoThrowString(object.to_string().c_str());
}
};
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...yes. 😑
Let me know if you dislike this removal. rle_vector uses strstream like all the other classes and TextAttribute has no string-serializer. I just removed it because it was easier. I could however write a full ToString method here if we want to.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah honestly I don't particularly care for all of these


template<typename T, typename S, typename Container>
class VerifyCompareTraits<::til::basic_rle<T, S, Container>, ::til::basic_rle<T, S, Container>>
{
Expand Down
Loading