refactor(tab): simplify mouse hold and release (#1185)

default-plugins/strider/src/main.rs

@@ -58,14 +58,12 @@ impl ZellijPlugin for State {
                 Mouse::ScrollUp(_) => {
                     *self.selected_mut() = self.selected().saturating_sub(1);
                 }
-                Mouse::Release(Some((line, _))) => {
+                Mouse::Release(line, _) => {
                     if line < 0 {
                         return;
                     }
                     let mut should_select = true;
-                    if let Some((Event::Mouse(Mouse::Release(Some((prev_line, _)))), t)) =
+                    if let Some((Event::Mouse(Mouse::Release(prev_line, _)), t)) = prev_event {
-                        prev_event
-                    {
                         if prev_line == line
                             && Instant::now().saturating_duration_since(t).as_millis() < 400
                         {

zellij-server/src/panes/grid.rs

@@ -1303,9 +1303,9 @@ impl Grid {
         self.mark_for_rerender();
     }
 
-    pub fn end_selection(&mut self, end: Option<&Position>) {
+    pub fn end_selection(&mut self, end: &Position) {
         let old_selection = self.selection;
-        self.selection.end(end);
+        self.selection.end(*end);
         self.update_selected_lines(&old_selection, &self.selection.clone());
         self.mark_for_rerender();
     }
@@ -1391,8 +1391,12 @@ impl Grid {
         // for now trim after building the selection to handle whitespace in wrapped lines
         let selection: Vec<_> = selection.iter().map(|l| l.trim_end()).collect();
 
+        if selection.is_empty() {
+            None
+        } else {
             Some(selection.join("\n"))
         }
+    }
 
     fn update_selected_lines(&mut self, old_selection: &Selection, new_selection: &Selection) {
         for l in old_selection.diff(new_selection, self.height) {

zellij-server/src/panes/plugin_pane.rs

@@ -360,14 +360,12 @@ impl Pane for PluginPane {
             ))
             .unwrap();
     }
-    fn end_selection(&mut self, end: Option<&Position>, client_id: ClientId) {
+    fn end_selection(&mut self, end: &Position, client_id: ClientId) {
         self.send_plugin_instructions
             .send(PluginInstruction::Update(
                 Some(self.pid),
                 Some(client_id),
-                Event::Mouse(Mouse::Release(
+                Event::Mouse(Mouse::Release(end.line(), end.column())),
-                    end.map(|Position { line, column }| (line.0, column.0)),
-                )),
             ))
             .unwrap();
     }

zellij-server/src/panes/selection.rs

@@ -32,11 +32,9 @@ impl Selection {
         self.end = to
     }
 
-    pub fn end(&mut self, to: Option<&Position>) {
+    pub fn end(&mut self, end: Position) {
         self.active = false;
-        if let Some(to) = to {
+        self.end = end;
-            self.end = *to
-        }
     }
 
     pub fn contains(&self, row: usize, col: usize) -> bool {

zellij-server/src/panes/terminal_pane.rs

@@ -431,7 +431,7 @@ impl Pane for TerminalPane {
         self.set_should_render(true);
     }
 
-    fn end_selection(&mut self, end: Option<&Position>, _client_id: ClientId) {
+    fn end_selection(&mut self, end: &Position, _client_id: ClientId) {
         self.grid.end_selection(end);
         self.set_should_render(true);
     }

zellij-server/src/panes/unit/grid_tests.rs

@@ -851,7 +851,7 @@ fn copy_selected_text_from_viewport() {
 
     grid.start_selection(&Position::new(23, 6));
     // check for widechar, 📦 occupies columns 34, 35, and gets selected even if only the first column is selected
-    grid.end_selection(Some(&Position::new(25, 35)));
+    grid.end_selection(&Position::new(25, 35));
     let text = grid.get_selected_text();
     assert_eq!(
         text.unwrap(),
@@ -875,7 +875,7 @@ fn copy_wrapped_selected_text_from_viewport() {
     }
 
     grid.start_selection(&Position::new(5, 0));
-    grid.end_selection(Some(&Position::new(8, 42)));
+    grid.end_selection(&Position::new(8, 42));
     let text = grid.get_selected_text();
     assert_eq!(
         text.unwrap(),
@@ -900,7 +900,7 @@ fn copy_selected_text_from_lines_above() {
 
     grid.start_selection(&Position::new(-2, 10));
     // check for widechar, 📦 occupies columns 34, 35, and gets selected even if only the first column is selected
-    grid.end_selection(Some(&Position::new(2, 8)));
+    grid.end_selection(&Position::new(2, 8));
     let text = grid.get_selected_text();
     assert_eq!(
         text.unwrap(),
@@ -927,7 +927,7 @@ fn copy_selected_text_from_lines_below() {
 
     grid.start_selection(&Position::new(63, 6));
     // check for widechar, 📦 occupies columns 34, 35, and gets selected even if only the first column is selected
-    grid.end_selection(Some(&Position::new(65, 35)));
+    grid.end_selection(&Position::new(65, 35));
     let text = grid.get_selected_text();
     assert_eq!(
         text.unwrap(),

zellij-server/src/panes/unit/selection_tests.rs

@@ -22,26 +22,15 @@ fn selection_to() {
 }
 
 #[test]
-fn selection_end_with_position() {
+fn selection_end() {
     let mut selection = Selection::default();
     selection.start(Position::new(10, 10));
-    selection.end(Some(&Position::new(20, 30)));
+    selection.end(Position::new(20, 30));
 
     assert!(!selection.active);
     assert_eq!(selection.end, Position::new(20, 30));
 }
 
-#[test]
-fn selection_end_without_position() {
-    let mut selection = Selection::default();
-    selection.start(Position::new(10, 10));
-    selection.to(Position::new(15, 100));
-    selection.end(None);
-
-    assert!(!selection.active);
-    assert_eq!(selection.end, Position::new(15, 100));
-}
-
 #[test]
 fn contains() {
     struct TestCase<'a> {

zellij-server/src/tab/mod.rs

@@ -199,7 +199,7 @@ pub trait Pane {
     }
     fn start_selection(&mut self, _start: &Position, _client_id: ClientId) {}
     fn update_selection(&mut self, _position: &Position, _client_id: ClientId) {}
-    fn end_selection(&mut self, _end: Option<&Position>, _client_id: ClientId) {}
+    fn end_selection(&mut self, _end: &Position, _client_id: ClientId) {}
     fn reset_selection(&mut self) {}
     fn get_selected_text(&self) -> Option<String> {
         None
@@ -2336,17 +2336,16 @@ impl Tab {
     pub fn handle_left_click(&mut self, position: &Position, client_id: ClientId) {
         self.focus_pane_at(position, client_id);
 
-        let show_floating_panes = self.floating_panes.panes_are_visible();
-        if show_floating_panes {
         let search_selectable = false;
-            if self
+        if self.floating_panes.panes_are_visible()
+            && self
                 .floating_panes
                 .move_pane_with_mouse(*position, search_selectable)
         {
             self.set_force_render();
             return;
         }
-        }
+
         if let Some(pane) = self.get_pane_at(position, false) {
             let relative_position = pane.relative_position(position);
             pane.start_selection(&relative_position, client_id);
@@ -2393,97 +2392,38 @@ impl Tab {
         }
     }
     pub fn handle_mouse_release(&mut self, position: &Position, client_id: ClientId) {
-        if self.floating_panes.panes_are_visible() {
+        if self.selecting_with_mouse {
-            self.floating_panes.stop_moving_pane_with_mouse(*position);
-        }
-        if !self.selecting_with_mouse {
-            return;
-        }
-
             let mut selected_text = None;
-        if self.floating_panes.panes_are_visible() {
+            let active_pane = self.get_active_pane_or_floating_pane_mut(client_id);
-            let active_pane_id = self
+            if let Some(active_pane) = active_pane {
-                .floating_panes
+                let relative_position = active_pane.relative_position(position);
-                .active_pane_id(client_id)
+                active_pane.end_selection(&relative_position, client_id);
-                .or_else(|| self.active_panes.get(&client_id).copied());
-            // on release, get the selected text from the active pane, and reset it's selection
-            let pane_id_at_position = self
-                .floating_panes
-                .get_pane_id_at(position, true)
-                .or_else(|| self.get_pane_id_at(position, true));
-            if active_pane_id != pane_id_at_position {
-                // release happened outside of pane
-                if let Some(active_pane_id) = active_pane_id {
-                    if let Some(active_pane) = self
-                        .floating_panes
-                        .get_mut(&active_pane_id)
-                        .or_else(|| self.panes.get_mut(&active_pane_id))
-                    {
-                        active_pane.end_selection(None, client_id);
                 selected_text = active_pane.get_selected_text();
                 active_pane.reset_selection();
             }
-                }
-            } else if let Some(pane) = pane_id_at_position.and_then(|pane_id_at_position| {
-                self.floating_panes
-                    .get_mut(&pane_id_at_position)
-                    .or_else(|| self.panes.get_mut(&pane_id_at_position))
-            }) {
-                // release happened inside of pane
-                let relative_position = pane.relative_position(position);
-                pane.end_selection(Some(&relative_position), client_id);
-                selected_text = pane.get_selected_text();
-                pane.reset_selection();
-            }
-        } else {
-            let active_pane_id = self.active_panes.get(&client_id);
-            // on release, get the selected text from the active pane, and reset it's selection
-            if active_pane_id != self.get_pane_id_at(position, true).as_ref() {
-                if let Some(active_pane_id) = active_pane_id {
-                    if let Some(active_pane) = self.panes.get_mut(&active_pane_id) {
-                        active_pane.end_selection(None, client_id);
-                        selected_text = active_pane.get_selected_text();
-                        active_pane.reset_selection();
-                    }
-                }
-            } else if let Some(pane) = self.get_pane_at(position, true) {
-                let relative_position = pane.relative_position(position);
-                pane.end_selection(Some(&relative_position), client_id);
-                selected_text = pane.get_selected_text();
-                pane.reset_selection();
-            }
-        }
 
             if let Some(selected_text) = selected_text {
                 self.write_selection_to_clipboard(&selected_text);
             }
             self.selecting_with_mouse = false;
+        } else if self.floating_panes.panes_are_visible() {
+            self.floating_panes.stop_moving_pane_with_mouse(*position);
+        }
     }
     pub fn handle_mouse_hold(&mut self, position_on_screen: &Position, client_id: ClientId) {
         let search_selectable = true;
-        if self
+        if self.selecting_with_mouse {
+            let active_pane = self.get_active_pane_or_floating_pane_mut(client_id);
+            if let Some(active_pane) = active_pane {
+                let relative_position = active_pane.relative_position(position_on_screen);
+                active_pane.update_selection(&relative_position, client_id);
+            }
+        } else if self.floating_panes.panes_are_visible()
+            && self
                 .floating_panes
                 .move_pane_with_mouse(*position_on_screen, search_selectable)
         {
             self.set_force_render();
-        } else if self.floating_panes.panes_are_visible() {
-            let active_pane = self
-                .floating_panes
-                .get_active_pane_mut(client_id)
-                .or_else(|| {
-                    self.active_panes
-                        .get(&client_id)
-                        .and_then(|pane_id| self.panes.get_mut(pane_id))
-                });
-            active_pane.map(|active_pane| {
-                let relative_position = active_pane.relative_position(position_on_screen);
-                active_pane.update_selection(&relative_position, client_id);
-            });
-        } else if let Some(active_pane_id) = self.get_active_pane_id(client_id) {
-            if let Some(active_pane) = self.panes.get_mut(&active_pane_id) {
-                let relative_position = active_pane.relative_position(position_on_screen);
-                active_pane.update_selection(&relative_position, client_id);
-            }
         }
     }
 

zellij-tile/src/data.rs

@@ -63,7 +63,7 @@ pub enum Mouse {
     LeftClick(isize, usize),  // line and column
     RightClick(isize, usize), // line and column
     Hold(isize, usize),       // line and column
-    Release(Option<(isize, usize)>), // line and column
+    Release(isize, usize),    // line and column
 }
 
 #[derive(Debug, Clone, PartialEq, EnumDiscriminants, ToString, Serialize, Deserialize)]