fix(terminal): route right-click to the app while mouse reporting is active (#347)

A TUI that turns mouse reporting on (vim with `set mouse=a`, lazygit,
tmux, …) draws its own right-button menus, and tty7 was delivering one
right-click to both consumers: `TerminalElement::register_mouse_handlers`
forwarded the press to the application, while the `.context_menu(…)` on
the terminal surface was attached unconditionally and popped tty7's own
menu over the top of it.

Gate the host menu on a single pure predicate, `should_show_context_menu`,
and call it from both sides so one click can only ever feed one consumer:
while reporting is active the unmodified right-click is the application's
alone, and Shift stays the escape hatch that reaches tty7 — the same
override Shift already provides for selection and for the wheel.

gpui-component's `ContextMenu` element owns the right mouse-down that
opens the popup: it wraps the terminal surface, so its listener fires
before any handler we can attach, and the builder closure it calls gets
no event to inspect. The verdict is therefore latched on our own right
mouse-down (safe, because the builder runs from a `window.defer` that
lands after the whole mouse dispatch has unwound) and a suppressed menu
is expressed as an item-less `PopupMenu`, which that element already
skips rendering.

Co-authored-by: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com>

src/terminal/element.rs

@@ -15,7 +15,7 @@ use gpui::{
 };
 use gpui_component::ActiveTheme as _;
 
-use super::view::TerminalView;
+use super::view::{TerminalView, should_show_context_menu};
 use crate::core::config::Config;
 
 const DIM_OPACITY: f32 = 0.66;
@@ -1268,7 +1268,10 @@ impl TerminalElement {
                 {
                     return;
                 }
-                if v.mouse_mode() && !mods.shift {
+                // The mirror image of the context-menu gate in
+                // `TerminalView::render`: a click the application gets is never
+                // also a click tty7 acts on, and vice versa.
+                if !should_show_context_menu(v.mouse_mode(), mods.shift) {
                     v.mouse_press(button, col, row, &mods);
                     return;
                 }

src/terminal/view.rs

@@ -122,6 +122,11 @@ pub struct TerminalView {
     last_mouse_cell: Option<(usize, usize)>,
     last_hover_cell: Option<(usize, usize)>,
     link_modifier_down: bool,
+    /// The verdict of `should_show_context_menu` for the most recent right
+    /// mouse-down, latched so the menu builder — which gpui-component runs on a
+    /// deferred callback, one turn after the click — can still see the
+    /// modifiers the user actually held.
+    context_menu_allowed: bool,
     scroll_debt: f32,
     pub(super) scroll_frac: f32,
     pub search: Option<SearchState>,
@@ -846,6 +851,7 @@ impl TerminalView {
             report_mouse,
             last_hover_cell: None,
             link_modifier_down: false,
+            context_menu_allowed: true,
             scroll_debt: 0.,
             scroll_frac: 0.,
             search: None,
@@ -4716,6 +4722,20 @@ impl Render for TerminalView {
                     window.focus(&this.focus_handle, cx);
                 }),
             )
+            // Latch the context-menu verdict for this click. gpui-component's
+            // `ContextMenu` element owns the right mouse-down that opens the
+            // popup and hands the builder no event to inspect, so the modifiers
+            // have to be recorded here. That element wraps this one, so its
+            // listener actually fires *before* this one — harmless, because it
+            // only builds the menu from a `window.defer` callback that runs
+            // once the whole mouse dispatch has unwound.
+            .on_mouse_down(
+                MouseButton::Right,
+                cx.listener(|this, ev: &MouseDownEvent, _window, _cx| {
+                    this.context_menu_allowed =
+                        should_show_context_menu(this.mouse_mode(), ev.modifiers.shift);
+                }),
+            )
             .drag_over::<ExternalPaths>(|s, _, _, cx| s.bg(cx.theme().drag_border.opacity(0.12)))
             .on_drop(cx.listener(|this, paths: &ExternalPaths, window, cx| {
                 window.focus(&this.focus_handle, cx);
@@ -4760,6 +4780,15 @@ impl Render for TerminalView {
             .children(reverse_search_menu)
             .children(integration_notice)
             .context_menu(move |menu, window, cx| {
+                // Suppressing the popup means handing back an item-less menu:
+                // gpui-component's `ContextMenu` element skips rendering the
+                // anchored overlay entirely when the built menu `is_empty()`,
+                // and its right mouse-down listener is registered after ours
+                // (it wraps this element), so we cannot out-order it or veto it
+                // any earlier. See `should_show_context_menu` for the rule.
+                if !menu_view.read(cx).context_menu_allowed {
+                    return menu;
+                }
                 let menu = menu
                     .min_w(px(220.))
                     .action_context(menu_focus.clone())
@@ -4914,6 +4943,24 @@ enum SelectEndCopy {
     Editor,
 }
 
+/// Whether a right-click belongs to tty7's own context menu rather than to the
+/// terminal application.
+///
+/// A TUI that has turned mouse reporting on (vim with `set mouse=a`, lazygit,
+/// tmux, …) draws its own right-button menus, so popping the host menu on top
+/// of the application's is a double delivery of one click — #251. While
+/// reporting is active the unmodified click is the application's alone; Shift
+/// is the escape hatch that reaches tty7, matching how Shift already overrides
+/// mouse reporting for selection (`register_mouse_handlers`) and for the wheel
+/// (`wheel_route`).
+///
+/// This is the exact complement of the "forward the press to the app" branch in
+/// `TerminalElement::register_mouse_handlers`, which is why both call it: one
+/// click must never feed both consumers.
+pub(super) fn should_show_context_menu(mouse_mode: bool, shift: bool) -> bool {
+    !mouse_mode || shift
+}
+
 fn select_end_copy(enabled: bool, grid: bool, editor: bool) -> SelectEndCopy {
     match (enabled, grid, editor) {
         (false, ..) => SelectEndCopy::None,
@@ -5264,7 +5311,8 @@ mod tests {
         drag_scroll_step, encode_mouse, escape_candidate, expand_file_command_template,
         fallback_chain, fig_icon_emoji, fig_icon_glyph, focus_report_bytes, input_overflow_shift,
         input_overlay_rows, menu_layout, paste_bytes, select_end_copy, shell_escape_path,
-        smooth_scroll_step, submit_bytes, trim_trailing_spaces, wheel_route, wrapped_click_index,
+        should_show_context_menu, smooth_scroll_step, submit_bytes, trim_trailing_spaces,
+        wheel_route, wrapped_click_index,
     };
     #[cfg(not(target_os = "macos"))]
     use super::{remote_paste_spec, staging_cache, staging_dir_is_safe};
@@ -5791,6 +5839,47 @@ mod tests {
         assert_eq!(wheel_route(everything, true, false), WheelRoute::Scrollback);
     }
 
+    #[test]
+    fn right_click_opens_the_host_menu_when_the_app_is_not_reporting_mouse() {
+        assert!(should_show_context_menu(false, false));
+        assert!(should_show_context_menu(false, true));
+    }
+
+    #[test]
+    fn right_click_belongs_to_the_app_while_mouse_reporting_is_active() {
+        assert!(
+            !should_show_context_menu(true, false),
+            "an unmodified right-click in mouse mode is the application's event, \
+             so tty7 must not also pop its own menu over it"
+        );
+    }
+
+    #[test]
+    fn shift_right_click_stays_the_escape_hatch_in_mouse_mode() {
+        assert!(
+            should_show_context_menu(true, true),
+            "Shift is the documented way to reach tty7's own menu inside a \
+             mouse-reporting TUI"
+        );
+    }
+
+    #[test]
+    fn context_menu_and_mouse_report_never_fire_for_the_same_click() {
+        // The render-time menu gate and the mouse-down forwarder in
+        // `TerminalElement` read the same predicate from opposite sides, so
+        // every (mouse_mode, shift) pair must land in exactly one of them.
+        for mouse_mode in [false, true] {
+            for shift in [false, true] {
+                let menu = should_show_context_menu(mouse_mode, shift);
+                let reported = mouse_mode && !shift;
+                assert_ne!(
+                    menu, reported,
+                    "mouse_mode={mouse_mode} shift={shift} must route to exactly one consumer"
+                );
+            }
+        }
+    }
+
     #[test]
     fn copy_on_select_copies_the_buffer_the_gesture_touched() {
         assert_eq!(select_end_copy(false, true, false), SelectEndCopy::None);