From ea8a3be91b66ef5e4deed1edad91eb5f3fbe114b Mon Sep 17 00:00:00 2001 From: Smit Barmase Date: Thu, 29 May 2025 09:24:39 +0530 Subject: [PATCH] recent_projects: Move SSH server entry to initialize once instead of every render (#31650) Currently, `RemoteEntry::SshConfig` for `ssh_config_servers` initializes on every render. This leads to side effects like a new focus handle being created on every render, which leads to breaking navigating up/down for `ssh_config_servers` items. This PR fixes it by moving the logic of remote entry for`ssh_config_servers` into `default_mode`, and only rebuilding it when `ssh_config_servers` actually changes. Before: https://github.com/user-attachments/assets/8c7187d3-16b5-4f96-aa73-fe4f8227b7d0 After: https://github.com/user-attachments/assets/21588628-8b1c-43fb-bcb8-0b93c70a1e2b Release Notes: - Fixed issue navigating SSH config servers in Remote Projects with keyboard. --- crates/recent_projects/src/remote_servers.rs | 100 ++++++++++++------- 1 file changed, 62 insertions(+), 38 deletions(-) diff --git a/crates/recent_projects/src/remote_servers.rs b/crates/recent_projects/src/remote_servers.rs index 017d3e3d2b..c1a731ee13 100644 --- a/crates/recent_projects/src/remote_servers.rs +++ b/crates/recent_projects/src/remote_servers.rs @@ -289,15 +289,18 @@ struct DefaultState { scrollbar: ScrollbarState, add_new_server: NavigableEntry, servers: Vec, - handle: ScrollHandle, } impl DefaultState { - fn new(cx: &mut App) -> Self { + fn new(ssh_config_servers: &BTreeSet, cx: &mut App) -> Self { let handle = ScrollHandle::new(); let scrollbar = ScrollbarState::new(handle.clone()); let add_new_server = NavigableEntry::new(&handle, cx); - let servers = SshSettings::get_global(cx) + + let ssh_settings = SshSettings::get_global(cx); + let read_ssh_config = ssh_settings.read_ssh_config; + + let mut servers: Vec = ssh_settings .ssh_connections() .map(|connection| { let open_folder = NavigableEntry::new(&handle, cx); @@ -316,11 +319,25 @@ impl DefaultState { }) .collect(); + if read_ssh_config { + let mut extra_servers_from_config = ssh_config_servers.clone(); + for server in &servers { + if let RemoteEntry::Project { connection, .. } = server { + extra_servers_from_config.remove(&connection.host); + } + } + servers.extend(extra_servers_from_config.into_iter().map(|host| { + RemoteEntry::SshConfig { + open_folder: NavigableEntry::new(&handle, cx), + host, + } + })); + } + Self { scrollbar, add_new_server, servers, - handle, } } } @@ -340,8 +357,8 @@ enum Mode { } impl Mode { - fn default_mode(cx: &mut App) -> Self { - Self::Default(DefaultState::new(cx)) + fn default_mode(ssh_config_servers: &BTreeSet, cx: &mut App) -> Self { + Self::Default(DefaultState::new(ssh_config_servers, cx)) } } impl RemoteServerProjects { @@ -404,7 +421,7 @@ impl RemoteServerProjects { }); Self { - mode: Mode::default_mode(cx), + mode: Mode::default_mode(&BTreeSet::new(), cx), focus_handle, workspace, retained_connections: Vec::new(), @@ -481,7 +498,7 @@ impl RemoteServerProjects { telemetry::event!("SSH Server Created"); this.retained_connections.push(client); this.add_ssh_server(connection_options, cx); - this.mode = Mode::default_mode(cx); + this.mode = Mode::default_mode(&this.ssh_config_servers, cx); this.focus_handle(cx).focus(window); cx.notify() }) @@ -648,7 +665,7 @@ impl RemoteServerProjects { } } }); - self.mode = Mode::default_mode(cx); + self.mode = Mode::default_mode(&self.ssh_config_servers, cx); self.focus_handle.focus(window); } } @@ -668,7 +685,7 @@ impl RemoteServerProjects { cx.notify(); } _ => { - self.mode = Mode::default_mode(cx); + self.mode = Mode::default_mode(&self.ssh_config_servers, cx); self.focus_handle(cx).focus(window); cx.notify(); } @@ -1229,7 +1246,10 @@ impl RemoteServerProjects { .ok(); remote_servers .update(cx, |this, cx| { - this.mode = Mode::default_mode(cx); + this.mode = Mode::default_mode( + &this.ssh_config_servers, + cx, + ); cx.notify(); }) .ok(); @@ -1281,7 +1301,7 @@ impl RemoteServerProjects { .id("ssh-options-copy-server-address") .track_focus(&entries[3].focus_handle) .on_action(cx.listener(|this, _: &menu::Confirm, window, cx| { - this.mode = Mode::default_mode(cx); + this.mode = Mode::default_mode(&this.ssh_config_servers, cx); cx.focus_self(window); cx.notify(); })) @@ -1297,7 +1317,8 @@ impl RemoteServerProjects { ) .child(Label::new("Go Back")) .on_click(cx.listener(|this, _, window, cx| { - this.mode = Mode::default_mode(cx); + this.mode = + Mode::default_mode(&this.ssh_config_servers, cx); cx.focus_self(window); cx.notify() })), @@ -1358,7 +1379,8 @@ impl RemoteServerProjects { cx: &mut Context, ) -> impl IntoElement { let ssh_settings = SshSettings::get_global(cx); - let read_ssh_config = ssh_settings.read_ssh_config; + let mut should_rebuild = false; + if ssh_settings .ssh_connections .as_ref() @@ -1373,32 +1395,34 @@ impl RemoteServerProjects { .ne(connections.iter()) }) { - self.mode = Mode::default_mode(cx); + should_rebuild = true; + }; + + if !should_rebuild && ssh_settings.read_ssh_config { + let current_ssh_hosts: BTreeSet = state + .servers + .iter() + .filter_map(|server| match server { + RemoteEntry::SshConfig { host, .. } => Some(host.clone()), + _ => None, + }) + .collect(); + let mut expected_ssh_hosts = self.ssh_config_servers.clone(); + for server in &state.servers { + if let RemoteEntry::Project { connection, .. } = server { + expected_ssh_hosts.remove(&connection.host); + } + } + should_rebuild = current_ssh_hosts != expected_ssh_hosts; + } + + if should_rebuild { + self.mode = Mode::default_mode(&self.ssh_config_servers, cx); if let Mode::Default(new_state) = &self.mode { state = new_state.clone(); } } - let mut extra_servers_from_config = if read_ssh_config { - self.ssh_config_servers.clone() - } else { - BTreeSet::new() - }; - let mut servers = state.servers.clone(); - for server in &servers { - if let RemoteEntry::Project { connection, .. } = server { - extra_servers_from_config.remove(&connection.host); - } - } - servers.extend( - extra_servers_from_config - .into_iter() - .map(|host| RemoteEntry::SshConfig { - open_folder: NavigableEntry::new(&state.handle, cx), - host, - }), - ); - let scroll_state = state.scrollbar.parent_entity(&cx.entity()); let connect_button = div() .id("ssh-connect-new-server-container") @@ -1455,7 +1479,7 @@ impl RemoteServerProjects { ) .into_any_element(), ) - .children(servers.iter().enumerate().map(|(ix, connection)| { + .children(state.servers.iter().enumerate().map(|(ix, connection)| { self.render_ssh_connection(ix, connection.clone(), window, cx) .into_any_element() })), @@ -1464,7 +1488,7 @@ impl RemoteServerProjects { ) .entry(state.add_new_server.clone()); - for server in &servers { + for server in &state.servers { match server { RemoteEntry::Project { open_folder, @@ -1556,7 +1580,7 @@ impl RemoteServerProjects { }, cx, ); - self.mode = Mode::default_mode(cx); + self.mode = Mode::default_mode(&self.ssh_config_servers, cx); new_ix.load(atomic::Ordering::Acquire) } }