Pin the DisplayName to Windows Terminal to fix the UAC prompt (#4995)

There's an issue in the UAC consent dialog where it cannot read an
application's name if it's stored in a resource. When it fails, it deems
us an "Unknown Program" and that looks pretty silly.

Fixes #2289.

(cherry picked from commit aaa4943112)

src/cascadia/CascadiaPackage/Package.appxmanifest

@@ -17,7 +17,7 @@
     Version="1.0.0.0" />
 
   <Properties>
-    <DisplayName>ms-resource:AppName</DisplayName>
+    <DisplayName>Windows Terminal</DisplayName>
     <PublisherDisplayName>Microsoft Corporation</PublisherDisplayName>
     <Logo>Images\StoreLogo.png</Logo>
   </Properties>

src/cascadia/TerminalApp/AppKeyBindingsSerialization.cpp

@@ -516,7 +516,7 @@ std::vector<::TerminalApp::SettingsLoadWarnings> winrt::TerminalApp::implementat
                 warnings.insert(warnings.end(), parseWarnings.begin(), parseWarnings.end());
 
                 // if an arg parser was registered, but failed, bail
-                if (args == nullptr)
+                if (pfn && args == nullptr)
                 {
                     continue;
                 }

src/cascadia/TerminalApp/Pane.cpp

@@ -803,10 +803,10 @@ void Pane::_CreateRowColDefinitions(const Size& rootSize)
         const auto paneSizes = _CalcChildrenSizes(rootSize.Width);
 
         auto firstColDef = Controls::ColumnDefinition();
-        firstColDef.Width(GridLengthHelper::FromPixels(paneSizes.first));
+        firstColDef.Width(GridLengthHelper::FromValueAndType(paneSizes.first, GridUnitType::Star));
 
         auto secondColDef = Controls::ColumnDefinition();
-        secondColDef.Width(GridLengthHelper::FromPixels(paneSizes.second));
+        secondColDef.Width(GridLengthHelper::FromValueAndType(paneSizes.second, GridUnitType::Star));
 
         _root.ColumnDefinitions().Append(firstColDef);
         _root.ColumnDefinitions().Append(secondColDef);
@@ -819,10 +819,10 @@ void Pane::_CreateRowColDefinitions(const Size& rootSize)
         const auto paneSizes = _CalcChildrenSizes(rootSize.Height);
 
         auto firstRowDef = Controls::RowDefinition();
-        firstRowDef.Height(GridLengthHelper::FromPixels(paneSizes.first));
+        firstRowDef.Height(GridLengthHelper::FromValueAndType(paneSizes.first, GridUnitType::Star));
 
         auto secondRowDef = Controls::RowDefinition();
-        secondRowDef.Height(GridLengthHelper::FromPixels(paneSizes.second));
+        secondRowDef.Height(GridLengthHelper::FromValueAndType(paneSizes.second, GridUnitType::Star));
 
         _root.RowDefinitions().Append(firstRowDef);
         _root.RowDefinitions().Append(secondRowDef);

src/cascadia/TerminalApp/WslDistroGenerator.cpp

@@ -63,13 +63,13 @@ std::vector<TerminalApp::Profile> WslDistroGenerator::GenerateProfiles()
                                              nullptr,
                                              &si,
                                              &pi));
