fix(plugins): multiple-select + compact-bar tooltip multiplayer issues (#4312)

* fix: allow stacking panes if root pane is floating

* fix: handle multiple client gracefully in multiple select

* style(fmt): rustfmt

* fix compact-bar tooltip multiuser duplication

* style(fmt): rustfmt

default-plugins/compact-bar/src/main.rs

@@ -59,6 +59,7 @@ struct State {
     persist: bool,
     is_first_run: bool,
     own_tab_index: Option<usize>,
+    own_client_id: u16,
 }
 
 struct TabRenderData {
@@ -72,10 +73,12 @@ register_plugin!(State);
 
 impl ZellijPlugin for State {
     fn load(&mut self, configuration: BTreeMap<String, String>) {
+        let plugin_ids = get_plugin_ids();
+        self.own_plugin_id = Some(plugin_ids.plugin_id);
+        self.own_client_id = plugin_ids.client_id;
         self.initialize_configuration(configuration);
         self.setup_subscriptions();
         self.configure_keybinds();
-        self.own_plugin_id = Some(get_plugin_ids().plugin_id);
     }
 
     fn update(&mut self, event: Event) -> bool {
@@ -104,14 +107,10 @@ impl ZellijPlugin for State {
         } else if message.name == MSG_TOGGLE_TOOLTIP
             && message.is_private
             && self.toggle_tooltip_key.is_some()
+            // only launch once per plugin instance
             && self.own_tab_index == Some(self.active_tab_idx.saturating_sub(1))
-        // only launch
-        // tooltip once
-        // even if there
-        // are a few
-        // instances of
-        // compact-bar
-        // running
+            // only launch once per client of plugin instance
+            && Some(format!("{}", self.own_client_id)) == message.payload
         {
             self.toggle_persisted_tooltip(self.mode_info.mode);
         }
@@ -166,7 +165,10 @@ impl State {
     fn configure_keybinds(&self) {
         if !self.is_tooltip && self.toggle_tooltip_key.is_some() {
             if let Some(toggle_key) = &self.toggle_tooltip_key {
-                reconfigure(bind_toggle_key_config(toggle_key), false);
+                reconfigure(
+                    bind_toggle_key_config(toggle_key, self.own_client_id),
+                    false,
+                );
             }
         }
     }
@@ -551,7 +553,7 @@ impl State {
     }
 }
 
-fn bind_toggle_key_config(toggle_key: &str) -> String {
+fn bind_toggle_key_config(toggle_key: &str, client_id: u16) -> String {
     format!(
         r#"
         keybinds {{
@@ -560,11 +562,12 @@ fn bind_toggle_key_config(toggle_key: &str) -> String {
                   MessagePlugin "compact-bar" {{
                       name "toggle_tooltip"
                       tooltip "{}"
+                      payload "{}"
                   }}
                 }}
             }}
         }}
     "#,
-        toggle_key, toggle_key
+        toggle_key, toggle_key, client_id
     )
 }

default-plugins/multiple-select/src/main.rs

@@ -11,6 +11,7 @@ pub struct App {
     total_tabs_in_session: Option<usize>,
     grouped_panes: Vec<PaneId>,
     grouped_panes_count: usize,
+    all_client_grouped_panes: BTreeMap<ClientId, Vec<PaneId>>,
     mode_info: ModeInfo,
     closing: bool,
     highlighted_at: Option<Instant>,
@@ -47,6 +48,8 @@ impl ZellijPlugin for App {
         if self.closing {
             return false;
         }
+        intercept_key_presses(); // we do this here so that all clients (even those connected after
+                                 // load) will have their keys intercepted
         match event {
             Event::ModeUpdate(mode_info) => self.handle_mode_update(mode_info),
             Event::PaneUpdate(pane_manifest) => self.handle_pane_update(pane_manifest),
@@ -59,6 +62,10 @@ impl ZellijPlugin for App {
 
     fn render(&mut self, rows: usize, cols: usize) {
         self.update_current_size(rows, cols);
+
+        if self.grouped_panes_count == 0 {
+            self.render_no_panes_message(rows, cols);
+        } else {
             let ui_width = self.calculate_ui_width();
             self.update_baseline_ui_width(ui_width);
             let base_x = cols.saturating_sub(self.baseline_ui_width) / 2;
@@ -68,6 +75,7 @@ impl ZellijPlugin for App {
             self.render_controls(base_x, base_y + 7);
         }
     }
+}
 
 impl App {
     fn update_current_size(&mut self, new_rows: usize, new_cols: usize) {
@@ -88,7 +96,7 @@ impl App {
         let controls_width = group_controls_length(&self.mode_info);
 
         let header_width = Self::header_text().0.len();
-        let shortcuts_max_width = Self::shortcuts_max_width();
+        let shortcuts_max_width = self.shortcuts_max_width();
 
         std::cmp::max(
             controls_width,
@@ -96,6 +104,20 @@ impl App {
         )
     }
 
+    fn render_no_panes_message(&self, rows: usize, cols: usize) {
+        let message = "PANES SELECTED FOR OTHER CLIENT";
+        let message_component = Text::new(message).color_all(2);
+        let base_x = cols.saturating_sub(message.len()) / 2;
+        let base_y = rows / 2;
+        print_text_with_coordinates(message_component, base_x, base_y, None, None);
+
+        let esc_message = "<ESC> - close";
+        let esc_message_component = Text::new(esc_message).color_substring(3, "<ESC>");
+        let esc_base_x = cols.saturating_sub(esc_message.len()) / 2;
+        let esc_base_y = base_y + 2;
+        print_text_with_coordinates(esc_message_component, esc_base_x, esc_base_y, None, None);
+    }
+
     fn header_text() -> (&'static str, Text) {
         let header_text = "<ESC> - cancel, <TAB> - move";
         let header_text_component = Text::new(header_text)
@@ -104,10 +126,10 @@ impl App {
         (header_text, header_text_component)
     }
 
-    fn shortcuts_max_width() -> usize {
+    fn shortcuts_max_width(&self) -> usize {
         std::cmp::max(
             std::cmp::max(
-                Self::group_actions_text().0.len(),
+                self.group_actions_text().0.len(),
                 Self::shortcuts_line1_text().0.len(),
             ),
             std::cmp::max(
@@ -117,10 +139,18 @@ impl App {
         )
     }
 
-    fn group_actions_text() -> (&'static str, Text) {
-        let text = "GROUP ACTIONS";
-        let component = Text::new(text).color_all(2);
-        (text, component)
+    fn group_actions_text(&self) -> (&'static str, Text) {
+        let count_text = if self.grouped_panes_count == 1 {
+            format!("GROUP ACTIONS ({} SELECTED PANE)", self.grouped_panes_count)
+        } else {
+            format!(
+                "GROUP ACTIONS ({} SELECTED PANES)",
+                self.grouped_panes_count
+            )
+        };
+
+        let component = Text::new(&count_text).color_all(2);
+        (Box::leak(count_text.into_boxed_str()), component)
     }
 
     fn shortcuts_line1_text() -> (&'static str, Text) {
@@ -164,7 +194,8 @@ impl App {
             return false;
         };
 
-        self.update_grouped_panes(&pane_manifest, own_client_id);
+        self.update_all_client_grouped_panes(&pane_manifest);
+        self.update_own_grouped_panes(&pane_manifest, own_client_id);
         self.update_tab_info(&pane_manifest);
         self.total_tabs_in_session = Some(pane_manifest.panes.keys().count());
 
@@ -183,7 +214,28 @@ impl App {
         false
     }
 
-    fn update_grouped_panes(&mut self, pane_manifest: &PaneManifest, own_client_id: ClientId) {
+    fn update_all_client_grouped_panes(&mut self, pane_manifest: &PaneManifest) {
+        self.all_client_grouped_panes.clear();
+
+        for (_tab_index, pane_infos) in &pane_manifest.panes {
+            for pane_info in pane_infos {
+                for (client_id, _index_in_pane_group) in &pane_info.index_in_pane_group {
+                    let pane_id = if pane_info.is_plugin {
+                        PaneId::Plugin(pane_info.id)
+                    } else {
+                        PaneId::Terminal(pane_info.id)
+                    };
+
+                    self.all_client_grouped_panes
+                        .entry(*client_id)
+                        .or_insert_with(Vec::new)
+                        .push(pane_id);
+                }
+            }
+        }
+    }
+
+    fn update_own_grouped_panes(&mut self, pane_manifest: &PaneManifest, own_client_id: ClientId) {
         self.grouped_panes.clear();
         let mut count = 0;
         let mut panes_with_index = Vec::new();
@@ -209,20 +261,15 @@ impl App {
             self.grouped_panes.push(pane_id);
         }
 
-        if count == 0 {
+        if self.all_clients_have_empty_groups() {
             self.close_self();
         }
 
         let previous_count = self.grouped_panes_count;
         self.grouped_panes_count = count;
         if let Some(own_plugin_id) = self.own_plugin_id {
-            let title = if count == 1 {
-                "SELECTED PANE"
-            } else {
-                "SELECTED PANES"
-            };
             if previous_count != count {
-                rename_plugin_pane(own_plugin_id, format!("{} {}", count, title));
+                rename_plugin_pane(own_plugin_id, "Multiple Pane Select".to_string());
             }
             if previous_count != 0 && count != 0 && previous_count != count {
                 if self.doherty_threshold_elapsed_since_highlight() {
@@ -234,6 +281,12 @@ impl App {
         }
     }
 
+    fn all_clients_have_empty_groups(&self) -> bool {
+        self.all_client_grouped_panes
+            .values()
+            .all(|panes| panes.is_empty())
+    }
+
     fn doherty_threshold_elapsed_since_highlight(&self) -> bool {
         self.highlighted_at
             .map(|h| h.elapsed() >= std::time::Duration::from_millis(400))
@@ -266,7 +319,7 @@ impl App {
             BareKey::Char('c') => self.close_grouped_panes(),
             BareKey::Tab => self.next_coordinates(),
             BareKey::Esc => {
-                self.ungroup_panes_in_zellij(&self.grouped_panes.clone());
+                self.ungroup_panes_in_zellij();
                 self.close_self();
             },
             _ => return false,
@@ -290,7 +343,7 @@ impl App {
 
     fn render_shortcuts(&self, base_x: usize, base_y: usize) {
         let mut running_y = base_y;
-        print_text_with_coordinates(Self::group_actions_text().1, base_x, running_y, None, None);
+        print_text_with_coordinates(self.group_actions_text().1, base_x, running_y, None, None);
         running_y += 1;
 
         print_text_with_coordinates(
@@ -337,28 +390,28 @@ impl App {
         self.execute_action_and_close(|pane_ids| {
             break_panes_to_new_tab(pane_ids, None, true);
         });
-        self.ungroup_panes_in_zellij(&self.grouped_panes.clone());
+        self.ungroup_panes_in_zellij();
     }
 
     pub fn stack_grouped_panes(&mut self) {
         self.execute_action_and_close(|pane_ids| {
             stack_panes(pane_ids.to_vec());
         });
-        self.ungroup_panes_in_zellij(&self.grouped_panes.clone());
+        self.ungroup_panes_in_zellij();
     }
 
     pub fn float_grouped_panes(&mut self) {
         self.execute_action_and_close(|pane_ids| {
             float_multiple_panes(pane_ids.to_vec());
         });
-        self.ungroup_panes_in_zellij(&self.grouped_panes.clone());
+        self.ungroup_panes_in_zellij();
     }
 
     pub fn embed_grouped_panes(&mut self) {
         self.execute_action_and_close(|pane_ids| {
             embed_multiple_panes(pane_ids.to_vec());
         });
-        self.ungroup_panes_in_zellij(&self.grouped_panes.clone());
+        self.ungroup_panes_in_zellij();
     }
 
     pub fn break_grouped_panes_right(&mut self) {
@@ -385,7 +438,7 @@ impl App {
         let pane_ids = self.grouped_panes.clone();
 
         if own_tab_index > 0 {
-            break_panes_to_tab_with_index(&pane_ids, own_tab_index - 1, true);
+            break_panes_to_tab_with_index(&pane_ids, own_tab_index.saturating_sub(1), true);
         } else {
             break_panes_to_new_tab(&pane_ids, None, true);
         }
@@ -399,9 +452,16 @@ impl App {
         });
     }
 
-    pub fn ungroup_panes_in_zellij(&mut self, pane_ids: &[PaneId]) {
-        group_and_ungroup_panes(vec![], pane_ids.to_vec());
+    pub fn ungroup_panes_in_zellij(&mut self) {
+        let all_grouped_panes: Vec<PaneId> = self
+            .all_client_grouped_panes
+            .values()
+            .flat_map(|panes| panes.iter().cloned())
+            .collect();
+        let for_all_clients = true;
+        group_and_ungroup_panes(vec![], all_grouped_panes, for_all_clients);
     }
+
     pub fn close_self(&mut self) {
         self.closing = true;
         close_self();

zellij-server/src/pane_groups.rs

@@ -94,6 +94,35 @@ impl PaneGroups {
             self.launch_plugin(screen_size, client_id);
         }
     }
+    pub fn group_and_ungroup_panes_for_all_clients(
+        &mut self,
+        pane_ids_to_group: Vec<PaneId>,
+        pane_ids_to_ungroup: Vec<PaneId>,
+        screen_size: Size,
+    ) {
+        let previous_groups = self.clone_inner();
+        let mut should_launch = false;
+        let all_connected_clients: Vec<ClientId> = self.panes_in_group.keys().copied().collect();
+
+        for client_id in &all_connected_clients {
+            let client_pane_group = self
+                .panes_in_group
+                .entry(*client_id)
+                .or_insert_with(|| vec![]);
+            client_pane_group.append(&mut pane_ids_to_group.clone());
+            client_pane_group.retain(|p| !pane_ids_to_ungroup.contains(p));
+
+            if self.should_launch_plugin(&previous_groups, &client_id) {
+                should_launch = true;
+            }
+        }
+
+        if should_launch {
+            if let Some(first_client) = all_connected_clients.first() {
+                self.launch_plugin(screen_size, first_client);
+            }
+        }
+    }
     pub fn override_groups_with(&mut self, new_pane_groups: HashMap<ClientId, Vec<PaneId>>) {
         self.panes_in_group = new_pane_groups;
     }

zellij-server/src/panes/grid.rs

@@ -1315,7 +1315,7 @@ impl Grid {
                 // the state is corrupted
                 return;
             }
-            if scroll_region_bottom == self.height - 1 && scroll_region_top == 0 {
+            if scroll_region_bottom == self.height.saturating_sub(1) && scroll_region_top == 0 {
                 if self.alternate_screen_state.is_none() {
                     self.transfer_rows_to_lines_above(1);
                 } else {
@@ -1547,7 +1547,7 @@ impl Grid {
         if y >= scroll_region_top && y <= scroll_region_bottom {
             self.cursor.y = std::cmp::min(scroll_region_bottom, y + y_offset);
         } else {
-            self.cursor.y = std::cmp::min(self.height - 1, y + y_offset);
+            self.cursor.y = std::cmp::min(self.height.saturating_sub(1), y + y_offset);
         }
         self.pad_lines_until(self.cursor.y, pad_character.clone());
         self.pad_current_line_until(self.cursor.x, pad_character);

zellij-server/src/plugins/zellij_exports.rs

@@ -444,13 +444,16 @@ fn host_run_plugin_command(mut caller: Caller<'_, PluginEnv>) {
                         close_plugin_after_replace,
                         context,
                     ),
-                    PluginCommand::GroupAndUngroupPanes(panes_to_group, panes_to_ungroup) => {
-                        group_and_ungroup_panes(
+                    PluginCommand::GroupAndUngroupPanes(
+                        panes_to_group,
+                        panes_to_ungroup,
+                        for_all_clients,
+                    ) => group_and_ungroup_panes(
                         env,
                         panes_to_group.into_iter().map(|p| p.into()).collect(),
                         panes_to_ungroup.into_iter().map(|p| p.into()).collect(),
-                        )
-                    },
+                        for_all_clients,
+                    ),
                     PluginCommand::HighlightAndUnhighlightPanes(
                         panes_to_highlight,
                         panes_to_unhighlight,
@@ -2270,12 +2273,14 @@ fn group_and_ungroup_panes(
     env: &PluginEnv,
     panes_to_group: Vec<PaneId>,
     panes_to_ungroup: Vec<PaneId>,
+    for_all_clients: bool,
 ) {
     let _ = env
         .senders
         .send_to_screen(ScreenInstruction::GroupAndUngroupPanes(
             panes_to_group,
             panes_to_ungroup,
+            for_all_clients,
             env.client_id,
         ));
 }

zellij-server/src/screen.rs

@@ -412,7 +412,7 @@ pub enum ScreenInstruction {
     ChangeFloatingPanesCoordinates(Vec<(PaneId, FloatingPaneCoordinates)>),
     AddHighlightPaneFrameColorOverride(Vec<PaneId>, Option<String>), // Option<String> => optional
     // message
-    GroupAndUngroupPanes(Vec<PaneId>, Vec<PaneId>, ClientId), // panes_to_group, panes_to_ungroup
+    GroupAndUngroupPanes(Vec<PaneId>, Vec<PaneId>, bool, ClientId), // panes_to_group, panes_to_ungroup, bool -> for all clients
     HighlightAndUnhighlightPanes(Vec<PaneId>, Vec<PaneId>, ClientId), // panes_to_highlight, panes_to_unhighlight
     FloatMultiplePanes(Vec<PaneId>, ClientId),
     EmbedMultiplePanes(Vec<PaneId>, ClientId),
@@ -2791,6 +2791,17 @@ impl Screen {
             log::error!("Failed to find tab for root_pane_id: {:?}", root_pane_id);
             return None;
         };
+        let root_pane_id_is_floating = self
+            .tabs
+            .get(&root_tab_id)
+            .map(|t| t.pane_id_is_floating(&root_pane_id))
+            .unwrap_or(false);
+
+        if root_pane_id_is_floating {
+            self.tabs.get_mut(&root_tab_id).map(|tab| {
+                let _ = tab.toggle_pane_embed_or_floating_for_pane_id(root_pane_id, None);
+            });
+        }
 
         let mut panes_to_stack = vec![];
         let target_tab_has_room_for_stack = self
@@ -3118,8 +3129,19 @@ impl Screen {
         &mut self,
         pane_ids_to_group: Vec<PaneId>,
         pane_ids_to_ungroup: Vec<PaneId>,
+        for_all_clients: bool,
         client_id: ClientId,
     ) {
+        if for_all_clients {
+            {
+                let mut current_pane_group = self.current_pane_group.borrow_mut();
+                current_pane_group.group_and_ungroup_panes_for_all_clients(
+                    pane_ids_to_group,
+                    pane_ids_to_ungroup,
+                    self.size,
+                );
+            }
+        } else {
             {
                 let mut current_pane_group = self.current_pane_group.borrow_mut();
                 current_pane_group.group_and_ungroup_panes(
@@ -3129,6 +3151,7 @@ impl Screen {
                     &client_id,
                 );
             }
+        }
         self.retain_only_existing_panes_in_pane_groups();
         let _ = self.log_and_report_session_state();
     }
@@ -5495,9 +5518,15 @@ pub(crate) fn screen_thread_main(
             ScreenInstruction::GroupAndUngroupPanes(
                 pane_ids_to_group,
                 pane_ids_to_ungroup,
+                for_all_clients,
                 client_id,
             ) => {
-                screen.group_and_ungroup_panes(pane_ids_to_group, pane_ids_to_ungroup, client_id);
+                screen.group_and_ungroup_panes(
+                    pane_ids_to_group,
+                    pane_ids_to_ungroup,
+                    for_all_clients,
+                    client_id,
+                );
                 let _ = screen.log_and_report_session_state();
             },
             ScreenInstruction::TogglePaneInGroup(client_id) => {

zellij-tile/src/shim.rs

@@ -1316,9 +1316,16 @@ pub fn stop_sharing_current_session() {
     unsafe { host_run_plugin_command() };
 }
 
-pub fn group_and_ungroup_panes(pane_ids_to_group: Vec<PaneId>, pane_ids_to_ungroup: Vec<PaneId>) {
-    let plugin_command =
-        PluginCommand::GroupAndUngroupPanes(pane_ids_to_group, pane_ids_to_ungroup);
+pub fn group_and_ungroup_panes(
+    pane_ids_to_group: Vec<PaneId>,
+    pane_ids_to_ungroup: Vec<PaneId>,
+    for_all_clients: bool,
+) {
+    let plugin_command = PluginCommand::GroupAndUngroupPanes(
+        pane_ids_to_group,
+        pane_ids_to_ungroup,
+        for_all_clients,
+    );
     let protobuf_plugin_command: ProtobufPluginCommand = plugin_command.try_into().unwrap();
     object_to_stdout(&protobuf_plugin_command.encode_to_vec());
     unsafe { host_run_plugin_command() };

zellij-utils/assets/prost/api.plugin_command.rs

@@ -292,6 +292,8 @@ pub struct GroupAndUngroupPanesPayload {
     pub pane_ids_to_group: ::prost::alloc::vec::Vec<PaneId>,
     #[prost(message, repeated, tag="2")]
     pub pane_ids_to_ungroup: ::prost::alloc::vec::Vec<PaneId>,
+    #[prost(bool, tag="3")]
+    pub for_all_clients: bool,
 }
 #[allow(clippy::derive_partial_eq_without_eq)]
 #[derive(Clone, PartialEq, ::prost::Message)]

zellij-utils/src/data.rs

@@ -2519,7 +2519,8 @@ pub enum PluginCommand {
     ShareCurrentSession,
     StopSharingCurrentSession,
     OpenFileInPlaceOfPlugin(FileToOpen, bool, Context), // bool -> close_plugin_after_replace
-    GroupAndUngroupPanes(Vec<PaneId>, Vec<PaneId>),     // panes to group, panes to ungroup
+    GroupAndUngroupPanes(Vec<PaneId>, Vec<PaneId>, bool), // panes to group, panes to ungroup,
+    // bool -> for all clients
     HighlightAndUnhighlightPanes(Vec<PaneId>, Vec<PaneId>), // panes to highlight, panes to
     // unhighlight
     CloseMultiplePanes(Vec<PaneId>),

zellij-utils/src/plugin_api/plugin_command.proto

@@ -316,6 +316,7 @@ message HighlightAndUnhighlightPanesPayload {
 message GroupAndUngroupPanesPayload {
   repeated PaneId pane_ids_to_group = 1;
   repeated PaneId pane_ids_to_ungroup = 2;
+  bool for_all_clients = 3;
 }
 
 message OpenFileInPlaceOfPluginPayload {

zellij-utils/src/plugin_api/plugin_command.rs

@@ -1591,6 +1591,7 @@ impl TryFrom<ProtobufPluginCommand> for PluginCommand {
                             .into_iter()
                             .filter_map(|p| p.try_into().ok())
                             .collect(),
+                        group_and_ungroup_panes_payload.for_all_clients,
                     ))
                 },
                 _ => Err("Mismatched payload for GroupAndUngroupPanes"),
@@ -2750,8 +2751,11 @@ impl TryFrom<PluginCommand> for ProtobufPluginCommand {
                     },
                 )),
             }),
-            PluginCommand::GroupAndUngroupPanes(panes_to_group, panes_to_ungroup) => {
-                Ok(ProtobufPluginCommand {
+            PluginCommand::GroupAndUngroupPanes(
+                panes_to_group,
+                panes_to_ungroup,
+                for_all_clients,
+            ) => Ok(ProtobufPluginCommand {
                 name: CommandName::GroupAndUngroupPanes as i32,
                 payload: Some(Payload::GroupAndUngroupPanesPayload(
                     GroupAndUngroupPanesPayload {
@@ -2763,10 +2767,10 @@ impl TryFrom<PluginCommand> for ProtobufPluginCommand {
                             .iter()
                             .filter_map(|&p| p.try_into().ok())
                             .collect(),
+                        for_all_clients,
                     },
                 )),
-                })
-            },
+            }),
             PluginCommand::StartWebServer => Ok(ProtobufPluginCommand {
                 name: CommandName::StartWebServer as i32,
                 payload: None,