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

Rewrite COOKED_READ_DATA #15783

Merged
merged 14 commits into from
Aug 25, 2023
Merged
Show file tree
Hide file tree
Changes from 7 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
1 change: 1 addition & 0 deletions .github/actions/spelling/expect/alphabet.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ BBBBCCCCC
BBGGRR
efg
EFG
efgh
EFGh
KLMNOQQQQQQQQQQ
QQQQQQQQQQABCDEFGHIJ
Expand Down
28 changes: 2 additions & 26 deletions .github/actions/spelling/expect/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ ADDALIAS
ADDREF
ADDSTRING
ADDTOOL
AEnd
AFew
AFill
AFX
Expand Down Expand Up @@ -307,7 +306,6 @@ coordnew
COPYCOLOR
CORESYSTEM
cotaskmem
countof
CPG
cpinfo
CPINFOEX
Expand Down Expand Up @@ -691,7 +689,7 @@ FSINFOCLASS
fte
Ftm
Fullscreens
fullwidth
Fullwidth
FUNCTIONCALL
fuzzer
fuzzmain
Expand Down Expand Up @@ -832,7 +830,6 @@ HMK
hmod
hmodule
hmon
homeglyphs
homoglyph
HORZ
hostable
Expand Down Expand Up @@ -936,7 +933,6 @@ itermcolors
ITerminal
itf
Ith
itoa
IUI
IUnknown
ivalid
Expand Down Expand Up @@ -965,7 +961,6 @@ kernelbasestaging
KEYBDINPUT
keychord
keydown
keyevent
KEYFIRST
KEYLAST
Keymapping
Expand Down Expand Up @@ -1118,6 +1113,7 @@ MDs
MEASUREITEM
megamix
memallocator
meme
MENUCHAR
MENUCONTROL
MENUDROPALIGNMENT
Expand Down Expand Up @@ -1318,7 +1314,6 @@ onecoreuuid
ONECOREWINDOWS
onehalf
oneseq
ONLCR
openbash
opencode
opencon
Expand All @@ -1328,13 +1323,6 @@ openps
openvt
ORIGINALFILENAME
osc
OSCBG
OSCCT
OSCFG
OSCRCC
OSCSCB
OSCSCC
OSCWT
OSDEPENDSROOT
OSG
OSGENG
Expand Down Expand Up @@ -1496,7 +1484,6 @@ propvariant
propvarutil
psa
PSECURITY
pseudocode
pseudoconsole
pseudoterminal
psh
Expand Down Expand Up @@ -1615,7 +1602,6 @@ rgrc
rgs
rgui
rgw
rgwch
RIGHTALIGN
RIGHTBUTTON
riid
Expand Down Expand Up @@ -1811,7 +1797,6 @@ STDMETHODCALLTYPE
STDMETHODIMP
STGM
stl
stoutapot
Stri
Stringable
STRINGTABLE
Expand Down Expand Up @@ -1867,7 +1852,6 @@ TBM
tchar
TCHFORMAT
TCI
tcome
tcommandline
tcommands
Tdd
Expand All @@ -1888,7 +1872,6 @@ TEs
testbuildplatform
testcon
testd
testdlls
testenv
testlab
testlist
Expand All @@ -1899,8 +1882,6 @@ testnameprefix
TESTNULL
testpass
testpasses
testtestabc
testtesttesttesttest
testtimeout
TEXCOORD
texel
Expand Down Expand Up @@ -1929,7 +1910,6 @@ TJson
TLambda
TLDP
TLEN
Tlgdata
TMAE
TMPF
TMult
Expand Down Expand Up @@ -2112,7 +2092,6 @@ vtseq
vtterm
vttest
VWX
waaay
waitable
WANSUNG
WANTARROWS
Expand All @@ -2134,7 +2113,6 @@ WDDMCONSOLECONTEXT
wdm
webpage
websites
websockets
wekyb
wex
wextest
Expand Down Expand Up @@ -2232,7 +2210,6 @@ wprp
wprpi
wregex
writeback
writechar
WRITECONSOLE
WRITECONSOLEINPUT
WRITECONSOLEOUTPUT
Expand Down Expand Up @@ -2306,7 +2283,6 @@ xunit
xutr
XVIRTUALSCREEN
XWalk
xxyyzz
yact
YCast
YCENTER
Expand Down
9 changes: 7 additions & 2 deletions src/buffer/out/Row.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,15 @@ LineRendition ROW::GetLineRendition() const noexcept
return _lineRendition;
}

uint16_t ROW::GetLineWidth() const noexcept
// Returns the index 1 past the last (technically) valid column in the row.
// The interplay between the old console and newer VT APIs which support line renditions is
// still unclear so it might be necessary to add two kinds of this function in the future.
// Console APIs treat the buffer as a large NxM matrix after all.
til::CoordType ROW::GetLineWidth() const noexcept
{
const auto scale = _lineRendition != LineRendition::SingleWidth ? 1 : 0;
return _columnCount >> scale;
const auto padding = _doubleBytePadded ? 1 : 0;
return (_columnCount - padding) >> scale;
}

// Routine Description:
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 @@ -106,7 +106,7 @@ class ROW final
bool WasDoubleBytePadded() const noexcept;
void SetLineRendition(const LineRendition lineRendition) noexcept;
LineRendition GetLineRendition() const noexcept;
uint16_t GetLineWidth() const noexcept;
til::CoordType GetLineWidth() const noexcept;

