Repository: microsoft/terminal (language: cpp)

Issue:
Scrolling in the Alternate Screen Buffer should send up/down keystrokes?
<!-- 
🚨🚨🚨🚨🚨🚨🚨🚨🚨🚨

I ACKNOWLEDGE THE FOLLOWING BEFORE PROCEEDING:
1. If I delete this entire template and go my own path, the core team may close my issue without further explanation or engagement.
2. If I list multiple bugs/concerns in this one issue, the core team may close my issue without further explanation or engagement.
3. If I write an issue that has many duplicates, the core team may close my issue without further explanation or engagement (and without necessarily spending time to find the exact duplicate ID number).
4. If I leave the title incomplete when filing the issue, the core team may close my issue without further explanation or engagement.
5. If I file something completely blank in the body, the core team may close my issue without further explanation or engagement.

All good? Then proceed!
-->

# Description of the new feature/enhancement

Many terminals will set different scrolling behavior within the Alternate Screen Buffer such that scrolling will send up/down key strokes. This seems to make more sense than scrolling through the main buffer's history.
xterm has this as an option (set by the 'alternatescroll' Xresource), VTE based terminals have this set as default behavior. 
Please consider adding this capability to Windows Terminal.

Depends on #381.


Discussion / hints:
FYI: Konsole supports this too. In all three of these emulators, you can toggle this behavior runtime using DECSET 1007.
I'd just like to add that the vintage console already does support this mode :)

I'd imagine that our mouse support will done in pieces, with basic mouse support coming first, and this being a follow-up. Once #376 and #545 are done, this would be the next logical follow-up task. 
> I'd just like to add that the vintage console already does support this mode :)

Note that there are issues with the current conhost implementation though. I just did a quick test in VIM (version 8.0.1453), and mouse wheel scrolling had no effect. There are two reasons for this:

1. VIM doesn't actually enable alternate scroll mode (private mode 1007), and for us it's disabled by default. I'm not sure whether we'd consider this a bug or not, since Xterm has the same default. However, a lot of other emulators do have it enabled by default, (or don't even have the option of turning it off), so that's maybe something worth considering.
2. Even if you enable the alternate scroll mode manually, it still doesn't work in VIM, because VIM turns on Cursor Keys Mode (`DECCKM`), and our alternate scroll implementation doesn't take that into account, so we are sending the wrong escape sequences. This is definitely a bug which we need to fix.
FYI, I've submitted a PR for the `DECCKM` problem mentioned in point 2 above. As for point 1, I think we were probably correct in leaving the scroll mode disabled by default as Xterm has done. I just discovered there is an option you can enable in VIM which will make it handle all the mouse scrolling itself. And if we were to enable the alternate scroll mode by default, we'd just end up breaking that functionality.
> FYI, I've submitted a PR for the DECCKM problem mentioned in point 2 above

Er... I'm no console guru, so I may have not totally understood the previous discussion...
@j4james, will your fix allow mouse scrolling within `less`?

> As for point 1, I think we were probably correct in leaving the scroll mode disabled by default as Xterm has done. I just discovered there is an option you can enable in VIM which will make it handle all the mouse scrolling itself. And if we were to enable the alternate scroll mode by default, we'd just end up breaking that functionality.

Are you sure about that? I just checked with Gnome Terminal (VTE based, I believe) and mintty. And they both behave correctly whether `set mouse=` or `set mouse=a`. i.e. when `set mouse=` they seem to send up/down key strokes, and when `set mouse=a` they seem to leave scroll handling to vim.

> Er... I'm no console guru, so I may have not totally understood the previous discussion...
> @j4james, will your fix allow mouse scrolling within `less`?

Yes, but only if you enable the alternate scrolling mode first (i.e. something like `printf "\e[?1007h"`). And note that this still only applies to conhost - it's not supported by Windows Terminal yet (that's why this issue is still open).
 
> Are you sure about that? I just checked with Gnome Terminal (VTE based, I believe) and mintty. And they both behave correctly whether `set mouse=` or `set mouse=a`. i.e. when `set mouse=` they seem to send up/down key strokes, and when `set mouse=a` they seem to leave scroll handling to vim.

I haven't tried Gnome Terminal, but Mintty definitely behaves weirdly for me with `set mouse=a`. It seems like it's generating up/down key strokes as well as passing the mouse events through, so some of the time the cursor moves and some of the time the screen scrolls.

That said, I don't feel strongly about what this defaults too, and maybe there is a way to have it enabled by default and still work well with applications that handle the scrolling themselves. But for now at least we're compatible with XTerm, and anyone that prefers it on by default can still trigger that manually.

Fix patch:
diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp
index 820686d322b..601ba834a49 100644
--- a/src/cascadia/TerminalControl/ControlCore.cpp
+++ b/src/cascadia/TerminalControl/ControlCore.cpp
@@ -1381,6 +1381,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     {
         return _terminal != nullptr && _terminal->IsTrackingMouseInput();
     }
