Debugged mouse reporting for now. Remaining bugs are GPUI level bugs

Mikayla Maki created

Change summary

crates/gpui/src/presenter.rs          |  21 +++
crates/terminal/src/connected_el.rs   | 164 ++++++++++++++++------------
crates/terminal/src/mappings/mouse.rs |  28 +++-
crates/terminal/src/terminal.rs       |  11 -
4 files changed, 139 insertions(+), 85 deletions(-)

Detailed changes

crates/gpui/src/presenter.rs 🔗

@@ -237,6 +237,7 @@ impl Presenter {
             let mut mouse_down_out_handlers = Vec::new();
             let mut mouse_moved_region = None;
             let mut mouse_down_region = None;
+            let mut mouse_up_region = None;
             let mut clicked_region = None;
             let mut dragged_region = None;
 
@@ -283,6 +284,15 @@ impl Presenter {
                         }
                     }
 
+                    for (region, _) in self.mouse_regions.iter().rev() {
+                        if region.bounds.contains_point(position) {
+                            invalidated_views.push(region.view_id);
+                            mouse_up_region =
+                                Some((region.clone(), MouseRegionEvent::Up(e.clone())));
+                            break;
+                        }
+                    }
+
                     if let Some(moved) = &mut self.last_mouse_moved_event {
                         if moved.pressed_button == Some(button) {
                             moved.pressed_button = None;
@@ -350,6 +360,17 @@ impl Presenter {
                 }
             }
 
+            if let Some((mouse_up_region, region_event)) = mouse_up_region {
+                handled = true;
+                if let Some(mouse_up_callback) =
+                    mouse_up_region.handlers.get(&region_event.handler_key())
+                {
+                    event_cx.with_current_view(mouse_up_region.view_id, |event_cx| {
+                        mouse_up_callback(region_event, event_cx);
+                    })
+                }
+            }
+
             if let Some((clicked_region, region_event)) = clicked_region {
                 handled = true;
                 if let Some(click_callback) =

crates/terminal/src/connected_el.rs 🔗

@@ -3,7 +3,10 @@ use alacritty_terminal::{
     grid::Dimensions,
     index::Point,
     selection::SelectionRange,
-    term::cell::{Cell, Flags},
+    term::{
+        cell::{Cell, Flags},
+        TermMode,
+    },
 };
 use editor::{Cursor, CursorShape, HighlightedRange, HighlightedRangeLine};
 use gpui::{
@@ -46,6 +49,7 @@ pub struct LayoutState {
     background_color: Color,
     selection_color: Color,
     size: TerminalSize,
+    mode: TermMode,
 }
 
 #[derive(Debug)]
@@ -428,41 +432,92 @@ impl TerminalEl {
         origin: Vector2F,
         view_id: usize,
         visible_bounds: RectF,
+        mode: TermMode,
         cx: &mut PaintContext,
     ) {
         let connection = self.terminal;
-        cx.scene.push_mouse_region(
-            MouseRegion::new(view_id, None, visible_bounds)
-                .on_move(move |event, cx| {
-                    if cx.is_parent_view_focused() {
-                        if let Some(conn_handle) = connection.upgrade(cx.app) {
-                            conn_handle.update(cx.app, |terminal, cx| {
-                                terminal.mouse_move(&event, origin);
-                                cx.notify();
-                            })
-                        }
+
+        let mut region = MouseRegion::new(view_id, None, visible_bounds);
+
+        //Terminal Emulator controlled behavior:
+        region = region
+            //Start selections
+            .on_down(
+                MouseButton::Left,
+                TerminalEl::generic_button_handler(
+                    connection,
+                    origin,
+                    move |terminal, origin, e, _cx| {
+                        terminal.mouse_down(&e, origin);
+                    },
+                ),
+            )
+            //Update drag selections
+            .on_drag(MouseButton::Left, move |_prev, event, cx| {
+                if cx.is_parent_view_focused() {
+                    if let Some(conn_handle) = connection.upgrade(cx.app) {
+                        conn_handle.update(cx.app, |terminal, cx| {
+                            terminal.mouse_drag(event, origin);
+                            cx.notify();
+                        })
                     }
-                })
-                .on_drag(MouseButton::Left, move |_prev, event, cx| {
-                    if cx.is_parent_view_focused() {
-                        if let Some(conn_handle) = connection.upgrade(cx.app) {
-                            conn_handle.update(cx.app, |terminal, cx| {
-                                terminal.mouse_drag(event, origin);
-                                cx.notify();
-                            })
-                        }
+                }
+            })
+            //Copy on up behavior
+            .on_up(
+                MouseButton::Left,
+                TerminalEl::generic_button_handler(
+                    connection,
+                    origin,
+                    move |terminal, origin, e, _cx| {
+                        terminal.mouse_up(&e, origin);
+                    },
+                ),
+            )
+            //Handle click based selections
+            .on_click(
+                MouseButton::Left,
+                TerminalEl::generic_button_handler(
+                    connection,
+                    origin,
+                    move |terminal, origin, e, _cx| {
+                        terminal.left_click(&e, origin);
+                    },
+                ),
+            )
+            //Context menu
+            .on_click(
+                MouseButton::Right,
+                move |e @ MouseButtonEvent { position, .. }, cx| {
+                    let mouse_mode = if let Some(conn_handle) = connection.upgrade(cx.app) {
+                        conn_handle.update(cx.app, |terminal, _cx| terminal.mouse_mode(e.shift))
+                    } else {
+                        //If we can't get the model handle, probably can't deploy the context menu
+                        true
+                    };
+                    if !mouse_mode {
+                        cx.dispatch_action(DeployContextMenu { position });
                     }
-                })
-                .on_down(
-                    MouseButton::Left,
-                    TerminalEl::generic_button_handler(
-                        connection,
-                        origin,
-                        move |terminal, origin, e, _cx| {
-                            terminal.mouse_down(&e, origin);
-                        },
-                    ),
-                )
+                },
+            )
+            //This handles both drag mode and mouse motion mode
+            //Mouse Move TODO
+            //This cannot be done conditionally for unknown reasons. Pending drag and drop rework.
+            //This also does not fire on right-mouse-down-move events wild.
+            .on_move(move |event, cx| {
+                dbg!(event);
+                if cx.is_parent_view_focused() {
+                    if let Some(conn_handle) = connection.upgrade(cx.app) {
+                        conn_handle.update(cx.app, |terminal, cx| {
+                            terminal.mouse_move(&event, origin);
+                            cx.notify();
+                        })
+                    }
+                }
+            });
+
+        if mode.contains(TermMode::MOUSE_MODE) {
+            region = region
                 .on_down(
                     MouseButton::Right,
                     TerminalEl::generic_button_handler(
@@ -483,16 +538,6 @@ impl TerminalEl {
                         },
                     ),
                 )
-                .on_up(
-                    MouseButton::Left,
-                    TerminalEl::generic_button_handler(
-                        connection,
-                        origin,
-                        move |terminal, origin, e, _cx| {
-                            terminal.mouse_up(&e, origin);
-                        },
-                    ),
-                )
                 .on_up(
                     MouseButton::Right,
                     TerminalEl::generic_button_handler(
@@ -513,31 +558,11 @@ impl TerminalEl {
                         },
                     ),
                 )
-                .on_click(
-                    MouseButton::Left,
-                    TerminalEl::generic_button_handler(
-                        connection,
-                        origin,
-                        move |terminal, origin, e, _cx| {
-                            terminal.left_click(&e, origin);
-                        },
-                    ),
-                )
-                .on_click(
-                    MouseButton::Right,
-                    move |e @ MouseButtonEvent { position, .. }, cx| {
-                        let mouse_mode = if let Some(conn_handle) = connection.upgrade(cx.app) {
-                            conn_handle.update(cx.app, |terminal, _cx| terminal.mouse_mode(e.shift))
-                        } else {
-                            //If we can't get the model handle, probably can't deploy the context menu
-                            true
-                        };
-                        if !mouse_mode {
-                            cx.dispatch_action(DeployContextMenu { position });
-                        }
-                    },
-                ),
-        );
+        }
+
+        //TODO: Mouse drag isn't correct
+        //TODO: Nor is mouse motion. Move events aren't happening??
+        cx.scene.push_mouse_region(region);
     }
 
     ///Configures a text style from the current settings.
@@ -601,7 +626,7 @@ impl Element for TerminalEl {
             terminal_theme.colors.background
         };
 
-        let (cells, selection, cursor, display_offset, cursor_text) = self
+        let (cells, selection, cursor, display_offset, cursor_text, mode) = self
             .terminal
             .upgrade(cx)
             .unwrap()
@@ -624,13 +649,13 @@ impl Element for TerminalEl {
                                 cell: ic.cell.clone(),
                             }),
                     );
-
                     (
                         cells,
                         content.selection,
                         content.cursor,
                         content.display_offset,
                         cursor_text,
+                        content.mode,
                     )
                 })
             });