void Reset(const TextAttribute& attr) noexcept;
void TransferAttributes(const til::small_rle<TextAttribute, uint16_t, 1>& attr, til::CoordType newWidth);
Expand Down
58 changes: 58 additions & 0 deletions src/buffer/out/textBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,64 @@ size_t TextBuffer::GraphemePrev(const std::wstring_view& chars, size_t position)
return til::utf16_iterate_prev(chars, position);
}

// Pretend as if `position` is a regular cursor in the TextBuffer.
// This function will then pretend as if you pressed the left/right arrow
// keys `distance` amount of times (negative = left, positive = right).
til::point TextBuffer::NavigateCursor(til::point position, til::CoordType distance)
Copy link
Member

Choose a reason for hiding this comment

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

oh lmao so it's like the opposite of the GetCellDistance thing I did

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep exactly! 😅 I use it to implement the whacky initialData argument for cooked read.

{
lhecker marked this conversation as resolved.
Show resolved Hide resolved
const til::CoordType maxX = _width - 1;
const til::CoordType maxY = _height - 1;
auto x = std::clamp(position.x, 0, maxX);
auto y = std::clamp(position.y, 0, maxY);
auto row = &GetRowByOffset(y);

if (distance < 0)
{
do
{
if (x > 0)
{
x = row->NavigateToPrevious(x);
}
else if (y <= 0)
{
break;
}
else
{
--y;
row = &GetRowByOffset(y);
x = row->GetLineWidth() - 1;
}
} while (++distance != 0);
}
else if (distance > 0)
{
auto rowWidth = row->GetLineWidth();

do
{
if (x < rowWidth)
{
x = row->NavigateToNext(x);
}
else if (y >= maxY)
{
break;
}
else
{
++y;
row = &GetRowByOffset(y);
rowWidth = row->GetLineWidth();
x = 0;
}
} while (--distance != 0);
}

return { x, y };
}

// This function is intended for writing regular "lines" of text as it'll set the wrap flag on the given row.
// You can continue calling the function on the same row as long as state.columnEnd < state.columnLimit.
void TextBuffer::Write(til::CoordType row, const TextAttribute& attributes, RowWriteState& state)
Expand Down
2 changes: 2 additions & 0 deletions src/buffer/out/textBuffer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ class TextBuffer final
static size_t GraphemeNext(const std::wstring_view& chars, size_t position) noexcept;
static size_t GraphemePrev(const std::wstring_view& chars, size_t position) noexcept;

til::point NavigateCursor(til::point position, til::CoordType distance);

// Text insertion functions
void Write(til::CoordType row, const TextAttribute& attributes, RowWriteState& state);
void FillRect(const til::rect& rect, const std::wstring_view& fill, const TextAttribute& attributes);
Expand Down
24 changes: 2 additions & 22 deletions src/cascadia/UnitTests_TerminalCore/ConptyRoundtripTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
#include "../host/readDataCooked.hpp"
#include "../host/output.h"
#include "../host/_stream.h" // For WriteCharsLegacy
#include "../host/cmdline.h" // For WC_INTERACTIVE
#include "test/CommonState.hpp"

#include "../cascadia/TerminalCore/Terminal.hpp"
Expand Down Expand Up @@ -3165,20 +3164,6 @@ void ConptyRoundtripTests::NewLinesAtBottomWithBackground()
verifyBuffer(*termTb, term->_mutableViewport.ToExclusive());
}

void doWriteCharsLegacy(SCREEN_INFORMATION& screenInfo, const std::wstring_view string, DWORD flags = 0)
{
auto dwNumBytes = string.size() * sizeof(wchar_t);
VERIFY_NT_SUCCESS(WriteCharsLegacy(screenInfo,
string.data(),
string.data(),
string.data(),
&dwNumBytes,
nullptr,
screenInfo.GetTextBuffer().GetCursor().GetPosition().x,
flags,
nullptr));
}

void ConptyRoundtripTests::WrapNewLineAtBottom()
{
// The actual bug case is
Expand Down Expand Up @@ -3220,11 +3205,6 @@ void ConptyRoundtripTests::WrapNewLineAtBottom()
return;
}

// I've tested this with 0x0, 0x4, 0x80, 0x84, and 0-8, and none of these
// flags seem to make a difference. So we're just assuming 0 here, so we
// don't test a bunch of redundant cases.
const auto writeCharsLegacyMode = 0;

// This test was originally written for
// https://github.com/microsoft/terminal/issues/5691
//
Expand Down Expand Up @@ -3263,7 +3243,7 @@ void ConptyRoundtripTests::WrapNewLineAtBottom()
}
else if (writingMethod == PrintWithWriteCharsLegacy)
{
doWriteCharsLegacy(si, str, writeCharsLegacyMode);
WriteCharsLegacy(si, str, false, nullptr);
}
};

Expand Down Expand Up @@ -3421,7 +3401,7 @@ void ConptyRoundtripTests::WrapNewLineAtBottomLikeMSYS()
}
else if (writingMethod == PrintWithWriteCharsLegacy)
{
doWriteCharsLegacy(si, str, WC_INTERACTIVE);
WriteCharsLegacy(si, str, true, nullptr);
}
};

Expand Down
Loading
Loading