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.

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

make filling chars (and, thus, erase line/char) unset wrap #2831

Merged
merged 4 commits into from
Oct 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 10 additions & 5 deletions src/buffer/out/Row.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,11 @@ const UnicodeStorage& ROW::GetUnicodeStorage() const noexcept
// Arguments:
// - it - custom console iterator to use for seeking input data. bool() false when it becomes invalid while seeking.
// - index - column in row to start writing at
// - setWrap - set the wrap flags if we hit the end of the row while writing and there's still more data in the iterator.
// - wrap - change the wrap flag if we hit the end of the row while writing and there's still more data in the iterator.
// - limitRight - right inclusive column ID for the last write in this row. (optional, will just write to the end of row if nullopt)
// Return Value:
// - iterator to first cell that was not written to this row.
OutputCellIterator ROW::WriteCells(OutputCellIterator it, const size_t index, const bool setWrap, std::optional<size_t> limitRight)
OutputCellIterator ROW::WriteCells(OutputCellIterator it, const size_t index, const std::optional<bool> wrap, std::optional<size_t> limitRight)
{
THROW_HR_IF(E_INVALIDARG, index >= _charRow.size());
THROW_HR_IF(E_INVALIDARG, limitRight.value_or(0) >= _charRow.size());
Expand Down Expand Up @@ -202,10 +202,15 @@ OutputCellIterator ROW::WriteCells(OutputCellIterator it, const size_t index, co
++it;
}

// If we're asked to set the wrap status and we just filled the last column with some text, set wrap status on the row.
if (setWrap && fillingLastColumn)
// If we're asked to (un)set the wrap status and we just filled the last column with some text...
// NOTE:
// - wrap = std::nullopt --> don't change the wrap value
// - wrap = true --> we're filling cells as a steam, consider this a wrap
// - wrap = false --> we're filling cells as a block, unwrap
if (wrap.has_value() && fillingLastColumn)
{
_charRow.SetWrapForced(true);
// set wrap status on the row to parameter's value.
_charRow.SetWrapForced(wrap.value());
}
}
else
Expand Down
2 changes: 1 addition & 1 deletion src/buffer/out/Row.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class ROW final
UnicodeStorage& GetUnicodeStorage() noexcept;
const UnicodeStorage& GetUnicodeStorage() const noexcept;

OutputCellIterator WriteCells(OutputCellIterator it, const size_t index, const bool setWrap, std::optional<size_t> limitRight = std::nullopt);
OutputCellIterator WriteCells(OutputCellIterator it, const size_t index, const std::optional<bool> wrap = std::nullopt, std::optional<size_t> limitRight = std::nullopt);

friend bool operator==(const ROW& a, const ROW& b) noexcept;

