From 646edc7f8f112ea96c6706e230887fcddce5c983 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 31 Oct 2023 14:25:41 +0100 Subject: [PATCH 1/5] AtlasEngine: Minor bug fixes (#16219) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit fixes 4 minor bugs: * Forgot to set the maximum swap chain latency. Without it, it defaults to up to 3 frames of latency. We don't need this, because our renderer is simple and fast and is expected to draw frames within <1ms. * ClearType treats the alpha channel as ignored, whereas custom shaders can manipulate the alpha channel freely. This meant that using both simultaneously would produce weird effects, like text having black background. We now force grayscale AA instead. * The builtin retro shader should not be effected by the previous point. * When the cbuffer is entirely unused in a custom shader, it has so far resulted in constant redraws. This happened because the D3D reflection `GetDesc` call will then return `E_FAIL` in this situation. The new code on the other hand will now assume that a failure to get the description is equal to the variable being unused. Closes #15960 ## Validation Steps Performed * A custom passthrough shader works with grayscale and ClearType AA while also changing the opacity with Ctrl+Shift+Scroll ✅ * Same for the builtin retro shader, but ClearType works ✅ * The passthrough shader doesn't result in constant redrawing ✅ (cherry picked from commit 0289cb043c3aac0f9199f667550d8811022e9bed) Service-Card-Id: 90915277 Service-Version: 1.19 --- src/renderer/atlas/AtlasEngine.api.cpp | 14 +++++++------- src/renderer/atlas/AtlasEngine.r.cpp | 4 +++- src/renderer/atlas/BackendD3D.cpp | 20 ++++++++++++-------- src/renderer/atlas/common.h | 2 +- 4 files changed, 23 insertions(+), 17 deletions(-) diff --git a/src/renderer/atlas/AtlasEngine.api.cpp b/src/renderer/atlas/AtlasEngine.api.cpp index 4459c97a8e5..c810dd115f6 100644 --- a/src/renderer/atlas/AtlasEngine.api.cpp +++ b/src/renderer/atlas/AtlasEngine.api.cpp @@ -490,20 +490,20 @@ void AtlasEngine::UpdateHyperlinkHoveredId(const uint16_t hoveredId) noexcept void AtlasEngine::_resolveTransparencySettings() noexcept { + // An opaque background allows us to use true "independent" flips. See AtlasEngine::_createSwapChain(). + // We can't enable them if custom shaders are specified, because it's unknown, whether they support opaque inputs. + const bool useAlpha = _api.enableTransparentBackground || !_api.s->misc->customPixelShaderPath.empty(); // If the user asks for ClearType, but also for a transparent background // (which our ClearType shader doesn't simultaneously support) // then we need to sneakily force the renderer to grayscale AA. - const auto antialiasingMode = _api.enableTransparentBackground && _api.antialiasingMode == AntialiasingMode::ClearType ? AntialiasingMode::Grayscale : _api.antialiasingMode; - const bool enableTransparentBackground = _api.enableTransparentBackground || !_api.s->misc->customPixelShaderPath.empty() || _api.s->misc->useRetroTerminalEffect; + const auto antialiasingMode = useAlpha && _api.antialiasingMode == AntialiasingMode::ClearType ? AntialiasingMode::Grayscale : _api.antialiasingMode; - if (antialiasingMode != _api.s->font->antialiasingMode || enableTransparentBackground != _api.s->target->enableTransparentBackground) + if (antialiasingMode != _api.s->font->antialiasingMode || useAlpha != _api.s->target->useAlpha) { const auto s = _api.s.write(); s->font.write()->antialiasingMode = antialiasingMode; - // An opaque background allows us to use true "independent" flips. See AtlasEngine::_createSwapChain(). - // We can't enable them if custom shaders are specified, because it's unknown, whether they support opaque inputs. - s->target.write()->enableTransparentBackground = enableTransparentBackground; - _api.backgroundOpaqueMixin = enableTransparentBackground ? 0x00000000 : 0xff000000; + s->target.write()->useAlpha = useAlpha; + _api.backgroundOpaqueMixin = useAlpha ? 0x00000000 : 0xff000000; } } diff --git a/src/renderer/atlas/AtlasEngine.r.cpp b/src/renderer/atlas/AtlasEngine.r.cpp index e7070b708e6..1fac8f0e834 100644 --- a/src/renderer/atlas/AtlasEngine.r.cpp +++ b/src/renderer/atlas/AtlasEngine.r.cpp @@ -329,7 +329,7 @@ void AtlasEngine::_createSwapChain() .SwapEffect = DXGI_SWAP_EFFECT_FLIP_SEQUENTIAL, // If our background is opaque we can enable "independent" flips by setting DXGI_ALPHA_MODE_IGNORE. // As our swap chain won't have to compose with DWM anymore it reduces the display latency dramatically. - .AlphaMode = _p.s->target->enableTransparentBackground ? DXGI_ALPHA_MODE_PREMULTIPLIED : DXGI_ALPHA_MODE_IGNORE, + .AlphaMode = _p.s->target->useAlpha ? DXGI_ALPHA_MODE_PREMULTIPLIED : DXGI_ALPHA_MODE_IGNORE, .Flags = swapChainFlags, }; @@ -360,6 +360,8 @@ void AtlasEngine::_createSwapChain() _p.swapChain.targetSize = _p.s->targetSize; _p.swapChain.waitForPresentation = true; + LOG_IF_FAILED(_p.swapChain.swapChain->SetMaximumFrameLatency(1)); + WaitUntilCanRender(); if (_p.swapChainChangedCallback) diff --git a/src/renderer/atlas/BackendD3D.cpp b/src/renderer/atlas/BackendD3D.cpp index 846567ca613..19334b3a9d4 100644 --- a/src/renderer/atlas/BackendD3D.cpp +++ b/src/renderer/atlas/BackendD3D.cpp @@ -403,23 +403,24 @@ void BackendD3D::_recreateCustomShader(const RenderingPayload& p) /* ppCode */ blob.addressof(), /* ppErrorMsgs */ error.addressof()); - // Unless we can determine otherwise, assume this shader requires evaluation every frame - _requiresContinuousRedraw = true; - if (SUCCEEDED(hr)) { - THROW_IF_FAILED(p.device->CreatePixelShader(blob->GetBufferPointer(), blob->GetBufferSize(), nullptr, _customPixelShader.put())); + THROW_IF_FAILED(p.device->CreatePixelShader(blob->GetBufferPointer(), blob->GetBufferSize(), nullptr, _customPixelShader.addressof())); // Try to determine whether the shader uses the Time variable wil::com_ptr reflector; - if (SUCCEEDED_LOG(D3DReflect(blob->GetBufferPointer(), blob->GetBufferSize(), IID_PPV_ARGS(reflector.put())))) + if (SUCCEEDED_LOG(D3DReflect(blob->GetBufferPointer(), blob->GetBufferSize(), IID_PPV_ARGS(reflector.addressof())))) { + // Depending on the version of the d3dcompiler_*.dll, the next two functions either return nullptr + // on failure or an instance of CInvalidSRConstantBuffer or CInvalidSRVariable respectively, + // which cause GetDesc() to return E_FAIL. In other words, we have to assume that any failure in the + // next few lines indicates that the cbuffer is entirely unused (--> _requiresContinuousRedraw=false). if (ID3D11ShaderReflectionConstantBuffer* constantBufferReflector = reflector->GetConstantBufferByIndex(0)) // shader buffer { if (ID3D11ShaderReflectionVariable* variableReflector = constantBufferReflector->GetVariableByIndex(0)) // time { D3D11_SHADER_VARIABLE_DESC variableDescriptor; - if (SUCCEEDED_LOG(variableReflector->GetDesc(&variableDescriptor))) + if (SUCCEEDED(variableReflector->GetDesc(&variableDescriptor))) { // only if time is used _requiresContinuousRedraw = WI_IsFlagSet(variableDescriptor.uFlags, D3D_SVF_USED); @@ -427,6 +428,11 @@ void BackendD3D::_recreateCustomShader(const RenderingPayload& p) } } } + else + { + // Unless we can determine otherwise, assume this shader requires evaluation every frame + _requiresContinuousRedraw = true; + } } else { @@ -447,8 +453,6 @@ void BackendD3D::_recreateCustomShader(const RenderingPayload& p) else if (p.s->misc->useRetroTerminalEffect) { THROW_IF_FAILED(p.device->CreatePixelShader(&custom_shader_ps[0], sizeof(custom_shader_ps), nullptr, _customPixelShader.put())); - // We know the built-in retro shader doesn't require continuous redraw. - _requiresContinuousRedraw = false; } if (_customPixelShader) diff --git a/src/renderer/atlas/common.h b/src/renderer/atlas/common.h index bb46ff61547..94434bffc9c 100644 --- a/src/renderer/atlas/common.h +++ b/src/renderer/atlas/common.h @@ -313,7 +313,7 @@ namespace Microsoft::Console::Render::Atlas struct TargetSettings { HWND hwnd = nullptr; - bool enableTransparentBackground = false; + bool useAlpha = false; bool useSoftwareRendering = false; }; From a608a9157160cf1b57c4d3eb486f5cb85e42bae9 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Mon, 6 Nov 2023 22:30:03 +0100 Subject: [PATCH 2/5] Fix the fix for the fix of nearby font loading (#16196) I still don't know how to reproduce it properly, but I'm slowly wrapping my head around how and why it happens. The issue isn't that `FindFamilyName` fails with `exists=FALSE`, but rather that any of the followup calls like `GetDesignGlyphMetrics` fails, which results in an exception and subsequently in an orderly fallback to Consolas. I've always thought that the issue is that even with the nearby font collection we get an `exists=FALSE`... I'm not sure why I thought that. This changeset also drops the fallback iteration for Lucida Console and Courier New, because I felt like the code looks neater that way and I think it's a reasonable expectation that Consolas is always installed. Closes #16058 (cherry picked from commit 9e86c9811f2cf0ad53c9c543b886274b1079dd00) Service-Card-Id: 90885607 Service-Version: 1.19 --- src/renderer/atlas/AtlasEngine.api.cpp | 65 ++++++++++++-------------- src/renderer/atlas/common.h | 1 - src/renderer/dx/DxFontInfo.cpp | 18 +++---- 3 files changed, 40 insertions(+), 44 deletions(-) diff --git a/src/renderer/atlas/AtlasEngine.api.cpp b/src/renderer/atlas/AtlasEngine.api.cpp index c810dd115f6..0e2f7941688 100644 --- a/src/renderer/atlas/AtlasEngine.api.cpp +++ b/src/renderer/atlas/AtlasEngine.api.cpp @@ -455,30 +455,34 @@ void AtlasEngine::SetWarningCallback(std::function pfn) noexcept [[nodiscard]] HRESULT AtlasEngine::UpdateFont(const FontInfoDesired& fontInfoDesired, FontInfo& fontInfo, const std::unordered_map& features, const std::unordered_map& axes) noexcept { - static constexpr std::array fallbackFaceNames{ static_cast(nullptr), L"Consolas", L"Lucida Console", L"Courier New" }; - auto it = fallbackFaceNames.begin(); - const auto end = fallbackFaceNames.end(); + try + { + _updateFont(fontInfoDesired.GetFaceName().c_str(), fontInfoDesired, fontInfo, features, axes); + return S_OK; + } + CATCH_LOG(); - for (;;) + if constexpr (Feature_NearbyFontLoading::IsEnabled()) { try { - _updateFont(*it, fontInfoDesired, fontInfo, features, axes); + // _resolveFontMetrics() checks `_api.s->font->fontCollection` for a pre-existing font collection, + // before falling back to using the system font collection. This way we can inject our custom one. See GH#9375. + // Doing it this way is a bit hacky, but it does have the benefit that we can cache a font collection + // instance across font changes, like when zooming the font size rapidly using the scroll wheel. + _api.s.write()->font.write()->fontCollection = FontCache::GetCached(); + _updateFont(fontInfoDesired.GetFaceName().c_str(), fontInfoDesired, fontInfo, features, axes); return S_OK; } - catch (...) - { - ++it; - if (it == end) - { - RETURN_CAUGHT_EXCEPTION(); - } - else - { - LOG_CAUGHT_EXCEPTION(); - } - } + CATCH_LOG(); } + + try + { + _updateFont(nullptr, fontInfoDesired, fontInfo, features, axes); + return S_OK; + } + CATCH_RETURN(); } void AtlasEngine::UpdateHyperlinkHoveredId(const uint16_t hoveredId) noexcept @@ -598,11 +602,7 @@ void AtlasEngine::_resolveFontMetrics(const wchar_t* requestedFaceName, const Fo if (!requestedFaceName) { - requestedFaceName = fontInfoDesired.GetFaceName().c_str(); - if (!requestedFaceName) - { - requestedFaceName = L"Consolas"; - } + requestedFaceName = L"Consolas"; } if (!requestedSize.height) { @@ -614,22 +614,19 @@ void AtlasEngine::_resolveFontMetrics(const wchar_t* requestedFaceName, const Fo requestedWeight = DWRITE_FONT_WEIGHT_NORMAL; } - wil::com_ptr fontCollection; - THROW_IF_FAILED(_p.dwriteFactory->GetSystemFontCollection(fontCollection.addressof(), FALSE)); + // UpdateFont() (and its NearbyFontLoading feature path specifically) sets `_api.s->font->fontCollection` + // to a custom font collection that includes .ttf files that are bundled with our app package. See GH#9375. + // Doing it this way is a bit hacky, but it does have the benefit that we can cache a font collection + // instance across font changes, like when zooming the font size rapidly using the scroll wheel. + auto fontCollection = _api.s->font->fontCollection; + if (!fontCollection) + { + THROW_IF_FAILED(_p.dwriteFactory->GetSystemFontCollection(fontCollection.addressof(), FALSE)); + } u32 index = 0; BOOL exists = false; THROW_IF_FAILED(fontCollection->FindFamilyName(requestedFaceName, &index, &exists)); - - if constexpr (Feature_NearbyFontLoading::IsEnabled()) - { - if (!exists) - { - fontCollection = FontCache::GetCached(); - THROW_IF_FAILED(fontCollection->FindFamilyName(requestedFaceName, &index, &exists)); - } - } - THROW_HR_IF(DWRITE_E_NOFONT, !exists); wil::com_ptr fontFamily; diff --git a/src/renderer/atlas/common.h b/src/renderer/atlas/common.h index 94434bffc9c..b8fa3ac0d59 100644 --- a/src/renderer/atlas/common.h +++ b/src/renderer/atlas/common.h @@ -470,7 +470,6 @@ namespace Microsoft::Console::Render::Atlas wil::com_ptr systemFontFallback; wil::com_ptr systemFontFallback1; // optional, might be nullptr wil::com_ptr textAnalyzer; - wil::com_ptr renderingParams; std::function warningCallback; std::function swapChainChangedCallback; diff --git a/src/renderer/dx/DxFontInfo.cpp b/src/renderer/dx/DxFontInfo.cpp index 4945939fd86..b1d4844a601 100644 --- a/src/renderer/dx/DxFontInfo.cpp +++ b/src/renderer/dx/DxFontInfo.cpp @@ -127,15 +127,6 @@ void DxFontInfo::SetFromEngine(const std::wstring_view familyName, { face = _FindFontFace(localeName); - if constexpr (Feature_NearbyFontLoading::IsEnabled()) - { - if (!face) - { - _fontCollection = FontCache::GetCached(); - face = _FindFontFace(localeName); - } - } - if (!face) { // If we missed, try looking a little more by trimming the last word off the requested family name a few times. @@ -167,6 +158,15 @@ void DxFontInfo::SetFromEngine(const std::wstring_view familyName, } CATCH_LOG(); + if constexpr (Feature_NearbyFontLoading::IsEnabled()) + { + if (!face) + { + _fontCollection = FontCache::GetCached(); + face = _FindFontFace(localeName); + } + } + // Alright, if our quick shot at trimming didn't work either... // move onto looking up a font from our hard-coded list of fonts // that should really always be available. From 95e4d0bbe0c633859a0ba288d060ed22d917f64a Mon Sep 17 00:00:00 2001 From: Taha Haksal <91548254+TahaHaksal@users.noreply.github.com> Date: Tue, 7 Nov 2023 01:42:56 +0300 Subject: [PATCH 3/5] Added selectionBackground to light color schemes (#16243) Add a selectionBackground property which is set to the scheme's brightBlack too all 3 of the light color schemes. Related to #8716 It does not close the bug because as mentioned in the issue, when you input numbers, they seem to be invisible in the light color schemes and selecting them with the cursor doesn't reveal them. (cherry picked from commit a5c269b2801a42f833f7a611ab7c624648f37054) Service-Card-Id: 91033167 Service-Version: 1.19 --- src/cascadia/TerminalSettingsModel/defaults.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/cascadia/TerminalSettingsModel/defaults.json b/src/cascadia/TerminalSettingsModel/defaults.json index cdd5d6e78dd..360f37afaad 100644 --- a/src/cascadia/TerminalSettingsModel/defaults.json +++ b/src/cascadia/TerminalSettingsModel/defaults.json @@ -174,6 +174,7 @@ "foreground": "#383A42", "background": "#FAFAFA", "cursorColor": "#4F525D", + "selectionBackground": "#4F525D", "black": "#383A42", "red": "#E45649", "green": "#50A14F", @@ -218,6 +219,7 @@ "foreground": "#657B83", "background": "#FDF6E3", "cursorColor": "#002B36", + "selectionBackground": "#073642", "black": "#002B36", "red": "#DC322F", "green": "#859900", @@ -262,6 +264,7 @@ "foreground": "#555753", "background": "#FFFFFF", "cursorColor": "#000000", + "selectionBackground": "#555753", "black": "#000000", "red": "#CC0000", "green": "#4E9A06", From 83a2bc181a154e744b8b2a76bc1d7301a8d07917 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 6 Nov 2023 16:45:24 -0600 Subject: [PATCH 4/5] Another theoretical fix for a crash (#16267) For history: > This is MSFT:46763065 internally. Dumps show this repros on 1.19 too. > > This was previously #16061 which had a theoretical fix in #16065. Looks like you're on Terminal Stable v1.18.2822.0, and https://github.com/microsoft/terminal/releases/tag/v1.18.2822.0 is supposed to have had that fix in it. Dang. > well this is embarrassing ... I never actually checked if we _still had a `_window`_. We're alive, yay! But we're still in the middle of refrigerating. So, there's no HWND anymore Attempt to fix this by actually ensuring there's a `_window` in `AppHost::_WindowInitializedHandler` Closes #16235 (cherry picked from commit 59dcbbe0e94814b94a7bfc5ae189d36c4bb0436f) Service-Card-Id: 91041359 Service-Version: 1.19 --- src/cascadia/WindowsTerminal/AppHost.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp index 5af3a80d50a..7cae9959998 100644 --- a/src/cascadia/WindowsTerminal/AppHost.cpp +++ b/src/cascadia/WindowsTerminal/AppHost.cpp @@ -979,7 +979,8 @@ winrt::Windows::Foundation::IAsyncOperation AppHost::_GetWindowL co_await wil::resume_foreground(_windowLogic.GetRoot().Dispatcher()); const auto strongThis = weakThis.lock(); - if (!strongThis) + // GH #16235: If we don't have a window logic, we're already refrigerating, and won't have our _window either. + if (!strongThis || _windowLogic == nullptr) { co_return layoutJson; } @@ -1267,7 +1268,8 @@ winrt::fire_and_forget AppHost::_QuitRequested(const winrt::Windows::Foundation: co_await wil::resume_foreground(_windowLogic.GetRoot().Dispatcher()); const auto strongThis = weakThis.lock(); - if (!strongThis) + // GH #16235: If we don't have a window logic, we're already refrigerating, and won't have our _window either. + if (!strongThis || _windowLogic == nullptr) { co_return; } @@ -1431,7 +1433,7 @@ winrt::fire_and_forget AppHost::_WindowInitializedHandler(const winrt::Windows:: // If we're gone on the other side of this co_await, well, that's fine. Just bail. const auto strongThis = weakThis.lock(); - if (!strongThis) + if (!strongThis || _window == nullptr) { co_return; } From db27348e27e9989eaae75c61517aa52921390c12 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 7 Nov 2023 18:51:13 +0100 Subject: [PATCH 5/5] 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 ✅ (cherry picked from commit 7a8dd90294e53b9d5466ff755611d13f4249ce0b) Service-Card-Id: 91033502 Service-Version: 1.19 --- 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);