@@ -709,6 +734,7 @@ impl Element for TerminalEl {
                 size: dimensions,
                 rects,
                 highlights,
+                mode,
             },
         )
     }
@@ -727,7 +753,7 @@ impl Element for TerminalEl {
             let origin = bounds.origin() + vec2f(layout.size.cell_width, 0.);
 
             //Elements are ephemeral, only at paint time do we know what could be clicked by a mouse
-            self.attach_mouse_handlers(origin, self.view.id(), visible_bounds, cx);
+            self.attach_mouse_handlers(origin, self.view.id(), visible_bounds, layout.mode, cx);
 
             cx.paint_layer(clip_bounds, |cx| {
                 //Start with a background color

crates/terminal/src/mappings/mouse.rs 🔗

@@ -1,4 +1,4 @@
-use std::cmp::min;
+use std::cmp::{max, min};
 use std::iter::repeat;
 
 use alacritty_terminal::grid::Dimensions;
@@ -60,6 +60,7 @@ impl MouseFormat {
     }
 }
 
+#[derive(Debug)]
 enum MouseButton {
     LeftButton = 0,
     MiddleButton = 1,
@@ -117,7 +118,7 @@ pub fn scroll_report(
     e: &ScrollWheelEvent,
     mode: TermMode,
 ) -> Option<impl Iterator<Item = Vec<u8>>> {
-    if mode.intersects(TermMode::MOUSE_MODE) && scroll_lines >= 1 {
+    if mode.intersects(TermMode::MOUSE_MODE) {
         mouse_report(
             point,
             MouseButton::from_scroll(e),
@@ -125,7 +126,7 @@ pub fn scroll_report(
             Modifiers::from_scroll(),
             MouseFormat::from_mode(mode),
         )
-        .map(|report| repeat(report).take(scroll_lines as usize))
+        .map(|report| repeat(report).take(max(scroll_lines, 1) as usize))
     } else {
         None
     }
@@ -165,14 +166,21 @@ pub fn mouse_button_report(
 
 pub fn mouse_moved_report(point: Point, e: &MouseMovedEvent, mode: TermMode) -> Option<Vec<u8>> {
     let button = MouseButton::from_move(e);
+    dbg!(&button);
+
     if !button.is_other() && mode.intersects(TermMode::MOUSE_MOTION | TermMode::MOUSE_DRAG) {
-        mouse_report(
-            point,
-            button,
-            true,
-            Modifiers::from_moved(e),
-            MouseFormat::from_mode(mode),
-        )
+        //Only drags are reported in drag mode, so block NoneMove.
+        if mode.contains(TermMode::MOUSE_DRAG) && matches!(button, MouseButton::NoneMove) {
+            None
+        } else {
+            mouse_report(
+                point,
+                button,
+                true,
+                Modifiers::from_moved(e),
+                MouseFormat::from_mode(mode),
+            )
+        }
     } else {
         None
     }

crates/terminal/src/terminal.rs 🔗

@@ -35,8 +35,8 @@ use thiserror::Error;
 use gpui::{
     geometry::vector::{vec2f, Vector2F},
     keymap::Keystroke,
-    ClipboardItem, Entity, ModelContext, MouseButtonEvent, MouseMovedEvent, MutableAppContext,
-    ScrollWheelEvent,
+    ClipboardItem, Entity, ModelContext, MouseButton, MouseButtonEvent, MouseMovedEvent,
+    MutableAppContext, ScrollWheelEvent,
 };
 
 use crate::mappings::{
@@ -627,6 +627,7 @@ impl Terminal {
     }
 
     pub fn mouse_move(&mut self, e: &MouseMovedEvent, origin: Vector2F) {
+        dbg!("term mouse_move");
         let position = e.position.sub(origin);
 
         let point = mouse_point(position, self.cur_size, self.last_offset);
@@ -653,7 +654,6 @@ impl Terminal {
 
     pub fn mouse_down(&mut self, e: &MouseButtonEvent, origin: Vector2F) {
         let position = e.position.sub(origin);
-
         let point = mouse_point(position, self.cur_size, self.last_offset);
         let side = mouse_side(position, self.cur_size);
 
@@ -661,7 +661,7 @@ impl Terminal {
             if let Some(bytes) = mouse_button_report(point, e, true, self.last_mode) {
                 self.pty_tx.notify(bytes);
             }
-        } else {
+        } else if e.button == MouseButton::Left {
             self.events
                 .push(InternalEvent::SetSelection(Some(Selection::new(
                     SelectionType::Simple,
@@ -695,14 +695,13 @@ impl Terminal {
 
     pub fn mouse_up(&mut self, e: &MouseButtonEvent, origin: Vector2F) {
         let position = e.position.sub(origin);
-
         if self.mouse_mode(e.shift) {
             let point = mouse_point(position, self.cur_size, self.last_offset);
 
             if let Some(bytes) = mouse_button_report(point, e, false, self.last_mode) {
                 self.pty_tx.notify(bytes);
             }
-        } else {
+        } else if e.button == MouseButton::Left {
             // Seems pretty standard to automatically copy on mouse_up for terminals,
             // so let's do that here
             self.copy();