-    switch (WaitForSingleObject(pi.hProcess, INFINITE))
+    switch (WaitForSingleObject(pi.hProcess, 2000))
     {
     case WAIT_OBJECT_0:
         break;
     case WAIT_ABANDONED:
     case WAIT_TIMEOUT:
-        THROW_HR(ERROR_CHILD_NOT_COMPLETE);
+        return profiles;
     case WAIT_FAILED:
         THROW_LAST_ERROR();
     default:

src/cascadia/TerminalControl/TermControl.cpp

@@ -724,6 +724,16 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
         const auto scanCode = gsl::narrow_cast<WORD>(e.KeyStatus().ScanCode);
         bool handled = false;
 
+        // Alt-Numpad# input will send us a character once the user releases Alt, so we should be ignoring the individual keydowns.
+        // The character will be sent through the TSFInputControl.
+        // See GH#1401 for more details
+        if (modifiers.IsAltPressed() && (e.OriginalKey() >= VirtualKey::NumberPad0 && e.OriginalKey() <= VirtualKey::NumberPad9))
+
+        {
+            e.Handled(true);
+            return;
+        }
+
         // GH#2235: Terminal::Settings hasn't been modified to differentiate between AltGr and Ctrl+Alt yet.
         // -> Don't check for key bindings if this is an AltGr key combination.
         if (!modifiers.IsAltGrPressed())

src/cascadia/TerminalCore/Terminal.cpp

@@ -173,15 +173,17 @@ void Terminal::UpdateSettings(winrt::Microsoft::Terminal::Settings::ICoreSetting
     {
         return S_FALSE;
     }
-    const auto dx = viewportSize.X - oldDimensions.X;
+
+    const auto dx = ::base::ClampSub(viewportSize.X, oldDimensions.X);
 
     const auto oldTop = _mutableViewport.Top();
 
-    const short newBufferHeight = viewportSize.Y + _scrollbackLines;
+    const short newBufferHeight = ::base::ClampAdd(viewportSize.Y, _scrollbackLines);
+
     COORD bufferSize{ viewportSize.X, newBufferHeight };
 
     // Save cursor's relative height versus the viewport
-    const short sCursorHeightInViewportBefore = _buffer->GetCursor().GetPosition().Y - _mutableViewport.Top();
+    const short sCursorHeightInViewportBefore = ::base::ClampSub(_buffer->GetCursor().GetPosition().Y, _mutableViewport.Top());
 
     // This will be used to determine where the viewport should be in the new buffer.
     const short oldViewportTop = _mutableViewport.Top();
@@ -266,7 +268,7 @@ void Terminal::UpdateSettings(winrt::Microsoft::Terminal::Settings::ICoreSetting
 
     const auto maxRow = std::max(newLastChar.Y, newCursorPos.Y);
 
-    const short proposedTopFromLastLine = ::base::saturated_cast<short>(maxRow - viewportSize.Y + 1);
+    const short proposedTopFromLastLine = ::base::ClampAdd(::base::ClampSub(maxRow, viewportSize.Y), 1);
     const short proposedTopFromScrollback = newViewportTop;
 
     short proposedTop = std::max(proposedTopFromLastLine,
@@ -294,7 +296,7 @@ void Terminal::UpdateSettings(winrt::Microsoft::Terminal::Settings::ICoreSetting
             {
                 try
                 {
-                    auto row = newTextBuffer->GetRowByOffset(::base::saturated_cast<short>(proposedTop - 1));
+                    auto row = newTextBuffer->GetRowByOffset(::base::ClampSub(proposedTop, 1));
                     if (row.GetCharRow().WasWrapForced())
                     {
                         proposedTop--;
@@ -324,7 +326,7 @@ void Terminal::UpdateSettings(winrt::Microsoft::Terminal::Settings::ICoreSetting
     const auto proposedBottom = newView.BottomExclusive();
     if (proposedBottom > bufferSize.Y)
     {
-        proposedTop = ::base::saturated_cast<short>(proposedTop - (proposedBottom - bufferSize.Y));
+        proposedTop = ::base::ClampSub(proposedTop, ::base::ClampSub(proposedBottom, bufferSize.Y));
     }
 
     _mutableViewport = Viewport::FromDimensions({ 0, proposedTop }, viewportSize);
@@ -339,7 +341,7 @@ void Terminal::UpdateSettings(winrt::Microsoft::Terminal::Settings::ICoreSetting
 
     // If the old scrolloffset was 0, then we weren't scrolled back at all
     // before, and shouldn't be now either.
-    _scrollOffset = originalOffsetWasZero ? 0 : _mutableViewport.Top() - newVisibleTop;
+    _scrollOffset = originalOffsetWasZero ? 0 : ::base::ClampSub(_mutableViewport.Top(), newVisibleTop);
     _NotifyScrollEvent();
 
     return S_OK;

src/cascadia/UnitTests_TerminalCore/ScreenSizeLimitsTest.cpp

@@ -11,6 +11,9 @@
 
 using namespace winrt::Microsoft::Terminal::Settings;
 using namespace Microsoft::Terminal::Core;
+using namespace WEX::Logging;
+using namespace WEX::TestExecution;
+using namespace WEX::Common;
 
 namespace TerminalCoreUnitTests
 {
@@ -21,66 +24,109 @@ namespace TerminalCoreUnitTests
     {
         TEST_CLASS(ScreenSizeLimitsTest);
 
-        TEST_METHOD(ScreenWidthAndHeightAreClampedToBounds)
-        {
-            DummyRenderTarget emptyRenderTarget;
+        TEST_METHOD(ScreenWidthAndHeightAreClampedToBounds);
+        TEST_METHOD(ScrollbackHistorySizeIsClampedToBounds);
 
-            // Negative values for initial visible row count or column count
-            // are clamped to 1. Too-large positive values are clamped to SHRT_MAX.
-            auto negativeColumnsSettings = winrt::make<MockTermSettings>(10000, 9999999, -1234);
-            Terminal negativeColumnsTerminal;
-            negativeColumnsTerminal.CreateFromSettings(negativeColumnsSettings, emptyRenderTarget);
-            COORD actualDimensions = negativeColumnsTerminal.GetViewport().Dimensions();
-            VERIFY_ARE_EQUAL(actualDimensions.Y, SHRT_MAX, L"Row count clamped to SHRT_MAX == " WCS(SHRT_MAX));
-            VERIFY_ARE_EQUAL(actualDimensions.X, 1, L"Column count clamped to 1");
-
-            // Zero values are clamped to 1 as well.
-            auto zeroRowsSettings = winrt::make<MockTermSettings>(10000, 0, 9999999);
-            Terminal zeroRowsTerminal;
-            zeroRowsTerminal.CreateFromSettings(zeroRowsSettings, emptyRenderTarget);
-            actualDimensions = zeroRowsTerminal.GetViewport().Dimensions();
-            VERIFY_ARE_EQUAL(actualDimensions.Y, 1, L"Row count clamped to 1");
-            VERIFY_ARE_EQUAL(actualDimensions.X, SHRT_MAX, L"Column count clamped to SHRT_MAX == " WCS(SHRT_MAX));
-        }
-
-        TEST_METHOD(ScrollbackHistorySizeIsClampedToBounds)
-        {
-            // What is actually clamped is the number of rows in the internal history buffer,
-            // which is the *sum* of the history size plus the number of rows
-            // actually visible on screen at the moment.
-
-            const unsigned int visibleRowCount = 100;
-            DummyRenderTarget emptyRenderTarget;
-
-            // Zero history size is acceptable.
-            auto noHistorySettings = winrt::make<MockTermSettings>(0, visibleRowCount, 100);
-            Terminal noHistoryTerminal;
-            noHistoryTerminal.CreateFromSettings(noHistorySettings, emptyRenderTarget);
-            VERIFY_ARE_EQUAL(noHistoryTerminal.GetTextBuffer().TotalRowCount(), visibleRowCount, L"History size of 0 is accepted");
-
-            // Negative history sizes are clamped to zero.
-            auto negativeHistorySizeSettings = winrt::make<MockTermSettings>(-100, visibleRowCount, 100);
-            Terminal negativeHistorySizeTerminal;
-            negativeHistorySizeTerminal.CreateFromSettings(negativeHistorySizeSettings, emptyRenderTarget);
-            VERIFY_ARE_EQUAL(negativeHistorySizeTerminal.GetTextBuffer().TotalRowCount(), visibleRowCount, L"Negative history size is clamped to 0");
-
-            // History size + initial visible rows == SHRT_MAX is acceptable.
-            auto maxHistorySizeSettings = winrt::make<MockTermSettings>(SHRT_MAX - visibleRowCount, visibleRowCount, 100);
-            Terminal maxHistorySizeTerminal;
-            maxHistorySizeTerminal.CreateFromSettings(maxHistorySizeSettings, emptyRenderTarget);
-            VERIFY_ARE_EQUAL(maxHistorySizeTerminal.GetTextBuffer().TotalRowCount(), static_cast<unsigned int>(SHRT_MAX), L"History size == SHRT_MAX - initial row count is accepted");
-
-            // History size + initial visible rows == SHRT_MAX + 1 will be clamped slightly.
-            auto justTooBigHistorySizeSettings = winrt::make<MockTermSettings>(SHRT_MAX - visibleRowCount + 1, visibleRowCount, 100);
-            Terminal justTooBigHistorySizeTerminal;
-            justTooBigHistorySizeTerminal.CreateFromSettings(justTooBigHistorySizeSettings, emptyRenderTarget);
-            VERIFY_ARE_EQUAL(justTooBigHistorySizeTerminal.GetTextBuffer().TotalRowCount(), static_cast<unsigned int>(SHRT_MAX), L"History size == 1 + SHRT_MAX - initial row count is clamped to SHRT_MAX - initial row count");
-
-            // Ridiculously large history sizes are also clamped.
-            auto farTooBigHistorySizeSettings = winrt::make<MockTermSettings>(99999999, visibleRowCount, 100);
-            Terminal farTooBigHistorySizeTerminal;
-            farTooBigHistorySizeTerminal.CreateFromSettings(farTooBigHistorySizeSettings, emptyRenderTarget);
-            VERIFY_ARE_EQUAL(farTooBigHistorySizeTerminal.GetTextBuffer().TotalRowCount(), static_cast<unsigned int>(SHRT_MAX), L"History size that is far too large is clamped to SHRT_MAX - initial row count");
-        }
+        TEST_METHOD(ResizeIsClampedToBounds);
     };
 }
+
+using namespace TerminalCoreUnitTests;
+
+void ScreenSizeLimitsTest::ScreenWidthAndHeightAreClampedToBounds()
+{
+    DummyRenderTarget emptyRenderTarget;
+
+    // Negative values for initial visible row count or column count
+    // are clamped to 1. Too-large positive values are clamped to SHRT_MAX.
+    auto negativeColumnsSettings = winrt::make<MockTermSettings>(10000, 9999999, -1234);
+    Terminal negativeColumnsTerminal;
+    negativeColumnsTerminal.CreateFromSettings(negativeColumnsSettings, emptyRenderTarget);
+    COORD actualDimensions = negativeColumnsTerminal.GetViewport().Dimensions();
+    VERIFY_ARE_EQUAL(actualDimensions.Y, SHRT_MAX, L"Row count clamped to SHRT_MAX == " WCS(SHRT_MAX));
+    VERIFY_ARE_EQUAL(actualDimensions.X, 1, L"Column count clamped to 1");
+
+    // Zero values are clamped to 1 as well.
+    auto zeroRowsSettings = winrt::make<MockTermSettings>(10000, 0, 9999999);
+    Terminal zeroRowsTerminal;
+    zeroRowsTerminal.CreateFromSettings(zeroRowsSettings, emptyRenderTarget);
+    actualDimensions = zeroRowsTerminal.GetViewport().Dimensions();
+    VERIFY_ARE_EQUAL(actualDimensions.Y, 1, L"Row count clamped to 1");
+    VERIFY_ARE_EQUAL(actualDimensions.X, SHRT_MAX, L"Column count clamped to SHRT_MAX == " WCS(SHRT_MAX));
+}
+
+void ScreenSizeLimitsTest::ScrollbackHistorySizeIsClampedToBounds()
+{
+    // What is actually clamped is the number of rows in the internal history buffer,
+    // which is the *sum* of the history size plus the number of rows
+    // actually visible on screen at the moment.
+
+    const unsigned int visibleRowCount = 100;
+    DummyRenderTarget emptyRenderTarget;
+
+    // Zero history size is acceptable.
+    auto noHistorySettings = winrt::make<MockTermSettings>(0, visibleRowCount, 100);
+    Terminal noHistoryTerminal;
+    noHistoryTerminal.CreateFromSettings(noHistorySettings, emptyRenderTarget);
+    VERIFY_ARE_EQUAL(noHistoryTerminal.GetTextBuffer().TotalRowCount(), visibleRowCount, L"History size of 0 is accepted");
+
+    // Negative history sizes are clamped to zero.
+    auto negativeHistorySizeSettings = winrt::make<MockTermSettings>(-100, visibleRowCount, 100);
+    Terminal negativeHistorySizeTerminal;
+    negativeHistorySizeTerminal.CreateFromSettings(negativeHistorySizeSettings, emptyRenderTarget);
+    VERIFY_ARE_EQUAL(negativeHistorySizeTerminal.GetTextBuffer().TotalRowCount(), visibleRowCount, L"Negative history size is clamped to 0");
+
+    // History size + initial visible rows == SHRT_MAX is acceptable.
+    auto maxHistorySizeSettings = winrt::make<MockTermSettings>(SHRT_MAX - visibleRowCount, visibleRowCount, 100);
+    Terminal maxHistorySizeTerminal;
+    maxHistorySizeTerminal.CreateFromSettings(maxHistorySizeSettings, emptyRenderTarget);
+    VERIFY_ARE_EQUAL(maxHistorySizeTerminal.GetTextBuffer().TotalRowCount(), static_cast<unsigned int>(SHRT_MAX), L"History size == SHRT_MAX - initial row count is accepted");
+
+    // History size + initial visible rows == SHRT_MAX + 1 will be clamped slightly.
+    auto justTooBigHistorySizeSettings = winrt::make<MockTermSettings>(SHRT_MAX - visibleRowCount + 1, visibleRowCount, 100);
+    Terminal justTooBigHistorySizeTerminal;
+    justTooBigHistorySizeTerminal.CreateFromSettings(justTooBigHistorySizeSettings, emptyRenderTarget);
+    VERIFY_ARE_EQUAL(justTooBigHistorySizeTerminal.GetTextBuffer().TotalRowCount(), static_cast<unsigned int>(SHRT_MAX), L"History size == 1 + SHRT_MAX - initial row count is clamped to SHRT_MAX - initial row count");
+
+    // Ridiculously large history sizes are also clamped.
+    auto farTooBigHistorySizeSettings = winrt::make<MockTermSettings>(99999999, visibleRowCount, 100);
+    Terminal farTooBigHistorySizeTerminal;
+    farTooBigHistorySizeTerminal.CreateFromSettings(farTooBigHistorySizeSettings, emptyRenderTarget);
+    VERIFY_ARE_EQUAL(farTooBigHistorySizeTerminal.GetTextBuffer().TotalRowCount(), static_cast<unsigned int>(SHRT_MAX), L"History size that is far too large is clamped to SHRT_MAX - initial row count");
+}
+
+void ScreenSizeLimitsTest::ResizeIsClampedToBounds()
+{
+    // What is actually clamped is the number of rows in the internal history buffer,
+    // which is the *sum* of the history size plus the number of rows
+    // actually visible on screen at the moment.
+    //
+    // This is a test for GH#2630, GH#2815.
+
+    const unsigned int initialVisibleColCount = 50;
+    const unsigned int initialVisibleRowCount = 50;
+    const auto historySize = SHRT_MAX - (initialVisibleRowCount * 2);
+    DummyRenderTarget emptyRenderTarget;
+
+    Log::Comment(L"Watch out - this test takes a while on debug, because "
+                 L"ResizeWithReflow takes a while on debug. This is expected.");
+
+    auto settings = winrt::make<MockTermSettings>(historySize, initialVisibleRowCount, initialVisibleColCount);
+    Log::Comment(L"First create a terminal with fewer than SHRT_MAX lines");
+    Terminal terminal;
+    terminal.CreateFromSettings(settings, emptyRenderTarget);
+    VERIFY_ARE_EQUAL(terminal.GetTextBuffer().TotalRowCount(), static_cast<unsigned int>(historySize + initialVisibleRowCount));
+
+    Log::Comment(L"Resize the terminal to have exactly SHRT_MAX lines");
+    VERIFY_SUCCEEDED(terminal.UserResize({ initialVisibleColCount, initialVisibleRowCount * 2 }));
+
+    VERIFY_ARE_EQUAL(terminal.GetTextBuffer().TotalRowCount(), static_cast<unsigned int>(SHRT_MAX));
+
+    Log::Comment(L"Resize the terminal to have MORE than SHRT_MAX lines - we should clamp to SHRT_MAX");
+    VERIFY_SUCCEEDED(terminal.UserResize({ initialVisibleColCount, initialVisibleRowCount * 3 }));
+    VERIFY_ARE_EQUAL(terminal.GetTextBuffer().TotalRowCount(), static_cast<unsigned int>(SHRT_MAX));
+
+    Log::Comment(L"Resize back down to the original size");
+    VERIFY_SUCCEEDED(terminal.UserResize({ initialVisibleColCount, initialVisibleRowCount }));
+    VERIFY_ARE_EQUAL(terminal.GetTextBuffer().TotalRowCount(), static_cast<unsigned int>(historySize + initialVisibleRowCount));
+}

src/host/ut_host/ConptyOutputTests.cpp

@@ -113,6 +113,7 @@ class ConptyOutputTests
     TEST_METHOD(SimpleWriteOutputTest);
     TEST_METHOD(WriteTwoLinesUsesNewline);
     TEST_METHOD(WriteAFewSimpleLines);
+    TEST_METHOD(InvalidateUntilOneBeforeEnd);
 
 private:
     bool _writeCallback(const char* const pch, size_t const cch);
@@ -124,10 +125,14 @@ private:
 
 bool ConptyOutputTests::_writeCallback(const char* const pch, size_t const cch)
 {
+    // Since rendering happens on a background thread that doesn't have the exception handler on it
+    // we need to rely on VERIFY's return codes instead of exceptions.
+    const WEX::TestExecution::DisableVerifyExceptions disableExceptionsScope;
+
     std::string actualString = std::string(pch, cch);
-    VERIFY_IS_GREATER_THAN(expectedOutput.size(),
-                           static_cast<size_t>(0),
-                           NoThrowString().Format(L"writing=\"%hs\", expecting %u strings", actualString.c_str(), expectedOutput.size()));
+    RETURN_BOOL_IF_FALSE(VERIFY_IS_GREATER_THAN(expectedOutput.size(),
+                                                static_cast<size_t>(0),
+                                                NoThrowString().Format(L"writing=\"%hs\", expecting %u strings", actualString.c_str(), expectedOutput.size())));
 
     std::string first = expectedOutput.front();
     expectedOutput.pop_front();
@@ -135,8 +140,8 @@ bool ConptyOutputTests::_writeCallback(const char* const pch, size_t const cch)
     Log::Comment(NoThrowString().Format(L"Expected =\t\"%hs\"", first.c_str()));
     Log::Comment(NoThrowString().Format(L"Actual =\t\"%hs\"", actualString.c_str()));
 
-    VERIFY_ARE_EQUAL(first.length(), cch);
-    VERIFY_ARE_EQUAL(first, actualString);
+    RETURN_BOOL_IF_FALSE(VERIFY_ARE_EQUAL(first.length(), cch));
+    RETURN_BOOL_IF_FALSE(VERIFY_ARE_EQUAL(first, actualString));
 
     return true;
 }
@@ -314,3 +319,55 @@ void ConptyOutputTests::WriteAFewSimpleLines()
 
     VERIFY_SUCCEEDED(renderer.PaintFrame());
 }
+
+void ConptyOutputTests::InvalidateUntilOneBeforeEnd()
+{
+    Log::Comment(NoThrowString().Format(
+        L"Make sure we don't use EL and wipe out the last column of text"));
+    VERIFY_IS_NOT_NULL(_pVtRenderEngine.get());
+
+    auto& g = ServiceLocator::LocateGlobals();
+    auto& renderer = *g.pRender;
+    auto& gci = g.getConsoleInformation();
+    auto& si = gci.GetActiveOutputBuffer();
+    auto& sm = si.GetStateMachine();
+    auto& tb = si.GetTextBuffer();
+
+    _flushFirstFrame();
+
+    // Move the cursor to width-15, draw 15 characters
+    sm.ProcessString(L"\x1b[1;66H");
+    sm.ProcessString(L"ABCDEFGHIJKLMNO");
+
+    {
+        auto iter = tb.GetCellDataAt({ 78, 0 });
+        VERIFY_ARE_EQUAL(L"N", (iter++)->Chars());
+        VERIFY_ARE_EQUAL(L"O", (iter++)->Chars());
+    }
+
+    expectedOutput.push_back("\x1b[65C");
+    expectedOutput.push_back("ABCDEFGHIJKLMNO");
+    expectedOutput.push_back("\x1b[1;80H"); // we move the cursor to the end of the line after paint
+
+    VERIFY_SUCCEEDED(renderer.PaintFrame());
+
+    // overstrike the first with X and the middle 8 with spaces
+    sm.ProcessString(L"\x1b[1;66H");
+    //                 ABCDEFGHIJKLMNO
+    sm.ProcessString(L"X             ");
+
+    {
+        auto iter = tb.GetCellDataAt({ 78, 0 });
+        VERIFY_ARE_EQUAL(L" ", (iter++)->Chars());
+        VERIFY_ARE_EQUAL(L"O", (iter++)->Chars());
+    }
+
+    expectedOutput.push_back("\x1b[1;66H");
+    expectedOutput.push_back("X"); // sequence optimizer should choose ECH here
+    expectedOutput.push_back("\x1b[13X");
+    expectedOutput.push_back("\x1b[13C");
+
+    expectedOutput.push_back("\x1b[?25h"); // we turn the cursor back on for good measure
+
+    VERIFY_SUCCEEDED(renderer.PaintFrame());
+}

src/renderer/vt/paint.cpp

@@ -529,7 +529,7 @@ using namespace Microsoft::Console::Types;
         //   before we need to print new text.
         _deferredCursorPos = { _lastText.X + sNumSpaces, _lastText.Y };
 
-        if (_deferredCursorPos.X < _lastViewport.RightInclusive())
+        if (_deferredCursorPos.X <= _lastViewport.RightInclusive())
         {
             RETURN_IF_FAILED(_EraseCharacter(sNumSpaces));
         }

src/terminal/adapter/ut_adapter/inputTest.cpp

@@ -45,6 +45,7 @@ public:
     TEST_METHOD(TerminalInputModifierKeyTests);
     TEST_METHOD(TerminalInputNullKeyTests);
     TEST_METHOD(DifferentModifiersTest);
+    TEST_METHOD(CtrlNumTest);
 
     wchar_t GetModifierChar(const bool fShift, const bool fAlt, const bool fCtrl)
     {
@@ -511,6 +512,13 @@ void InputTest::TerminalInputModifierKeyTests()
                 s_pwsInputBuffer[1] = wchShifted;
                 fExpectedKeyHandled = true;
             }
+            else if (ControlPressed(uiKeystate) && (vkey >= '1' && vkey <= '9'))
+            {
+                // The C-# keys get translated into very specific control
+                // characters that don't play nicely with this test. These keys
+                // are tested in the CtrlNumTest Test instead.
+                continue;
+            }
             else
             {
                 fExpectedKeyHandled = false;
@@ -684,4 +692,79 @@ void InputTest::DifferentModifiersTest()
     TestKey(pInput, uiKeystate, vkey, L'/');
     uiKeystate = RIGHT_ALT_PRESSED;
     TestKey(pInput, uiKeystate, vkey, L'/');
+
+    // See https://github.com/microsoft/terminal/pull/4947#issuecomment-600382856
+    // C-? -> DEL -> 0x7f
+    Log::Comment(NoThrowString().Format(L"Checking C-?"));
+    // Use SHIFT_PRESSED to force us into differentiating between '/' and '?'
+    vkey = LOBYTE(VkKeyScan(L'?'));
+    s_pwszInputExpected = L"\x7f";
+    TestKey(pInput, SHIFT_PRESSED | LEFT_CTRL_PRESSED, vkey, L'?');
+    TestKey(pInput, SHIFT_PRESSED | RIGHT_CTRL_PRESSED, vkey, L'?');
+
+    // C-M-/ -> 0x1b0x1f
+    Log::Comment(NoThrowString().Format(L"Checking C-M-/"));
+    uiKeystate = LEFT_CTRL_PRESSED | LEFT_ALT_PRESSED;
+    vkey = LOBYTE(VkKeyScan(L'/'));
+    s_pwszInputExpected = L"\x1b\x1f";
+    TestKey(pInput, LEFT_CTRL_PRESSED | LEFT_ALT_PRESSED, vkey, L'/');
+    TestKey(pInput, RIGHT_CTRL_PRESSED | LEFT_ALT_PRESSED, vkey, L'/');
+    // LEFT_CTRL_PRESSED | RIGHT_ALT_PRESSED is skipped because that's AltGr
+    TestKey(pInput, RIGHT_CTRL_PRESSED | RIGHT_ALT_PRESSED, vkey, L'/');
+
+    // C-M-? -> 0x1b0x7f
+    Log::Comment(NoThrowString().Format(L"Checking C-M-?"));
+    uiKeystate = LEFT_CTRL_PRESSED | LEFT_ALT_PRESSED;
+    vkey = LOBYTE(VkKeyScan(L'?'));
+    s_pwszInputExpected = L"\x1b\x7f";
+    TestKey(pInput, SHIFT_PRESSED | LEFT_CTRL_PRESSED | LEFT_ALT_PRESSED, vkey, L'?');
+    TestKey(pInput, SHIFT_PRESSED | RIGHT_CTRL_PRESSED | LEFT_ALT_PRESSED, vkey, L'?');
+    // LEFT_CTRL_PRESSED | RIGHT_ALT_PRESSED is skipped because that's AltGr
+    TestKey(pInput, SHIFT_PRESSED | RIGHT_CTRL_PRESSED | RIGHT_ALT_PRESSED, vkey, L'?');
+}
+
+void InputTest::CtrlNumTest()
+{
+    Log::Comment(L"Starting test...");
+
+    const TerminalInput* const pInput = new TerminalInput(s_TerminalInputTestCallback);
+
+    Log::Comment(L"Sending the various Ctrl+Num keys.");
+
+    unsigned int uiKeystate = LEFT_CTRL_PRESSED;
+    BYTE vkey = static_cast<WORD>('1');
+    s_pwszInputExpected = L"1";
+    TestKey(pInput, uiKeystate, vkey);
+
+    Log::Comment(NoThrowString().Format(
+        L"Skipping Ctrl+2, since that's supposed to send NUL, and doesn't play "
+        L"nicely with this test. Ctrl+2 is covered by other tests in this class."));
+
+    vkey = static_cast<WORD>('3');
+    s_pwszInputExpected = L"\x1b";
+    TestKey(pInput, uiKeystate, vkey);
+
+    vkey = static_cast<WORD>('4');
+    s_pwszInputExpected = L"\x1c";
+    TestKey(pInput, uiKeystate, vkey);
+
+    vkey = static_cast<WORD>('5');
+    s_pwszInputExpected = L"\x1d";
+    TestKey(pInput, uiKeystate, vkey);
+
+    vkey = static_cast<WORD>('6');
+    s_pwszInputExpected = L"\x1e";
+    TestKey(pInput, uiKeystate, vkey);
+
+    vkey = static_cast<WORD>('7');
+    s_pwszInputExpected = L"\x1f";
+    TestKey(pInput, uiKeystate, vkey);
+
+    vkey = static_cast<WORD>('8');
+    s_pwszInputExpected = L"\x7f";
+    TestKey(pInput, uiKeystate, vkey);
+
+    vkey = static_cast<WORD>('9');
+    s_pwszInputExpected = L"9";
+    TestKey(pInput, uiKeystate, vkey);
 }

src/terminal/input/terminalInput.cpp

@@ -185,19 +185,40 @@ static constexpr std::array<TermKeyMap, 22> s_modifierKeyMapping{
 // These sequences are not later updated to encode the modifier state in the
 //      sequence itself, they are just weird exceptional cases to the general
 //      rules above.
-static constexpr std::array<TermKeyMap, 6> s_simpleModifiedKeyMapping{
+static constexpr std::array<TermKeyMap, 14> s_simpleModifiedKeyMapping{
     TermKeyMap{ VK_BACK, CTRL_PRESSED, L"\x8" },
     TermKeyMap{ VK_BACK, ALT_PRESSED, L"\x1b\x7f" },
     TermKeyMap{ VK_BACK, CTRL_PRESSED | ALT_PRESSED, L"\x1b\x8" },
     TermKeyMap{ VK_TAB, CTRL_PRESSED, L"\t" },
     TermKeyMap{ VK_TAB, SHIFT_PRESSED, L"\x1b[Z" },
     TermKeyMap{ VK_DIVIDE, CTRL_PRESSED, L"\x1F" },
+
+    // GH#3507 - We should also be encoding Ctrl+# according to the following table:
+    // https://vt100.net/docs/vt220-rm/table3-5.html
+    // * 1 and 9 do not send any special characters, but they _should_ send
+    //   through the character unmodified.
+    // * 0 doesn't seem to send even an unmodified '0' through.
+    // * Ctrl+2 is already special-cased below in `HandleKey`, so it's not
+    //   included here.
+    TermKeyMap{ static_cast<WORD>('1'), CTRL_PRESSED, L"1" },
+    // TermKeyMap{ static_cast<WORD>('2'), CTRL_PRESSED, L"\x00" },
+    TermKeyMap{ static_cast<WORD>('3'), CTRL_PRESSED, L"\x1B" },
+    TermKeyMap{ static_cast<WORD>('4'), CTRL_PRESSED, L"\x1C" },
+    TermKeyMap{ static_cast<WORD>('5'), CTRL_PRESSED, L"\x1D" },
+    TermKeyMap{ static_cast<WORD>('6'), CTRL_PRESSED, L"\x1E" },
+    TermKeyMap{ static_cast<WORD>('7'), CTRL_PRESSED, L"\x1F" },
+    TermKeyMap{ static_cast<WORD>('8'), CTRL_PRESSED, L"\x7F" },
+    TermKeyMap{ static_cast<WORD>('9'), CTRL_PRESSED, L"9" },
+
     // These two are not implemented here, because they are system keys.
     // TermKeyMap{ VK_TAB, ALT_PRESSED, L""}, This is the Windows system shortcut for switching windows.
     // TermKeyMap{ VK_ESCAPE, ALT_PRESSED, L""}, This is another Windows system shortcut for switching windows.
 };
 
 const wchar_t* const CTRL_SLASH_SEQUENCE = L"\x1f";
+const wchar_t* const CTRL_QUESTIONMARK_SEQUENCE = L"\x7F";
+const wchar_t* const CTRL_ALT_SLASH_SEQUENCE = L"\x1b\x1f";
+const wchar_t* const CTRL_ALT_QUESTIONMARK_SEQUENCE = L"\x1b\x7F";
 
 void TerminalInput::ChangeKeypadMode(const bool applicationMode) noexcept
 {
@@ -323,13 +344,73 @@ static bool _searchWithModifier(const KeyEvent& keyEvent, InputSender sender)
         }
         else
         {
-            // One last check: C-/ is supposed to be C-_
-            // But '/' is not the same VKEY on all keyboards. So we have to
-            //      figure out the vkey at runtime.
-            const BYTE slashVkey = LOBYTE(VkKeyScan(L'/'));
-            if (keyEvent.GetVirtualKeyCode() == slashVkey && keyEvent.IsCtrlPressed())
+            // One last check:
+            // * C-/ is supposed to be ^_ (the C0 character US)
+            // * C-? is supposed to be DEL
+            // * C-M-/ is supposed to be ^[^_
+            // * C-M-? is supposed to be ^[^?
+            //
+            // But this whole scenario is tricky. '/' is not the same VKEY on
+            // all keyboards. On USASCII keyboards, '/' and '?' share the _same_
+            // key. So we have to figure out the vkey at runtime, and we have to
+            // determine if the key that was pressed was '?' with some
+            // modifiers, or '/' with some modifiers.
+            //
+            // These translations are not in s_simpleModifiedKeyMapping, because
+            // the aformentioned fact that they aren't the same VKEY on all
+            // keyboards.
+            //
+            // See GH#3079 for details.
+            // Also see https://github.com/microsoft/terminal/pull/4947#issuecomment-600382856
+
+            // VkKeyScan will give us both the Vkey of the key needed for this
+            // character, and the modifiers the user might need to press to get
+            // this character.
+            const auto slashKeyScan = VkKeyScan(L'/'); // On USASCII: 0x00bf
+            const auto questionMarkKeyScan = VkKeyScan(L'?'); //On USASCII: 0x01bf
+
+            const auto slashVkey = LOBYTE(slashKeyScan);
+            const auto questionMarkVkey = LOBYTE(questionMarkKeyScan);
+
+            const auto ctrl = keyEvent.IsCtrlPressed();
+            const auto alt = keyEvent.IsAltPressed();
+            const bool shift = keyEvent.IsShiftPressed();
+
+            // From the KeyEvent we're translating, synthesize the equivalent VkKeyScan result
+            const auto vkey = keyEvent.GetVirtualKeyCode();
+            const short keyScanFromEvent = vkey |
+                                           (shift ? 0x100 : 0) |
+                                           (ctrl ? 0x200 : 0) |
+                                           (alt ? 0x400 : 0);
+
+            // Make sure the VKEY is an _exact_ match, and that the modifier
+            // bits also match. This handles the hypothetical case we get a
+            // keyscan back that's ctrl+alt+some_random_VK, and some_random_VK
+            // has bits that are a superset of the bits set for question mark.
+            const bool wasQuestionMark = vkey == questionMarkVkey && WI_AreAllFlagsSet(keyScanFromEvent, questionMarkKeyScan);
+            const bool wasSlash = vkey == slashVkey && WI_AreAllFlagsSet(keyScanFromEvent, slashKeyScan);
+
+            // If the key pressed was exactly the ? key, then try to send the
+            // appropriate sequence for a modified '?'. Otherwise, check if this
+            // was a modified '/' keypress. These mappings don't need to be
+            // changed at all.
+            if ((ctrl && alt) && wasQuestionMark)
+            {
+                sender(CTRL_ALT_QUESTIONMARK_SEQUENCE);
+                success = true;
+            }
+            else if (ctrl && wasQuestionMark)
+            {
+                sender(CTRL_QUESTIONMARK_SEQUENCE);
+                success = true;
+            }
+            else if ((ctrl && alt) && wasSlash)
+            {
+                sender(CTRL_ALT_SLASH_SEQUENCE);
+                success = true;
+            }
+            else if (ctrl && wasSlash)
             {
-                // This mapping doesn't need to be changed at all.
                 sender(CTRL_SLASH_SEQUENCE);
                 success = true;
             }