Skip to content

Commit

Permalink
Remove IsGlyphFullWidth from InputBuffer
Browse files Browse the repository at this point in the history
  • Loading branch information
lhecker committed Aug 7, 2024
1 parent 2fab986 commit 7641a08
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 19 deletions.
11 changes: 3 additions & 8 deletions src/host/inputBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include "misc.h"
#include "stream.h"
#include "../interactivity/inc/ServiceLocator.hpp"
#include "../types/inc/GlyphWidth.hpp"

#define INPUT_BUFFER_DEFAULT_INPUT_MODE (ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT | ENABLE_ECHO_INPUT | ENABLE_MOUSE_INPUT)

Expand Down Expand Up @@ -804,13 +803,9 @@ bool InputBuffer::_CoalesceEvent(const INPUT_RECORD& inEvent) noexcept
(lastKey.wVirtualScanCode == inKey.wVirtualScanCode || WI_IsFlagSet(inKey.dwControlKeyState, NLS_IME_CONVERSION)) &&
lastKey.uChar.UnicodeChar == inKey.uChar.UnicodeChar &&
lastKey.dwControlKeyState == inKey.dwControlKeyState &&
// TODO:GH#8000 This behavior is an import from old conhost v1 and has been broken for decades.
// This is probably the outdated idea that any wide glyph is being represented by 2 characters (DBCS) and likely
// resulted from conhost originally being split into a ASCII/OEM and a DBCS variant with preprocessor flags.
// You can't update the repeat count of such a A,B pair, because they're stored as A,A,B,B (down-down, up-up).
// I believe the proper approach is to store pairs of characters as pairs, update their combined
// repeat count and only when they're being read de-coalesce them into their alternating form.
!IsGlyphFullWidth(inKey.uChar.UnicodeChar))
// A single repeat count cannot represent two INPUT_RECORDs simultaneously,
// and so it cannot represent a surrogate pair either.
!til::is_surrogate(inKey.uChar.UnicodeChar))
{
lastKey.wRepeatCount += inKey.wRepeatCount;
return true;
Expand Down
19 changes: 8 additions & 11 deletions src/host/ut_host/InputBufferTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -219,22 +219,19 @@ class InputBufferTests
}
}

TEST_METHOD(InputBufferDoesNotCoalesceFullWidthChars)
TEST_METHOD(InputBufferDoesNotCoalesceSurrogatePairs)
{
InputBuffer inputBuffer;
WCHAR hiraganaA = 0x3042; // U+3042 hiragana A
auto record = MakeKeyEvent(true, 1, hiraganaA, 0, hiraganaA, 0);

// send a bunch of identical events
inputBuffer.Flush();
for (size_t i = 0; i < RECORD_INSERT_COUNT; ++i)
{
VERIFY_IS_GREATER_THAN(inputBuffer.Write(record), 0u);
VERIFY_ARE_EQUAL(inputBuffer._storage.back(), record);
}
// U+1F44D thumbs up emoji
inputBuffer.Write(MakeKeyEvent(true, 1, 0xD83D, 0, 0xD83D, 0));
inputBuffer.Write(MakeKeyEvent(true, 1, 0xDC4D, 0, 0xDC4D, 0));

// Should not coalese despite otherwise matching perfectly.

Check failure

Code scanning / check-spelling

Unrecognized Spelling Error

coalese is not a recognized word. (unrecognized-spelling)
inputBuffer.Write(MakeKeyEvent(true, 1, 0xDC4D, 0, 0xDC4D, 0));

// The events shouldn't be coalesced
VERIFY_ARE_EQUAL(inputBuffer.GetNumberOfReadyEvents(), RECORD_INSERT_COUNT);
VERIFY_ARE_EQUAL(inputBuffer.GetNumberOfReadyEvents(), 3);
}

TEST_METHOD(CanFlushAllOutput)
Expand Down

0 comments on commit 7641a08

Please sign in to comment.