maxmustermann/terminal
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

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

Browse source 
EraseInLine calls `FillConsoleOutputCharacterW()`. In filling the row with
chars, we were setting the wrap flag. We need to specifically not do this on
ANY _FILL_ operation. Now a fill operation UNSETS the wrap flag if we fill to
the end of the line.

Originally, we had a boolean `setWrap` that would mean...
- **true**: if writing to the end of the row, SET the wrap value to true
- **false**: if writing to the end of the row, DON'T CHANGE the wrap value

Now we're making this bool a std::optional to allow for a ternary state. This
allows for us to handle the following cases completely. Refer to the table
below:

,- current wrap value
|     ,- are we filling the last cell in the row?
|     |     ,- new wrap value
|     |     |     ,- comments
|--   |--   |--   |
| 0   | 0   | 0   |
| 0   | 1   | 0   |
| 0   | 1   | 1   | THIS CASE WAS HANDLED CORRECTLY
| 1   | 0   | 0   | THIS CASE WAS UNHANDLED
| 1   | 0   | 1   |
| 1   | 1   | 1   |

To handle that special case (1-0-0), we need to UNSET the wrap. So now, we have
~setWrap~ `wrap` mean the following:
- **true**: if writing to the end of the row, SET the wrap value to TRUE
- **false**: if writing to the end of the row, SET the wrap value to FALSE
- **nullopt**: leave the wrap value as it is

Closes #1126
This commit is contained in:
Carlos Zamora 2019-09-30 18:16:31 -07:00 committed by Dustin L. Howett (MSFT)
parent a2f8a943b4
commit 4dd9f9c180
9 changed files with 228 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
src/buffer/out/Row.cpp
View file

@ -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());
@ -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

2
src/buffer/out/Row.hpp
View file

@ -57,7 +57,7 @@ public:
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;

13
src/buffer/out/textBuffer.cpp
View file

@ -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;
@ -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;
@ -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.
@ -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);

5
src/buffer/out/textBuffer.hpp
View file

@ -89,11 +89,12 @@ public:
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);

5
src/host/_output.cpp
View file

@ -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

121
src/host/ft_host/API_FillOutputTests.cpp
View file

@ -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
src/host/screenInfo.cpp
View file

@ -2599,14 +2599,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:

3
src/host/screenInfo.hpp
View file

@ -131,7 +131,8 @@ public:
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);

74
src/host/ut_host/TextBufferTests.cpp
View file

@ -77,6 +77,8 @@ class TextBufferTests
TEST_METHOD(TestWrapFlag);
TEST_METHOD(TestWrapThroughWriteLine);
TEST_METHOD(TestDoubleBytePadFlag);
void DoBoundaryTest(PWCHAR const pwszInputString,
@ -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();