From 5edb40c7a8dc718ef6eb29a3f6d109eeacb3a496 Mon Sep 17 00:00:00 2001 From: KyleBarton Date: Fri, 3 Apr 2026 11:51:31 -0700 Subject: [PATCH 1/8] Use an object for docker compose ports rather than raw string (#53090) Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Closes #53048 Release Notes: - Fixed serialization error with Docker Compose for dev containers --- .../src/devcontainer_manifest.rs | 77 +++++++++++++++---- crates/dev_container/src/docker.rs | 76 +++++++++++++++++- 2 files changed, 137 insertions(+), 16 deletions(-) diff --git a/crates/dev_container/src/devcontainer_manifest.rs b/crates/dev_container/src/devcontainer_manifest.rs index 8529604be9b1f3728b9638c2ca6852ff741c6ce2..0ba7e8c82a036477103e18db0940f8950fb875d2 100644 --- a/crates/dev_container/src/devcontainer_manifest.rs +++ b/crates/dev_container/src/devcontainer_manifest.rs @@ -20,7 +20,8 @@ use crate::{ }, docker::{ Docker, DockerClient, DockerComposeConfig, DockerComposeService, DockerComposeServiceBuild, - DockerComposeVolume, DockerInspect, DockerPs, get_remote_dir_from_config, + DockerComposeServicePort, DockerComposeVolume, DockerInspect, DockerPs, + get_remote_dir_from_config, }, features::{DevContainerFeatureJson, FeatureManifest, parse_oci_feature_ref}, get_oci_token, @@ -1137,18 +1138,30 @@ RUN sed -i -E 's/((^|\s)PATH=)([^\$]*)$/\1\${{PATH:-\3}}/g' /etc/profile || true // If the main service uses a different service's network bridge, append to that service's ports instead if let Some(network_service_name) = network_mode_service { if let Some(service) = service_declarations.get_mut(network_service_name) { - service.ports.push(format!("{port}:{port}")); + service.ports.push(DockerComposeServicePort { + target: port.clone(), + published: port.clone(), + ..Default::default() + }); } else { service_declarations.insert( network_service_name.to_string(), DockerComposeService { - ports: vec![format!("{port}:{port}")], + ports: vec![DockerComposeServicePort { + target: port.clone(), + published: port.clone(), + ..Default::default() + }], ..Default::default() }, ); } } else { - main_service.ports.push(format!("{port}:{port}")); + main_service.ports.push(DockerComposeServicePort { + target: port.clone(), + published: port.clone(), + ..Default::default() + }); } } let other_service_ports: Vec<(&str, &str)> = forward_ports @@ -1171,12 +1184,20 @@ RUN sed -i -E 's/((^|\s)PATH=)([^\$]*)$/\1\${{PATH:-\3}}/g' /etc/profile || true .collect(); for (service_name, port) in other_service_ports { if let Some(service) = service_declarations.get_mut(service_name) { - service.ports.push(format!("{port}:{port}")); + service.ports.push(DockerComposeServicePort { + target: port.to_string(), + published: port.to_string(), + ..Default::default() + }); } else { service_declarations.insert( service_name.to_string(), DockerComposeService { - ports: vec![format!("{port}:{port}")], + ports: vec![DockerComposeServicePort { + target: port.to_string(), + published: port.to_string(), + ..Default::default() + }], ..Default::default() }, ); @@ -1186,18 +1207,30 @@ RUN sed -i -E 's/((^|\s)PATH=)([^\$]*)$/\1\${{PATH:-\3}}/g' /etc/profile || true if let Some(port) = &self.dev_container().app_port { if let Some(network_service_name) = network_mode_service { if let Some(service) = service_declarations.get_mut(network_service_name) { - service.ports.push(format!("{port}:{port}")); + service.ports.push(DockerComposeServicePort { + target: port.clone(), + published: port.clone(), + ..Default::default() + }); } else { service_declarations.insert( network_service_name.to_string(), DockerComposeService { - ports: vec![format!("{port}:{port}")], + ports: vec![DockerComposeServicePort { + target: port.clone(), + published: port.clone(), + ..Default::default() + }], ..Default::default() }, ); } } else { - main_service.ports.push(format!("{port}:{port}")); + main_service.ports.push(DockerComposeServicePort { + target: port.clone(), + published: port.clone(), + ..Default::default() + }); } } @@ -3278,6 +3311,8 @@ chmod +x ./install.sh #[cfg(not(target_os = "windows"))] #[gpui::test] async fn test_spawns_devcontainer_with_docker_compose(cx: &mut TestAppContext) { + use crate::docker::DockerComposeServicePort; + cx.executor().allow_parking(); env_logger::try_init().ok(); let given_devcontainer_contents = r#" @@ -3540,10 +3575,26 @@ ENV DOCKER_BUILDKIT=1 "db".to_string(), DockerComposeService { ports: vec![ - "8083:8083".to_string(), - "5432:5432".to_string(), - "1234:1234".to_string(), - "8084:8084".to_string() + DockerComposeServicePort { + target: "8083".to_string(), + published: "8083".to_string(), + ..Default::default() + }, + DockerComposeServicePort { + target: "5432".to_string(), + published: "5432".to_string(), + ..Default::default() + }, + DockerComposeServicePort { + target: "1234".to_string(), + published: "1234".to_string(), + ..Default::default() + }, + DockerComposeServicePort { + target: "8084".to_string(), + published: "8084".to_string(), + ..Default::default() + }, ], ..Default::default() }, diff --git a/crates/dev_container/src/docker.rs b/crates/dev_container/src/docker.rs index 1658acfadc059327e2e7b43d393324e9f37d42db..9320ec360968425cf85644e96b12c1d089c1f05f 100644 --- a/crates/dev_container/src/docker.rs +++ b/crates/dev_container/src/docker.rs @@ -86,6 +86,43 @@ pub(crate) struct DockerComposeServiceBuild { pub(crate) additional_contexts: Option>, } +#[derive(Debug, Clone, Deserialize, Serialize, Eq, PartialEq, Default)] +pub(crate) struct DockerComposeServicePort { + #[serde(deserialize_with = "deserialize_string_or_int")] + pub(crate) target: String, + #[serde(deserialize_with = "deserialize_string_or_int")] + pub(crate) published: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub(crate) mode: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub(crate) protocol: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub(crate) host_ip: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub(crate) app_protocol: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub(crate) name: Option, +} + +fn deserialize_string_or_int<'de, D>(deserializer: D) -> Result +where + D: serde::Deserializer<'de>, +{ + use serde::Deserialize; + + #[derive(Deserialize)] + #[serde(untagged)] + enum StringOrInt { + String(String), + Int(u32), + } + + match StringOrInt::deserialize(deserializer)? { + StringOrInt::String(s) => Ok(s), + StringOrInt::Int(b) => Ok(b.to_string()), + } +} + #[derive(Debug, Clone, Deserialize, Serialize, Eq, PartialEq, Default)] pub(crate) struct DockerComposeService { pub(crate) image: Option, @@ -109,7 +146,7 @@ pub(crate) struct DockerComposeService { #[serde(skip_serializing_if = "Option::is_none")] pub(crate) env_file: Option>, #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub(crate) ports: Vec, + pub(crate) ports: Vec, #[serde(skip_serializing_if = "Option::is_none")] pub(crate) network_mode: Option, } @@ -491,8 +528,8 @@ mod test { command_json::deserialize_json_output, devcontainer_json::MountDefinition, docker::{ - Docker, DockerComposeConfig, DockerComposeService, DockerComposeVolume, DockerInspect, - DockerPs, get_remote_dir_from_config, + Docker, DockerComposeConfig, DockerComposeService, DockerComposeServicePort, + DockerComposeVolume, DockerInspect, DockerPs, get_remote_dir_from_config, }, }; @@ -879,6 +916,22 @@ mod test { "POSTGRES_PORT": "5432", "POSTGRES_USER": "postgres" }, + "ports": [ + { + "target": "5443", + "published": "5442" + }, + { + "name": "custom port", + "protocol": "udp", + "host_ip": "127.0.0.1", + "app_protocol": "http", + "mode": "host", + "target": "8081", + "published": "8083" + + } + ], "image": "mcr.microsoft.com/devcontainers/rust:2-1-bookworm", "network_mode": "service:db", "volumes": [ @@ -943,6 +996,23 @@ mod test { target: "/workspaces".to_string(), }], network_mode: Some("service:db".to_string()), + + ports: vec![ + DockerComposeServicePort { + target: "5443".to_string(), + published: "5442".to_string(), + ..Default::default() + }, + DockerComposeServicePort { + target: "8081".to_string(), + published: "8083".to_string(), + mode: Some("host".to_string()), + protocol: Some("udp".to_string()), + host_ip: Some("127.0.0.1".to_string()), + app_protocol: Some("http".to_string()), + name: Some("custom port".to_string()), + }, + ], ..Default::default() }, ), From e4ebd3aae5c243087940f48de5cf27c46a569525 Mon Sep 17 00:00:00 2001 From: Oleksiy Syvokon Date: Fri, 3 Apr 2026 22:31:03 +0300 Subject: [PATCH 2/8] Fix crash in WgpuAtlas when viewing a screen share (#53088) When atlas tiles are rapidly allocated and freed (e.g. watching a shared screen in Collab), a texture can become unreferenced and be removed while GPU uploads for it are still pending. On the next frame, `flush_uploads` indexes into the now-empty texture slot and panics: ``` thread 'main' panicked at crates/gpui_wgpu/src/wgpu_atlas.rs:231:40: texture must exist... #11 core::option::expect_failed #12 gpui_wgpu::wgpu_atlas::WgpuAtlas::before_frame #13 gpui_wgpu::wgpu_renderer::WgpuRenderer::draw ``` This change drains pending uploads for a texture when it becomes unreferenced in `remove`, and skips uploads for missing textures in `flush_uploads` as a safety net. Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - Fixed occasional crashes when viewing a screen share --- crates/gpui_wgpu/src/wgpu_atlas.rs | 82 +++++++++++++++++++++++++++++- 1 file changed, 81 insertions(+), 1 deletion(-) diff --git a/crates/gpui_wgpu/src/wgpu_atlas.rs b/crates/gpui_wgpu/src/wgpu_atlas.rs index 3eba5c533f80d727425cc87ae89b754afa8722b1..55f6edee21b9f2da02268c66c665c34d5b52066a 100644 --- a/crates/gpui_wgpu/src/wgpu_atlas.rs +++ b/crates/gpui_wgpu/src/wgpu_atlas.rs @@ -115,6 +115,8 @@ impl PlatformAtlas for WgpuAtlas { if let Some(mut texture) = texture_slot.take() { texture.decrement_ref_count(); if texture.is_unreferenced() { + lock.pending_uploads + .retain(|upload| upload.id != texture.id); lock.storage[id.kind] .free_list .push(texture.id.index as usize); @@ -228,7 +230,9 @@ impl WgpuAtlasState { fn flush_uploads(&mut self) { for upload in self.pending_uploads.drain(..) { - let texture = &self.storage[upload.id]; + let Some(texture) = self.storage.get(upload.id) else { + continue; + }; let bytes_per_pixel = texture.bytes_per_pixel(); self.queue.write_texture( @@ -286,6 +290,15 @@ impl ops::IndexMut for WgpuAtlasStorage { } } +impl WgpuAtlasStorage { + fn get(&self, id: AtlasTextureId) -> Option<&WgpuAtlasTexture> { + self[id.kind] + .textures + .get(id.index as usize) + .and_then(|t| t.as_ref()) + } +} + impl ops::Index for WgpuAtlasStorage { type Output = WgpuAtlasTexture; fn index(&self, id: AtlasTextureId) -> &Self::Output { @@ -341,3 +354,70 @@ impl WgpuAtlasTexture { self.live_atlas_keys == 0 } } + +#[cfg(all(test, not(target_family = "wasm")))] +mod tests { + use super::*; + use gpui::{ImageId, RenderImageParams}; + use pollster::block_on; + use std::sync::Arc; + + fn test_device_and_queue() -> anyhow::Result<(Arc, Arc)> { + block_on(async { + let instance = wgpu::Instance::new(wgpu::InstanceDescriptor { + backends: wgpu::Backends::all(), + flags: wgpu::InstanceFlags::default(), + backend_options: wgpu::BackendOptions::default(), + memory_budget_thresholds: wgpu::MemoryBudgetThresholds::default(), + display: None, + }); + let adapter = instance + .request_adapter(&wgpu::RequestAdapterOptions { + power_preference: wgpu::PowerPreference::LowPower, + compatible_surface: None, + force_fallback_adapter: false, + }) + .await + .map_err(|error| anyhow::anyhow!("failed to request adapter: {error}"))?; + let (device, queue) = adapter + .request_device(&wgpu::DeviceDescriptor { + label: Some("wgpu_atlas_test_device"), + required_features: wgpu::Features::empty(), + required_limits: wgpu::Limits::downlevel_defaults() + .using_resolution(adapter.limits()) + .using_alignment(adapter.limits()), + memory_hints: wgpu::MemoryHints::MemoryUsage, + trace: wgpu::Trace::Off, + experimental_features: wgpu::ExperimentalFeatures::disabled(), + }) + .await + .map_err(|error| anyhow::anyhow!("failed to request device: {error}"))?; + Ok((Arc::new(device), Arc::new(queue))) + }) + } + + #[test] + fn before_frame_skips_uploads_for_removed_texture() -> anyhow::Result<()> { + let (device, queue) = test_device_and_queue()?; + + let atlas = WgpuAtlas::new(device, queue); + let key = AtlasKey::Image(RenderImageParams { + image_id: ImageId(1), + frame_index: 0, + }); + let size = Size { + width: DevicePixels(1), + height: DevicePixels(1), + }; + let mut build = || Ok(Some((size, Cow::Owned(vec![0, 0, 0, 255])))); + + // Regression test: before the fix, this panicked in flush_uploads + atlas + .get_or_insert_with(&key, &mut build)? + .expect("tile should be created"); + atlas.remove(&key); + atlas.before_frame(); + + Ok(()) + } +} From 203f48d25cfd1a923a70941bdb388109f06d6e13 Mon Sep 17 00:00:00 2001 From: Josh Robson Chase Date: Fri, 3 Apr 2026 15:42:00 -0400 Subject: [PATCH 3/8] workspace: Implement focus-follows-mouse for panes (#46740) Implements basic focus-follows-mouse behavior. Right now, it's only applied in the `workspace` crate for `Pane`s, so anything that lives outside of that container (panels and such for the most part) won't have this behavior applied. The core logic is implemented as an extension trait, and should be trivial to apply to other elements as it makes sense. https://github.com/user-attachments/assets/d338fa30-7f9c-439f-8b50-1720e3f509b1 Closes #8167 Release Notes: - Added "Focus Follows Mouse" for editor and terminal panes --------- Co-authored-by: Conrad Irwin --- assets/settings/default.json | 5 ++ crates/settings/src/vscode_import.rs | 1 + crates/settings_content/src/workspace.rs | 10 +++ crates/settings_ui/src/page_data.rs | 48 +++++++++++++- crates/workspace/src/dock.rs | 10 ++- crates/workspace/src/focus_follows_mouse.rs | 71 +++++++++++++++++++++ crates/workspace/src/pane.rs | 13 +++- crates/workspace/src/workspace.rs | 5 +- crates/workspace/src/workspace_settings.rs | 23 ++++++- 9 files changed, 178 insertions(+), 8 deletions(-) create mode 100644 crates/workspace/src/focus_follows_mouse.rs diff --git a/assets/settings/default.json b/assets/settings/default.json index 74a4e15a044fa5686441f2e8a587595936ea08fb..e9d21eb0dcc18ae939a41e3415b93eaeba1e4546 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -225,6 +225,11 @@ // 3. Hide on both typing and cursor movement: // "on_typing_and_movement" "hide_mouse": "on_typing_and_movement", + // Determines whether the focused panel follows the mouse location. + "focus_follows_mouse": { + "enabled": false, + "debounce_ms": 250, + }, // Determines how snippets are sorted relative to other completion items. // // 1. Place snippets at the top of the completion list: diff --git a/crates/settings/src/vscode_import.rs b/crates/settings/src/vscode_import.rs index c40b38c460a17f30b1fce26c50b40a893f7724a8..1211cbd8a4519ea295773eb0d979b48258908311 100644 --- a/crates/settings/src/vscode_import.rs +++ b/crates/settings/src/vscode_import.rs @@ -999,6 +999,7 @@ impl VsCodeSettings { } }), zoomed_padding: None, + focus_follows_mouse: None, } } diff --git a/crates/settings_content/src/workspace.rs b/crates/settings_content/src/workspace.rs index ef00a44790fd10b8c56278362a2f552a40f52cbb..0bae7c260f6607f2015f750e5bb9dec7cc26342d 100644 --- a/crates/settings_content/src/workspace.rs +++ b/crates/settings_content/src/workspace.rs @@ -122,6 +122,9 @@ pub struct WorkspaceSettingsContent { /// What draws window decorations/titlebar, the client application (Zed) or display server /// Default: client pub window_decorations: Option, + /// Whether the focused panel follows the mouse location + /// Default: false + pub focus_follows_mouse: Option, } #[with_fallible_options] @@ -928,3 +931,10 @@ impl DocumentSymbols { self == &Self::On } } + +#[with_fallible_options] +#[derive(Copy, Clone, PartialEq, Default, Serialize, Deserialize, JsonSchema, MergeFrom, Debug)] +pub struct FocusFollowsMouse { + pub enabled: Option, + pub debounce_ms: Option, +} diff --git a/crates/settings_ui/src/page_data.rs b/crates/settings_ui/src/page_data.rs index f0cf87c403b340dacd33e2c04b043ab8085a461a..828a574115c4664b3ab2f37f32ad4087363b3978 100644 --- a/crates/settings_ui/src/page_data.rs +++ b/crates/settings_ui/src/page_data.rs @@ -4159,7 +4159,7 @@ fn window_and_layout_page() -> SettingsPage { ] } - fn layout_section() -> [SettingsPageItem; 4] { + fn layout_section() -> [SettingsPageItem; 6] { [ SettingsPageItem::SectionHeader("Layout"), SettingsPageItem::SettingItem(SettingItem { @@ -4223,6 +4223,52 @@ fn window_and_layout_page() -> SettingsPage { }), metadata: None, }), + SettingsPageItem::SettingItem(SettingItem { + title: "Focus Follows Mouse", + description: "Whether to change focus to a pane when the mouse hovers over it.", + field: Box::new(SettingField { + json_path: Some("focus_follows_mouse.enabled"), + pick: |settings_content| { + settings_content + .workspace + .focus_follows_mouse + .as_ref() + .and_then(|s| s.enabled.as_ref()) + }, + write: |settings_content, value| { + settings_content + .workspace + .focus_follows_mouse + .get_or_insert_default() + .enabled = value; + }, + }), + metadata: None, + files: USER, + }), + SettingsPageItem::SettingItem(SettingItem { + title: "Focus Follows Mouse Debounce ms", + description: "Amount of time to wait before changing focus.", + field: Box::new(SettingField { + json_path: Some("focus_follows_mouse.debounce_ms"), + pick: |settings_content| { + settings_content + .workspace + .focus_follows_mouse + .as_ref() + .and_then(|s| s.debounce_ms.as_ref()) + }, + write: |settings_content, value| { + settings_content + .workspace + .focus_follows_mouse + .get_or_insert_default() + .debounce_ms = value; + }, + }), + metadata: None, + files: USER, + }), ] } diff --git a/crates/workspace/src/dock.rs b/crates/workspace/src/dock.rs index e36b48f06fd3ca0983b13ddb564af08ddab9fba5..e58b4b59100c05085c93993370b85a788fc159ca 100644 --- a/crates/workspace/src/dock.rs +++ b/crates/workspace/src/dock.rs @@ -1,5 +1,6 @@ +use crate::focus_follows_mouse::FocusFollowsMouse as _; use crate::persistence::model::DockData; -use crate::{DraggedDock, Event, ModalLayer, Pane}; +use crate::{DraggedDock, Event, FocusFollowsMouse, ModalLayer, Pane, WorkspaceSettings}; use crate::{Workspace, status_bar::StatusItemView}; use anyhow::Context as _; use client::proto; @@ -12,7 +13,7 @@ use gpui::{ px, }; use serde::{Deserialize, Serialize}; -use settings::SettingsStore; +use settings::{Settings, SettingsStore}; use std::sync::Arc; use ui::{ ContextMenu, CountBadge, Divider, DividerColor, IconButton, Tooltip, prelude::*, @@ -252,6 +253,7 @@ pub struct Dock { is_open: bool, active_panel_index: Option, focus_handle: FocusHandle, + focus_follows_mouse: FocusFollowsMouse, pub(crate) serialized_dock: Option, zoom_layer_open: bool, modal_layer: Entity, @@ -376,6 +378,7 @@ impl Dock { active_panel_index: None, is_open: false, focus_handle: focus_handle.clone(), + focus_follows_mouse: WorkspaceSettings::get_global(cx).focus_follows_mouse, _subscriptions: [focus_subscription, zoom_subscription], serialized_dock: None, zoom_layer_open: false, @@ -1086,8 +1089,10 @@ impl Render for Dock { }; div() + .id("dock-panel") .key_context(dispatch_context) .track_focus(&self.focus_handle(cx)) + .focus_follows_mouse(self.focus_follows_mouse, cx) .flex() .bg(cx.theme().colors().panel_background) .border_color(cx.theme().colors().border) @@ -1121,6 +1126,7 @@ impl Render for Dock { }) } else { div() + .id("dock-panel") .key_context(dispatch_context) .track_focus(&self.focus_handle(cx)) } diff --git a/crates/workspace/src/focus_follows_mouse.rs b/crates/workspace/src/focus_follows_mouse.rs new file mode 100644 index 0000000000000000000000000000000000000000..da433cefcf059960181c190da83b06260651b063 --- /dev/null +++ b/crates/workspace/src/focus_follows_mouse.rs @@ -0,0 +1,71 @@ +use gpui::{ + AnyWindowHandle, AppContext as _, Context, FocusHandle, Focusable, Global, + StatefulInteractiveElement, Task, +}; + +use crate::workspace_settings; + +#[derive(Default)] +struct FfmState { + // The window and element to be focused + handles: Option<(AnyWindowHandle, FocusHandle)>, + // The debounced task which will do the focusing + _debounce_task: Option>, +} + +impl Global for FfmState {} + +pub trait FocusFollowsMouse: StatefulInteractiveElement { + fn focus_follows_mouse( + self, + settings: workspace_settings::FocusFollowsMouse, + cx: &Context, + ) -> Self { + if settings.enabled { + self.on_hover(cx.listener(move |this, enter, window, cx| { + if *enter { + let window_handle = window.window_handle(); + let focus_handle = this.focus_handle(cx); + + let state = cx.try_global::(); + + // Only replace the target if the new handle doesn't contain the existing one. + // This ensures that hovering over a parent (e.g., Dock) doesn't override + // a more specific child target (e.g., a Pane inside the Dock). + let should_replace = state + .and_then(|s| s.handles.as_ref()) + .map(|(_, existing)| !focus_handle.contains(existing, window)) + .unwrap_or(true); + + if !should_replace { + return; + } + + let debounce_task = cx.spawn(async move |_this, cx| { + cx.background_executor().timer(settings.debounce).await; + + cx.update(|cx| { + let state = cx.default_global::(); + let Some((window, focus)) = state.handles.take() else { + return; + }; + + let _ = cx.update_window(window, move |_view, window, cx| { + window.focus(&focus, cx); + }); + }); + }); + + cx.set_global(FfmState { + handles: Some((window_handle, focus_handle)), + _debounce_task: Some(debounce_task), + }); + } + })) + } else { + self + } + } +} + +impl FocusFollowsMouse for T {} diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index deb7e1efef37acff992d8f5be5825741e887b979..92f0781f82234ce79d47db08785b6592fb53f566 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -2,6 +2,7 @@ use crate::{ CloseWindow, NewFile, NewTerminal, OpenInTerminal, OpenOptions, OpenTerminal, OpenVisible, SplitDirection, ToggleFileFinder, ToggleProjectSymbols, ToggleZoom, Workspace, WorkspaceItemBuilder, ZoomIn, ZoomOut, + focus_follows_mouse::FocusFollowsMouse as _, invalid_item_view::InvalidItemView, item::{ ActivateOnClose, ClosePosition, Item, ItemBufferKind, ItemHandle, ItemSettings, @@ -11,7 +12,7 @@ use crate::{ move_item, notifications::NotifyResultExt, toolbar::Toolbar, - workspace_settings::{AutosaveSetting, TabBarSettings, WorkspaceSettings}, + workspace_settings::{AutosaveSetting, FocusFollowsMouse, TabBarSettings, WorkspaceSettings}, }; use anyhow::Result; use collections::{BTreeSet, HashMap, HashSet, VecDeque}; @@ -443,6 +444,7 @@ pub struct Pane { pinned_tab_count: usize, diagnostics: HashMap, zoom_out_on_close: bool, + focus_follows_mouse: FocusFollowsMouse, diagnostic_summary_update: Task<()>, /// If a certain project item wants to get recreated with specific data, it can persist its data before the recreation here. pub project_item_restoration_data: HashMap>, @@ -615,6 +617,7 @@ impl Pane { pinned_tab_count: 0, diagnostics: Default::default(), zoom_out_on_close: true, + focus_follows_mouse: WorkspaceSettings::get_global(cx).focus_follows_mouse, diagnostic_summary_update: Task::ready(()), project_item_restoration_data: HashMap::default(), welcome_page: None, @@ -782,7 +785,6 @@ impl Pane { fn settings_changed(&mut self, window: &mut Window, cx: &mut Context) { let tab_bar_settings = TabBarSettings::get_global(cx); - let new_max_tabs = WorkspaceSettings::get_global(cx).max_tabs; if let Some(display_nav_history_buttons) = self.display_nav_history_buttons.as_mut() { *display_nav_history_buttons = tab_bar_settings.show_nav_history_buttons; @@ -795,6 +797,12 @@ impl Pane { self.nav_history.0.lock().preview_item_id = None; } + let workspace_settings = WorkspaceSettings::get_global(cx); + + self.focus_follows_mouse = workspace_settings.focus_follows_mouse; + + let new_max_tabs = workspace_settings.max_tabs; + if self.use_max_tabs && new_max_tabs != self.max_tabs { self.max_tabs = new_max_tabs; self.close_items_on_settings_change(window, cx); @@ -4460,6 +4468,7 @@ impl Render for Pane { placeholder.child(self.welcome_page.clone().unwrap()) } } + .focus_follows_mouse(self.focus_follows_mouse, cx) }) .child( // drag target diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index e5b927cbbbc571966d2483e82d98ce61adb06cda..1bf0d2bc4a09a2c6417ce2b35e46372d274c6161 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -19,6 +19,7 @@ mod security_modal; pub mod shared_screen; use db::smol::future::yield_now; pub use shared_screen::SharedScreen; +pub mod focus_follows_mouse; mod status_bar; pub mod tasks; mod theme_preview; @@ -147,8 +148,8 @@ use util::{ }; use uuid::Uuid; pub use workspace_settings::{ - AutosaveSetting, BottomDockLayout, RestoreOnStartupBehavior, StatusBarSettings, TabBarSettings, - WorkspaceSettings, + AutosaveSetting, BottomDockLayout, FocusFollowsMouse, RestoreOnStartupBehavior, + StatusBarSettings, TabBarSettings, WorkspaceSettings, }; use zed_actions::{Spawn, feedback::FileBugReport, theme::ToggleMode}; diff --git a/crates/workspace/src/workspace_settings.rs b/crates/workspace/src/workspace_settings.rs index d78b233229800b571ccc37f87719d09125f1c4c3..ee0e80336d744cadaecdf0201525deddb8d5eec9 100644 --- a/crates/workspace/src/workspace_settings.rs +++ b/crates/workspace/src/workspace_settings.rs @@ -1,4 +1,4 @@ -use std::num::NonZeroUsize; +use std::{num::NonZeroUsize, time::Duration}; use crate::DockPosition; use collections::HashMap; @@ -35,6 +35,13 @@ pub struct WorkspaceSettings { pub use_system_window_tabs: bool, pub zoomed_padding: bool, pub window_decorations: settings::WindowDecorations, + pub focus_follows_mouse: FocusFollowsMouse, +} + +#[derive(Copy, Clone, Deserialize)] +pub struct FocusFollowsMouse { + pub enabled: bool, + pub debounce: Duration, } #[derive(Copy, Clone, PartialEq, Debug, Default)] @@ -113,6 +120,20 @@ impl Settings for WorkspaceSettings { use_system_window_tabs: workspace.use_system_window_tabs.unwrap(), zoomed_padding: workspace.zoomed_padding.unwrap(), window_decorations: workspace.window_decorations.unwrap(), + focus_follows_mouse: FocusFollowsMouse { + enabled: workspace + .focus_follows_mouse + .unwrap() + .enabled + .unwrap_or(false), + debounce: Duration::from_millis( + workspace + .focus_follows_mouse + .unwrap() + .debounce_ms + .unwrap_or(250), + ), + }, } } } From 2fbf83049ff924b8612365fa7c19b65c7d7e1e07 Mon Sep 17 00:00:00 2001 From: Danilo Leal <67129314+danilo-leal@users.noreply.github.com> Date: Fri, 3 Apr 2026 18:27:52 -0300 Subject: [PATCH 4/8] gpui: Refactor follow_tail implementation to fix scroll snapping bugs (#53101) Follow up to https://github.com/zed-industries/zed/pull/53017 This PR does some significant refactoring of the `follow_tail` feature in the GPUI list. That's only used by the agent panel's thread view and given to the height-changing nature of streaming agent responses, we were seeing some scroll snapping bugs upon scrolling while the thread is generating. In the process of fixing it, we introduced a `remeasure_items` method as an alternative to `splice` so that we could get the remeasurement fix without scroll position changes. We already had a `remeasure` method that did that for all of the indexes, but we needed something more scoped out for the agent panel case, so as to not remeasure the entire list's content on every new streamed token. Effectively, this ends up reverting what the PR linked above introduced, but it improved the API in the process. Release Notes: - N/A Co-authored-by: Mikayla Maki --- crates/agent_ui/src/conversation_view.rs | 63 +-- .../src/conversation_view/thread_view.rs | 28 +- crates/gpui/src/elements/list.rs | 426 +++++++++++++++--- 3 files changed, 390 insertions(+), 127 deletions(-) diff --git a/crates/agent_ui/src/conversation_view.rs b/crates/agent_ui/src/conversation_view.rs index 1b9d364e9ce03702b47c63e8a856f0ba4b8aba87..ce125a5d7c901ccb6fc89f405f482cbf52b94f5d 100644 --- a/crates/agent_ui/src/conversation_view.rs +++ b/crates/agent_ui/src/conversation_view.rs @@ -831,6 +831,8 @@ impl ConversationView { let count = thread.read(cx).entries().len(); let list_state = ListState::new(0, gpui::ListAlignment::Top, px(2048.0)); + list_state.set_follow_mode(gpui::FollowMode::Tail); + entry_view_state.update(cx, |view_state, cx| { for ix in 0..count { view_state.sync_entry(ix, &thread, window, cx); @@ -844,7 +846,7 @@ impl ConversationView { if let Some(scroll_position) = thread.read(cx).ui_scroll_position() { list_state.scroll_to(scroll_position); } else { - list_state.set_follow_tail(true); + list_state.scroll_to_end(); } AgentDiff::set_active_thread(&self.workspace, thread.clone(), window, cx); @@ -1243,15 +1245,15 @@ impl ConversationView { if let Some(active) = self.thread_view(&thread_id) { let entry_view_state = active.read(cx).entry_view_state.clone(); let list_state = active.read(cx).list_state.clone(); - notify_entry_changed( - &entry_view_state, - &list_state, - index..index, - index, - thread, - window, - cx, - ); + entry_view_state.update(cx, |view_state, cx| { + view_state.sync_entry(index, thread, window, cx); + list_state.splice_focusable( + index..index, + [view_state + .entry(index) + .and_then(|entry| entry.focus_handle(cx))], + ); + }); active.update(cx, |active, cx| { active.sync_editor_mode_for_empty_state(cx); }); @@ -1261,15 +1263,10 @@ impl ConversationView { if let Some(active) = self.thread_view(&thread_id) { let entry_view_state = active.read(cx).entry_view_state.clone(); let list_state = active.read(cx).list_state.clone(); - notify_entry_changed( - &entry_view_state, - &list_state, - *index..*index + 1, - *index, - thread, - window, - cx, - ); + entry_view_state.update(cx, |view_state, cx| { + view_state.sync_entry(*index, thread, window, cx); + }); + list_state.remeasure_items(*index..*index + 1); active.update(cx, |active, cx| { active.auto_expand_streaming_thought(cx); }); @@ -1313,7 +1310,6 @@ impl ConversationView { active.clear_auto_expand_tracking(); if active.list_state.is_following_tail() { active.list_state.scroll_to_end(); - active.list_state.set_follow_tail(false); } } active.sync_generating_indicator(cx); @@ -1391,7 +1387,6 @@ impl ConversationView { active.thread_retry_status.take(); if active.list_state.is_following_tail() { active.list_state.scroll_to_end(); - active.list_state.set_follow_tail(false); } } active.sync_generating_indicator(cx); @@ -2608,32 +2603,6 @@ impl ConversationView { } } -/// Syncs an entry's view state with the latest thread data and splices -/// the list item so the list knows to re-measure it on the next paint. -/// -/// Used by both `NewEntry` (splice range `index..index` to insert) and -/// `EntryUpdated` (splice range `index..index+1` to replace), which is -/// why the caller provides the splice range. -fn notify_entry_changed( - entry_view_state: &Entity, - list_state: &ListState, - splice_range: std::ops::Range, - index: usize, - thread: &Entity, - window: &mut Window, - cx: &mut App, -) { - entry_view_state.update(cx, |view_state, cx| { - view_state.sync_entry(index, thread, window, cx); - list_state.splice_focusable( - splice_range, - [view_state - .entry(index) - .and_then(|entry| entry.focus_handle(cx))], - ); - }); -} - fn loading_contents_spinner(size: IconSize) -> AnyElement { Icon::new(IconName::LoadCircle) .size(size) diff --git a/crates/agent_ui/src/conversation_view/thread_view.rs b/crates/agent_ui/src/conversation_view/thread_view.rs index c113eb0b768ee143eb69b5e705c15c91e367e6c2..53e63268c51aa1aa5537a87b6055dea62ecd630e 100644 --- a/crates/agent_ui/src/conversation_view/thread_view.rs +++ b/crates/agent_ui/src/conversation_view/thread_view.rs @@ -541,24 +541,15 @@ impl ThreadView { let thread_view = cx.entity().downgrade(); this.list_state - .set_scroll_handler(move |event, _window, cx| { + .set_scroll_handler(move |_event, _window, cx| { let list_state = list_state_for_scroll.clone(); let thread_view = thread_view.clone(); - let is_following_tail = event.is_following_tail; // N.B. We must defer because the scroll handler is called while the // ListState's RefCell is mutably borrowed. Reading logical_scroll_top() // directly would panic from a double borrow. cx.defer(move |cx| { let scroll_top = list_state.logical_scroll_top(); let _ = thread_view.update(cx, |this, cx| { - if !is_following_tail { - let is_generating = - matches!(this.thread.read(cx).status(), ThreadStatus::Generating); - - if list_state.is_at_bottom() && is_generating { - list_state.set_follow_tail(true); - } - } if let Some(thread) = this.as_native_thread(cx) { thread.update(cx, |thread, _cx| { thread.set_ui_scroll_position(Some(scroll_top)); @@ -1070,7 +1061,7 @@ impl ThreadView { })?; let _ = this.update(cx, |this, cx| { - this.list_state.set_follow_tail(true); + this.list_state.scroll_to_end(); cx.notify(); }); @@ -4945,7 +4936,7 @@ impl ThreadView { } pub fn scroll_to_end(&mut self, cx: &mut Context) { - self.list_state.set_follow_tail(true); + self.list_state.scroll_to_end(); cx.notify(); } @@ -4967,7 +4958,6 @@ impl ThreadView { } pub(crate) fn scroll_to_top(&mut self, cx: &mut Context) { - self.list_state.set_follow_tail(false); self.list_state.scroll_to(ListOffset::default()); cx.notify(); } @@ -4979,7 +4969,6 @@ impl ThreadView { cx: &mut Context, ) { let page_height = self.list_state.viewport_bounds().size.height; - self.list_state.set_follow_tail(false); self.list_state.scroll_by(-page_height * 0.9); cx.notify(); } @@ -4991,11 +4980,7 @@ impl ThreadView { cx: &mut Context, ) { let page_height = self.list_state.viewport_bounds().size.height; - self.list_state.set_follow_tail(false); self.list_state.scroll_by(page_height * 0.9); - if self.list_state.is_at_bottom() { - self.list_state.set_follow_tail(true); - } cx.notify(); } @@ -5005,7 +4990,6 @@ impl ThreadView { window: &mut Window, cx: &mut Context, ) { - self.list_state.set_follow_tail(false); self.list_state.scroll_by(-window.line_height() * 3.); cx.notify(); } @@ -5016,11 +5000,7 @@ impl ThreadView { window: &mut Window, cx: &mut Context, ) { - self.list_state.set_follow_tail(false); self.list_state.scroll_by(window.line_height() * 3.); - if self.list_state.is_at_bottom() { - self.list_state.set_follow_tail(true); - } cx.notify(); } @@ -5054,7 +5034,6 @@ impl ThreadView { .rev() .find(|&i| matches!(entries.get(i), Some(AgentThreadEntry::UserMessage(_)))) { - self.list_state.set_follow_tail(false); self.list_state.scroll_to(ListOffset { item_ix: target_ix, offset_in_item: px(0.), @@ -5074,7 +5053,6 @@ impl ThreadView { if let Some(target_ix) = (current_ix + 1..entries.len()) .find(|&i| matches!(entries.get(i), Some(AgentThreadEntry::UserMessage(_)))) { - self.list_state.set_follow_tail(false); self.list_state.scroll_to(ListOffset { item_ix: target_ix, offset_in_item: px(0.), diff --git a/crates/gpui/src/elements/list.rs b/crates/gpui/src/elements/list.rs index b4c8e7ca9015190fb8bb1698f79f1b025bfa4829..5525f5c17d2ad33e1ce9696afded1cea5447020c 100644 --- a/crates/gpui/src/elements/list.rs +++ b/crates/gpui/src/elements/list.rs @@ -72,7 +72,7 @@ struct StateInner { scrollbar_drag_start_height: Option, measuring_behavior: ListMeasuringBehavior, pending_scroll: Option, - follow_tail: bool, + follow_state: FollowState, } /// Keeps track of a fractional scroll position within an item for restoration @@ -84,6 +84,49 @@ struct PendingScrollFraction { fraction: f32, } +/// Controls whether the list automatically follows new content at the end. +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] +pub enum FollowMode { + /// Normal scrolling — no automatic following. + #[default] + Normal, + /// The list should auto-scroll along with the tail, when scrolled to bottom. + Tail, +} + +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] +enum FollowState { + #[default] + Normal, + Tail { + is_following: bool, + }, +} + +impl FollowState { + fn is_following(&self) -> bool { + matches!(self, FollowState::Tail { is_following: true }) + } + + fn has_stopped_following(&self) -> bool { + matches!( + self, + FollowState::Tail { + is_following: false + } + ) + } + + fn start_following(&mut self) { + if let FollowState::Tail { + is_following: false, + } = self + { + *self = FollowState::Tail { is_following: true }; + } + } +} + /// Whether the list is scrolling from top to bottom or bottom to top. #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum ListAlignment { @@ -169,6 +212,7 @@ pub struct ListPrepaintState { #[derive(Clone)] enum ListItem { Unmeasured { + size_hint: Option>, focus_handle: Option, }, Measured { @@ -186,9 +230,16 @@ impl ListItem { } } + fn size_hint(&self) -> Option> { + match self { + ListItem::Measured { size, .. } => Some(*size), + ListItem::Unmeasured { size_hint, .. } => *size_hint, + } + } + fn focus_handle(&self) -> Option { match self { - ListItem::Unmeasured { focus_handle } | ListItem::Measured { focus_handle, .. } => { + ListItem::Unmeasured { focus_handle, .. } | ListItem::Measured { focus_handle, .. } => { focus_handle.clone() } } @@ -196,7 +247,7 @@ impl ListItem { fn contains_focused(&self, window: &Window, cx: &App) -> bool { match self { - ListItem::Unmeasured { focus_handle } | ListItem::Measured { focus_handle, .. } => { + ListItem::Unmeasured { focus_handle, .. } | ListItem::Measured { focus_handle, .. } => { focus_handle .as_ref() .is_some_and(|handle| handle.contains_focused(window, cx)) @@ -240,7 +291,7 @@ impl ListState { scrollbar_drag_start_height: None, measuring_behavior: ListMeasuringBehavior::default(), pending_scroll: None, - follow_tail: false, + follow_state: FollowState::default(), }))); this.splice(0..0, item_count); this @@ -275,37 +326,63 @@ impl ListState { /// Use this when item heights may have changed (e.g., font size changes) /// but the number and identity of items remains the same. pub fn remeasure(&self) { - let state = &mut *self.0.borrow_mut(); + let count = self.item_count(); + self.remeasure_items(0..count); + } - let new_items = state.items.iter().map(|item| ListItem::Unmeasured { - focus_handle: item.focus_handle(), - }); + /// Mark items in `range` as needing remeasurement while preserving + /// the current scroll position. Unlike [`Self::splice`], this does + /// not change the number of items or blow away `logical_scroll_top`. + /// + /// Use this when an item's content has changed and its rendered + /// height may be different (e.g., streaming text, tool results + /// loading), but the item itself still exists at the same index. + pub fn remeasure_items(&self, range: Range) { + let state = &mut *self.0.borrow_mut(); - // If there's a `logical_scroll_top`, we need to keep track of it as a - // `PendingScrollFraction`, so we can later preserve that scroll - // position proportionally to the item, in case the item's height - // changes. + // If the scroll-top item falls within the remeasured range, + // store a fractional offset so the layout can restore the + // proportional scroll position after the item is re-rendered + // at its new height. if let Some(scroll_top) = state.logical_scroll_top { - let mut cursor = state.items.cursor::(()); - cursor.seek(&Count(scroll_top.item_ix), Bias::Right); + if range.contains(&scroll_top.item_ix) { + let mut cursor = state.items.cursor::(()); + cursor.seek(&Count(scroll_top.item_ix), Bias::Right); - if let Some(item) = cursor.item() { - if let Some(size) = item.size() { - let fraction = if size.height.0 > 0.0 { - (scroll_top.offset_in_item.0 / size.height.0).clamp(0.0, 1.0) - } else { - 0.0 - }; - - state.pending_scroll = Some(PendingScrollFraction { - item_ix: scroll_top.item_ix, - fraction, - }); + if let Some(item) = cursor.item() { + if let Some(size) = item.size() { + let fraction = if size.height.0 > 0.0 { + (scroll_top.offset_in_item.0 / size.height.0).clamp(0.0, 1.0) + } else { + 0.0 + }; + + state.pending_scroll = Some(PendingScrollFraction { + item_ix: scroll_top.item_ix, + fraction, + }); + } } } } - state.items = SumTree::from_iter(new_items, ()); + // Rebuild the tree, replacing items in the range with + // Unmeasured copies that keep their focus handles. + let new_items = { + let mut cursor = state.items.cursor::(()); + let mut new_items = cursor.slice(&Count(range.start), Bias::Right); + let invalidated = cursor.slice(&Count(range.end), Bias::Right); + new_items.extend( + invalidated.iter().map(|item| ListItem::Unmeasured { + size_hint: item.size_hint(), + focus_handle: item.focus_handle(), + }), + (), + ); + new_items.append(cursor.suffix(), ()); + new_items + }; + state.items = new_items; state.measuring_behavior.reset(); } @@ -339,7 +416,10 @@ impl ListState { new_items.extend( focus_handles.into_iter().map(|focus_handle| { spliced_count += 1; - ListItem::Unmeasured { focus_handle } + ListItem::Unmeasured { + size_hint: None, + focus_handle, + } }), (), ); @@ -414,24 +494,37 @@ impl ListState { }); } - /// Set whether the list should automatically follow the tail (auto-scroll to the end). - pub fn set_follow_tail(&self, follow: bool) { - self.0.borrow_mut().follow_tail = follow; - if follow { - self.scroll_to_end(); + /// Set the follow mode for the list. In `Tail` mode, the list + /// will auto-scroll to the end and re-engage after the user + /// scrolls back to the bottom. In `Normal` mode, no automatic + /// following occurs. + pub fn set_follow_mode(&self, mode: FollowMode) { + let state = &mut *self.0.borrow_mut(); + + match mode { + FollowMode::Normal => { + state.follow_state = FollowState::Normal; + } + FollowMode::Tail => { + state.follow_state = FollowState::Tail { is_following: true }; + if matches!(mode, FollowMode::Tail) { + let item_count = state.items.summary().count; + state.logical_scroll_top = Some(ListOffset { + item_ix: item_count, + offset_in_item: px(0.), + }); + } + } } } - /// Returns whether the list is currently in follow-tail mode (auto-scrolling to the end). + /// Returns whether the list is currently actively following the + /// tail (snapping to the end on each layout). pub fn is_following_tail(&self) -> bool { - self.0.borrow().follow_tail - } - - /// Returns whether the list is scrolled to the bottom (within 1px). - pub fn is_at_bottom(&self) -> bool { - let current_offset = self.scroll_px_offset_for_scrollbar().y.abs(); - let max_offset = self.max_offset_for_scrollbar().y; - current_offset >= max_offset - px(1.0) + matches!( + self.0.borrow().follow_state, + FollowState::Tail { is_following: true } + ) } /// Scroll the list to the given offset @@ -599,6 +692,7 @@ impl StateInner { if self.reset { return; } + let padding = self.last_padding.unwrap_or_default(); let scroll_max = (self.items.summary().height + padding.top + padding.bottom - height).max(px(0.)); @@ -620,8 +714,10 @@ impl StateInner { }); } - if self.follow_tail && delta.y > px(0.) { - self.follow_tail = false; + if let FollowState::Tail { is_following } = &mut self.follow_state { + if delta.y > px(0.) { + *is_following = false; + } } if let Some(handler) = self.scroll_handler.as_mut() { @@ -631,7 +727,10 @@ impl StateInner { visible_range, count: self.items.summary().count, is_scrolled: self.logical_scroll_top.is_some(), - is_following_tail: self.follow_tail, + is_following_tail: matches!( + self.follow_state, + FollowState::Tail { is_following: true } + ), }, window, cx, @@ -722,7 +821,7 @@ impl StateInner { let mut max_item_width = px(0.); let mut scroll_top = self.logical_scroll_top(); - if self.follow_tail { + if self.follow_state.is_following() { scroll_top = ListOffset { item_ix: self.items.summary().count, offset_in_item: px(0.), @@ -875,6 +974,18 @@ impl StateInner { new_items.append(cursor.suffix(), ()); self.items = new_items; + // If follow_tail mode is on but the user scrolled away + // (is_following is false), check whether the current scroll + // position has returned to the bottom. + if self.follow_state.has_stopped_following() { + let padding = self.last_padding.unwrap_or_default(); + let total_height = self.items.summary().height + padding.top + padding.bottom; + let scroll_offset = self.scroll_top(&scroll_top); + if scroll_offset + available_height >= total_height - px(1.0) { + self.follow_state.start_following(); + } + } + // If none of the visible items are focused, check if an off-screen item is focused // and include it to be rendered after the visible items so keyboard interaction continues // to work for it. @@ -1011,7 +1122,7 @@ impl StateInner { content_height - self.scrollbar_drag_start_height.unwrap_or(content_height); let new_scroll_top = (point.y - drag_offset).abs().max(px(0.)).min(scroll_max); - self.follow_tail = false; + self.follow_state = FollowState::Normal; if self.alignment == ListAlignment::Bottom && new_scroll_top == scroll_max { self.logical_scroll_top = None; @@ -1159,6 +1270,7 @@ impl Element for List { { let new_items = SumTree::from_iter( state.items.iter().map(|item| ListItem::Unmeasured { + size_hint: None, focus_handle: item.focus_handle(), }), (), @@ -1245,11 +1357,18 @@ impl sum_tree::Item for ListItem { fn summary(&self, _: ()) -> Self::Summary { match self { - ListItem::Unmeasured { focus_handle } => ListItemSummary { + ListItem::Unmeasured { + size_hint, + focus_handle, + } => ListItemSummary { count: 1, rendered_count: 0, unrendered_count: 1, - height: px(0.), + height: if let Some(size) = size_hint { + size.height + } else { + px(0.) + }, has_focus_handles: focus_handle.is_some(), }, ListItem::Measured { @@ -1319,8 +1438,8 @@ mod test { use std::rc::Rc; use crate::{ - self as gpui, AppContext, Context, Element, IntoElement, ListState, Render, Styled, - TestAppContext, Window, div, list, point, px, size, + self as gpui, AppContext, Context, Element, FollowMode, IntoElement, ListState, Render, + Styled, TestAppContext, Window, div, list, point, px, size, }; #[gpui::test] @@ -1545,7 +1664,7 @@ mod test { }) }); - state.set_follow_tail(true); + state.set_follow_mode(FollowMode::Tail); // First paint — items are 50px, total 500px, viewport 200px. // Follow-tail should anchor to the end. @@ -1599,7 +1718,7 @@ mod test { } } - state.set_follow_tail(true); + state.set_follow_mode(FollowMode::Tail); // Paint with follow-tail — scroll anchored to the bottom. cx.draw(point(px(0.), px(0.)), size(px(100.), px(200.)), |_, cx| { @@ -1641,7 +1760,7 @@ mod test { let view = cx.update(|_, cx| cx.new(|_| TestView(state.clone()))); - state.set_follow_tail(true); + state.set_follow_mode(FollowMode::Tail); // Paint with follow-tail — scroll anchored to the bottom. cx.draw(point(px(0.), px(0.)), size(px(100.), px(200.)), |_, _| { @@ -1709,7 +1828,7 @@ mod test { // Enable follow-tail — this should immediately snap the scroll anchor // to the end, like the user just sent a prompt. - state.set_follow_tail(true); + state.set_follow_mode(FollowMode::Tail); cx.draw(point(px(0.), px(0.)), size(px(100.), px(200.)), |_, _| { view.into_any_element() @@ -1764,4 +1883,201 @@ mod test { -scroll_offset.y, max_offset.y, ); } + + /// When the user scrolls away from the bottom during follow_tail, + /// follow_tail suspends. If they scroll back to the bottom, the + /// next paint should re-engage follow_tail using fresh measurements. + #[gpui::test] + fn test_follow_tail_reengages_when_scrolled_back_to_bottom(cx: &mut TestAppContext) { + let cx = cx.add_empty_window(); + + // 10 items × 50px = 500px total, 200px viewport. + let state = ListState::new(10, crate::ListAlignment::Top, px(0.)); + + struct TestView(ListState); + impl Render for TestView { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + list(self.0.clone(), |_, _, _| { + div().h(px(50.)).w_full().into_any() + }) + .w_full() + .h_full() + } + } + + let view = cx.update(|_, cx| cx.new(|_| TestView(state.clone()))); + + state.set_follow_mode(FollowMode::Tail); + + cx.draw(point(px(0.), px(0.)), size(px(100.), px(200.)), |_, _| { + view.clone().into_any_element() + }); + assert!(state.is_following_tail()); + + // Scroll up — follow_tail should suspend (not fully disengage). + cx.simulate_event(ScrollWheelEvent { + position: point(px(50.), px(100.)), + delta: ScrollDelta::Pixels(point(px(0.), px(50.))), + ..Default::default() + }); + assert!(!state.is_following_tail()); + + // Scroll back down to the bottom. + cx.simulate_event(ScrollWheelEvent { + position: point(px(50.), px(100.)), + delta: ScrollDelta::Pixels(point(px(0.), px(-10000.))), + ..Default::default() + }); + + // After a paint, follow_tail should re-engage because the + // layout confirmed we're at the true bottom. + cx.draw(point(px(0.), px(0.)), size(px(100.), px(200.)), |_, _| { + view.clone().into_any_element() + }); + assert!( + state.is_following_tail(), + "follow_tail should re-engage after scrolling back to the bottom" + ); + } + + /// When an item is spliced to unmeasured (0px) while follow_tail + /// is suspended, the re-engagement check should still work correctly + #[gpui::test] + fn test_follow_tail_reengagement_not_fooled_by_unmeasured_items(cx: &mut TestAppContext) { + let cx = cx.add_empty_window(); + + // 20 items × 50px = 1000px total, 200px viewport, 1000px + // overdraw so all items get measured during the follow_tail + // paint (matching realistic production settings). + let state = ListState::new(20, crate::ListAlignment::Top, px(1000.)); + + struct TestView(ListState); + impl Render for TestView { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + list(self.0.clone(), |_, _, _| { + div().h(px(50.)).w_full().into_any() + }) + .w_full() + .h_full() + } + } + + let view = cx.update(|_, cx| cx.new(|_| TestView(state.clone()))); + + state.set_follow_mode(FollowMode::Tail); + + cx.draw(point(px(0.), px(0.)), size(px(100.), px(200.)), |_, _| { + view.clone().into_any_element() + }); + assert!(state.is_following_tail()); + + // Scroll up a meaningful amount — suspends follow_tail. + // 20 items × 50px = 1000px. viewport 200px. scroll_max = 800px. + // Scrolling up 200px puts us at 600px, clearly not at bottom. + cx.simulate_event(ScrollWheelEvent { + position: point(px(50.), px(100.)), + delta: ScrollDelta::Pixels(point(px(0.), px(200.))), + ..Default::default() + }); + assert!(!state.is_following_tail()); + + // Invalidate the last item (simulates EntryUpdated calling + // remeasure_items). This makes items.summary().height + // temporarily wrong (0px for the invalidated item). + state.remeasure_items(19..20); + + // Paint — layout re-measures the invalidated item with its true + // height. The re-engagement check uses these fresh measurements. + // Since we scrolled 200px up from the 800px max, we're at + // ~600px — NOT at the bottom, so follow_tail should NOT + // re-engage. + cx.draw(point(px(0.), px(0.)), size(px(100.), px(200.)), |_, _| { + view.clone().into_any_element() + }); + assert!( + !state.is_following_tail(), + "follow_tail should not falsely re-engage due to an unmeasured item \ + reducing items.summary().height" + ); + } + + /// Calling `set_follow_mode(FollowState::Normal)` or dragging the scrollbar should + /// fully disengage follow_tail — clearing any suspended state so + /// follow_tail won’t auto-re-engage. + #[gpui::test] + fn test_follow_tail_suspended_state_cleared_by_explicit_actions(cx: &mut TestAppContext) { + let cx = cx.add_empty_window(); + + // 10 items × 50px = 500px total, 200px viewport. + let state = ListState::new(10, crate::ListAlignment::Top, px(0.)).measure_all(); + + struct TestView(ListState); + impl Render for TestView { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + list(self.0.clone(), |_, _, _| { + div().h(px(50.)).w_full().into_any() + }) + .w_full() + .h_full() + } + } + + let view = cx.update(|_, cx| cx.new(|_| TestView(state.clone()))); + + state.set_follow_mode(FollowMode::Tail); + // --- Part 1: set_follow_mode(FollowState::Normal) clears suspended state --- + + cx.draw(point(px(0.), px(0.)), size(px(100.), px(200.)), |_, _| { + view.clone().into_any_element() + }); + + // Scroll up — suspends follow_tail. + cx.simulate_event(ScrollWheelEvent { + position: point(px(50.), px(100.)), + delta: ScrollDelta::Pixels(point(px(0.), px(50.))), + ..Default::default() + }); + assert!(!state.is_following_tail()); + + // Scroll back to the bottom — should re-engage follow_tail. + cx.simulate_event(ScrollWheelEvent { + position: point(px(50.), px(100.)), + delta: ScrollDelta::Pixels(point(px(0.), px(-10000.))), + ..Default::default() + }); + + cx.draw(point(px(0.), px(0.)), size(px(100.), px(200.)), |_, _| { + view.clone().into_any_element() + }); + assert!( + state.is_following_tail(), + "follow_tail should re-engage after scrolling back to the bottom" + ); + + // --- Part 2: scrollbar drag clears suspended state --- + + cx.draw(point(px(0.), px(0.)), size(px(100.), px(200.)), |_, _| { + view.clone().into_any_element() + }); + + // Drag the scrollbar to the middle — should clear suspended state. + state.set_offset_from_scrollbar(point(px(0.), px(150.))); + + // Scroll to the bottom. + cx.simulate_event(ScrollWheelEvent { + position: point(px(50.), px(100.)), + delta: ScrollDelta::Pixels(point(px(0.), px(-10000.))), + ..Default::default() + }); + + // Paint — should NOT re-engage because the scrollbar drag + // cleared the suspended state. + cx.draw(point(px(0.), px(0.)), size(px(100.), px(200.)), |_, _| { + view.clone().into_any_element() + }); + assert!( + !state.is_following_tail(), + "follow_tail should not re-engage after scrollbar drag cleared the suspended state" + ); + } } From e9b280afe00815cced8c50ca6e97d7987e5782ec Mon Sep 17 00:00:00 2001 From: KyleBarton Date: Fri, 3 Apr 2026 14:55:12 -0700 Subject: [PATCH 5/8] Account for windows absolute paths in bind mounts (#53093) Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Addresses an auxiliary windows bug found in #52924 - bind mounts are not working in Windows because MountDefinition is not accounting for absolute Windows paths. Release Notes: - Fixed windows bind mount issue with dev containers --- crates/dev_container/src/devcontainer_json.rs | 54 ++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/crates/dev_container/src/devcontainer_json.rs b/crates/dev_container/src/devcontainer_json.rs index 4429c63a37a87d1b54455b8169359ddf40511e24..f034026a8de4c4a6c3186c97870e25f3510ebc85 100644 --- a/crates/dev_container/src/devcontainer_json.rs +++ b/crates/dev_container/src/devcontainer_json.rs @@ -72,7 +72,11 @@ impl Display for MountDefinition { f, "type={},source={},target={},consistency=cached", self.mount_type.clone().unwrap_or_else(|| { - if self.source.starts_with('/') { + if self.source.starts_with('/') + || self.source.starts_with("\\\\") + || self.source.get(1..3) == Some(":\\") + || self.source.get(1..3) == Some(":/") + { "bind".to_string() } else { "volume".to_string() @@ -1355,4 +1359,52 @@ mod test { assert_eq!(devcontainer.build_type(), DevContainerBuildType::Dockerfile); } + + #[test] + fn mount_definition_should_use_bind_type_for_unix_absolute_paths() { + let mount = MountDefinition { + source: "/home/user/project".to_string(), + target: "/workspaces/project".to_string(), + mount_type: None, + }; + + let rendered = mount.to_string(); + + assert!( + rendered.starts_with("type=bind,"), + "Expected mount type 'bind' for Unix absolute path, but got: {rendered}" + ); + } + + #[test] + fn mount_definition_should_use_bind_type_for_windows_unc_paths() { + let mount = MountDefinition { + source: "\\\\server\\share\\project".to_string(), + target: "/workspaces/project".to_string(), + mount_type: None, + }; + + let rendered = mount.to_string(); + + assert!( + rendered.starts_with("type=bind,"), + "Expected mount type 'bind' for Windows UNC path, but got: {rendered}" + ); + } + + #[test] + fn mount_definition_should_use_bind_type_for_windows_absolute_paths() { + let mount = MountDefinition { + source: "C:\\Users\\mrg\\cli".to_string(), + target: "/workspaces/cli".to_string(), + mount_type: None, + }; + + let rendered = mount.to_string(); + + assert!( + rendered.starts_with("type=bind,"), + "Expected mount type 'bind' for Windows absolute path, but got: {rendered}" + ); + } } From eeb87cb17768c4a1372047ad67a1e9a1d7dd507b Mon Sep 17 00:00:00 2001 From: Saketh <126517689+SAKETH11111@users.noreply.github.com> Date: Fri, 3 Apr 2026 17:55:40 -0500 Subject: [PATCH 6/8] remote: Use SSH nicknames in display names (#53103) Closes #52943 ## Summary - Prefer SSH nicknames over raw hosts in remote connection display names - Add regression tests for nickname and host fallback behavior ## Why The `nickname` field is documented as the user-facing label for SSH connections, but `RemoteConnectionOptions::display_name()` always returned the raw host. That meant recent-projects UI surfaces kept ignoring nicknames even when they were configured. ## Validation - `cargo test -p remote ssh_display_name` - `cargo test -p remote` Release Notes: - Fixed SSH recent-project labels to show configured nicknames instead of raw hosts when available. --- crates/remote/src/remote_client.rs | 31 +++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/crates/remote/src/remote_client.rs b/crates/remote/src/remote_client.rs index e746d82aac857d3174a4bab14c937a7538b2f1b4..c04d3630f92bcc27afb01a619176d3ae79d3fac7 100644 --- a/crates/remote/src/remote_client.rs +++ b/crates/remote/src/remote_client.rs @@ -1285,7 +1285,10 @@ pub enum RemoteConnectionOptions { impl RemoteConnectionOptions { pub fn display_name(&self) -> String { match self { - RemoteConnectionOptions::Ssh(opts) => opts.host.to_string(), + RemoteConnectionOptions::Ssh(opts) => opts + .nickname + .clone() + .unwrap_or_else(|| opts.host.to_string()), RemoteConnectionOptions::Wsl(opts) => opts.distro_name.clone(), RemoteConnectionOptions::Docker(opts) => { if opts.use_podman { @@ -1300,6 +1303,32 @@ impl RemoteConnectionOptions { } } +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_ssh_display_name_prefers_nickname() { + let options = RemoteConnectionOptions::Ssh(SshConnectionOptions { + host: "1.2.3.4".into(), + nickname: Some("My Cool Project".to_string()), + ..Default::default() + }); + + assert_eq!(options.display_name(), "My Cool Project"); + } + + #[test] + fn test_ssh_display_name_falls_back_to_host() { + let options = RemoteConnectionOptions::Ssh(SshConnectionOptions { + host: "1.2.3.4".into(), + ..Default::default() + }); + + assert_eq!(options.display_name(), "1.2.3.4"); + } +} + impl From for RemoteConnectionOptions { fn from(opts: SshConnectionOptions) -> Self { RemoteConnectionOptions::Ssh(opts) From 5ae174fa5f811c154ed8f05de1c75c5ad5160790 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 3 Apr 2026 16:02:55 -0700 Subject: [PATCH 7/8] Rework sidebar rendering to use MultiWorkspace's project groups (#53096) Release Notes: * [x] It's possible to get into a state where agent panel shows a thread that is archived - N/A --------- Co-authored-by: Eric Holk Co-authored-by: Mikayla Maki --- Cargo.lock | 1 - crates/agent_ui/src/agent_panel.rs | 4 + crates/project/src/project.rs | 6 +- crates/sidebar/Cargo.toml | 1 - crates/sidebar/src/project_group_builder.rs | 282 -------- crates/sidebar/src/sidebar.rs | 739 ++++++++++---------- crates/sidebar/src/sidebar_tests.rs | 582 +++++++-------- crates/workspace/src/multi_workspace.rs | 20 + 8 files changed, 713 insertions(+), 922 deletions(-) delete mode 100644 crates/sidebar/src/project_group_builder.rs diff --git a/Cargo.lock b/Cargo.lock index aae7afecc5ea6f6ba3d63453321c829b677e1c58..906c5e65456c604e5123bfde9ac1c39e261eedfd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -15871,7 +15871,6 @@ dependencies = [ "agent_ui", "anyhow", "chrono", - "collections", "editor", "feature_flags", "fs", diff --git a/crates/agent_ui/src/agent_panel.rs b/crates/agent_ui/src/agent_panel.rs index 118f0dce6cb53c4e7851c79513cf936d6023a711..5fd39509df4ec2263e47c7e87b3e4b7852eaf154 100644 --- a/crates/agent_ui/src/agent_panel.rs +++ b/crates/agent_ui/src/agent_panel.rs @@ -2076,6 +2076,10 @@ impl AgentPanel { window: &mut Window, cx: &mut Context, ) { + if let Some(store) = ThreadMetadataStore::try_global(cx) { + store.update(cx, |store, cx| store.unarchive(&session_id, cx)); + } + if let Some(conversation_view) = self.background_threads.remove(&session_id) { self.set_active_view( ActiveView::AgentThread { conversation_view }, diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 41f57299835f37b001575b682118aa17a6516ad9..c5b1f982ceacc59a60ff1303faffc972a3ce505d 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -6049,11 +6049,7 @@ impl ProjectGroupKey { /// Creates a new `ProjectGroupKey` with the given path list. /// /// The path list should point to the git main worktree paths for a project. - /// - /// This should be used only in a few places to make sure we can ensure the - /// main worktree path invariant. Namely, this should only be called from - /// [`Workspace`]. - pub(crate) fn new(host: Option, paths: PathList) -> Self { + pub fn new(host: Option, paths: PathList) -> Self { Self { paths, host } } diff --git a/crates/sidebar/Cargo.toml b/crates/sidebar/Cargo.toml index 04ed8808a14d4c6853b08669523d55a2ebba4482..d76fd139557dd10438d7cf98f9168d87dcae9804 100644 --- a/crates/sidebar/Cargo.toml +++ b/crates/sidebar/Cargo.toml @@ -23,7 +23,6 @@ agent_settings.workspace = true agent_ui = { workspace = true, features = ["audio"] } anyhow.workspace = true chrono.workspace = true -collections.workspace = true editor.workspace = true feature_flags.workspace = true fs.workspace = true diff --git a/crates/sidebar/src/project_group_builder.rs b/crates/sidebar/src/project_group_builder.rs deleted file mode 100644 index 20919647c185ce7014f056a99bb9c85ae595c560..0000000000000000000000000000000000000000 --- a/crates/sidebar/src/project_group_builder.rs +++ /dev/null @@ -1,282 +0,0 @@ -//! The sidebar groups threads by a canonical path list. -//! -//! Threads have a path list associated with them, but this is the absolute path -//! of whatever worktrees they were associated with. In the sidebar, we want to -//! group all threads by their main worktree, and then we add a worktree chip to -//! the sidebar entry when that thread is in another worktree. -//! -//! This module is provides the functions and structures necessary to do this -//! lookup and mapping. - -use collections::{HashMap, HashSet, vecmap::VecMap}; -use gpui::{App, Entity}; -use project::ProjectGroupKey; -use std::{ - path::{Path, PathBuf}, - sync::Arc, -}; -use workspace::{MultiWorkspace, PathList, Workspace}; - -#[derive(Default)] -pub struct ProjectGroup { - pub workspaces: Vec>, - /// Root paths of all open workspaces in this group. Used to skip - /// redundant thread-store queries for linked worktrees that already - /// have an open workspace. - covered_paths: HashSet>, -} - -impl ProjectGroup { - fn add_workspace(&mut self, workspace: &Entity, cx: &App) { - if !self.workspaces.contains(workspace) { - self.workspaces.push(workspace.clone()); - } - for path in workspace.read(cx).root_paths(cx) { - self.covered_paths.insert(path); - } - } - - pub fn first_workspace(&self) -> &Entity { - self.workspaces - .first() - .expect("groups always have at least one workspace") - } - - pub fn main_workspace(&self, cx: &App) -> &Entity { - self.workspaces - .iter() - .find(|ws| { - !crate::root_repository_snapshots(ws, cx) - .any(|snapshot| snapshot.is_linked_worktree()) - }) - .unwrap_or_else(|| self.first_workspace()) - } -} - -pub struct ProjectGroupBuilder { - /// Maps git repositories' work_directory_abs_path to their original_repo_abs_path - directory_mappings: HashMap, - project_groups: VecMap, -} - -impl ProjectGroupBuilder { - fn new() -> Self { - Self { - directory_mappings: HashMap::default(), - project_groups: VecMap::new(), - } - } - - pub fn from_multiworkspace(mw: &MultiWorkspace, cx: &App) -> Self { - let mut builder = Self::new(); - // First pass: collect all directory mappings from every workspace - // so we know how to canonicalize any path (including linked - // worktree paths discovered by the main repo's workspace). - for workspace in mw.workspaces() { - builder.add_workspace_mappings(workspace.read(cx), cx); - } - - // Second pass: group each workspace using canonical paths derived - // from the full set of mappings. - for workspace in mw.workspaces() { - let group_name = workspace.read(cx).project_group_key(cx); - builder - .project_group_entry(&group_name) - .add_workspace(workspace, cx); - } - builder - } - - fn project_group_entry(&mut self, name: &ProjectGroupKey) -> &mut ProjectGroup { - self.project_groups.entry_ref(name).or_insert_default() - } - - fn add_mapping(&mut self, work_directory: &Path, original_repo: &Path) { - let old = self - .directory_mappings - .insert(PathBuf::from(work_directory), PathBuf::from(original_repo)); - if let Some(old) = old { - debug_assert_eq!( - &old, original_repo, - "all worktrees should map to the same main worktree" - ); - } - } - - pub fn add_workspace_mappings(&mut self, workspace: &Workspace, cx: &App) { - for repo in workspace.project().read(cx).repositories(cx).values() { - let snapshot = repo.read(cx).snapshot(); - - self.add_mapping( - &snapshot.work_directory_abs_path, - &snapshot.original_repo_abs_path, - ); - - for worktree in snapshot.linked_worktrees.iter() { - self.add_mapping(&worktree.path, &snapshot.original_repo_abs_path); - } - } - } - - pub fn canonicalize_path<'a>(&'a self, path: &'a Path) -> &'a Path { - self.directory_mappings - .get(path) - .map(AsRef::as_ref) - .unwrap_or(path) - } - - /// Whether the given group should load threads for a linked worktree - /// at `worktree_path`. Returns `false` if the worktree already has an - /// open workspace in the group (its threads are loaded via the - /// workspace loop) or if the worktree's canonical path list doesn't - /// match `group_path_list`. - pub fn group_owns_worktree( - &self, - group: &ProjectGroup, - group_path_list: &PathList, - worktree_path: &Path, - ) -> bool { - if group.covered_paths.contains(worktree_path) { - return false; - } - let canonical = self.canonicalize_path_list(&PathList::new(&[worktree_path])); - canonical == *group_path_list - } - - /// Canonicalizes every path in a [`PathList`] using the builder's - /// directory mappings. - fn canonicalize_path_list(&self, path_list: &PathList) -> PathList { - let paths: Vec<_> = path_list - .paths() - .iter() - .map(|p| self.canonicalize_path(p).to_path_buf()) - .collect(); - PathList::new(&paths) - } - - pub fn groups(&self) -> impl Iterator { - self.project_groups.iter() - } -} - -#[cfg(test)] -mod tests { - use std::sync::Arc; - - use super::*; - use fs::FakeFs; - use gpui::TestAppContext; - use settings::SettingsStore; - - fn init_test(cx: &mut TestAppContext) { - cx.update(|cx| { - let settings_store = SettingsStore::test(cx); - cx.set_global(settings_store); - theme_settings::init(theme::LoadThemes::JustBase, cx); - }); - } - - async fn create_fs_with_main_and_worktree(cx: &mut TestAppContext) -> Arc { - let fs = FakeFs::new(cx.executor()); - fs.insert_tree( - "/project", - serde_json::json!({ - ".git": { - "worktrees": { - "feature-a": { - "commondir": "../../", - "HEAD": "ref: refs/heads/feature-a", - }, - }, - }, - "src": {}, - }), - ) - .await; - fs.insert_tree( - "/wt/feature-a", - serde_json::json!({ - ".git": "gitdir: /project/.git/worktrees/feature-a", - "src": {}, - }), - ) - .await; - fs.add_linked_worktree_for_repo( - std::path::Path::new("/project/.git"), - false, - git::repository::Worktree { - path: std::path::PathBuf::from("/wt/feature-a"), - ref_name: Some("refs/heads/feature-a".into()), - sha: "abc".into(), - is_main: false, - }, - ) - .await; - fs - } - - #[gpui::test] - async fn test_main_repo_maps_to_itself(cx: &mut TestAppContext) { - init_test(cx); - let fs = create_fs_with_main_and_worktree(cx).await; - cx.update(|cx| ::set_global(fs.clone(), cx)); - - let project = project::Project::test(fs.clone(), ["/project".as_ref()], cx).await; - project - .update(cx, |project, cx| project.git_scans_complete(cx)) - .await; - - let (multi_workspace, cx) = cx.add_window_view(|window, cx| { - workspace::MultiWorkspace::test_new(project.clone(), window, cx) - }); - - multi_workspace.read_with(cx, |mw, cx| { - let mut canonicalizer = ProjectGroupBuilder::new(); - for workspace in mw.workspaces() { - canonicalizer.add_workspace_mappings(workspace.read(cx), cx); - } - - // The main repo path should canonicalize to itself. - assert_eq!( - canonicalizer.canonicalize_path(Path::new("/project")), - Path::new("/project"), - ); - - // An unknown path returns None. - assert_eq!( - canonicalizer.canonicalize_path(Path::new("/something/else")), - Path::new("/something/else"), - ); - }); - } - - #[gpui::test] - async fn test_worktree_checkout_canonicalizes_to_main_repo(cx: &mut TestAppContext) { - init_test(cx); - let fs = create_fs_with_main_and_worktree(cx).await; - cx.update(|cx| ::set_global(fs.clone(), cx)); - - // Open the worktree checkout as its own project. - let project = project::Project::test(fs.clone(), ["/wt/feature-a".as_ref()], cx).await; - project - .update(cx, |project, cx| project.git_scans_complete(cx)) - .await; - - let (multi_workspace, cx) = cx.add_window_view(|window, cx| { - workspace::MultiWorkspace::test_new(project.clone(), window, cx) - }); - - multi_workspace.read_with(cx, |mw, cx| { - let mut canonicalizer = ProjectGroupBuilder::new(); - for workspace in mw.workspaces() { - canonicalizer.add_workspace_mappings(workspace.read(cx), cx); - } - - // The worktree checkout path should canonicalize to the main repo. - assert_eq!( - canonicalizer.canonicalize_path(Path::new("/wt/feature-a")), - Path::new("/project"), - ); - }); - } -} diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index 6816898ffc55bbf81b2c17719b3bde6eb8b58e68..25a2b7ecb75ae11a551caa221609e8c5bfa1751e 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/crates/sidebar/src/sidebar.rs @@ -23,7 +23,9 @@ use gpui::{ use menu::{ Cancel, Confirm, SelectChild, SelectFirst, SelectLast, SelectNext, SelectParent, SelectPrevious, }; -use project::{AgentId, AgentRegistryStore, Event as ProjectEvent, linked_worktree_short_name}; +use project::{ + AgentId, AgentRegistryStore, Event as ProjectEvent, ProjectGroupKey, linked_worktree_short_name, +}; use recent_projects::sidebar_recent_projects::SidebarRecentProjects; use remote::RemoteConnectionOptions; use ui::utils::platform_title_bar_height; @@ -54,10 +56,6 @@ use zed_actions::agents_sidebar::{FocusSidebarFilter, ToggleThreadSwitcher}; use crate::thread_switcher::{ThreadSwitcher, ThreadSwitcherEntry, ThreadSwitcherEvent}; -use crate::project_group_builder::ProjectGroupBuilder; - -mod project_group_builder; - #[cfg(test)] mod sidebar_tests; @@ -136,13 +134,7 @@ impl ActiveEntry { (ActiveEntry::Thread { session_id, .. }, ListEntry::Thread(thread)) => { thread.metadata.session_id == *session_id } - ( - ActiveEntry::Draft(workspace), - ListEntry::NewThread { - workspace: entry_workspace, - .. - }, - ) => workspace == entry_workspace, + (ActiveEntry::Draft(_workspace), ListEntry::DraftThread { .. }) => true, _ => false, } } @@ -209,9 +201,8 @@ impl ThreadEntry { #[derive(Clone)] enum ListEntry { ProjectHeader { - path_list: PathList, + key: ProjectGroupKey, label: SharedString, - workspace: Entity, highlight_positions: Vec, has_running_threads: bool, waiting_thread_count: usize, @@ -219,30 +210,25 @@ enum ListEntry { }, Thread(ThreadEntry), ViewMore { - path_list: PathList, + key: ProjectGroupKey, is_fully_expanded: bool, }, + /// The user's active draft thread. Shows a prefix of the currently-typed + /// prompt, or "Untitled Thread" if the prompt is empty. + DraftThread { + worktrees: Vec, + }, + /// A convenience row for starting a new thread. Shown when a project group + /// has no threads, or when the active workspace contains linked worktrees + /// with no threads for that specific worktree set. NewThread { - path_list: PathList, - workspace: Entity, + key: project::ProjectGroupKey, worktrees: Vec, }, } #[cfg(test)] impl ListEntry { - fn workspace(&self) -> Option> { - match self { - ListEntry::ProjectHeader { workspace, .. } => Some(workspace.clone()), - ListEntry::Thread(thread_entry) => match &thread_entry.workspace { - ThreadEntryWorkspace::Open(workspace) => Some(workspace.clone()), - ThreadEntryWorkspace::Closed(_) => None, - }, - ListEntry::ViewMore { .. } => None, - ListEntry::NewThread { workspace, .. } => Some(workspace.clone()), - } - } - fn session_id(&self) -> Option<&acp::SessionId> { match self { ListEntry::Thread(thread_entry) => Some(&thread_entry.metadata.session_id), @@ -321,27 +307,32 @@ fn workspace_path_list(workspace: &Entity, cx: &App) -> PathList { /// Derives worktree display info from a thread's stored path list. /// -/// For each path in the thread's `folder_paths` that canonicalizes to a -/// different path (i.e. it's a git worktree), produces a [`WorktreeInfo`] -/// with the short worktree name and full path. +/// For each path in the thread's `folder_paths` that is not one of the +/// group's main paths (i.e. it's a git linked worktree), produces a +/// [`WorktreeInfo`] with the short worktree name and full path. fn worktree_info_from_thread_paths( folder_paths: &PathList, - project_groups: &ProjectGroupBuilder, + group_key: &project::ProjectGroupKey, ) -> Vec { + let main_paths = group_key.path_list().paths(); folder_paths .paths() .iter() .filter_map(|path| { - let canonical = project_groups.canonicalize_path(path); - if canonical != path.as_path() { - Some(WorktreeInfo { - name: linked_worktree_short_name(canonical, path).unwrap_or_default(), - full_path: SharedString::from(path.display().to_string()), - highlight_positions: Vec::new(), - }) - } else { - None + if main_paths.iter().any(|mp| mp.as_path() == path.as_path()) { + return None; } + // Find the main path whose file name matches this linked + // worktree's file name, falling back to the first main path. + let main_path = main_paths + .iter() + .find(|mp| mp.file_name() == path.file_name()) + .or(main_paths.first())?; + Some(WorktreeInfo { + name: linked_worktree_short_name(main_path, path).unwrap_or_default(), + full_path: SharedString::from(path.display().to_string()), + highlight_positions: Vec::new(), + }) }) .collect() } @@ -677,10 +668,41 @@ impl Sidebar { result } + /// Finds an open workspace whose project group key matches the given path list. + fn workspace_for_group(&self, path_list: &PathList, cx: &App) -> Option> { + let mw = self.multi_workspace.upgrade()?; + let mw = mw.read(cx); + mw.workspaces() + .iter() + .find(|ws| ws.read(cx).project_group_key(cx).path_list() == path_list) + .cloned() + } + + /// Opens a new workspace for a group that has no open workspaces. + fn open_workspace_for_group( + &mut self, + path_list: &PathList, + window: &mut Window, + cx: &mut Context, + ) { + let Some(multi_workspace) = self.multi_workspace.upgrade() else { + return; + }; + + let paths: Vec = + path_list.paths().iter().map(|p| p.to_path_buf()).collect(); + + multi_workspace + .update(cx, |mw, cx| { + mw.open_project(paths, workspace::OpenMode::Activate, window, cx) + }) + .detach_and_log_err(cx); + } + /// Rebuilds the sidebar contents from current workspace and thread state. /// - /// Uses [`ProjectGroupBuilder`] to group workspaces by their main git - /// repository, then populates thread entries from the metadata store and + /// Iterates [`MultiWorkspace::project_group_keys`] to determine project + /// groups, then populates thread entries from the metadata store and /// merges live thread info from active agent panels. /// /// Aim for a single forward pass over workspaces and threads plus an @@ -764,11 +786,6 @@ impl Sidebar { let mut current_session_ids: HashSet = HashSet::new(); let mut project_header_indices: Vec = Vec::new(); - // Use ProjectGroupBuilder to canonically group workspaces by their - // main git repository. This replaces the manual absorbed-workspace - // detection that was here before. - let project_groups = ProjectGroupBuilder::from_multiworkspace(mw, cx); - let has_open_projects = workspaces .iter() .any(|ws| !workspace_path_list(ws, cx).paths().is_empty()); @@ -785,38 +802,28 @@ impl Sidebar { (icon, icon_from_external_svg) }; - for (group_name, group) in project_groups.groups() { - let path_list = group_name.path_list().clone(); + for (group_key, group_workspaces) in mw.project_groups(cx) { + let path_list = group_key.path_list().clone(); if path_list.paths().is_empty() { continue; } - let label = group_name.display_name(); + let label = group_key.display_name(); let is_collapsed = self.collapsed_groups.contains(&path_list); let should_load_threads = !is_collapsed || !query.is_empty(); let is_active = active_workspace .as_ref() - .is_some_and(|active| group.workspaces.contains(active)); - - // Pick a representative workspace for the group: prefer the active - // workspace if it belongs to this group, otherwise use the main - // repo workspace (not a linked worktree). - let representative_workspace = active_workspace - .as_ref() - .filter(|_| is_active) - .unwrap_or_else(|| group.main_workspace(cx)); + .is_some_and(|active| group_workspaces.contains(active)); // Collect live thread infos from all workspaces in this group. - let live_infos: Vec<_> = group - .workspaces + let live_infos: Vec<_> = group_workspaces .iter() .flat_map(|ws| all_thread_infos_for_workspace(ws, cx)) .collect(); let mut threads: Vec = Vec::new(); - let mut threadless_workspaces: Vec<(Entity, Vec)> = Vec::new(); let mut has_running_threads = false; let mut waiting_thread_count: usize = 0; @@ -824,61 +831,88 @@ impl Sidebar { let mut seen_session_ids: HashSet = HashSet::new(); let thread_store = ThreadMetadataStore::global(cx); - // Load threads from each workspace in the group. - for workspace in &group.workspaces { - let ws_path_list = workspace_path_list(workspace, cx); - let mut workspace_rows = thread_store - .read(cx) - .entries_for_path(&ws_path_list) - .cloned() - .peekable(); - if workspace_rows.peek().is_none() { - let worktrees = - worktree_info_from_thread_paths(&ws_path_list, &project_groups); - threadless_workspaces.push((workspace.clone(), worktrees)); + // Build a lookup from workspace root paths to their workspace + // entity, used to assign ThreadEntryWorkspace::Open for threads + // whose folder_paths match an open workspace. + let workspace_by_path_list: HashMap> = + group_workspaces + .iter() + .map(|ws| (workspace_path_list(ws, cx), ws)) + .collect(); + + // Resolve a ThreadEntryWorkspace for a thread row. If any open + // workspace's root paths match the thread's folder_paths, use + // Open; otherwise use Closed. + let resolve_workspace = |row: &ThreadMetadata| -> ThreadEntryWorkspace { + workspace_by_path_list + .get(&row.folder_paths) + .map(|ws| ThreadEntryWorkspace::Open((*ws).clone())) + .unwrap_or_else(|| ThreadEntryWorkspace::Closed(row.folder_paths.clone())) + }; + + // Build a ThreadEntry from a metadata row. + let make_thread_entry = |row: ThreadMetadata, + workspace: ThreadEntryWorkspace| + -> ThreadEntry { + let (icon, icon_from_external_svg) = resolve_agent_icon(&row.agent_id); + let worktrees = worktree_info_from_thread_paths(&row.folder_paths, &group_key); + ThreadEntry { + metadata: row, + icon, + icon_from_external_svg, + status: AgentThreadStatus::default(), + workspace, + is_live: false, + is_background: false, + is_title_generating: false, + highlight_positions: Vec::new(), + worktrees, + diff_stats: DiffStats::default(), } - for row in workspace_rows { - if !seen_session_ids.insert(row.session_id.clone()) { - continue; - } - let (icon, icon_from_external_svg) = resolve_agent_icon(&row.agent_id); - let worktrees = - worktree_info_from_thread_paths(&row.folder_paths, &project_groups); - threads.push(ThreadEntry { - metadata: row, - icon, - icon_from_external_svg, - status: AgentThreadStatus::default(), - workspace: ThreadEntryWorkspace::Open(workspace.clone()), - is_live: false, - is_background: false, - is_title_generating: false, - highlight_positions: Vec::new(), - worktrees, - diff_stats: DiffStats::default(), - }); + }; + + // === Main code path: one query per group via main_worktree_paths === + // The main_worktree_paths column is set on all new threads and + // points to the group's canonical paths regardless of which + // linked worktree the thread was opened in. + for row in thread_store + .read(cx) + .entries_for_main_worktree_path(&path_list) + .cloned() + { + if !seen_session_ids.insert(row.session_id.clone()) { + continue; } + let workspace = resolve_workspace(&row); + threads.push(make_thread_entry(row, workspace)); } - // Load threads from linked git worktrees whose - // canonical paths belong to this group. - let linked_worktree_queries = group - .workspaces - .iter() - .flat_map(|ws| root_repository_snapshots(ws, cx)) - .filter(|snapshot| !snapshot.is_linked_worktree()) - .flat_map(|snapshot| { - snapshot - .linked_worktrees() - .iter() - .filter(|wt| { - project_groups.group_owns_worktree(group, &path_list, &wt.path) - }) - .map(|wt| PathList::new(std::slice::from_ref(&wt.path))) - .collect::>() - }); + // Legacy threads did not have `main_worktree_paths` populated, so they + // must be queried by their `folder_paths`. + + // Load any legacy threads for the main worktrees of this project group. + for row in thread_store.read(cx).entries_for_path(&path_list).cloned() { + if !seen_session_ids.insert(row.session_id.clone()) { + continue; + } + let workspace = resolve_workspace(&row); + threads.push(make_thread_entry(row, workspace)); + } - for worktree_path_list in linked_worktree_queries { + // Load any legacy threads for any single linked wortree of this project group. + let mut linked_worktree_paths = HashSet::new(); + for workspace in &group_workspaces { + if workspace.read(cx).visible_worktrees(cx).count() != 1 { + continue; + } + for snapshot in root_repository_snapshots(workspace, cx) { + for linked_worktree in snapshot.linked_worktrees() { + linked_worktree_paths.insert(linked_worktree.path.clone()); + } + } + } + for path in linked_worktree_paths { + let worktree_path_list = PathList::new(std::slice::from_ref(&path)); for row in thread_store .read(cx) .entries_for_path(&worktree_path_list) @@ -887,67 +921,10 @@ impl Sidebar { if !seen_session_ids.insert(row.session_id.clone()) { continue; } - let (icon, icon_from_external_svg) = resolve_agent_icon(&row.agent_id); - let worktrees = - worktree_info_from_thread_paths(&row.folder_paths, &project_groups); - threads.push(ThreadEntry { - metadata: row, - icon, - icon_from_external_svg, - status: AgentThreadStatus::default(), - workspace: ThreadEntryWorkspace::Closed(worktree_path_list.clone()), - is_live: false, - is_background: false, - is_title_generating: false, - highlight_positions: Vec::new(), - worktrees, - diff_stats: DiffStats::default(), - }); - } - } - - // Load threads from main worktrees when a workspace in this - // group is itself a linked worktree checkout. - let main_repo_queries: Vec = group - .workspaces - .iter() - .flat_map(|ws| root_repository_snapshots(ws, cx)) - .filter(|snapshot| snapshot.is_linked_worktree()) - .map(|snapshot| { - PathList::new(std::slice::from_ref(&snapshot.original_repo_abs_path)) - }) - .collect(); - - for main_repo_path_list in main_repo_queries { - let folder_path_matches = thread_store - .read(cx) - .entries_for_path(&main_repo_path_list) - .cloned(); - let main_worktree_path_matches = thread_store - .read(cx) - .entries_for_main_worktree_path(&main_repo_path_list) - .cloned(); - - for row in folder_path_matches.chain(main_worktree_path_matches) { - if !seen_session_ids.insert(row.session_id.clone()) { - continue; - } - let (icon, icon_from_external_svg) = resolve_agent_icon(&row.agent_id); - let worktrees = - worktree_info_from_thread_paths(&row.folder_paths, &project_groups); - threads.push(ThreadEntry { - metadata: row, - icon, - icon_from_external_svg, - status: AgentThreadStatus::default(), - workspace: ThreadEntryWorkspace::Closed(main_repo_path_list.clone()), - is_live: false, - is_background: false, - is_title_generating: false, - highlight_positions: Vec::new(), - worktrees, - diff_stats: DiffStats::default(), - }); + threads.push(make_thread_entry( + row, + ThreadEntryWorkspace::Closed(worktree_path_list.clone()), + )); } } @@ -1051,9 +1028,8 @@ impl Sidebar { project_header_indices.push(entries.len()); entries.push(ListEntry::ProjectHeader { - path_list: path_list.clone(), + key: group_key.clone(), label, - workspace: representative_workspace.clone(), highlight_positions: workspace_highlight_positions, has_running_threads, waiting_thread_count, @@ -1065,15 +1041,13 @@ impl Sidebar { entries.push(thread.into()); } } else { - let is_draft_for_workspace = is_active - && matches!(&self.active_entry, Some(ActiveEntry::Draft(_))) - && self.active_entry_workspace() == Some(representative_workspace); + let is_draft_for_group = is_active + && matches!(&self.active_entry, Some(ActiveEntry::Draft(ws)) if group_workspaces.contains(ws)); project_header_indices.push(entries.len()); entries.push(ListEntry::ProjectHeader { - path_list: path_list.clone(), + key: group_key.clone(), label, - workspace: representative_workspace.clone(), highlight_positions: Vec::new(), has_running_threads, waiting_thread_count, @@ -1084,25 +1058,61 @@ impl Sidebar { continue; } - // Emit "New Thread" entries for threadless workspaces - // and active drafts, right after the header. - for (workspace, worktrees) in &threadless_workspaces { - entries.push(ListEntry::NewThread { - path_list: path_list.clone(), - workspace: workspace.clone(), - worktrees: worktrees.clone(), - }); + // Emit a DraftThread entry when the active draft belongs to this group. + if is_draft_for_group { + if let Some(ActiveEntry::Draft(draft_ws)) = &self.active_entry { + let ws_path_list = workspace_path_list(draft_ws, cx); + let worktrees = worktree_info_from_thread_paths(&ws_path_list, &group_key); + entries.push(ListEntry::DraftThread { worktrees }); + } } - if is_draft_for_workspace - && !threadless_workspaces - .iter() - .any(|(ws, _)| ws == representative_workspace) + + // Emit a NewThread entry when: + // 1. The group has zero threads (convenient affordance). + // 2. The active workspace has linked worktrees but no threads + // for the active workspace's specific set of worktrees. + let group_has_no_threads = threads.is_empty() && !group_workspaces.is_empty(); + let active_ws_has_threadless_linked_worktrees = is_active + && !is_draft_for_group + && active_workspace.as_ref().is_some_and(|active_ws| { + let ws_path_list = workspace_path_list(active_ws, cx); + let has_linked_worktrees = + !worktree_info_from_thread_paths(&ws_path_list, &group_key).is_empty(); + if !has_linked_worktrees { + return false; + } + let thread_store = ThreadMetadataStore::global(cx); + let has_threads_for_ws = thread_store + .read(cx) + .entries_for_path(&ws_path_list) + .next() + .is_some() + || thread_store + .read(cx) + .entries_for_main_worktree_path(&ws_path_list) + .next() + .is_some(); + !has_threads_for_ws + }); + + if !is_draft_for_group + && (group_has_no_threads || active_ws_has_threadless_linked_worktrees) { - let ws_path_list = workspace_path_list(representative_workspace, cx); - let worktrees = worktree_info_from_thread_paths(&ws_path_list, &project_groups); + let worktrees = if active_ws_has_threadless_linked_worktrees { + active_workspace + .as_ref() + .map(|ws| { + worktree_info_from_thread_paths( + &workspace_path_list(ws, cx), + &group_key, + ) + }) + .unwrap_or_default() + } else { + Vec::new() + }; entries.push(ListEntry::NewThread { - path_list: path_list.clone(), - workspace: representative_workspace.clone(), + key: group_key.clone(), worktrees, }); } @@ -1148,7 +1158,7 @@ impl Sidebar { if total > DEFAULT_THREADS_SHOWN { entries.push(ListEntry::ViewMore { - path_list: path_list.clone(), + key: group_key.clone(), is_fully_expanded, }); } @@ -1236,9 +1246,8 @@ impl Sidebar { let rendered = match entry { ListEntry::ProjectHeader { - path_list, + key, label, - workspace, highlight_positions, has_running_threads, waiting_thread_count, @@ -1246,9 +1255,8 @@ impl Sidebar { } => self.render_project_header( ix, false, - path_list, + key, label, - workspace, highlight_positions, *has_running_threads, *waiting_thread_count, @@ -1258,22 +1266,15 @@ impl Sidebar { ), ListEntry::Thread(thread) => self.render_thread(ix, thread, is_active, is_selected, cx), ListEntry::ViewMore { - path_list, + key, is_fully_expanded, - } => self.render_view_more(ix, path_list, *is_fully_expanded, is_selected, cx), - ListEntry::NewThread { - path_list, - workspace, - worktrees, - } => self.render_new_thread( - ix, - path_list, - workspace, - is_active, - worktrees, - is_selected, - cx, - ), + } => self.render_view_more(ix, key.path_list(), *is_fully_expanded, is_selected, cx), + ListEntry::DraftThread { worktrees, .. } => { + self.render_draft_thread(ix, is_active, worktrees, is_selected, cx) + } + ListEntry::NewThread { key, worktrees, .. } => { + self.render_new_thread(ix, key, worktrees, is_selected, cx) + } }; if is_group_header_after_first { @@ -1291,13 +1292,9 @@ impl Sidebar { fn render_remote_project_icon( &self, ix: usize, - workspace: &Entity, - cx: &mut Context, + host: Option<&RemoteConnectionOptions>, ) -> Option { - let project = workspace.read(cx).project().read(cx); - let remote_connection_options = project.remote_connection_options(cx)?; - - let remote_icon_per_type = match remote_connection_options { + let remote_icon_per_type = match host? { RemoteConnectionOptions::Wsl(_) => IconName::Linux, RemoteConnectionOptions::Docker(_) => IconName::Box, _ => IconName::Server, @@ -1320,9 +1317,8 @@ impl Sidebar { &self, ix: usize, is_sticky: bool, - path_list: &PathList, + key: &ProjectGroupKey, label: &SharedString, - workspace: &Entity, highlight_positions: &[usize], has_running_threads: bool, waiting_thread_count: usize, @@ -1330,6 +1326,9 @@ impl Sidebar { is_focused: bool, cx: &mut Context, ) -> AnyElement { + let path_list = key.path_list(); + let host = key.host(); + let id_prefix = if is_sticky { "sticky-" } else { "" }; let id = SharedString::from(format!("{id_prefix}project-header-{ix}")); let disclosure_id = SharedString::from(format!("disclosure-{ix}")); @@ -1342,16 +1341,15 @@ impl Sidebar { (IconName::ChevronDown, "Collapse Project") }; - let has_new_thread_entry = self - .contents - .entries - .get(ix + 1) - .is_some_and(|entry| matches!(entry, ListEntry::NewThread { .. })); + let has_new_thread_entry = self.contents.entries.get(ix + 1).is_some_and(|entry| { + matches!( + entry, + ListEntry::NewThread { .. } | ListEntry::DraftThread { .. } + ) + }); let show_new_thread_button = !has_new_thread_entry && !self.has_filter_query(cx); - let workspace_for_remove = workspace.clone(); - let workspace_for_menu = workspace.clone(); - let workspace_for_open = workspace.clone(); + let workspace = self.workspace_for_group(path_list, cx); let path_list_for_toggle = path_list.clone(); let path_list_for_collapse = path_list.clone(); @@ -1408,7 +1406,7 @@ impl Sidebar { ) .child(label) .when_some( - self.render_remote_project_icon(ix, workspace, cx), + self.render_remote_project_icon(ix, host.as_ref()), |this, icon| this.child(icon), ) .when(is_collapsed, |this| { @@ -1452,13 +1450,13 @@ impl Sidebar { .on_mouse_down(gpui::MouseButton::Left, |_, _, cx| { cx.stop_propagation(); }) - .child(self.render_project_header_menu( - ix, - id_prefix, - &workspace_for_menu, - &workspace_for_remove, - cx, - )) + .when_some(workspace, |this, workspace| { + this.child( + self.render_project_header_menu( + ix, id_prefix, &workspace, &workspace, cx, + ), + ) + }) .when(view_more_expanded && !is_collapsed, |this| { this.child( IconButton::new( @@ -1480,52 +1478,56 @@ impl Sidebar { })), ) }) - .when(show_new_thread_button, |this| { - this.child( - IconButton::new( - SharedString::from(format!( - "{id_prefix}project-header-new-thread-{ix}", + .when( + show_new_thread_button && workspace_for_new_thread.is_some(), + |this| { + let workspace_for_new_thread = + workspace_for_new_thread.clone().unwrap(); + let path_list_for_new_thread = path_list_for_new_thread.clone(); + this.child( + IconButton::new( + SharedString::from(format!( + "{id_prefix}project-header-new-thread-{ix}", + )), + IconName::Plus, + ) + .icon_size(IconSize::Small) + .tooltip(Tooltip::text("New Thread")) + .on_click(cx.listener( + move |this, _, window, cx| { + this.collapsed_groups.remove(&path_list_for_new_thread); + this.selection = None; + this.create_new_thread( + &workspace_for_new_thread, + window, + cx, + ); + }, )), - IconName::Plus, ) - .icon_size(IconSize::Small) - .tooltip(Tooltip::text("New Thread")) - .on_click(cx.listener({ - let workspace_for_new_thread = workspace_for_new_thread.clone(); - let path_list_for_new_thread = path_list_for_new_thread.clone(); - move |this, _, window, cx| { - // Uncollapse the group if collapsed so - // the new-thread entry becomes visible. - this.collapsed_groups.remove(&path_list_for_new_thread); - this.selection = None; - this.create_new_thread(&workspace_for_new_thread, window, cx); - } - })), - ) - }) + }, + ) }) .when(!is_active, |this| { + let path_list_for_open = path_list.clone(); this.cursor_pointer() .hover(|s| s.bg(hover_color)) - .tooltip(Tooltip::text("Activate Workspace")) - .on_click(cx.listener({ - move |this, _, window, cx| { - this.active_entry = - Some(ActiveEntry::Draft(workspace_for_open.clone())); + .tooltip(Tooltip::text("Open Workspace")) + .on_click(cx.listener(move |this, _, window, cx| { + if let Some(workspace) = this.workspace_for_group(&path_list_for_open, cx) { + this.active_entry = Some(ActiveEntry::Draft(workspace.clone())); if let Some(multi_workspace) = this.multi_workspace.upgrade() { multi_workspace.update(cx, |multi_workspace, cx| { - multi_workspace.activate( - workspace_for_open.clone(), - window, - cx, - ); + multi_workspace.activate(workspace.clone(), window, cx); }); } - if AgentPanel::is_visible(&workspace_for_open, cx) { - workspace_for_open.update(cx, |workspace, cx| { + if AgentPanel::is_visible(&workspace, cx) { + workspace.update(cx, |workspace, cx| { workspace.focus_panel::(window, cx); }); } + } else { + this.open_workspace_for_group(&path_list_for_open, window, cx); } })) }) @@ -1720,9 +1722,8 @@ impl Sidebar { } let ListEntry::ProjectHeader { - path_list, + key, label, - workspace, highlight_positions, has_running_threads, waiting_thread_count, @@ -1738,9 +1739,8 @@ impl Sidebar { let header_element = self.render_project_header( header_idx, true, - &path_list, + key, &label, - workspace, &highlight_positions, *has_running_threads, *waiting_thread_count, @@ -1961,8 +1961,8 @@ impl Sidebar { }; match entry { - ListEntry::ProjectHeader { path_list, .. } => { - let path_list = path_list.clone(); + ListEntry::ProjectHeader { key, .. } => { + let path_list = key.path_list().clone(); self.toggle_collapse(&path_list, window, cx); } ListEntry::Thread(thread) => { @@ -1983,11 +1983,11 @@ impl Sidebar { } } ListEntry::ViewMore { - path_list, + key, is_fully_expanded, .. } => { - let path_list = path_list.clone(); + let path_list = key.path_list().clone(); if *is_fully_expanded { self.expanded_groups.remove(&path_list); } else { @@ -1997,9 +1997,16 @@ impl Sidebar { self.serialize(cx); self.update_entries(cx); } - ListEntry::NewThread { workspace, .. } => { - let workspace = workspace.clone(); - self.create_new_thread(&workspace, window, cx); + ListEntry::DraftThread { .. } => { + // Already active — nothing to do. + } + ListEntry::NewThread { key, .. } => { + let path_list = key.path_list().clone(); + if let Some(workspace) = self.workspace_for_group(&path_list, cx) { + self.create_new_thread(&workspace, window, cx); + } else { + self.open_workspace_for_group(&path_list, window, cx); + } } } } @@ -2251,9 +2258,9 @@ impl Sidebar { let Some(ix) = self.selection else { return }; match self.contents.entries.get(ix) { - Some(ListEntry::ProjectHeader { path_list, .. }) => { - if self.collapsed_groups.contains(path_list) { - let path_list = path_list.clone(); + Some(ListEntry::ProjectHeader { key, .. }) => { + if self.collapsed_groups.contains(key.path_list()) { + let path_list = key.path_list().clone(); self.collapsed_groups.remove(&path_list); self.update_entries(cx); } else if ix + 1 < self.contents.entries.len() { @@ -2275,23 +2282,23 @@ impl Sidebar { let Some(ix) = self.selection else { return }; match self.contents.entries.get(ix) { - Some(ListEntry::ProjectHeader { path_list, .. }) => { - if !self.collapsed_groups.contains(path_list) { - let path_list = path_list.clone(); - self.collapsed_groups.insert(path_list); + Some(ListEntry::ProjectHeader { key, .. }) => { + if !self.collapsed_groups.contains(key.path_list()) { + self.collapsed_groups.insert(key.path_list().clone()); self.update_entries(cx); } } Some( - ListEntry::Thread(_) | ListEntry::ViewMore { .. } | ListEntry::NewThread { .. }, + ListEntry::Thread(_) + | ListEntry::ViewMore { .. } + | ListEntry::NewThread { .. } + | ListEntry::DraftThread { .. }, ) => { for i in (0..ix).rev() { - if let Some(ListEntry::ProjectHeader { path_list, .. }) = - self.contents.entries.get(i) + if let Some(ListEntry::ProjectHeader { key, .. }) = self.contents.entries.get(i) { - let path_list = path_list.clone(); self.selection = Some(i); - self.collapsed_groups.insert(path_list); + self.collapsed_groups.insert(key.path_list().clone()); self.update_entries(cx); break; } @@ -2313,7 +2320,10 @@ impl Sidebar { let header_ix = match self.contents.entries.get(ix) { Some(ListEntry::ProjectHeader { .. }) => Some(ix), Some( - ListEntry::Thread(_) | ListEntry::ViewMore { .. } | ListEntry::NewThread { .. }, + ListEntry::Thread(_) + | ListEntry::ViewMore { .. } + | ListEntry::NewThread { .. } + | ListEntry::DraftThread { .. }, ) => (0..ix).rev().find(|&i| { matches!( self.contents.entries.get(i), @@ -2324,15 +2334,14 @@ impl Sidebar { }; if let Some(header_ix) = header_ix { - if let Some(ListEntry::ProjectHeader { path_list, .. }) = - self.contents.entries.get(header_ix) + if let Some(ListEntry::ProjectHeader { key, .. }) = self.contents.entries.get(header_ix) { - let path_list = path_list.clone(); - if self.collapsed_groups.contains(&path_list) { - self.collapsed_groups.remove(&path_list); + let path_list = key.path_list(); + if self.collapsed_groups.contains(path_list) { + self.collapsed_groups.remove(path_list); } else { self.selection = Some(header_ix); - self.collapsed_groups.insert(path_list); + self.collapsed_groups.insert(path_list.clone()); } self.update_entries(cx); } @@ -2346,8 +2355,8 @@ impl Sidebar { cx: &mut Context, ) { for entry in &self.contents.entries { - if let ListEntry::ProjectHeader { path_list, .. } = entry { - self.collapsed_groups.insert(path_list.clone()); + if let ListEntry::ProjectHeader { key, .. } = entry { + self.collapsed_groups.insert(key.path_list().clone()); } } self.update_entries(cx); @@ -2402,17 +2411,18 @@ impl Sidebar { }); // Find the workspace that owns this thread's project group by - // walking backwards to the nearest ProjectHeader. We must use - // *this* workspace (not the active workspace) because the user - // might be archiving a thread in a non-active group. + // walking backwards to the nearest ProjectHeader and looking up + // an open workspace for that group's path_list. let group_workspace = current_pos.and_then(|pos| { - self.contents.entries[..pos] - .iter() - .rev() - .find_map(|e| match e { - ListEntry::ProjectHeader { workspace, .. } => Some(workspace.clone()), - _ => None, - }) + let path_list = + self.contents.entries[..pos] + .iter() + .rev() + .find_map(|e| match e { + ListEntry::ProjectHeader { key, .. } => Some(key.path_list()), + _ => None, + })?; + self.workspace_for_group(path_list, cx) }); let next_thread = current_pos.and_then(|pos| { @@ -2527,28 +2537,26 @@ impl Sidebar { .insert(session_id.clone(), Utc::now()); } - fn mru_threads_for_switcher(&self, _cx: &App) -> Vec { + fn mru_threads_for_switcher(&self, cx: &App) -> Vec { let mut current_header_label: Option = None; - let mut current_header_workspace: Option> = None; + let mut current_header_path_list: Option = None; let mut entries: Vec = self .contents .entries .iter() .filter_map(|entry| match entry { - ListEntry::ProjectHeader { - label, workspace, .. - } => { + ListEntry::ProjectHeader { label, key, .. } => { current_header_label = Some(label.clone()); - current_header_workspace = Some(workspace.clone()); + current_header_path_list = Some(key.path_list().clone()); None } ListEntry::Thread(thread) => { let workspace = match &thread.workspace { - ThreadEntryWorkspace::Open(workspace) => workspace.clone(), - ThreadEntryWorkspace::Closed(_) => { - current_header_workspace.as_ref()?.clone() - } - }; + ThreadEntryWorkspace::Open(workspace) => Some(workspace.clone()), + ThreadEntryWorkspace::Closed(_) => current_header_path_list + .as_ref() + .and_then(|pl| self.workspace_for_group(pl, cx)), + }?; let notified = self .contents .is_thread_notified(&thread.metadata.session_id); @@ -3055,7 +3063,9 @@ impl Sidebar { .rev() .find(|&&header_ix| header_ix <= selected_ix) .and_then(|&header_ix| match &self.contents.entries[header_ix] { - ListEntry::ProjectHeader { workspace, .. } => Some(workspace.clone()), + ListEntry::ProjectHeader { key, .. } => { + self.workspace_for_group(key.path_list(), cx) + } _ => None, }) } else { @@ -3098,11 +3108,9 @@ impl Sidebar { }); } - fn render_new_thread( + fn render_draft_thread( &self, ix: usize, - _path_list: &PathList, - workspace: &Entity, is_active: bool, worktrees: &[WorktreeInfo], is_selected: bool, @@ -3110,12 +3118,48 @@ impl Sidebar { ) -> AnyElement { let label: SharedString = if is_active { self.active_draft_text(cx) - .unwrap_or_else(|| DEFAULT_THREAD_TITLE.into()) + .unwrap_or_else(|| "Untitled Thread".into()) } else { - DEFAULT_THREAD_TITLE.into() + "Untitled Thread".into() }; - let workspace = workspace.clone(); + let id = SharedString::from(format!("draft-thread-btn-{}", ix)); + + let thread_item = ThreadItem::new(id, label) + .icon(IconName::Plus) + .icon_color(Color::Custom(cx.theme().colors().icon_muted.opacity(0.8))) + .worktrees( + worktrees + .iter() + .map(|wt| ThreadItemWorktreeInfo { + name: wt.name.clone(), + full_path: wt.full_path.clone(), + highlight_positions: wt.highlight_positions.clone(), + }) + .collect(), + ) + .selected(true) + .focused(is_selected); + + div() + .on_mouse_down(gpui::MouseButton::Left, |_, _, cx| { + cx.stop_propagation(); + }) + .child(thread_item) + .into_any_element() + } + + fn render_new_thread( + &self, + ix: usize, + key: &ProjectGroupKey, + worktrees: &[WorktreeInfo], + is_selected: bool, + cx: &mut Context, + ) -> AnyElement { + let label: SharedString = DEFAULT_THREAD_TITLE.into(); + let path_list = key.path_list().clone(); + let id = SharedString::from(format!("new-thread-btn-{}", ix)); let thread_item = ThreadItem::new(id, label) @@ -3131,25 +3175,18 @@ impl Sidebar { }) .collect(), ) - .selected(is_active) + .selected(false) .focused(is_selected) - .when(!is_active, |this| { - this.on_click(cx.listener(move |this, _, window, cx| { - this.selection = None; + .on_click(cx.listener(move |this, _, window, cx| { + this.selection = None; + if let Some(workspace) = this.workspace_for_group(&path_list, cx) { this.create_new_thread(&workspace, window, cx); - })) - }); + } else { + this.open_workspace_for_group(&path_list, window, cx); + } + })); - if is_active { - div() - .on_mouse_down(gpui::MouseButton::Left, |_, _, cx| { - cx.stop_propagation(); - }) - .child(thread_item) - .into_any_element() - } else { - thread_item.into_any_element() - } + thread_item.into_any_element() } fn render_no_results(&self, cx: &mut Context) -> impl IntoElement { diff --git a/crates/sidebar/src/sidebar_tests.rs b/crates/sidebar/src/sidebar_tests.rs index 1499fc48a9fd094b07d181701866ab941c5968f3..cf1ee8a0f524d9d94edf83c24ecea900f3261fb8 100644 --- a/crates/sidebar/src/sidebar_tests.rs +++ b/crates/sidebar/src/sidebar_tests.rs @@ -88,14 +88,18 @@ fn setup_sidebar( sidebar } -async fn save_n_test_threads(count: u32, path_list: &PathList, cx: &mut gpui::VisualTestContext) { +async fn save_n_test_threads( + count: u32, + project: &Entity, + cx: &mut gpui::VisualTestContext, +) { for i in 0..count { save_thread_metadata( acp::SessionId::new(Arc::from(format!("thread-{}", i))), format!("Thread {}", i + 1).into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, i).unwrap(), None, - path_list.clone(), + project, cx, ) } @@ -104,7 +108,7 @@ async fn save_n_test_threads(count: u32, path_list: &PathList, cx: &mut gpui::Vi async fn save_test_thread_metadata( session_id: &acp::SessionId, - path_list: PathList, + project: &Entity, cx: &mut TestAppContext, ) { save_thread_metadata( @@ -112,7 +116,7 @@ async fn save_test_thread_metadata( "Test".into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, - path_list, + project, cx, ) } @@ -120,7 +124,7 @@ async fn save_test_thread_metadata( async fn save_named_thread_metadata( session_id: &str, title: &str, - path_list: &PathList, + project: &Entity, cx: &mut gpui::VisualTestContext, ) { save_thread_metadata( @@ -128,7 +132,7 @@ async fn save_named_thread_metadata( SharedString::from(title.to_string()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, - path_list.clone(), + project, cx, ); cx.run_until_parked(); @@ -139,21 +143,31 @@ fn save_thread_metadata( title: SharedString, updated_at: DateTime, created_at: Option>, - path_list: PathList, + project: &Entity, cx: &mut TestAppContext, ) { - let metadata = ThreadMetadata { - session_id, - agent_id: agent::ZED_AGENT_ID.clone(), - title, - updated_at, - created_at, - folder_paths: path_list, - main_worktree_paths: PathList::default(), - archived: false, - }; cx.update(|cx| { - ThreadMetadataStore::global(cx).update(cx, |store, cx| store.save_manually(metadata, cx)) + let (folder_paths, main_worktree_paths) = { + let project_ref = project.read(cx); + let paths: Vec> = project_ref + .visible_worktrees(cx) + .map(|worktree| worktree.read(cx).abs_path()) + .collect(); + let folder_paths = PathList::new(&paths); + let main_worktree_paths = project_ref.project_group_key(cx).path_list().clone(); + (folder_paths, main_worktree_paths) + }; + let metadata = ThreadMetadata { + session_id, + agent_id: agent::ZED_AGENT_ID.clone(), + title, + updated_at, + created_at, + folder_paths, + main_worktree_paths, + archived: false, + }; + ThreadMetadataStore::global(cx).update(cx, |store, cx| store.save_manually(metadata, cx)); }); cx.run_until_parked(); } @@ -193,11 +207,11 @@ fn visible_entries_as_strings( match entry { ListEntry::ProjectHeader { label, - path_list, + key, highlight_positions: _, .. } => { - let icon = if sidebar.collapsed_groups.contains(path_list) { + let icon = if sidebar.collapsed_groups.contains(key.path_list()) { ">" } else { "v" @@ -248,6 +262,22 @@ fn visible_entries_as_strings( format!(" + View More{}", selected) } } + ListEntry::DraftThread { worktrees, .. } => { + let worktree = if worktrees.is_empty() { + String::new() + } else { + let mut seen = Vec::new(); + let mut chips = Vec::new(); + for wt in worktrees { + if !seen.contains(&wt.name) { + seen.push(wt.name.clone()); + chips.push(format!("{{{}}}", wt.name)); + } + } + format!(" {}", chips.join(", ")) + }; + format!(" [~ Draft{}]{}", worktree, selected) + } ListEntry::NewThread { worktrees, .. } => { let worktree = if worktrees.is_empty() { String::new() @@ -274,11 +304,14 @@ fn visible_entries_as_strings( async fn test_serialization_round_trip(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)); + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let sidebar = setup_sidebar(&multi_workspace, cx); - let path_list = PathList::new(&[std::path::PathBuf::from("/my-project")]); - save_n_test_threads(3, &path_list, cx).await; + save_n_test_threads(3, &project, cx).await; + + let path_list = project.read_with(cx, |project, cx| { + project.project_group_key(cx).path_list().clone() + }); // Set a custom width, collapse the group, and expand "View More". sidebar.update_in(cx, |sidebar, window, cx| { @@ -437,17 +470,15 @@ async fn test_single_workspace_no_threads(cx: &mut TestAppContext) { async fn test_single_workspace_with_saved_threads(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)); + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let sidebar = setup_sidebar(&multi_workspace, cx); - let path_list = PathList::new(&[std::path::PathBuf::from("/my-project")]); - save_thread_metadata( acp::SessionId::new(Arc::from("thread-1")), "Fix crash in project panel".into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 3, 0, 0, 0).unwrap(), None, - path_list.clone(), + &project, cx, ); @@ -456,7 +487,7 @@ async fn test_single_workspace_with_saved_threads(cx: &mut TestAppContext) { "Add inline diff view".into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 2, 0, 0, 0).unwrap(), None, - path_list, + &project, cx, ); cx.run_until_parked(); @@ -478,18 +509,16 @@ async fn test_single_workspace_with_saved_threads(cx: &mut TestAppContext) { async fn test_workspace_lifecycle(cx: &mut TestAppContext) { let project = init_test_project("/project-a", cx).await; let (multi_workspace, cx) = - cx.add_window_view(|window, cx| MultiWorkspace::test_new(project, window, cx)); + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let sidebar = setup_sidebar(&multi_workspace, cx); // Single workspace with a thread - let path_list = PathList::new(&[std::path::PathBuf::from("/project-a")]); - save_thread_metadata( acp::SessionId::new(Arc::from("thread-a1")), "Thread A1".into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, - path_list, + &project, cx, ); cx.run_until_parked(); @@ -530,11 +559,10 @@ async fn test_workspace_lifecycle(cx: &mut TestAppContext) { async fn test_view_more_pagination(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)); + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let sidebar = setup_sidebar(&multi_workspace, cx); - let path_list = PathList::new(&[std::path::PathBuf::from("/my-project")]); - save_n_test_threads(12, &path_list, cx).await; + save_n_test_threads(12, &project, cx).await; multi_workspace.update_in(cx, |_, _window, cx| cx.notify()); cx.run_until_parked(); @@ -557,12 +585,15 @@ async fn test_view_more_pagination(cx: &mut TestAppContext) { async fn test_view_more_batched_expansion(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)); + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let sidebar = setup_sidebar(&multi_workspace, cx); - let path_list = PathList::new(&[std::path::PathBuf::from("/my-project")]); // Create 17 threads: initially shows 5, then 10, then 15, then all 17 with Collapse - save_n_test_threads(17, &path_list, cx).await; + save_n_test_threads(17, &project, cx).await; + + let path_list = project.read_with(cx, |project, cx| { + project.project_group_key(cx).path_list().clone() + }); multi_workspace.update_in(cx, |_, _window, cx| cx.notify()); cx.run_until_parked(); @@ -629,11 +660,14 @@ async fn test_view_more_batched_expansion(cx: &mut TestAppContext) { async fn test_collapse_and_expand_group(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)); + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), 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; + save_n_test_threads(1, &project, cx).await; + + let path_list = project.read_with(cx, |project, cx| { + project.project_group_key(cx).path_list().clone() + }); multi_workspace.update_in(cx, |_, _window, cx| cx.notify()); cx.run_until_parked(); @@ -685,9 +719,8 @@ async fn test_visible_entries_as_strings(cx: &mut TestAppContext) { s.contents.entries = vec![ // Expanded project header ListEntry::ProjectHeader { - path_list: expanded_path.clone(), + key: project::ProjectGroupKey::new(None, expanded_path.clone()), label: "expanded-project".into(), - workspace: workspace.clone(), highlight_positions: Vec::new(), has_running_threads: false, waiting_thread_count: 0, @@ -809,14 +842,13 @@ async fn test_visible_entries_as_strings(cx: &mut TestAppContext) { }), // View More entry ListEntry::ViewMore { - path_list: expanded_path.clone(), + key: project::ProjectGroupKey::new(None, expanded_path.clone()), is_fully_expanded: false, }, // Collapsed project header ListEntry::ProjectHeader { - path_list: collapsed_path.clone(), + key: project::ProjectGroupKey::new(None, collapsed_path.clone()), label: "collapsed-project".into(), - workspace: workspace.clone(), highlight_positions: Vec::new(), has_running_threads: false, waiting_thread_count: 0, @@ -872,11 +904,10 @@ async fn test_visible_entries_as_strings(cx: &mut TestAppContext) { async fn test_keyboard_select_next_and_previous(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)); + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let sidebar = setup_sidebar(&multi_workspace, cx); - let path_list = PathList::new(&[std::path::PathBuf::from("/my-project")]); - save_n_test_threads(3, &path_list, cx).await; + save_n_test_threads(3, &project, cx).await; multi_workspace.update_in(cx, |_, _window, cx| cx.notify()); cx.run_until_parked(); @@ -932,11 +963,10 @@ async fn test_keyboard_select_next_and_previous(cx: &mut TestAppContext) { async fn test_keyboard_select_first_and_last(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)); + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let sidebar = setup_sidebar(&multi_workspace, cx); - let path_list = PathList::new(&[std::path::PathBuf::from("/my-project")]); - save_n_test_threads(3, &path_list, cx).await; + save_n_test_threads(3, &project, cx).await; multi_workspace.update_in(cx, |_, _window, cx| cx.notify()); cx.run_until_parked(); @@ -987,11 +1017,10 @@ async fn test_keyboard_focus_in_does_not_set_selection(cx: &mut TestAppContext) async fn test_keyboard_confirm_on_project_header_toggles_collapse(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)); + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), 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; + save_n_test_threads(1, &project, cx).await; multi_workspace.update_in(cx, |_, _window, cx| cx.notify()); cx.run_until_parked(); @@ -1029,11 +1058,10 @@ async fn test_keyboard_confirm_on_project_header_toggles_collapse(cx: &mut TestA async fn test_keyboard_confirm_on_view_more_expands(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)); + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let sidebar = setup_sidebar(&multi_workspace, cx); - let path_list = PathList::new(&[std::path::PathBuf::from("/my-project")]); - save_n_test_threads(8, &path_list, cx).await; + save_n_test_threads(8, &project, cx).await; multi_workspace.update_in(cx, |_, _window, cx| cx.notify()); cx.run_until_parked(); @@ -1064,11 +1092,10 @@ async fn test_keyboard_confirm_on_view_more_expands(cx: &mut TestAppContext) { async fn test_keyboard_expand_and_collapse_selected_entry(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)); + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), 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; + save_n_test_threads(1, &project, cx).await; multi_workspace.update_in(cx, |_, _window, cx| cx.notify()); cx.run_until_parked(); @@ -1109,11 +1136,10 @@ async fn test_keyboard_expand_and_collapse_selected_entry(cx: &mut TestAppContex async fn test_keyboard_collapse_from_child_selects_parent(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)); + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), 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; + save_n_test_threads(1, &project, cx).await; multi_workspace.update_in(cx, |_, _window, cx| cx.notify()); cx.run_until_parked(); @@ -1177,11 +1203,10 @@ async fn test_keyboard_navigation_on_empty_list(cx: &mut TestAppContext) { async fn test_selection_clamps_after_entry_removal(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)); + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), 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; + save_n_test_threads(1, &project, cx).await; multi_workspace.update_in(cx, |_, _window, cx| cx.notify()); cx.run_until_parked(); @@ -1254,15 +1279,13 @@ async fn test_parallel_threads_shown_with_live_status(cx: &mut TestAppContext) { cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let (sidebar, panel) = setup_sidebar_with_agent_panel(&multi_workspace, cx); - let path_list = PathList::new(&[std::path::PathBuf::from("/my-project")]); - // Open thread A and keep it generating. let connection = StubAgentConnection::new(); open_thread_with_connection(&panel, connection.clone(), cx); send_message(&panel, cx); let session_id_a = active_session_id(&panel, cx); - save_test_thread_metadata(&session_id_a, path_list.clone(), cx).await; + save_test_thread_metadata(&session_id_a, &project, cx).await; cx.update(|_, cx| { connection.send_update( @@ -1281,7 +1304,7 @@ async fn test_parallel_threads_shown_with_live_status(cx: &mut TestAppContext) { send_message(&panel, cx); let session_id_b = active_session_id(&panel, cx); - save_test_thread_metadata(&session_id_b, path_list.clone(), cx).await; + save_test_thread_metadata(&session_id_b, &project, cx).await; cx.run_until_parked(); @@ -1300,15 +1323,13 @@ async fn test_background_thread_completion_triggers_notification(cx: &mut TestAp cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a.clone(), window, cx)); let (sidebar, panel_a) = setup_sidebar_with_agent_panel(&multi_workspace, cx); - let path_list_a = PathList::new(&[std::path::PathBuf::from("/project-a")]); - // Open thread on workspace A and keep it generating. let connection_a = StubAgentConnection::new(); open_thread_with_connection(&panel_a, connection_a.clone(), cx); send_message(&panel_a, cx); let session_id_a = active_session_id(&panel_a, cx); - save_test_thread_metadata(&session_id_a, path_list_a.clone(), cx).await; + save_test_thread_metadata(&session_id_a, &project_a, cx).await; cx.update(|_, cx| { connection_a.send_update( @@ -1358,11 +1379,9 @@ fn type_in_search(sidebar: &Entity, query: &str, cx: &mut gpui::VisualT async fn test_search_narrows_visible_threads_to_matches(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)); + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let sidebar = setup_sidebar(&multi_workspace, cx); - let path_list = PathList::new(&[std::path::PathBuf::from("/my-project")]); - for (id, title, hour) in [ ("t-1", "Fix crash in project panel", 3), ("t-2", "Add inline diff view", 2), @@ -1373,7 +1392,7 @@ async fn test_search_narrows_visible_threads_to_matches(cx: &mut TestAppContext) title.into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, hour, 0, 0).unwrap(), None, - path_list.clone(), + &project, cx, ); } @@ -1411,17 +1430,15 @@ async fn test_search_matches_regardless_of_case(cx: &mut TestAppContext) { // Search should match case-insensitively so they can still find it. 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)); + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let sidebar = setup_sidebar(&multi_workspace, cx); - let path_list = PathList::new(&[std::path::PathBuf::from("/my-project")]); - save_thread_metadata( acp::SessionId::new(Arc::from("thread-1")), "Fix Crash In Project Panel".into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, - path_list, + &project, cx, ); cx.run_until_parked(); @@ -1453,18 +1470,16 @@ async fn test_escape_clears_search_and_restores_full_list(cx: &mut TestAppContex // to dismiss the filter and see the full list again. 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)); + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let sidebar = setup_sidebar(&multi_workspace, cx); - let path_list = PathList::new(&[std::path::PathBuf::from("/my-project")]); - for (id, title, hour) in [("t-1", "Alpha thread", 2), ("t-2", "Beta thread", 1)] { save_thread_metadata( acp::SessionId::new(Arc::from(id)), title.into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, hour, 0, 0).unwrap(), None, - path_list.clone(), + &project, cx, ) } @@ -1502,11 +1517,9 @@ async fn test_escape_clears_search_and_restores_full_list(cx: &mut TestAppContex async fn test_search_only_shows_workspace_headers_with_matches(cx: &mut TestAppContext) { let project_a = init_test_project("/project-a", cx).await; let (multi_workspace, cx) = - cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a, window, cx)); + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a.clone(), window, cx)); let sidebar = setup_sidebar(&multi_workspace, cx); - let path_list_a = PathList::new(&[std::path::PathBuf::from("/project-a")]); - for (id, title, hour) in [ ("a1", "Fix bug in sidebar", 2), ("a2", "Add tests for editor", 1), @@ -1516,7 +1529,7 @@ async fn test_search_only_shows_workspace_headers_with_matches(cx: &mut TestAppC title.into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, hour, 0, 0).unwrap(), None, - path_list_a.clone(), + &project_a, cx, ) } @@ -1527,7 +1540,8 @@ async fn test_search_only_shows_workspace_headers_with_matches(cx: &mut TestAppC }); cx.run_until_parked(); - let path_list_b = PathList::new::(&[]); + let project_b = + multi_workspace.read_with(cx, |mw, cx| mw.workspaces()[1].read(cx).project().clone()); for (id, title, hour) in [ ("b1", "Refactor sidebar layout", 3), @@ -1538,7 +1552,7 @@ async fn test_search_only_shows_workspace_headers_with_matches(cx: &mut TestAppC title.into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, hour, 0, 0).unwrap(), None, - path_list_b.clone(), + &project_b, cx, ) } @@ -1584,11 +1598,9 @@ async fn test_search_only_shows_workspace_headers_with_matches(cx: &mut TestAppC async fn test_search_matches_workspace_name(cx: &mut TestAppContext) { let project_a = init_test_project("/alpha-project", cx).await; let (multi_workspace, cx) = - cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a, window, cx)); + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a.clone(), window, cx)); let sidebar = setup_sidebar(&multi_workspace, cx); - let path_list_a = PathList::new(&[std::path::PathBuf::from("/alpha-project")]); - for (id, title, hour) in [ ("a1", "Fix bug in sidebar", 2), ("a2", "Add tests for editor", 1), @@ -1598,7 +1610,7 @@ async fn test_search_matches_workspace_name(cx: &mut TestAppContext) { title.into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, hour, 0, 0).unwrap(), None, - path_list_a.clone(), + &project_a, cx, ) } @@ -1609,7 +1621,8 @@ async fn test_search_matches_workspace_name(cx: &mut TestAppContext) { }); cx.run_until_parked(); - let path_list_b = PathList::new::(&[]); + let project_b = + multi_workspace.read_with(cx, |mw, cx| mw.workspaces()[1].read(cx).project().clone()); for (id, title, hour) in [ ("b1", "Refactor sidebar layout", 3), @@ -1620,7 +1633,7 @@ async fn test_search_matches_workspace_name(cx: &mut TestAppContext) { title.into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, hour, 0, 0).unwrap(), None, - path_list_b.clone(), + &project_b, cx, ) } @@ -1686,11 +1699,9 @@ async fn test_search_matches_workspace_name(cx: &mut TestAppContext) { async fn test_search_finds_threads_hidden_behind_view_more(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)); + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let sidebar = setup_sidebar(&multi_workspace, cx); - let path_list = PathList::new(&[std::path::PathBuf::from("/my-project")]); - // Create 8 threads. The oldest one has a unique name and will be // behind View More (only 5 shown by default). for i in 0..8u32 { @@ -1704,7 +1715,7 @@ async fn test_search_finds_threads_hidden_behind_view_more(cx: &mut TestAppConte title.into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, i).unwrap(), None, - path_list.clone(), + &project, cx, ) } @@ -1738,17 +1749,15 @@ async fn test_search_finds_threads_hidden_behind_view_more(cx: &mut TestAppConte async fn test_search_finds_threads_inside_collapsed_groups(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)); + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let sidebar = setup_sidebar(&multi_workspace, cx); - let path_list = PathList::new(&[std::path::PathBuf::from("/my-project")]); - save_thread_metadata( acp::SessionId::new(Arc::from("thread-1")), "Important thread".into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, - path_list, + &project, cx, ); cx.run_until_parked(); @@ -1779,11 +1788,9 @@ async fn test_search_finds_threads_inside_collapsed_groups(cx: &mut TestAppConte async fn test_search_then_keyboard_navigate_and_confirm(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)); + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let sidebar = setup_sidebar(&multi_workspace, cx); - let path_list = PathList::new(&[std::path::PathBuf::from("/my-project")]); - for (id, title, hour) in [ ("t-1", "Fix crash in panel", 3), ("t-2", "Fix lint warnings", 2), @@ -1794,7 +1801,7 @@ async fn test_search_then_keyboard_navigate_and_confirm(cx: &mut TestAppContext) title.into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, hour, 0, 0).unwrap(), None, - path_list.clone(), + &project, cx, ) } @@ -1841,7 +1848,7 @@ async fn test_search_then_keyboard_navigate_and_confirm(cx: &mut TestAppContext) async fn test_confirm_on_historical_thread_activates_workspace(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)); + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let sidebar = setup_sidebar(&multi_workspace, cx); multi_workspace.update_in(cx, |mw, window, cx| { @@ -1849,14 +1856,12 @@ async fn test_confirm_on_historical_thread_activates_workspace(cx: &mut TestAppC }); cx.run_until_parked(); - let path_list = PathList::new(&[std::path::PathBuf::from("/my-project")]); - save_thread_metadata( acp::SessionId::new(Arc::from("hist-1")), "Historical Thread".into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 6, 1, 0, 0, 0).unwrap(), None, - path_list, + &project, cx, ); cx.run_until_parked(); @@ -1899,17 +1904,15 @@ async fn test_confirm_on_historical_thread_activates_workspace(cx: &mut TestAppC async fn test_click_clears_selection_and_focus_in_restores_it(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)); + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let sidebar = setup_sidebar(&multi_workspace, cx); - let path_list = PathList::new(&[std::path::PathBuf::from("/my-project")]); - save_thread_metadata( acp::SessionId::new(Arc::from("t-1")), "Thread A".into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 2, 0, 0, 0).unwrap(), None, - path_list.clone(), + &project, cx, ); @@ -1918,7 +1921,7 @@ async fn test_click_clears_selection_and_focus_in_restores_it(cx: &mut TestAppCo "Thread B".into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, - path_list, + &project, cx, ); @@ -1966,8 +1969,6 @@ async fn test_thread_title_update_propagates_to_sidebar(cx: &mut TestAppContext) cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let (sidebar, panel) = setup_sidebar_with_agent_panel(&multi_workspace, cx); - let path_list = PathList::new(&[std::path::PathBuf::from("/my-project")]); - let connection = StubAgentConnection::new(); connection.set_next_prompt_updates(vec![acp::SessionUpdate::AgentMessageChunk( acp::ContentChunk::new("Hi there!".into()), @@ -1976,7 +1977,7 @@ async fn test_thread_title_update_propagates_to_sidebar(cx: &mut TestAppContext) send_message(&panel, cx); let session_id = active_session_id(&panel, cx); - save_test_thread_metadata(&session_id, path_list.clone(), cx).await; + save_test_thread_metadata(&session_id, &project, cx).await; cx.run_until_parked(); assert_eq!( @@ -2014,8 +2015,6 @@ async fn test_focused_thread_tracks_user_intent(cx: &mut TestAppContext) { cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a.clone(), window, cx)); let (sidebar, panel_a) = setup_sidebar_with_agent_panel(&multi_workspace, cx); - let path_list_a = PathList::new(&[std::path::PathBuf::from("/project-a")]); - // Save a thread so it appears in the list. let connection_a = StubAgentConnection::new(); connection_a.set_next_prompt_updates(vec![acp::SessionUpdate::AgentMessageChunk( @@ -2024,7 +2023,7 @@ async fn test_focused_thread_tracks_user_intent(cx: &mut TestAppContext) { open_thread_with_connection(&panel_a, connection_a, cx); send_message(&panel_a, cx); let session_id_a = active_session_id(&panel_a, cx); - save_test_thread_metadata(&session_id_a, path_list_a.clone(), cx).await; + save_test_thread_metadata(&session_id_a, &project_a, cx).await; // Add a second workspace with its own agent panel. let fs = cx.update(|_, cx| ::global(cx)); @@ -2099,8 +2098,7 @@ async fn test_focused_thread_tracks_user_intent(cx: &mut TestAppContext) { open_thread_with_connection(&panel_b, connection_b, cx); send_message(&panel_b, cx); let session_id_b = active_session_id(&panel_b, cx); - let path_list_b = PathList::new(&[std::path::PathBuf::from("/project-b")]); - save_test_thread_metadata(&session_id_b, path_list_b.clone(), cx).await; + save_test_thread_metadata(&session_id_b, &project_b, cx).await; cx.run_until_parked(); // Workspace A is currently active. Click a thread in workspace B, @@ -2161,7 +2159,7 @@ async fn test_focused_thread_tracks_user_intent(cx: &mut TestAppContext) { open_thread_with_connection(&panel_b, connection_b2, cx); send_message(&panel_b, cx); let session_id_b2 = active_session_id(&panel_b, cx); - save_test_thread_metadata(&session_id_b2, path_list_b.clone(), cx).await; + save_test_thread_metadata(&session_id_b2, &project_b, cx).await; cx.run_until_parked(); // Panel B is not the active workspace's panel (workspace A is @@ -2243,8 +2241,6 @@ async fn test_new_thread_button_works_after_adding_folder(cx: &mut TestAppContex cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let (sidebar, panel) = setup_sidebar_with_agent_panel(&multi_workspace, cx); - let path_list_a = PathList::new(&[std::path::PathBuf::from("/project-a")]); - // Start a thread and send a message so it has history. let connection = StubAgentConnection::new(); connection.set_next_prompt_updates(vec![acp::SessionUpdate::AgentMessageChunk( @@ -2253,7 +2249,7 @@ async fn test_new_thread_button_works_after_adding_folder(cx: &mut TestAppContex open_thread_with_connection(&panel, connection, cx); send_message(&panel, cx); let session_id = active_session_id(&panel, cx); - save_test_thread_metadata(&session_id, path_list_a.clone(), cx).await; + save_test_thread_metadata(&session_id, &project, cx).await; cx.run_until_parked(); // Verify the thread appears in the sidebar. @@ -2287,9 +2283,15 @@ async fn test_new_thread_button_works_after_adding_folder(cx: &mut TestAppContex // The workspace path_list is now [project-a, project-b]. The active // thread's metadata was re-saved with the new paths by the agent panel's // project subscription, so it stays visible under the updated group. + // The old [project-a] group persists in the sidebar (empty) because + // project_group_keys is append-only. assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [project-a, project-b]", " Hello *",] + vec![ + "v [project-a, project-b]", // + " Hello *", + "v [project-a]", + ] ); // The "New Thread" button must still be clickable (not stuck in @@ -2334,8 +2336,6 @@ async fn test_cmd_n_shows_new_thread_entry(cx: &mut TestAppContext) { cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let (sidebar, panel) = setup_sidebar_with_agent_panel(&multi_workspace, cx); - let path_list = PathList::new(&[std::path::PathBuf::from("/my-project")]); - // Create a non-empty thread (has messages). let connection = StubAgentConnection::new(); connection.set_next_prompt_updates(vec![acp::SessionUpdate::AgentMessageChunk( @@ -2345,7 +2345,7 @@ async fn test_cmd_n_shows_new_thread_entry(cx: &mut TestAppContext) { send_message(&panel, cx); let session_id = active_session_id(&panel, cx); - save_test_thread_metadata(&session_id, path_list.clone(), cx).await; + save_test_thread_metadata(&session_id, &project, cx).await; cx.run_until_parked(); assert_eq!( @@ -2365,8 +2365,8 @@ async fn test_cmd_n_shows_new_thread_entry(cx: &mut TestAppContext) { assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [my-project]", " [+ New Thread]", " Hello *"], - "After Cmd-N the sidebar should show a highlighted New Thread entry" + vec!["v [my-project]", " [~ Draft]", " Hello *"], + "After Cmd-N the sidebar should show a highlighted Draft entry" ); sidebar.read_with(cx, |sidebar, _cx| { @@ -2385,8 +2385,6 @@ async fn test_draft_with_server_session_shows_as_draft(cx: &mut TestAppContext) cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let (sidebar, panel) = setup_sidebar_with_agent_panel(&multi_workspace, cx); - let path_list = PathList::new(&[std::path::PathBuf::from("/my-project")]); - // Create a saved thread so the workspace has history. let connection = StubAgentConnection::new(); connection.set_next_prompt_updates(vec![acp::SessionUpdate::AgentMessageChunk( @@ -2395,7 +2393,7 @@ async fn test_draft_with_server_session_shows_as_draft(cx: &mut TestAppContext) open_thread_with_connection(&panel, connection, cx); send_message(&panel, cx); let saved_session_id = active_session_id(&panel, cx); - save_test_thread_metadata(&saved_session_id, path_list.clone(), cx).await; + save_test_thread_metadata(&saved_session_id, &project, cx).await; cx.run_until_parked(); assert_eq!( @@ -2412,8 +2410,7 @@ async fn test_draft_with_server_session_shows_as_draft(cx: &mut TestAppContext) assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [my-project]", " [+ New Thread]", " Hello *"], - "Draft with a server session should still show as [+ New Thread]" + vec!["v [my-project]", " [~ Draft]", " Hello *"], ); let workspace = multi_workspace.read_with(cx, |mw, _cx| mw.workspace().clone()); @@ -2503,17 +2500,12 @@ async fn test_cmd_n_shows_new_thread_entry_in_absorbed_worktree(cx: &mut TestApp send_message(&worktree_panel, cx); let session_id = active_session_id(&worktree_panel, cx); - let wt_path_list = PathList::new(&[std::path::PathBuf::from("/wt-feature-a")]); - save_test_thread_metadata(&session_id, wt_path_list, cx).await; + save_test_thread_metadata(&session_id, &worktree_project, cx).await; cx.run_until_parked(); assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec![ - "v [project]", - " [+ New Thread]", - " Hello {wt-feature-a} *" - ] + vec!["v [project]", " Hello {wt-feature-a} *"] ); // Simulate Cmd-N in the worktree workspace. @@ -2529,12 +2521,11 @@ async fn test_cmd_n_shows_new_thread_entry_in_absorbed_worktree(cx: &mut TestApp visible_entries_as_strings(&sidebar, cx), vec![ "v [project]", - " [+ New Thread]", - " [+ New Thread {wt-feature-a}]", + " [~ Draft {wt-feature-a}]", " Hello {wt-feature-a} *" ], "After Cmd-N in an absorbed worktree, the sidebar should show \ - a highlighted New Thread entry under the main repo header" + a highlighted Draft entry under the main repo header" ); sidebar.read_with(cx, |sidebar, _cx| { @@ -2586,14 +2577,17 @@ async fn test_search_matches_worktree_name(cx: &mut TestAppContext) { .update(cx, |project, cx| project.git_scans_complete(cx)) .await; + let worktree_project = project::Project::test(fs.clone(), ["/wt/rosewood".as_ref()], cx).await; + worktree_project + .update(cx, |p, cx| p.git_scans_complete(cx)) + .await; + let (multi_workspace, cx) = cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let sidebar = setup_sidebar(&multi_workspace, cx); - let main_paths = PathList::new(&[std::path::PathBuf::from("/project")]); - let wt_paths = PathList::new(&[std::path::PathBuf::from("/wt/rosewood")]); - save_named_thread_metadata("main-t", "Unrelated Thread", &main_paths, cx).await; - save_named_thread_metadata("wt-t", "Fix Bug", &wt_paths, cx).await; + save_named_thread_metadata("main-t", "Unrelated Thread", &project, cx).await; + save_named_thread_metadata("wt-t", "Fix Bug", &worktree_project, cx).await; multi_workspace.update_in(cx, |_, _window, cx| cx.notify()); cx.run_until_parked(); @@ -2615,13 +2609,17 @@ async fn test_git_worktree_added_live_updates_sidebar(cx: &mut TestAppContext) { .update(cx, |project, cx| project.git_scans_complete(cx)) .await; + let worktree_project = project::Project::test(fs.clone(), ["/wt/rosewood".as_ref()], cx).await; + worktree_project + .update(cx, |p, cx| p.git_scans_complete(cx)) + .await; + let (multi_workspace, cx) = cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let sidebar = setup_sidebar(&multi_workspace, cx); // Save a thread against a worktree path that doesn't exist yet. - let wt_paths = PathList::new(&[std::path::PathBuf::from("/wt/rosewood")]); - save_named_thread_metadata("wt-thread", "Worktree Thread", &wt_paths, cx).await; + save_named_thread_metadata("wt-thread", "Worktree Thread", &worktree_project, cx).await; multi_workspace.update_in(cx, |_, _window, cx| cx.notify()); cx.run_until_parked(); @@ -2650,11 +2648,7 @@ async fn test_git_worktree_added_live_updates_sidebar(cx: &mut TestAppContext) { assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec![ - "v [project]", - " [+ New Thread]", - " Worktree Thread {rosewood}", - ] + vec!["v [project]", " Worktree Thread {rosewood}",] ); } @@ -2714,10 +2708,8 @@ async fn test_two_worktree_workspaces_absorbed_when_main_added(cx: &mut TestAppC }); let sidebar = setup_sidebar(&multi_workspace, cx); - let paths_a = PathList::new(&[std::path::PathBuf::from("/wt-feature-a")]); - let paths_b = PathList::new(&[std::path::PathBuf::from("/wt-feature-b")]); - save_named_thread_metadata("thread-a", "Thread A", &paths_a, cx).await; - save_named_thread_metadata("thread-b", "Thread B", &paths_b, cx).await; + save_named_thread_metadata("thread-a", "Thread A", &project_a, cx).await; + save_named_thread_metadata("thread-b", "Thread B", &project_b, cx).await; multi_workspace.update_in(cx, |_, _window, cx| cx.notify()); cx.run_until_parked(); @@ -2748,7 +2740,6 @@ async fn test_two_worktree_workspaces_absorbed_when_main_added(cx: &mut TestAppC visible_entries_as_strings(&sidebar, cx), vec![ "v [project]", - " [+ New Thread]", " Thread A {wt-feature-a}", " Thread B {wt-feature-b}", ] @@ -2813,8 +2804,7 @@ async fn test_threadless_workspace_shows_new_thread_with_worktree_chip(cx: &mut let sidebar = setup_sidebar(&multi_workspace, cx); // Only save a thread for workspace A. - let paths_a = PathList::new(&[std::path::PathBuf::from("/wt-feature-a")]); - save_named_thread_metadata("thread-a", "Thread A", &paths_a, cx).await; + save_named_thread_metadata("thread-a", "Thread A", &project_a, cx).await; multi_workspace.update_in(cx, |_, _window, cx| cx.notify()); cx.run_until_parked(); @@ -2894,11 +2884,7 @@ async fn test_multi_worktree_thread_shows_multiple_chips(cx: &mut TestAppContext let sidebar = setup_sidebar(&multi_workspace, cx); // Save a thread under the same paths as the workspace roots. - let thread_paths = PathList::new(&[ - std::path::PathBuf::from("/worktrees/project_a/olivetti/project_a"), - std::path::PathBuf::from("/worktrees/project_b/selectric/project_b"), - ]); - save_named_thread_metadata("wt-thread", "Cross Worktree Thread", &thread_paths, cx).await; + save_named_thread_metadata("wt-thread", "Cross Worktree Thread", &project, cx).await; multi_workspace.update_in(cx, |_, _window, cx| cx.notify()); cx.run_until_parked(); @@ -2971,11 +2957,7 @@ async fn test_same_named_worktree_chips_are_deduplicated(cx: &mut TestAppContext let sidebar = setup_sidebar(&multi_workspace, cx); // Thread with roots in both repos' "olivetti" worktrees. - let thread_paths = PathList::new(&[ - std::path::PathBuf::from("/worktrees/project_a/olivetti/project_a"), - std::path::PathBuf::from("/worktrees/project_b/olivetti/project_b"), - ]); - save_named_thread_metadata("wt-thread", "Same Branch Thread", &thread_paths, cx).await; + save_named_thread_metadata("wt-thread", "Same Branch Thread", &project, cx).await; multi_workspace.update_in(cx, |_, _window, cx| cx.notify()); cx.run_until_parked(); @@ -3070,8 +3052,7 @@ async fn test_absorbed_worktree_running_thread_shows_live_status(cx: &mut TestAp let session_id = active_session_id(&worktree_panel, cx); // Save metadata so the sidebar knows about this thread. - let wt_paths = PathList::new(&[std::path::PathBuf::from("/wt-feature-a")]); - save_test_thread_metadata(&session_id, wt_paths, cx).await; + save_test_thread_metadata(&session_id, &worktree_project, cx).await; // Keep the thread generating by sending a chunk without ending // the turn. @@ -3091,7 +3072,7 @@ async fn test_absorbed_worktree_running_thread_shows_live_status(cx: &mut TestAp entries, vec![ "v [project]", - " [+ New Thread]", + " [~ Draft]", " Hello {wt-feature-a} * (running)", ] ); @@ -3164,8 +3145,7 @@ async fn test_absorbed_worktree_completion_triggers_notification(cx: &mut TestAp send_message(&worktree_panel, cx); let session_id = active_session_id(&worktree_panel, cx); - let wt_paths = PathList::new(&[std::path::PathBuf::from("/wt-feature-a")]); - save_test_thread_metadata(&session_id, wt_paths, cx).await; + save_test_thread_metadata(&session_id, &worktree_project, cx).await; cx.update(|_, cx| { connection.send_update( @@ -3180,7 +3160,7 @@ async fn test_absorbed_worktree_completion_triggers_notification(cx: &mut TestAp visible_entries_as_strings(&sidebar, cx), vec![ "v [project]", - " [+ New Thread]", + " [~ Draft]", " Hello {wt-feature-a} * (running)", ] ); @@ -3190,11 +3170,7 @@ async fn test_absorbed_worktree_completion_triggers_notification(cx: &mut TestAp assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec![ - "v [project]", - " [+ New Thread]", - " Hello {wt-feature-a} * (!)", - ] + vec!["v [project]", " [~ Draft]", " Hello {wt-feature-a} * (!)",] ); } @@ -3232,13 +3208,17 @@ async fn test_clicking_worktree_thread_opens_workspace_when_none_exists(cx: &mut .update(cx, |p, cx| p.git_scans_complete(cx)) .await; + let worktree_project = project::Project::test(fs.clone(), ["/wt-feature-a".as_ref()], cx).await; + worktree_project + .update(cx, |p, cx| p.git_scans_complete(cx)) + .await; + let (multi_workspace, cx) = cx.add_window_view(|window, cx| MultiWorkspace::test_new(main_project.clone(), window, cx)); let sidebar = setup_sidebar(&multi_workspace, cx); // Save a thread for the worktree path (no workspace for it). - let paths_wt = PathList::new(&[std::path::PathBuf::from("/wt-feature-a")]); - save_named_thread_metadata("thread-wt", "WT Thread", &paths_wt, cx).await; + save_named_thread_metadata("thread-wt", "WT Thread", &worktree_project, cx).await; multi_workspace.update_in(cx, |_, _window, cx| cx.notify()); cx.run_until_parked(); @@ -3246,11 +3226,7 @@ async fn test_clicking_worktree_thread_opens_workspace_when_none_exists(cx: &mut // Thread should appear under the main repo with a worktree chip. assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec![ - "v [project]", - " [+ New Thread]", - " WT Thread {wt-feature-a}" - ], + vec!["v [project]", " WT Thread {wt-feature-a}"], ); // Only 1 workspace should exist. @@ -3262,7 +3238,7 @@ async fn test_clicking_worktree_thread_opens_workspace_when_none_exists(cx: &mut // Focus the sidebar and select the worktree thread. open_and_focus_sidebar(&sidebar, cx); sidebar.update_in(cx, |sidebar, _window, _cx| { - sidebar.selection = Some(2); // index 0 is header, 1 is new thread, 2 is the thread + sidebar.selection = Some(1); // index 0 is header, 1 is the thread }); // Confirm to open the worktree thread. @@ -3323,28 +3299,28 @@ async fn test_clicking_worktree_thread_does_not_briefly_render_as_separate_proje .update(cx, |p, cx| p.git_scans_complete(cx)) .await; + let worktree_project = project::Project::test(fs.clone(), ["/wt-feature-a".as_ref()], cx).await; + worktree_project + .update(cx, |p, cx| p.git_scans_complete(cx)) + .await; + let (multi_workspace, cx) = cx.add_window_view(|window, cx| MultiWorkspace::test_new(main_project.clone(), window, cx)); let sidebar = setup_sidebar(&multi_workspace, cx); - let paths_wt = PathList::new(&[std::path::PathBuf::from("/wt-feature-a")]); - save_named_thread_metadata("thread-wt", "WT Thread", &paths_wt, cx).await; + save_named_thread_metadata("thread-wt", "WT Thread", &worktree_project, cx).await; multi_workspace.update_in(cx, |_, _window, cx| cx.notify()); cx.run_until_parked(); assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec![ - "v [project]", - " [+ New Thread]", - " WT Thread {wt-feature-a}" - ], + vec!["v [project]", " WT Thread {wt-feature-a}"], ); open_and_focus_sidebar(&sidebar, cx); sidebar.update_in(cx, |sidebar, _window, _cx| { - sidebar.selection = Some(2); + sidebar.selection = Some(1); // index 0 is header, 1 is the thread }); let assert_sidebar_state = |sidebar: &mut Sidebar, _cx: &mut Context| { @@ -3400,7 +3376,7 @@ async fn test_clicking_worktree_thread_does_not_briefly_render_as_separate_proje ListEntry::ViewMore { .. } => { panic!("unexpected `View More` entry while opening linked worktree thread"); } - ListEntry::NewThread { .. } => {} + ListEntry::DraftThread { .. } | ListEntry::NewThread { .. } => {} } } @@ -3480,10 +3456,8 @@ async fn test_clicking_absorbed_worktree_thread_activates_worktree_workspace( let sidebar = setup_sidebar(&multi_workspace, cx); - let paths_main = PathList::new(&[std::path::PathBuf::from("/project")]); - let paths_wt = PathList::new(&[std::path::PathBuf::from("/wt-feature-a")]); - save_named_thread_metadata("thread-main", "Main Thread", &paths_main, cx).await; - save_named_thread_metadata("thread-wt", "WT Thread", &paths_wt, cx).await; + save_named_thread_metadata("thread-main", "Main Thread", &main_project, cx).await; + save_named_thread_metadata("thread-wt", "WT Thread", &worktree_project, cx).await; multi_workspace.update_in(cx, |_, _window, cx| cx.notify()); cx.run_until_parked(); @@ -3544,18 +3518,17 @@ async fn test_activate_archived_thread_with_saved_paths_activates_matching_works 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)); + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a.clone(), window, cx)); multi_workspace.update_in(cx, |mw, window, cx| { - mw.test_add_workspace(project_b, window, cx); + mw.test_add_workspace(project_b.clone(), window, cx); }); let sidebar = setup_sidebar(&multi_workspace, cx); // Save a thread with path_list pointing to project-b. - let path_list_b = PathList::new(&[std::path::PathBuf::from("/project-b")]); let session_id = acp::SessionId::new(Arc::from("archived-1")); - save_test_thread_metadata(&session_id, path_list_b.clone(), cx).await; + save_test_thread_metadata(&session_id, &project_b, cx).await; // Ensure workspace A is active. multi_workspace.update_in(cx, |mw, window, cx| { @@ -4093,7 +4066,7 @@ async fn test_archive_thread_uses_next_threads_own_workspace(cx: &mut TestAppCon "Thread 2".into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 2, 0, 0, 0).unwrap(), None, - PathList::new(&[std::path::PathBuf::from("/project")]), + &main_project, cx, ); @@ -4105,7 +4078,7 @@ async fn test_archive_thread_uses_next_threads_own_workspace(cx: &mut TestAppCon "Thread 1".into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, - PathList::new(&[std::path::PathBuf::from("/wt-feature-a")]), + &worktree_project, cx, ); @@ -4215,6 +4188,11 @@ async fn test_linked_worktree_threads_not_duplicated_across_groups(cx: &mut Test .update(cx, |p, cx| p.git_scans_complete(cx)) .await; + let worktree_project = project::Project::test(fs.clone(), ["/wt-feature-a".as_ref()], cx).await; + worktree_project + .update(cx, |p, cx| p.git_scans_complete(cx)) + .await; + let (multi_workspace, cx) = cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_only.clone(), window, cx)); multi_workspace.update_in(cx, |mw, window, cx| { @@ -4223,8 +4201,7 @@ async fn test_linked_worktree_threads_not_duplicated_across_groups(cx: &mut Test let sidebar = setup_sidebar(&multi_workspace, cx); // Save a thread under the linked worktree path. - let wt_paths = PathList::new(&[std::path::PathBuf::from("/wt-feature-a")]); - save_named_thread_metadata("wt-thread", "Worktree Thread", &wt_paths, cx).await; + save_named_thread_metadata("wt-thread", "Worktree Thread", &worktree_project, cx).await; multi_workspace.update_in(cx, |_, _window, cx| cx.notify()); cx.run_until_parked(); @@ -4234,11 +4211,10 @@ async fn test_linked_worktree_threads_not_duplicated_across_groups(cx: &mut Test assert_eq!( visible_entries_as_strings(&sidebar, cx), vec![ - "v [project]", - " [+ New Thread]", - " Worktree Thread {wt-feature-a}", "v [other, project]", " [+ New Thread]", + "v [project]", + " Worktree Thread {wt-feature-a}", ] ); } @@ -4250,8 +4226,6 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) { cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let (sidebar, panel) = setup_sidebar_with_agent_panel(&multi_workspace, cx); - let path_list = PathList::new(&[std::path::PathBuf::from("/my-project")]); - let switcher_ids = |sidebar: &Entity, cx: &mut gpui::VisualTestContext| -> Vec { sidebar.read_with(cx, |sidebar, cx| { @@ -4298,7 +4272,7 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) { "Thread C".into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), Some(chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap()), - path_list.clone(), + &project, cx, ); @@ -4314,7 +4288,7 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) { "Thread B".into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 2, 0, 0, 0).unwrap(), Some(chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 2, 0, 0, 0).unwrap()), - path_list.clone(), + &project, cx, ); @@ -4330,7 +4304,7 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) { "Thread A".into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 3, 0, 0, 0).unwrap(), Some(chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 3, 0, 0, 0).unwrap()), - path_list.clone(), + &project, cx, ); @@ -4516,7 +4490,7 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) { "Historical Thread".into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 6, 1, 0, 0, 0).unwrap(), Some(chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 6, 1, 0, 0, 0).unwrap()), - path_list.clone(), + &project, cx, ); @@ -4557,7 +4531,7 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) { "Old Historical Thread".into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2023, 6, 1, 0, 0, 0).unwrap(), Some(chrono::TimeZone::with_ymd_and_hms(&Utc, 2023, 6, 1, 0, 0, 0).unwrap()), - path_list, + &project, cx, ); @@ -4591,17 +4565,15 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) { async fn test_archive_thread_keeps_metadata_but_hides_from_sidebar(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)); + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let sidebar = setup_sidebar(&multi_workspace, cx); - let path_list = PathList::new(&[std::path::PathBuf::from("/my-project")]); - save_thread_metadata( acp::SessionId::new(Arc::from("thread-to-archive")), "Thread To Archive".into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, - path_list, + &project, cx, ); cx.run_until_parked(); @@ -4643,17 +4615,15 @@ async fn test_archive_thread_keeps_metadata_but_hides_from_sidebar(cx: &mut Test async fn test_archived_threads_excluded_from_sidebar_entries(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)); + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let sidebar = setup_sidebar(&multi_workspace, cx); - let path_list = PathList::new(&[std::path::PathBuf::from("/my-project")]); - save_thread_metadata( acp::SessionId::new(Arc::from("visible-thread")), "Visible Thread".into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 2, 0, 0, 0).unwrap(), None, - path_list.clone(), + &project, cx, ); @@ -4663,7 +4633,7 @@ async fn test_archived_threads_excluded_from_sidebar_entries(cx: &mut TestAppCon "Archived Thread".into(), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, - path_list, + &project, cx, ); @@ -4756,18 +4726,21 @@ async fn test_linked_worktree_workspace_shows_main_worktree_threads(cx: &mut Tes .update(cx, |p, cx| p.git_scans_complete(cx)) .await; + let main_project = project::Project::test(fs.clone(), ["/project".as_ref()], cx).await; + main_project + .update(cx, |p, cx| p.git_scans_complete(cx)) + .await; + let (multi_workspace, cx) = cx.add_window_view(|window, cx| { MultiWorkspace::test_new(worktree_project.clone(), window, cx) }); let sidebar = setup_sidebar(&multi_workspace, cx); // Save a thread against the MAIN repo path. - let main_paths = PathList::new(&[std::path::PathBuf::from("/project")]); - save_named_thread_metadata("main-thread", "Main Repo Thread", &main_paths, cx).await; + save_named_thread_metadata("main-thread", "Main Repo Thread", &main_project, cx).await; // Save a thread against the linked worktree path. - let wt_paths = PathList::new(&[std::path::PathBuf::from("/wt-feature-a")]); - save_named_thread_metadata("wt-thread", "Worktree Thread", &wt_paths, cx).await; + save_named_thread_metadata("wt-thread", "Worktree Thread", &worktree_project, cx).await; multi_workspace.update_in(cx, |_, _window, cx| cx.notify()); cx.run_until_parked(); @@ -4788,7 +4761,6 @@ async fn test_linked_worktree_workspace_shows_main_worktree_threads(cx: &mut Tes mod property_test { use super::*; - use gpui::EntityId; struct UnopenedWorktree { path: String, @@ -4922,7 +4894,7 @@ mod property_test { fn save_thread_to_path( state: &mut TestState, - path_list: PathList, + project: &Entity, cx: &mut gpui::VisualTestContext, ) { let session_id = state.next_thread_id(); @@ -4930,7 +4902,7 @@ mod property_test { let updated_at = chrono::TimeZone::with_ymd_and_hms(&chrono::Utc, 2024, 1, 1, 0, 0, 0) .unwrap() + chrono::Duration::seconds(state.thread_counter as i64); - save_thread_metadata(session_id, title, updated_at, None, path_list, cx); + save_thread_metadata(session_id, title, updated_at, None, project, cx); } fn save_thread_to_path_with_main( @@ -4970,11 +4942,10 @@ mod property_test { ) { match operation { Operation::SaveThread { workspace_index } => { - let workspace = - multi_workspace.read_with(cx, |mw, _| mw.workspaces()[workspace_index].clone()); - let path_list = workspace - .read_with(cx, |workspace, cx| PathList::new(&workspace.root_paths(cx))); - save_thread_to_path(state, path_list, cx); + let project = multi_workspace.read_with(cx, |mw, cx| { + mw.workspaces()[workspace_index].read(cx).project().clone() + }); + save_thread_to_path(state, &project, cx); } Operation::SaveWorktreeThread { worktree_index } => { let worktree = &state.unopened_worktrees[worktree_index]; @@ -5147,7 +5118,7 @@ mod property_test { .entries .iter() .filter_map(|entry| match entry { - ListEntry::ProjectHeader { path_list, .. } => Some(path_list.clone()), + ListEntry::ProjectHeader { key, .. } => Some(key.path_list().clone()), _ => None, }) .collect(); @@ -5173,31 +5144,32 @@ mod property_test { anyhow::bail!("sidebar should still have an associated multi-workspace"); }; - let workspaces = multi_workspace.read(cx).workspaces().to_vec(); + let mw = multi_workspace.read(cx); - // Workspaces with no root paths are not shown because the - // sidebar skips empty path lists. All other workspaces should - // appear — either via a Thread entry or a NewThread entry for - // threadless workspaces. - let expected_workspaces: HashSet = workspaces - .iter() - .filter(|ws| !workspace_path_list(ws, cx).paths().is_empty()) - .map(|ws| ws.entity_id()) + // Every project group key in the multi-workspace that has a + // non-empty path list should appear as a ProjectHeader in the + // sidebar. + let expected_keys: HashSet<&project::ProjectGroupKey> = mw + .project_group_keys() + .filter(|k| !k.path_list().paths().is_empty()) .collect(); - let sidebar_workspaces: HashSet = sidebar + let sidebar_keys: HashSet<&project::ProjectGroupKey> = sidebar .contents .entries .iter() - .filter_map(|entry| entry.workspace().map(|ws| ws.entity_id())) + .filter_map(|entry| match entry { + ListEntry::ProjectHeader { key, .. } => Some(key), + _ => None, + }) .collect(); - let missing = &expected_workspaces - &sidebar_workspaces; - let stray = &sidebar_workspaces - &expected_workspaces; + let missing = &expected_keys - &sidebar_keys; + let stray = &sidebar_keys - &expected_keys; anyhow::ensure!( missing.is_empty() && stray.is_empty(), - "sidebar workspaces don't match multi-workspace.\n\ + "sidebar project groups don't match multi-workspace.\n\ Only in multi-workspace (missing): {:?}\n\ Only in sidebar (stray): {:?}", missing, @@ -5222,33 +5194,79 @@ mod property_test { .collect(); let mut metadata_thread_ids: HashSet = HashSet::default(); + + // Query using the same approach as the sidebar: iterate project + // group keys, then do main + legacy queries per group. + let mw = multi_workspace.read(cx); + let mut workspaces_by_group: HashMap>> = + HashMap::default(); for workspace in &workspaces { - let path_list = workspace_path_list(workspace, cx); + let key = workspace.read(cx).project_group_key(cx); + workspaces_by_group + .entry(key) + .or_default() + .push(workspace.clone()); + } + + for group_key in mw.project_group_keys() { + let path_list = group_key.path_list().clone(); if path_list.paths().is_empty() { continue; } + + let group_workspaces = workspaces_by_group + .get(group_key) + .map(|ws| ws.as_slice()) + .unwrap_or_default(); + + // Main code path queries (run for all groups, even without workspaces). + for metadata in thread_store + .read(cx) + .entries_for_main_worktree_path(&path_list) + { + metadata_thread_ids.insert(metadata.session_id.clone()); + } for metadata in thread_store.read(cx).entries_for_path(&path_list) { metadata_thread_ids.insert(metadata.session_id.clone()); } - for snapshot in root_repository_snapshots(workspace, cx) { - for linked_worktree in snapshot.linked_worktrees() { - let worktree_path_list = - PathList::new(std::slice::from_ref(&linked_worktree.path)); - for metadata in thread_store.read(cx).entries_for_path(&worktree_path_list) { + + // Legacy: per-workspace queries for different root paths. + let covered_paths: HashSet = group_workspaces + .iter() + .flat_map(|ws| { + ws.read(cx) + .root_paths(cx) + .into_iter() + .map(|p| p.to_path_buf()) + }) + .collect(); + + for workspace in group_workspaces { + let ws_path_list = workspace_path_list(workspace, cx); + if ws_path_list != path_list { + for metadata in thread_store.read(cx).entries_for_path(&ws_path_list) { metadata_thread_ids.insert(metadata.session_id.clone()); } } - if snapshot.is_linked_worktree() { - let main_path_list = - PathList::new(std::slice::from_ref(&snapshot.original_repo_abs_path)); - for metadata in thread_store.read(cx).entries_for_path(&main_path_list) { - metadata_thread_ids.insert(metadata.session_id.clone()); + } + + for workspace in group_workspaces { + for snapshot in root_repository_snapshots(workspace, cx) { + let repo_path_list = + PathList::new(&[snapshot.original_repo_abs_path.to_path_buf()]); + if repo_path_list != path_list { + continue; } - for metadata in thread_store - .read(cx) - .entries_for_main_worktree_path(&main_path_list) - { - metadata_thread_ids.insert(metadata.session_id.clone()); + for linked_worktree in snapshot.linked_worktrees() { + if covered_paths.contains(&*linked_worktree.path) { + continue; + } + let worktree_path_list = + PathList::new(std::slice::from_ref(&linked_worktree.path)); + for metadata in thread_store.read(cx).entries_for_path(&worktree_path_list) + { + metadata_thread_ids.insert(metadata.session_id.clone()); + } } } } diff --git a/crates/workspace/src/multi_workspace.rs b/crates/workspace/src/multi_workspace.rs index 6aa369774b63dd0d250ba67ba4a5b69a335a2de9..d1bfcf2652d4d7c77d1f83ca2bc9d9603e3a2eed 100644 --- a/crates/workspace/src/multi_workspace.rs +++ b/crates/workspace/src/multi_workspace.rs @@ -474,6 +474,26 @@ impl MultiWorkspace { self.project_group_keys.iter() } + /// Returns the project groups, ordered by most recently added. + pub fn project_groups( + &self, + cx: &App, + ) -> impl Iterator>)> { + let mut groups = self + .project_group_keys + .iter() + .rev() + .map(|key| (key.clone(), Vec::new())) + .collect::>(); + for workspace in &self.workspaces { + let key = workspace.read(cx).project_group_key(cx); + if let Some((_, workspaces)) = groups.iter_mut().find(|(k, _)| k == &key) { + workspaces.push(workspace.clone()); + } + } + groups.into_iter() + } + pub fn workspace(&self) -> &Entity { &self.workspaces[self.active_workspace_index] } From c8a81dac7a9985cc50ac3497d4892c6cfb6e79ae Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Fri, 3 Apr 2026 21:02:26 -0400 Subject: [PATCH 8/8] Fix panic on non-ASCII thread titles in archive search The archive view's fuzzy_match_positions used chars().enumerate() which produces character indices, not byte indices. When thread titles contain multi-byte UTF-8 characters (emoji, CJK, etc.), these character indices don't correspond to valid byte boundaries, causing a panic in HighlightedLabel::new which asserts that highlight indices are valid UTF-8 boundaries. Switch to char_indices() and eq_ignore_ascii_case() to produce correct byte positions, matching the approach used by the sidebar's version of the same function. --- crates/agent_ui/src/threads_archive_view.rs | 70 +++++++++++++++++++-- 1 file changed, 64 insertions(+), 6 deletions(-) diff --git a/crates/agent_ui/src/threads_archive_view.rs b/crates/agent_ui/src/threads_archive_view.rs index 9aca31e1edbe729fccecfc0dd8f0530d2aed2564..f0c02eefc34a03c5c45730ac4b53645c5b15a2e1 100644 --- a/crates/agent_ui/src/threads_archive_view.rs +++ b/crates/agent_ui/src/threads_archive_view.rs @@ -91,14 +91,16 @@ impl TimeBucket { } fn fuzzy_match_positions(query: &str, text: &str) -> Option> { - let query = query.to_lowercase(); - let text_lower = text.to_lowercase(); let mut positions = Vec::new(); let mut query_chars = query.chars().peekable(); - for (i, c) in text_lower.chars().enumerate() { - if query_chars.peek() == Some(&c) { - positions.push(i); - query_chars.next(); + for (byte_idx, candidate_char) in text.char_indices() { + if let Some(&query_char) = query_chars.peek() { + if candidate_char.eq_ignore_ascii_case(&query_char) { + positions.push(byte_idx); + query_chars.next(); + } + } else { + break; } } if query_chars.peek().is_none() { @@ -1283,3 +1285,59 @@ impl PickerDelegate for ProjectPickerDelegate { ) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_fuzzy_match_positions_returns_byte_indices() { + // "🔥abc" — the fire emoji is 4 bytes, so 'a' starts at byte 4, 'b' at 5, 'c' at 6. + let text = "🔥abc"; + let positions = fuzzy_match_positions("ab", text).expect("should match"); + assert_eq!(positions, vec![4, 5]); + + // Verify positions are valid char boundaries (this is the assertion that + // panicked before the fix). + for &pos in &positions { + assert!( + text.is_char_boundary(pos), + "position {pos} is not a valid UTF-8 boundary in {text:?}" + ); + } + } + + #[test] + fn test_fuzzy_match_positions_ascii_still_works() { + let positions = fuzzy_match_positions("he", "hello").expect("should match"); + assert_eq!(positions, vec![0, 1]); + } + + #[test] + fn test_fuzzy_match_positions_case_insensitive() { + let positions = fuzzy_match_positions("HE", "hello").expect("should match"); + assert_eq!(positions, vec![0, 1]); + } + + #[test] + fn test_fuzzy_match_positions_no_match() { + assert!(fuzzy_match_positions("xyz", "hello").is_none()); + } + + #[test] + fn test_fuzzy_match_positions_multi_byte_interior() { + // "café" — 'é' is 2 bytes (0xC3 0xA9), so 'f' starts at byte 4, 'é' at byte 5. + let text = "café"; + let positions = fuzzy_match_positions("fé", text).expect("should match"); + // 'c'=0, 'a'=1, 'f'=2, 'é'=3..4 — wait, let's verify: + // Actually: c=1 byte, a=1 byte, f=1 byte, é=2 bytes + // So byte positions: c=0, a=1, f=2, é=3 + assert_eq!(positions, vec![2, 3]); + for &pos in &positions { + assert!( + text.is_char_boundary(pos), + "position {pos} is not a valid UTF-8 boundary in {text:?}" + ); + } + } +}