Expand Down
13 changes: 8 additions & 5 deletions src/buffer/out/textBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -318,10 +318,12 @@ OutputCellIterator TextBuffer::Write(const OutputCellIterator givenIt)
// Arguments:
// - givenIt - Iterator representing output cell data to write
// - target - the row/column to start writing the text to
// - wrap - change the wrap flag if we hit the end of the row while writing and there's still more data
// Return Value:
// - The final position of the iterator
OutputCellIterator TextBuffer::Write(const OutputCellIterator givenIt,
const COORD target)
const COORD target,
const std::optional<bool> wrap)
{
// Make mutable copy so we can walk.
auto it = givenIt;
Expand All @@ -336,7 +338,8 @@ OutputCellIterator TextBuffer::Write(const OutputCellIterator givenIt,
while (it && size.IsInBounds(lineTarget))
{
// Attempt to write as much data as possible onto this line.
it = WriteLine(it, lineTarget, true);
// NOTE: if wrap = true/false, we want to set the line's wrap to true/false (respectively) if we reach the end of the line
it = WriteLine(it, lineTarget, wrap);

// Move to the next line down.
lineTarget.X = 0;
Expand All @@ -351,13 +354,13 @@ OutputCellIterator TextBuffer::Write(const OutputCellIterator givenIt,
// Arguments:
// - givenIt - The iterator that will dereference into cell data to insert
// - target - Coordinate targeted within output buffer
// - setWrap - Whether we should try to set the wrap flag if we write up to the end of the line and have more data
// - wrap - change the wrap flag if we hit the end of the row while writing and there's still more data in the iterator.
// - limitRight - Optionally restrict the right boundary for writing (e.g. stop writing earlier than the end of line)
// Return Value:
// - The iterator, but advanced to where we stopped writing. Use to find input consumed length or cells written length.
OutputCellIterator TextBuffer::WriteLine(const OutputCellIterator givenIt,
const COORD target,
const bool setWrap,
const std::optional<bool> wrap,
std::optional<size_t> limitRight)
{
// If we're not in bounds, exit early.
Expand All @@ -368,7 +371,7 @@ OutputCellIterator TextBuffer::WriteLine(const OutputCellIterator givenIt,

// Get the row and write the cells
ROW& row = GetRowByOffset(target.Y);
const auto newIt = row.WriteCells(givenIt, target.X, setWrap, limitRight);
const auto newIt = row.WriteCells(givenIt, target.X, wrap, limitRight);

// Take the cell distance written and notify that it needs to be repainted.
const auto written = newIt.GetCellDistance(givenIt);
Expand Down
5 changes: 3 additions & 2 deletions src/buffer/out/textBuffer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,12 @@ class TextBuffer final
OutputCellIterator Write(const OutputCellIterator givenIt);

OutputCellIterator Write(const OutputCellIterator givenIt,
const COORD target);
const COORD target,
const std::optional<bool> wrap = true);

OutputCellIterator WriteLine(const OutputCellIterator givenIt,
const COORD target,
const bool setWrap = false,
const std::optional<bool> setWrap = std::nullopt,
const std::optional<size_t> limitRight = std::nullopt);

bool InsertCharacter(const wchar_t wch, const DbcsAttribute dbcsAttribute, const TextAttribute attr);
Expand Down
5 changes: 4 additions & 1 deletion src/host/_output.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,10 @@ void WriteToScreen(SCREEN_INFORMATION& screenInfo, const Viewport& region)
try
{
const OutputCellIterator it(character, lengthToWrite);
const auto done = screenInfo.Write(it, startingCoordinate);

// when writing to the buffer, specifically unset wrap if we get to the last column.
// a fill operation should UNSET wrap in that scenario. See GH #1126 for more details.
const auto done = screenInfo.Write(it, startingCoordinate, false);
cellsModified = done.GetInputDistance(it);

// Notify accessibility
Expand Down
121 changes: 121 additions & 0 deletions src/host/ft_host/API_FillOutputTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,125 @@ class FillOutputTests
&charsWritten));
VERIFY_ARE_EQUAL(1u, charsWritten);
}

TEST_METHOD(UnsetWrap)
{
// WARNING: If this test suddenly decides to start failing,
// this is because the wrap registry key is not set.
// TODO GH #2859: Get/Set Registry Key for Wrap

HANDLE hConsole = GetStdOutputHandle();
DWORD charsWritten = 0;

CONSOLE_SCREEN_BUFFER_INFOEX sbiex = { 0 };
sbiex.cbSize = sizeof(sbiex);
VERIFY_WIN32_BOOL_SUCCEEDED(GetConsoleScreenBufferInfoEx(hConsole, &sbiex));

const auto consoleWidth = sbiex.dwSize.X;

std::wstring input(consoleWidth + 2, L'a');
std::wstring filled(consoleWidth, L'b');

// Write until a wrap occurs
VERIFY_WIN32_BOOL_SUCCEEDED(WriteConsoleW(hConsole,
input.data(),
gsl::narrow_cast<DWORD>(input.size()),
&charsWritten,
nullptr));

// Verify wrap occurred
std::unique_ptr<wchar_t[]> bufferText = std::make_unique<wchar_t[]>(consoleWidth);
DWORD readSize = 0;
VERIFY_WIN32_BOOL_SUCCEEDED(ReadConsoleOutputCharacterW(hConsole,
bufferText.get(),
consoleWidth,
{ 0, 0 },
&readSize));

WEX::Common::String expected(input.c_str(), readSize);
WEX::Common::String actual(bufferText.get(), readSize);
VERIFY_ARE_EQUAL(expected, actual);

bufferText = std::make_unique<wchar_t[]>(2);
VERIFY_WIN32_BOOL_SUCCEEDED(ReadConsoleOutputCharacterW(hConsole,
bufferText.get(),
2,
{ 0, 1 },
&readSize));

VERIFY_ARE_EQUAL(2u, readSize);
expected = WEX::Common::String(input.c_str(), readSize);
actual = WEX::Common::String(bufferText.get(), readSize);
VERIFY_ARE_EQUAL(expected, actual);

// Fill Console Line with 'b's
VERIFY_WIN32_BOOL_SUCCEEDED(FillConsoleOutputCharacterW(hConsole,
L'b',
consoleWidth,
{ 2, 0 },
&charsWritten));

// Verify first line is full of 'a's then 'b's
bufferText = std::make_unique<wchar_t[]>(consoleWidth);
VERIFY_WIN32_BOOL_SUCCEEDED(ReadConsoleOutputCharacterW(hConsole,
bufferText.get(),
consoleWidth,
{ 0, 0 },
&readSize));

expected = WEX::Common::String(input.c_str(), 2);
actual = WEX::Common::String(bufferText.get(), 2);
VERIFY_ARE_EQUAL(expected, actual);

expected = WEX::Common::String(filled.c_str(), consoleWidth - 2);
actual = WEX::Common::String(&bufferText[2], readSize - 2);
VERIFY_ARE_EQUAL(expected, actual);

// Verify second line is still has 'a's that wrapped over
bufferText = std::make_unique<wchar_t[]>(2);
VERIFY_WIN32_BOOL_SUCCEEDED(ReadConsoleOutputCharacterW(hConsole,
bufferText.get(),
static_cast<SHORT>(2),
{ 0, 0 },
&readSize));

VERIFY_ARE_EQUAL(2u, readSize);
expected = WEX::Common::String(input.c_str(), 2);
actual = WEX::Common::String(bufferText.get(), readSize);
VERIFY_ARE_EQUAL(expected, actual);

// Resize to be smaller by 2
sbiex.srWindow.Right -= 2;
sbiex.dwSize.X -= 2;
VERIFY_WIN32_BOOL_SUCCEEDED(SetConsoleScreenBufferInfoEx(hConsole, &sbiex));

// Verify first line is full of 'a's then 'b's
bufferText = std::make_unique<wchar_t[]>(consoleWidth - 2);
VERIFY_WIN32_BOOL_SUCCEEDED(ReadConsoleOutputCharacterW(hConsole,
bufferText.get(),
consoleWidth - static_cast<SHORT>(2),
{ 0, 0 },
&readSize));

expected = WEX::Common::String(input.c_str(), 2);
actual = WEX::Common::String(bufferText.get(), 2);
VERIFY_ARE_EQUAL(expected, actual);

expected = WEX::Common::String(filled.c_str(), consoleWidth - 4);
actual = WEX::Common::String(&bufferText[2], readSize - 2);
VERIFY_ARE_EQUAL(expected, actual);

// Verify second line is still has 'a's ('b's didn't wrap over)
bufferText = std::make_unique<wchar_t[]>(static_cast<SHORT>(2));
VERIFY_WIN32_BOOL_SUCCEEDED(ReadConsoleOutputCharacterW(hConsole,
bufferText.get(),
static_cast<SHORT>(2),
{ 0, 0 },
&readSize));

VERIFY_ARE_EQUAL(2u, readSize);
expected = WEX::Common::String(input.c_str(), 2);
actual = WEX::Common::String(bufferText.get(), readSize);
VERIFY_ARE_EQUAL(expected, actual);
}
};
7 changes: 5 additions & 2 deletions src/host/screenInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2586,14 +2586,17 @@ OutputCellIterator SCREEN_INFORMATION::Write(const OutputCellIterator it)
// Arguments:
// - it - Iterator representing output cell data to write.
// - target - The position to start writing at
// - wrap - change the wrap flag if we hit the end of the row while writing and there's still more data
// Return Value:
// - the iterator at its final position
// Note:
// - will throw exception on error.
OutputCellIterator SCREEN_INFORMATION::Write(const OutputCellIterator it,
const COORD target)
const COORD target,
const std::optional<bool> wrap)
{
return _textBuffer->Write(it, target);
// NOTE: if wrap = true/false, we want to set the line's wrap to true/false (respectively) if we reach the end of the line
return _textBuffer->Write(it, target, wrap);
}

// Routine Description:
Expand Down
3 changes: 2 additions & 1 deletion src/host/screenInfo.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@ class SCREEN_INFORMATION : public ConsoleObjectHeader, public Microsoft::Console
OutputCellIterator Write(const OutputCellIterator it);

OutputCellIterator Write(const OutputCellIterator it,
const COORD target);
const COORD target,
const std::optional<bool> wrap = true);

