From 7a8dd90294e53b9d5466ff755611d13f4249ce0b Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 7 Nov 2023 18:51:13 +0100 Subject: [PATCH] Fix tabs being printed in cmd.exe prompts (#16273) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A late change in #16105 wrapped `_buffer` into a class to better track its dirty state, but I failed to notice that in this one instance we intentionally manipulated `_buffer` without marking it as dirty. This fixes the issue by adding a call to `MarkAsClean()`. This changeset also adds the test instructions from #15783 as a document to this repository. I've extended the list with two bugs we've found in the implementation since then. ## Validation Steps Performed * In cmd.exe, with an empty prompt in an empty directory: Pressing tab produces an audible bing and prints no text ✅ --- doc/COOKED_READ_DATA.md | 90 +++++++++++++++++++++++++++++++++++++ src/host/readDataCooked.cpp | 7 ++- 2 files changed, 95 insertions(+), 2 deletions(-) create mode 100644 doc/COOKED_READ_DATA.md diff --git a/doc/COOKED_READ_DATA.md b/doc/COOKED_READ_DATA.md new file mode 100644 index 00000000000..113a51bb749 --- /dev/null +++ b/doc/COOKED_READ_DATA.md @@ -0,0 +1,90 @@ +# COOKED_READ_DATA, aka conhost's readline implementation + +## Test instructions + +All of the following ✅ marks must be fulfilled during manual testing: +* ASCII input +* Chinese input (中文維基百科) ❔ + * Resizing the window properly wraps/unwraps wide glyphs ❌ + Broken due to `TextBuffer::Reflow` bugs +* Surrogate pair input (🙂) ❔ + * Resizing the window properly wraps/unwraps surrogate pairs ❌ + Broken due to `TextBuffer::Reflow` bugs +* In cmd.exe + * Create 2 file: "a😊b.txt" and "a😟b.txt" + * Press tab: Autocomplete to "a😊b.txt" ✅ + * Navigate the cursor right past the "a" + * Press tab twice: Autocomplete to "a😟b.txt" ✅ +* Backspace deletes preceding glyphs ✅ +* Ctrl+Backspace deletes preceding words ✅ +* Escape clears input ✅ +* Home navigates to start ✅ +* Ctrl+Home deletes text between cursor and start ✅ +* End navigates to end ✅ +* Ctrl+End deletes text between cursor and end ✅ +* Left navigates over previous code points ✅ +* Ctrl+Left navigates to previous word-starts ✅ +* Right and F1 navigate over next code points ✅ + * Pressing right at the end of input copies characters + from the previous command ✅ +* Ctrl+Right navigates to next word-ends ✅ +* Insert toggles overwrite mode ✅ +* Delete deletes next code point ✅ +* Up and F5 cycle through history ✅ + * Doesn't crash with no history ✅ + * Stops at first entry ✅ +* Down cycles through history ✅ + * Doesn't crash with no history ✅ + * Stops at last entry ✅ +* PageUp retrieves the oldest command ✅ +* PageDown retrieves the newest command ✅ +* F2 starts "copy to char" prompt ✅ + * Escape dismisses prompt ✅ + * Typing a character copies text from the previous command up + until that character into the current buffer (acts identical + to F3, but with automatic character search) ✅ +* F3 copies the previous command into the current buffer, + starting at the current cursor position, + for as many characters as possible ✅ + * Doesn't erase trailing text if the current buffer + is longer than the previous command ✅ + * Puts the cursor at the end of the copied text ✅ +* F4 starts "copy from char" prompt ✅ + * Escape dismisses prompt ✅ + * Erases text between the current cursor position and the + first instance of a given char (but not including it) ✅ +* F6 inserts Ctrl+Z ✅ +* F7 without modifiers starts "command list" prompt ✅ + * Escape dismisses prompt ✅ + * Minimum size of 40x10 characters ✅ + * Width expands to fit the widest history command ✅ + * Height expands up to 20 rows with longer histories ✅ + * F9 starts "command number" prompt ✅ + * Left/Right paste replace the buffer with the given command ✅ + * And put cursor at the end of the buffer ✅ + * Up/Down navigate selection through history ✅ + * Stops at start/end with <10 entries ✅ + * Stops at start/end with >20 entries ✅ + * Wide text rendering during pagination with >20 entries ✅ + * Shift+Up/Down moves history items around ✅ + * Home navigates to first entry ✅ + * End navigates to last entry ✅ + * PageUp navigates by 20 items at a time or to first ✅ + * PageDown navigates by 20 items at a time or to last ✅ +* Alt+F7 clears command history ✅ +* F8 cycles through commands that start with the same text as + the current buffer up until the current cursor position ✅ + * Doesn't crash with no history ✅ +* F9 starts "command number" prompt ✅ + * Escape dismisses prompt ✅ + * Ignores non-ASCII-decimal characters ✅ + * Allows entering between 1 and 5 digits ✅ + * Pressing Enter fetches the given command from the history ✅ +* Alt+F10 clears doskey aliases ✅ +* In cmd.exe, with an empty prompt in an empty directory: + Pressing tab produces an audible bing and prints no text ✅ +* When Narrator is enabled, in cmd.exe: + * Typing individual characters announces only + exactly each character that is being typed ✅ + * Backspacing at the end of a prompt announces + only exactly each deleted character ✅ diff --git a/src/host/readDataCooked.cpp b/src/host/readDataCooked.cpp index 3644dfe78db..c842bab312d 100644 --- a/src/host/readDataCooked.cpp +++ b/src/host/readDataCooked.cpp @@ -436,14 +436,17 @@ void COOKED_READ_DATA::_handleChar(wchar_t wch, const DWORD modifiers) if (_ctrlWakeupMask != 0 && wch < L' ' && (_ctrlWakeupMask & (1 << wch))) { - _flushBuffer(); - // The old implementation (all the way since the 90s) overwrote the character at the current cursor position with the given wch. // But simultaneously it incremented the buffer length, which would have only worked if it was written at the end of the buffer. // Press tab past the "f" in the string "foo" and you'd get "f\to " (a trailing whitespace; the initial contents of the buffer back then). // It's unclear whether the original intention was to write at the end of the buffer at all times or to implement an insert mode. // I went with insert mode. + // + // It is important that we don't actually print that character out though, as it's only for the calling application to see. + // That's why we flush the contents before the insertion and then ensure that the _flushBuffer() call in Read() exits early. + _flushBuffer(); _buffer.Replace(_buffer.GetCursorPosition(), 0, &wch, 1); + _buffer.MarkAsClean(); _controlKeyState = modifiers; _transitionState(State::DoneWithWakeupMask);