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

Jump to bottom

Fix colors getting lost on reflow #17568

Merged
merged 3 commits into from
Jul 30, 2024
+11 −2
Merged

Fix colors getting lost on reflow #17568

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
98c2e10
Fix colors getting lost on reflow
lhecker Jul 15, 2024
cc152cb
Merge remote-tracking branch 'origin/main' into dev/lhecker/color-reflow
lhecker Jul 30, 2024
b6b8184
Fix unit test
lhecker Jul 30, 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
9 changes: 9 additions & 0 deletions src/buffer/out/textBuffer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2855,6 +2855,15 @@ void TextBuffer::Reflow(TextBuffer& oldBuffer, TextBuffer& newBuffer, const View
}
}

// The for loop right after this if condition will copy entire rows of attributes at a time.
// This assumes of course that the "write cursor" (newX, newY) is at the start of a row.
// If we didn't check for this, we may otherwise copy attributes from a later row into a previous one.
if (newX != 0)
{
newX = 0;
newY++;
}

// Finish copying buffer attributes to remaining rows below the last
// printable character. This is to fix the `color 2f` scenario, where you
// change the buffer colors then resize and everything below the last
Expand Down
4 changes: 2 additions & 2 deletions src/buffer/out/ut_textbuffer/ReflowTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -589,13 +589,13 @@ namespace
TestBuffer{
{ 2, 5 }, // reduce width aggressively
{
{ L" ", false },
{ L" ", true },
{ L" ", true },
{ L" ", true },
{ L" ", true },
{ L" ", false },
},
{ 1, 0 },
{ 1, 4 },
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not entirely sure if I was able to think the test fix 100% through, but generally speaking this new value makes sense to me. If you expand this diff hunk above, you see the original buffer. The cursor is at the tail end beyond the "Q" in the bottom right corner. There are 12 spaces between the "Q" and the cursor. When we resize to a size of 2x5 the entire buffer ends up being filled with spaces. It's only natural then, that the cursor should end up at the bottom right corner of the new buffer.

},
},
},
Expand Down
Loading