OutputCellIterator WriteRect(const OutputCellIterator it,
const Microsoft::Console::Types::Viewport viewport);
Expand Down
74 changes: 74 additions & 0 deletions src/host/ut_host/TextBufferTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ class TextBufferTests

TEST_METHOD(TestWrapFlag);

TEST_METHOD(TestWrapThroughWriteLine);

TEST_METHOD(TestDoubleBytePadFlag);

void DoBoundaryTest(PWCHAR const pwszInputString,
Expand Down Expand Up @@ -197,6 +199,78 @@ void TextBufferTests::TestWrapFlag()
VERIFY_IS_FALSE(Row.GetCharRow().WasWrapForced());
}

void TextBufferTests::TestWrapThroughWriteLine()
{
TextBuffer& textBuffer = GetTbi();

auto VerifyWrap = [&](bool expected) {
ROW& Row = textBuffer._GetFirstRow();

if (expected)
{
VERIFY_IS_TRUE(Row.GetCharRow().WasWrapForced());
}
else
{
VERIFY_IS_FALSE(Row.GetCharRow().WasWrapForced());
}
};

// Construct string for testing
const auto width = textBuffer.GetSize().Width();
std::wstring chars = L"";
for (auto i = 0; i < width; i++)
{
chars.append(L"a");
}
const auto lineOfText = std::move(chars);

Log::Comment(L"Case 1 : Implicit wrap (false)");
{
TextAttribute expectedAttr(FOREGROUND_RED);
OutputCellIterator it(lineOfText, expectedAttr);

textBuffer.WriteLine(it, { 0, 0 });
VerifyWrap(false);
}

Log::Comment(L"Case 2 : wrap = true");
{
TextAttribute expectedAttr(FOREGROUND_RED);
OutputCellIterator it(lineOfText, expectedAttr);

textBuffer.WriteLine(it, { 0, 0 }, true);
VerifyWrap(true);
}

Log::Comment(L"Case 3: wrap = nullopt (remain as TRUE)");
{
TextAttribute expectedAttr(FOREGROUND_RED);
OutputCellIterator it(lineOfText, expectedAttr);

textBuffer.WriteLine(it, { 0, 0 }, std::nullopt);
VerifyWrap(true);
}

Log::Comment(L"Case 4: wrap = false");
{
TextAttribute expectedAttr(FOREGROUND_RED);
OutputCellIterator it(lineOfText, expectedAttr);

textBuffer.WriteLine(it, { 0, 0 }, false);
VerifyWrap(false);
}

Log::Comment(L"Case 5: wrap = nullopt (remain as false)");
{
TextAttribute expectedAttr(FOREGROUND_RED);
OutputCellIterator it(lineOfText, expectedAttr);

textBuffer.WriteLine(it, { 0, 0 }, std::nullopt);
VerifyWrap(false);
}
}

void TextBufferTests::TestDoubleBytePadFlag()
{
TextBuffer& textBuffer = GetTbi();
Expand Down