Cargo.lock 🔗
@@ -15994,7 +15994,6 @@ dependencies = [
"project",
"prompt_store",
"recent_projects",
- "serde",
"serde_json",
"settings",
"theme",
Address review feedback for sidebar collapsed state persistence
Richard Feldman created
- Fix broken doc comment on Sidebar struct (blank line split the comment)
- Move collapsed groups persistence from global KVP to per-window
MultiWorkspaceState, eliminating cross-window race conditions and
enabling proper per-window isolation with cross-session restore
- Prune stale collapsed groups during serialization by filtering
against currently known project headers
- Add tests for mixed collapsed/expanded state across multiple groups
and for empty state after uncollapsing all groupsChange summary
Cargo.lock | 1
crates/sidebar/Cargo.toml | 3
crates/sidebar/src/sidebar.rs | 173 ++++++++++++++++++------
crates/util/src/path_list.rs | 2
crates/workspace/src/multi_workspace.rs | 20 ++
crates/workspace/src/persistence.rs | 2
crates/workspace/src/persistence/model.rs | 2
crates/workspace/src/workspace.rs | 9 +
8 files changed, 158 insertions(+), 54 deletions(-)
Detailed changes
crates/sidebar/Cargo.toml 🔗
@@ -23,7 +23,6 @@ agent_settings.workspace = true
agent_ui.workspace = true
anyhow.workspace = true
chrono.workspace = true
-db.workspace = true
editor.workspace = true
feature_flags.workspace = true
fs.workspace = true
@@ -32,8 +31,6 @@ gpui.workspace = true
menu.workspace = true
project.workspace = true
recent_projects.workspace = true
-serde.workspace = true
-serde_json.workspace = true
settings.workspace = true
theme.workspace = true
ui.workspace = true
crates/sidebar/src/sidebar.rs 🔗
@@ -10,12 +10,12 @@ use agent_ui::{
Agent, AgentPanel, AgentPanelEvent, DEFAULT_THREAD_TITLE, NewThread, RemoveSelectedThread,
};
use chrono::Utc;
-use db::kvp::KeyValueStore;
+
use editor::Editor;
use feature_flags::{AgentV2FeatureFlag, FeatureFlagViewExt as _};
use gpui::{
Action as _, AnyElement, App, Context, Entity, FocusHandle, Focusable, KeyContext, ListState,
- Pixels, Render, SharedString, Task, WeakEntity, Window, WindowHandle, list, prelude::*, px,
+ Pixels, Render, SharedString, WeakEntity, Window, WindowHandle, list, prelude::*, px,
};
use menu::{
@@ -25,7 +25,6 @@ use project::{Event as ProjectEvent, linked_worktree_short_name};
use recent_projects::sidebar_recent_projects::SidebarRecentProjects;
use ui::utils::platform_title_bar_height;
-use serde::{Deserialize, Serialize};
use settings::Settings as _;
use std::collections::{HashMap, HashSet};
use std::mem;
@@ -35,12 +34,12 @@ use ui::{
AgentThreadStatus, CommonAnimationExt, ContextMenu, Divider, HighlightedLabel, KeyBinding,
PopoverMenu, PopoverMenuHandle, Tab, ThreadItem, TintColor, Tooltip, WithScrollbar, prelude::*,
};
+use util::ResultExt as _;
use util::path_list::PathList;
-use util::{ResultExt as _, TryFutureExt as _};
use workspace::{
AddFolderToProject, FocusWorkspaceSidebar, MultiWorkspace, MultiWorkspaceEvent, Open,
- SerializedPathList, Sidebar as WorkspaceSidebar, SidebarSide, ToggleWorkspaceSidebar,
- Workspace, WorkspaceId, sidebar_side_context_menu,
+ Sidebar as WorkspaceSidebar, SidebarSide, ToggleWorkspaceSidebar, Workspace, WorkspaceId,
+ sidebar_side_context_menu,
};
use zed_actions::OpenRecent;
@@ -66,8 +65,6 @@ const DEFAULT_WIDTH: Pixels = px(300.0);
const MIN_WIDTH: Pixels = px(200.0);
const MAX_WIDTH: Pixels = px(800.0);
const DEFAULT_THREADS_SHOWN: usize = 5;
-const SIDEBAR_COLLAPSED_GROUPS_NAMESPACE: &str = "agents_sidebar_collapsed_groups";
-const SIDEBAR_COLLAPSED_GROUPS_KEY: &str = "global";
#[derive(Debug, Default)]
enum SidebarView {
@@ -76,12 +73,6 @@ enum SidebarView {
Archive(Entity<ThreadsArchiveView>),
}
-#[derive(Debug, Serialize, Deserialize)]
-struct SerializedSidebarState {
- #[serde(default)]
- collapsed_groups: Vec<SerializedPathList>,
-}
-
#[derive(Clone, Debug)]
struct ActiveThreadInfo {
session_id: acp::SessionId,
@@ -240,24 +231,7 @@ fn workspace_path_list(workspace: &Entity<Workspace>, cx: &App) -> PathList {
PathList::new(&workspace.read(cx).root_paths(cx))
}
-fn load_collapsed_groups(kvp: &KeyValueStore) -> HashSet<PathList> {
- kvp.scoped(SIDEBAR_COLLAPSED_GROUPS_NAMESPACE)
- .read(SIDEBAR_COLLAPSED_GROUPS_KEY)
- .log_err()
- .flatten()
- .and_then(|json| serde_json::from_str::<SerializedSidebarState>(&json).log_err())
- .map(|state| {
- state
- .collapsed_groups
- .into_iter()
- .map(|path_list| PathList::deserialize(&path_list))
- .collect()
- })
- .unwrap_or_default()
-}
-
/// The sidebar re-derives its entire entry list from scratch on every
-
/// change via `update_entries` → `rebuild_contents`. Avoid adding
/// incremental or inter-event coordination state — if something can
/// be computed from the current world state, compute it in the rebuild.
@@ -282,7 +256,6 @@ pub struct Sidebar {
hovered_thread_index: Option<usize>,
collapsed_groups: HashSet<PathList>,
expanded_groups: HashMap<PathList, usize>,
- pending_serialization: Task<Option<()>>,
view: SidebarView,
recent_projects_popover_handle: PopoverMenuHandle<SidebarRecentProjects>,
project_header_menu_ix: Option<usize>,
@@ -310,7 +283,7 @@ impl Sidebar {
cx.subscribe_in(
&multi_workspace,
window,
- |this, _multi_workspace, event: &MultiWorkspaceEvent, window, cx| match event {
+ |this, multi_workspace, event: &MultiWorkspaceEvent, window, cx| match event {
MultiWorkspaceEvent::ActiveWorkspaceChanged => {
this.observe_draft_editor(cx);
this.update_entries(cx);
@@ -322,6 +295,18 @@ impl Sidebar {
MultiWorkspaceEvent::WorkspaceRemoved(_) => {
this.update_entries(cx);
}
+ MultiWorkspaceEvent::SidebarCollapsedGroupsChanged => {
+ let groups: HashSet<PathList> = multi_workspace
+ .read(cx)
+ .sidebar_collapsed_groups()
+ .iter()
+ .map(|s| PathList::deserialize(s))
+ .collect();
+ if groups != this.collapsed_groups {
+ this.collapsed_groups = groups;
+ this.update_entries(cx);
+ }
+ }
},
)
.detach();
@@ -353,7 +338,12 @@ impl Sidebar {
})
.detach();
- let collapsed_groups = load_collapsed_groups(&KeyValueStore::global(cx));
+ let collapsed_groups: HashSet<PathList> = multi_workspace
+ .read(cx)
+ .sidebar_collapsed_groups()
+ .iter()
+ .map(|s| PathList::deserialize(s))
+ .collect();
let workspaces = multi_workspace.read(cx).workspaces().to_vec();
cx.defer_in(window, move |this, window, cx| {
@@ -377,7 +367,6 @@ impl Sidebar {
hovered_thread_index: None,
collapsed_groups,
expanded_groups: HashMap::new(),
- pending_serialization: Task::ready(None),
view: SidebarView::default(),
recent_projects_popover_handle: PopoverMenuHandle::default(),
project_header_menu_ix: None,
@@ -1719,24 +1708,27 @@ impl Sidebar {
}
fn serialize_collapsed_groups(&mut self, cx: &mut Context<Self>) {
+ let known_path_lists: HashSet<&PathList> = self
+ .contents
+ .entries
+ .iter()
+ .filter_map(|entry| match entry {
+ ListEntry::ProjectHeader { path_list, .. } => Some(path_list),
+ _ => None,
+ })
+ .collect();
let collapsed_groups = self
.collapsed_groups
.iter()
+ .filter(|path_list| known_path_lists.contains(path_list))
.map(PathList::serialize)
.collect::<Vec<_>>();
- let kvp = KeyValueStore::global(cx);
- self.pending_serialization = cx.background_spawn(
- async move {
- kvp.scoped(SIDEBAR_COLLAPSED_GROUPS_NAMESPACE)
- .write(
- SIDEBAR_COLLAPSED_GROUPS_KEY.to_string(),
- serde_json::to_string(&SerializedSidebarState { collapsed_groups })?,
- )
- .await?;
- anyhow::Ok(())
- }
- .log_err(),
- );
+ if let Some(multi_workspace) = self.multi_workspace.upgrade() {
+ multi_workspace.update(cx, |mw, cx| {
+ mw.set_sidebar_collapsed_groups(collapsed_groups, cx);
+ mw.serialize(cx);
+ });
+ }
}
fn toggle_collapse(
@@ -3843,6 +3835,91 @@ mod tests {
);
}
+ #[gpui::test]
+ async fn test_collapsed_groups_mixed_state_restore(cx: &mut TestAppContext) {
+ init_test(cx);
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_tree("/project-a", serde_json::json!({ "src": {} }))
+ .await;
+ fs.insert_tree("/project-b", serde_json::json!({ "src": {} }))
+ .await;
+ cx.update(|cx| <dyn fs::Fs>::set_global(fs.clone(), cx));
+
+ let project_a = project::Project::test(fs.clone(), ["/project-a".as_ref()], cx).await;
+ let project_b = project::Project::test(fs.clone(), ["/project-b".as_ref()], cx).await;
+
+ let (multi_workspace, cx) =
+ cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a, window, cx));
+ multi_workspace.update_in(cx, |mw, window, cx| {
+ mw.test_add_workspace(project_b, window, cx);
+ });
+ let sidebar = setup_sidebar(&multi_workspace, cx);
+
+ let path_a = PathList::new(&[std::path::PathBuf::from("/project-a")]);
+ let path_b = PathList::new(&[std::path::PathBuf::from("/project-b")]);
+ save_n_test_threads(1, &path_a, cx).await;
+ save_named_thread_metadata("thread-b", "Thread B", &path_b, cx).await;
+
+ multi_workspace.update_in(cx, |_, _window, cx| cx.notify());
+ cx.run_until_parked();
+
+ // Collapse only project-a, leave project-b expanded
+ sidebar.update_in(cx, |s, window, cx| {
+ s.toggle_collapse(&path_a, window, cx);
+ });
+ cx.run_until_parked();
+
+ assert_eq!(
+ visible_entries_as_strings(&sidebar, cx),
+ vec!["> [project-a]", "v [project-b]", " Thread B"]
+ );
+
+ // Recreate sidebar to simulate restart
+ let restored_sidebar = setup_sidebar(&multi_workspace, cx);
+ multi_workspace.update_in(cx, |_, _window, cx| cx.notify());
+ cx.run_until_parked();
+
+ // project-a should still be collapsed, project-b should still be expanded
+ assert_eq!(
+ visible_entries_as_strings(&restored_sidebar, cx),
+ vec!["> [project-a]", "v [project-b]", " Thread B"]
+ );
+ }
+
+ #[gpui::test]
+ async fn test_collapsed_groups_empty_after_uncollapse_all(cx: &mut TestAppContext) {
+ let project = init_test_project("/my-project", cx).await;
+ let (multi_workspace, cx) =
+ cx.add_window_view(|window, cx| MultiWorkspace::test_new(project, window, cx));
+ let sidebar = setup_sidebar(&multi_workspace, cx);
+
+ let path_list = PathList::new(&[std::path::PathBuf::from("/my-project")]);
+ save_n_test_threads(1, &path_list, cx).await;
+
+ multi_workspace.update_in(cx, |_, _window, cx| cx.notify());
+ cx.run_until_parked();
+
+ // Collapse, then uncollapse
+ sidebar.update_in(cx, |s, window, cx| {
+ s.toggle_collapse(&path_list, window, cx);
+ });
+ cx.run_until_parked();
+ sidebar.update_in(cx, |s, window, cx| {
+ s.toggle_collapse(&path_list, window, cx);
+ });
+ cx.run_until_parked();
+
+ // Recreate sidebar — everything should be expanded
+ let restored_sidebar = setup_sidebar(&multi_workspace, cx);
+ multi_workspace.update_in(cx, |_, _window, cx| cx.notify());
+ cx.run_until_parked();
+
+ assert_eq!(
+ visible_entries_as_strings(&restored_sidebar, cx),
+ vec!["v [my-project]", " Thread 1"]
+ );
+ }
+
#[gpui::test]
async fn test_visible_entries_as_strings(cx: &mut TestAppContext) {
let project = init_test_project("/my-project", cx).await;
crates/util/src/path_list.rs 🔗
@@ -38,7 +38,7 @@ impl Hash for PathList {
}
}
-#[derive(Debug, Serialize, Deserialize)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct SerializedPathList {
pub paths: String,
pub order: String,
crates/workspace/src/multi_workspace.rs 🔗
@@ -15,6 +15,7 @@ use std::path::PathBuf;
use std::sync::Arc;
use ui::prelude::*;
use util::ResultExt;
+use util::path_list::SerializedPathList;
use zed_actions::agents_sidebar::MoveWorkspaceToNewWindow;
use agent_settings::AgentSettings;
@@ -89,6 +90,7 @@ pub enum MultiWorkspaceEvent {
ActiveWorkspaceChanged,
WorkspaceAdded(Entity<Workspace>),
WorkspaceRemoved(EntityId),
+ SidebarCollapsedGroupsChanged,
}
pub trait Sidebar: Focusable + Render + Sized {
@@ -177,6 +179,7 @@ pub struct MultiWorkspace {
active_workspace_index: usize,
sidebar: Option<Box<dyn SidebarHandle>>,
sidebar_open: bool,
+ sidebar_collapsed_groups: Vec<SerializedPathList>,
pending_removal_tasks: Vec<Task<()>>,
_serialize_task: Option<Task<()>>,
_subscriptions: Vec<Subscription>,
@@ -225,6 +228,7 @@ impl MultiWorkspace {
active_workspace_index: 0,
sidebar: None,
sidebar_open: false,
+ sidebar_collapsed_groups: Vec::new(),
pending_removal_tasks: Vec::new(),
_serialize_task: None,
_subscriptions: vec![
@@ -251,6 +255,19 @@ impl MultiWorkspace {
self.sidebar_open
}
+ pub fn sidebar_collapsed_groups(&self) -> &[SerializedPathList] {
+ &self.sidebar_collapsed_groups
+ }
+
+ pub fn set_sidebar_collapsed_groups(
+ &mut self,
+ groups: Vec<SerializedPathList>,
+ cx: &mut Context<Self>,
+ ) {
+ self.sidebar_collapsed_groups = groups;
+ cx.emit(MultiWorkspaceEvent::SidebarCollapsedGroupsChanged);
+ }
+
pub fn sidebar_has_notifications(&self, cx: &App) -> bool {
self.sidebar
.as_ref()
@@ -487,11 +504,12 @@ impl MultiWorkspace {
self.cycle_workspace(-1, window, cx);
}
- fn serialize(&mut self, cx: &mut App) {
+ pub fn serialize(&mut self, cx: &mut App) {
let window_id = self.window_id;
let state = crate::persistence::model::MultiWorkspaceState {
active_workspace_id: self.workspace().read(cx).database_id(),
sidebar_open: self.sidebar_open,
+ collapsed_sidebar_groups: self.sidebar_collapsed_groups.clone(),
};
let kvp = db::kvp::KeyValueStore::global(cx);
self._serialize_task = Some(cx.background_spawn(async move {
crates/workspace/src/persistence.rs 🔗
@@ -3978,6 +3978,7 @@ mod tests {
MultiWorkspaceState {
active_workspace_id: Some(WorkspaceId(2)),
sidebar_open: true,
+ collapsed_sidebar_groups: Vec::new(),
},
)
.await;
@@ -3988,6 +3989,7 @@ mod tests {
MultiWorkspaceState {
active_workspace_id: Some(WorkspaceId(3)),
sidebar_open: false,
+ collapsed_sidebar_groups: Vec::new(),
},
)
.await;
crates/workspace/src/persistence/model.rs 🔗
@@ -64,6 +64,8 @@ pub struct SessionWorkspace {
pub struct MultiWorkspaceState {
pub active_workspace_id: Option<WorkspaceId>,
pub sidebar_open: bool,
+ #[serde(default)]
+ pub collapsed_sidebar_groups: Vec<util::path_list::SerializedPathList>,
}
/// The serialized state of a single MultiWorkspace window from a previous session:
crates/workspace/src/workspace.rs 🔗
@@ -8610,6 +8610,15 @@ pub async fn restore_multiworkspace(
.ok();
}
+ if !state.collapsed_sidebar_groups.is_empty() {
+ let collapsed_groups = state.collapsed_sidebar_groups;
+ window_handle
+ .update(cx, |multi_workspace, _, cx| {
+ multi_workspace.set_sidebar_collapsed_groups(collapsed_groups, cx);
+ })
+ .ok();
+ }
+
window_handle
.update(cx, |_, window, _cx| {
window.activate_window();