+    bool ControlCore::ShouldSendAlternateScroll(const unsigned int uiButton,
+                                                const int32_t delta) const
+    {
+        return _terminal != nullptr && _terminal->ShouldSendAlternateScroll(uiButton, delta);
+    }
 
     Core::Point ControlCore::CursorPosition() const
     {
diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h
index a8cd204f1ef..926375286f3 100644
--- a/src/cascadia/TerminalControl/ControlCore.h
+++ b/src/cascadia/TerminalControl/ControlCore.h
@@ -138,6 +138,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         void CursorOn(const bool isCursorOn);
 
         bool IsVtMouseModeEnabled() const;
+        bool ShouldSendAlternateScroll(const unsigned int uiButton, const int32_t delta) const;
         Core::Point CursorPosition() const;
 
         bool HasSelection() const;
diff --git a/src/cascadia/TerminalControl/ControlInteractivity.cpp b/src/cascadia/TerminalControl/ControlInteractivity.cpp
index de992e1b379..015942e4ac1 100644
--- a/src/cascadia/TerminalControl/ControlInteractivity.cpp
+++ b/src/cascadia/TerminalControl/ControlInteractivity.cpp
@@ -409,8 +409,14 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     {
         const til::point terminalPosition = _getTerminalPosition(til::point{ pixelPosition });
 
-        // Short-circuit isReadOnly check to avoid warning dialog
-        if (!_core->IsInReadOnlyMode() && _canSendVTMouseInput(modifiers))
+        // Short-circuit isReadOnly check to avoid warning dialog.
+        //
+        // GH#3321: Alternate scroll mode is a special type of mouse input mode
+        // where the terminal sends arrow keys when the user mouse wheels, but
+        // the client app doesn't care for other mouse input. It's tracked
+        // separately from _canSendVTMouseInput.
+        if (!_core->IsInReadOnlyMode() &&
+            (_canSendVTMouseInput(modifiers) || _shouldSendAlternateScroll(modifiers, delta)))
         {
             // Most mouse event handlers call
             //      _trySendMouseEvent(point);
@@ -571,6 +577,17 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         return _core->IsVtMouseModeEnabled();
     }
 
+    bool ControlInteractivity::_shouldSendAlternateScroll(const ::Microsoft::Terminal::Core::ControlKeyStates modifiers, const int32_t delta)
+    {
+        // If the user is holding down Shift, suppress mouse events
+        // TODO GH#4875: disable/customize this functionality
+        if (modifiers.IsShiftPressed())
+        {
+            return false;
+        }
+        return _core->ShouldSendAlternateScroll(WM_MOUSEWHEEL, delta);
+    }
+
     // Method Description:
     // - Sets selection's end position to match supplied cursor position, e.g. while mouse dragging.
     // Arguments:
diff --git a/src/cascadia/TerminalControl/ControlInteractivity.h b/src/cascadia/TerminalControl/ControlInteractivity.h
index 59de34ae96b..200498a37ab 100644
--- a/src/cascadia/TerminalControl/ControlInteractivity.h
+++ b/src/cascadia/TerminalControl/ControlInteractivity.h
@@ -140,6 +140,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
         void _hyperlinkHandler(const std::wstring_view uri);
         bool _canSendVTMouseInput(const ::Microsoft::Terminal::Core::ControlKeyStates modifiers);
+        bool _shouldSendAlternateScroll(const ::Microsoft::Terminal::Core::ControlKeyStates modifiers, const int32_t delta);
 
         void _sendPastedTextToConnection(std::wstring_view wstr);
         til::point _getTerminalPosition(const til::point pixelPosition);
diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp
index 41951c72acf..6ebaa29eb59 100644
--- a/src/cascadia/TerminalCore/Terminal.cpp
+++ b/src/cascadia/TerminalCore/Terminal.cpp
@@ -536,6 +536,20 @@ bool Terminal::IsTrackingMouseInput() const noexcept
     return _terminalInput->IsTrackingMouseInput();
 }
 
+// Routine Description:
+// - Relays if we are in alternate scroll mode, a special type of mouse input
+//   mode where scrolling sends the arrow keypresses, but the app doesn't
+//   otherwise want mouse input.
+// Parameters:
+// - <none>
+// Return value:
+// - true, if we are tracking mouse input. False, otherwise
+bool Terminal::ShouldSendAlternateScroll(const unsigned int uiButton,
+                                         const int32_t delta) const noexcept
+{
+    return _terminalInput->ShouldSendAlternateScroll(uiButton, ::base::saturated_cast<short>(delta));
+}
+
 // Method Description:
 // - Given a coord, get the URI at that location
 // Arguments:
diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp
index 4fd6a20ded6..03690e63781 100644
--- a/src/cascadia/TerminalCore/Terminal.hpp
+++ b/src/cascadia/TerminalCore/Terminal.hpp
@@ -154,6 +154,7 @@ class Microsoft::Terminal::Core::Terminal final :
 
     void TrySnapOnInput() override;
     bool IsTrackingMouseInput() const noexcept;
+    bool ShouldSendAlternateScroll(const unsigned int uiButton, const int32_t delta) const noexcept;
 
     std::wstring GetHyperlinkAtPosition(const COORD position);
     uint16_t GetHyperlinkIdAtPosition(const COORD position);
diff --git a/src/cascadia/TerminalCore/TerminalApi.cpp b/src/cascadia/TerminalCore/TerminalApi.cpp
index 36fae0efd42..7550332c92c 100644
--- a/src/cascadia/TerminalCore/TerminalApi.cpp
+++ b/src/cascadia/TerminalCore/TerminalApi.cpp
@@ -630,6 +630,10 @@ void Terminal::UseAlternateScreenBuffer()
     // update all the hyperlinks on the screen
     _updateUrlDetection();
 
+    // GH#3321: Make sure we let the TerminalInput know that we switched
+    // buffers. This might affect how we interpret certain mouse events.
+    _terminalInput->UseAlternateScreenBuffer();
+
     // Update scrollbars
     _NotifyScrollEvent();
 
@@ -674,6 +678,10 @@ void Terminal::UseMainScreenBuffer()
     _mainBuffer->ClearPatternRecognizers();
     _updateUrlDetection();
 
+    // GH#3321: Make sure we let the TerminalInput know that we switched
+    // buffers. This might affect how we interpret certain mouse events.
+    _terminalInput->UseMainScreenBuffer();
+
     // Update scrollbars
     _NotifyScrollEvent();
 
diff --git a/src/terminal/input/mouseInput.cpp b/src/terminal/input/mouseInput.cpp
index 82d8c358303..cbfbd81282b 100644
--- a/src/terminal/input/mouseInput.cpp
+++ b/src/terminal/input/mouseInput.cpp
@@ -321,7 +321,7 @@ bool TerminalInput::HandleMouse(const COORD position,
             // on the wheel, accumulate delta until we hit the amount required to dispatch one
             // "line" worth of scroll.
             // Mark the event as "handled" if we would have otherwise emitted a scroll event.
-            return IsTrackingMouseInput() || _ShouldSendAlternateScroll(button, delta);
+            return IsTrackingMouseInput() || ShouldSendAlternateScroll(button, delta);
         }
 
         // We're ready to send this event through, but first we need to clear the accumulated;
@@ -330,7 +330,7 @@ bool TerminalInput::HandleMouse(const COORD position,
     }
 
     bool success = false;
-    if (_ShouldSendAlternateScroll(button, delta))
+    if (ShouldSendAlternateScroll(button, delta))
     {
         success = _SendAlternateScroll(delta);
     }
@@ -539,11 +539,12 @@ std::wstring TerminalInput::_GenerateSGRSequence(const COORD position,
 // - delta: The scroll wheel delta of the input event
 // Return value:
 // True iff the alternate buffer is active and alternate scroll mode is enabled and the event is a mouse wheel event.
-bool TerminalInput::_ShouldSendAlternateScroll(const unsigned int button, const short delta) const noexcept
+bool TerminalInput::ShouldSendAlternateScroll(const unsigned int button, const short delta) const noexcept
 {
-    return _mouseInputState.inAlternateBuffer &&
-           _inputMode.test(Mode::AlternateScroll) &&
-           (button == WM_MOUSEWHEEL || button == WM_MOUSEHWHEEL) && delta != 0;
+    const bool inAltBuffer{ _mouseInputState.inAlternateBuffer };
+    const bool inAltScroll{ _inputMode.test(Mode::AlternateScroll) };
+    const bool wasMouseWheel{ (button == WM_MOUSEWHEEL || button == WM_MOUSEHWHEEL) && delta != 0 };
+    return inAltBuffer && inAltScroll && wasMouseWheel;
 }
 
 // Routine Description:
diff --git a/src/terminal/input/terminalInput.hpp b/src/terminal/input/terminalInput.hpp
index 4221c40708f..6b18f5ebc90 100644
--- a/src/terminal/input/terminalInput.hpp
+++ b/src/terminal/input/terminalInput.hpp
@@ -73,6 +73,7 @@ namespace Microsoft::Console::VirtualTerminal
                          const MouseButtonState state);
 
         bool IsTrackingMouseInput() const noexcept;
+        bool ShouldSendAlternateScroll(const unsigned int button, const short delta) const noexcept;
 #pragma endregion
 
 #pragma region MouseInputState Management
@@ -127,7 +128,6 @@ namespace Microsoft::Console::VirtualTerminal
                                                  const short modifierKeyState,
                                                  const short delta);
 
-        bool _ShouldSendAlternateScroll(const unsigned int button, const short delta) const noexcept;
         bool _SendAlternateScroll(const short delta) const noexcept;
 
         static constexpr unsigned int s_GetPressedButton(const MouseButtonState state) noexcept;


Test patch: