Skip to content

Commit db27348

Browse files
lheckerDHowett
authored andcommitted
Fix tabs being printed in cmd.exe prompts (#16273)
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 ✅ (cherry picked from commit 7a8dd90) Service-Card-Id: 91033502 Service-Version: 1.19
1 parent 83a2bc1 commit db27348

File tree

2 files changed

+95
-2
lines changed

2 files changed

+95
-2
lines changed

doc/COOKED_READ_DATA.md

+90
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
# COOKED_READ_DATA, aka conhost's readline implementation
2+
3+
## Test instructions
4+
5+
All of the following ✅ marks must be fulfilled during manual testing:
6+
* ASCII input
7+
* Chinese input (中文維基百科) ❔
8+
* Resizing the window properly wraps/unwraps wide glyphs ❌
9+
Broken due to `TextBuffer::Reflow` bugs
10+
* Surrogate pair input (🙂) ❔
11+
* Resizing the window properly wraps/unwraps surrogate pairs ❌
12+
Broken due to `TextBuffer::Reflow` bugs
13+
* In cmd.exe
14+
* Create 2 file: "a😊b.txt" and "a😟b.txt"
15+
* Press tab: Autocomplete to "a😊b.txt" ✅
16+
* Navigate the cursor right past the "a"
17+
* Press tab twice: Autocomplete to "a😟b.txt" ✅
18+
* Backspace deletes preceding glyphs ✅
19+
* Ctrl+Backspace deletes preceding words ✅
20+
* Escape clears input ✅
21+
* Home navigates to start ✅
22+
* Ctrl+Home deletes text between cursor and start ✅
23+
* End navigates to end ✅
24+
* Ctrl+End deletes text between cursor and end ✅
25+
* Left navigates over previous code points ✅
26+
* Ctrl+Left navigates to previous word-starts ✅
27+
* Right and F1 navigate over next code points ✅
28+
* Pressing right at the end of input copies characters
29+
from the previous command ✅
30+
* Ctrl+Right navigates to next word-ends ✅
31+
* Insert toggles overwrite mode ✅
32+
* Delete deletes next code point ✅
33+
* Up and F5 cycle through history ✅
34+
* Doesn't crash with no history ✅
35+
* Stops at first entry ✅
36+
* Down cycles through history ✅
37+
* Doesn't crash with no history ✅
38+
* Stops at last entry ✅
39+
* PageUp retrieves the oldest command ✅
40+
* PageDown retrieves the newest command ✅
41+
* F2 starts "copy to char" prompt ✅
42+
* Escape dismisses prompt ✅
43+
* Typing a character copies text from the previous command up
44+
until that character into the current buffer (acts identical
45+
to F3, but with automatic character search) ✅
46+
* F3 copies the previous command into the current buffer,
47+
starting at the current cursor position,
48+
for as many characters as possible ✅
49+
* Doesn't erase trailing text if the current buffer
50+
is longer than the previous command ✅
51+
* Puts the cursor at the end of the copied text ✅
52+
* F4 starts "copy from char" prompt ✅
53+
* Escape dismisses prompt ✅
54+
* Erases text between the current cursor position and the
55+
first instance of a given char (but not including it) ✅
56+
* F6 inserts Ctrl+Z ✅
57+
* F7 without modifiers starts "command list" prompt ✅
58+
* Escape dismisses prompt ✅
59+
* Minimum size of 40x10 characters ✅
60+
* Width expands to fit the widest history command ✅
61+
* Height expands up to 20 rows with longer histories ✅
62+
* F9 starts "command number" prompt ✅
63+
* Left/Right paste replace the buffer with the given command ✅
64+
* And put cursor at the end of the buffer ✅
65+
* Up/Down navigate selection through history ✅
66+
* Stops at start/end with <10 entries ✅
67+
* Stops at start/end with >20 entries ✅
68+
* Wide text rendering during pagination with >20 entries ✅
69+
* Shift+Up/Down moves history items around ✅
70+
* Home navigates to first entry ✅
71+
* End navigates to last entry ✅
72+
* PageUp navigates by 20 items at a time or to first ✅
73+
* PageDown navigates by 20 items at a time or to last ✅
74+
* Alt+F7 clears command history ✅
75+
* F8 cycles through commands that start with the same text as
76+
the current buffer up until the current cursor position ✅
77+
* Doesn't crash with no history ✅
78+
* F9 starts "command number" prompt ✅
79+
* Escape dismisses prompt ✅
80+
* Ignores non-ASCII-decimal characters ✅
81+
* Allows entering between 1 and 5 digits ✅
82+
* Pressing Enter fetches the given command from the history ✅
83+
* Alt+F10 clears doskey aliases ✅
84+
* In cmd.exe, with an empty prompt in an empty directory:
85+
Pressing tab produces an audible bing and prints no text ✅
86+
* When Narrator is enabled, in cmd.exe:
87+
* Typing individual characters announces only
88+
exactly each character that is being typed ✅
89+
* Backspacing at the end of a prompt announces
90+
only exactly each deleted character ✅

src/host/readDataCooked.cpp

+5-2
Original file line numberDiff line numberDiff line change
@@ -436,14 +436,17 @@ void COOKED_READ_DATA::_handleChar(wchar_t wch, const DWORD modifiers)
436436

437437
if (_ctrlWakeupMask != 0 && wch < L' ' && (_ctrlWakeupMask & (1 << wch)))
438438
{
439-
_flushBuffer();
440-
441439
// The old implementation (all the way since the 90s) overwrote the character at the current cursor position with the given wch.
442440
// But simultaneously it incremented the buffer length, which would have only worked if it was written at the end of the buffer.
443441
// 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).
444442
// 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.
445443
// I went with insert mode.
444+
//
445+
// It is important that we don't actually print that character out though, as it's only for the calling application to see.
446+
// That's why we flush the contents before the insertion and then ensure that the _flushBuffer() call in Read() exits early.
447+
_flushBuffer();
446448
_buffer.Replace(_buffer.GetCursorPosition(), 0, &wch, 1);
449+
_buffer.MarkAsClean();
447450

448451
_controlKeyState = modifiers;
449452
_transitionState(State::DoneWithWakeupMask);

0 commit comments

Comments
 (0)