From 74798c68d5c63d31e2ccca5c8f5ec0a02c90679c Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 8 Jul 2026 08:46:41 -0700 Subject: [PATCH 01/61] gpui: Add run_embedded and ApplicationHandle for externally driven run loops (#60574) On ordinary platforms, `Platform::run` blocks for the lifetime of the app, and `Application::run`'s stack frame keeps the app state alive. Embedded platforms invert that: the run loop belongs to someone else. `Application::run_embedded` supports that shape: it starts the app exactly like `run()`, but returns an `ApplicationHandle` holding the strong app handle. Release Notes: - N/A --- crates/gpui/src/app.rs | 47 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index e6ca25ecae0..0467c3e4fd8 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -143,6 +143,31 @@ impl Drop for AppRefMut<'_> { /// You won't interact with this type much outside of initial configuration and startup. pub struct Application(Rc); +/// A strong handle to an [`Application`] started with [`Application::run_embedded`]. +/// +/// Dropping this handle releases the app, so an embedder must hold it for as long as the +/// app should run. While held, it is the embedder's entry point back into GPUI each time +/// the external run loop gives it control. +pub struct ApplicationHandle { + app: Rc, +} + +impl ApplicationHandle { + /// Invoke `f` with the app context. Must not be called re-entrantly from code that + /// is already inside an update; the app state is a `RefCell` and will panic on a + /// double borrow. + pub fn update(&self, f: impl FnOnce(&mut App) -> R) -> R { + let cx = &mut *self.app.borrow_mut(); + f(cx) + } + + /// An [`AsyncApp`] for use across await points. It holds the app weakly; keeping the + /// app alive remains this handle's job. + pub fn to_async(&self) -> AsyncApp { + self.update(|cx| cx.to_async()) + } +} + /// Represents an application before it is fully launched. Once your app is /// configured, you'll start the app with `App::run`. impl Application { @@ -209,6 +234,28 @@ impl Application { })); } + /// Start the application for an embedder that drives the run loop itself. + /// + /// On ordinary platforms `Platform::run` blocks for the lifetime of the app, and the + /// app state is kept alive by [`Application::run`]'s stack frame. Embedded platforms — + /// where the run loop belongs to someone else, e.g. GPUI compiled into a Wasm guest, + /// or a GPUI view hosted inside a foreign native application — implement + /// `Platform::run` to invoke the launch callback and return immediately. This method + /// supports that shape: it returns an [`ApplicationHandle`] that keeps the app alive + /// and lets the embedder re-enter it whenever the external run loop yields control. + pub fn run_embedded(self, on_finish_launching: F) -> ApplicationHandle + where + F: 'static + FnOnce(&mut App), + { + let this = self.0.clone(); + let platform = self.0.borrow().platform.clone(); + platform.run(Box::new(move || { + let cx = &mut *this.borrow_mut(); + on_finish_launching(cx); + })); + ApplicationHandle { app: self.0 } + } + /// Register a handler to be invoked when the platform instructs the application /// to open one or more URLs. pub fn on_open_urls(&self, mut callback: F) -> &Self From c80020231bcc38c03db562d3c619dae20f1ab193 Mon Sep 17 00:00:00 2001 From: soddygo Date: Wed, 8 Jul 2026 23:52:26 +0800 Subject: [PATCH 02/61] agent_ui: Fix expanded message editor staying auto-height during streaming (#55916) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments (Unsafe: none) - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [ ] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable ## Summary When the agent is streaming output, expanding the message editor can appear to work (the container grows) but the input editor itself remains constrained to auto-height (only a few lines). Toggling minimize → expand makes it render correctly. This PR ensures the message editor stays in full mode while expanded, preventing automatic editor mode syncing on thread updates from overriding the user's explicit expand action during streaming. ## Steps to reproduce 1. Start an agent thread and send a message that causes streaming output 2. While streaming, click “Expand Message Editor” 3. Observe the input editor still shows only a few lines (auto-height) 4. Click “Minimize Message Editor” and then expand again; it becomes fully expanded ## Test plan - Start an agent thread and let it stream - Click “Expand Message Editor” while streaming - Verify the editor actually expands (not limited to the auto-height max lines) - Toggle minimize/expand multiple times during streaming; verify it remains correct ## Additional context Repro video: https://github.com/user-attachments/assets/6e328fa8-d431-456a-b70a-864ae617ea88 Release Notes: - Fixed message editor not fully expanding during agent generation --------- Co-authored-by: Smit Barmase --- crates/agent_ui/src/conversation_view.rs | 4 +- .../src/conversation_view/thread_view.rs | 38 +++++++------------ 2 files changed, 16 insertions(+), 26 deletions(-) diff --git a/crates/agent_ui/src/conversation_view.rs b/crates/agent_ui/src/conversation_view.rs index 65632e9406b..110961b34f1 100644 --- a/crates/agent_ui/src/conversation_view.rs +++ b/crates/agent_ui/src/conversation_view.rs @@ -1614,7 +1614,7 @@ impl ConversationView { }); active.update(cx, |active, cx| { active.sync_elicitation_state_for_entry(index, window, cx); - active.sync_editor_mode_for_empty_state(cx); + active.sync_editor_mode(cx); active.sync_generating_indicator(cx); }); } @@ -1641,7 +1641,7 @@ impl ConversationView { entry_view_state.update(cx, |view_state, _cx| view_state.remove(range.clone())); list_state.splice(range.clone(), 0); active.update(cx, |active, cx| { - active.sync_editor_mode_for_empty_state(cx); + active.sync_editor_mode(cx); }); } } diff --git a/crates/agent_ui/src/conversation_view/thread_view.rs b/crates/agent_ui/src/conversation_view/thread_view.rs index 90ea600ff5f..fe8b7b29233 100644 --- a/crates/agent_ui/src/conversation_view/thread_view.rs +++ b/crates/agent_ui/src/conversation_view/thread_view.rs @@ -1036,7 +1036,7 @@ impl ThreadView { }; this.sync_generating_indicator(cx); - this.sync_editor_mode_for_empty_state(cx); + this.sync_editor_mode(cx); this.sync_existing_elicitation_states(window, cx); let list_state_for_scroll = this.list_state.clone(); let thread_view = cx.entity().downgrade(); @@ -2373,27 +2373,7 @@ impl ThreadView { pub fn set_editor_is_expanded(&mut self, is_expanded: bool, cx: &mut Context) { self.editor_expanded = is_expanded; - self.message_editor.update(cx, |editor, cx| { - if is_expanded { - editor.set_mode( - EditorMode::Full { - scale_ui_elements_with_buffer_font_size: false, - show_active_line_background: false, - sizing_behavior: SizingBehavior::ExcludeOverscrollMargin, - }, - cx, - ) - } else { - let agent_settings = AgentSettings::get_global(cx); - editor.set_mode( - EditorMode::AutoHeight { - min_lines: agent_settings.message_editor_min_lines, - max_lines: Some(agent_settings.set_message_editor_max_lines()), - }, - cx, - ) - } - }); + self.sync_editor_mode(cx); cx.notify(); } @@ -7104,11 +7084,21 @@ impl ThreadView { open_markdown_in_workspace(thread_title, markdown, workspace, window, cx) } - pub(crate) fn sync_editor_mode_for_empty_state(&mut self, cx: &mut Context) { + pub(crate) fn sync_editor_mode(&mut self, cx: &mut Context) { let has_messages = self.list_state.item_count() > 0; let v2_empty_state = !has_messages; - let mode = if v2_empty_state { + if !has_messages { + self.editor_expanded = false; + } + + let mode = if self.editor_expanded { + EditorMode::Full { + scale_ui_elements_with_buffer_font_size: false, + show_active_line_background: false, + sizing_behavior: SizingBehavior::ExcludeOverscrollMargin, + } + } else if v2_empty_state { EditorMode::Full { scale_ui_elements_with_buffer_font_size: false, show_active_line_background: false, From 74b5207744ef95505fd75c56bf2c9d4736e931ee Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 8 Jul 2026 08:52:48 -0700 Subject: [PATCH 03/61] Unify Render and RenderOnce into View (#58087) This allows us to build powerful and flexible Input and TextArea components 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: - N/A --------- Co-authored-by: Claude Opus 4.8 (1M context) --- crates/gpui/Cargo.toml | 4 + .../examples/view_example/example_editor.rs | 549 ++++++++++++++++++ .../examples/view_example/example_input.rs | 121 ++++ .../examples/view_example/example_tests.rs | 131 +++++ .../view_example/example_text_area.rs | 118 ++++ .../view_example/view_example_main.rs | 173 ++++++ crates/gpui/src/element.rs | 114 +--- crates/gpui/src/view.rs | 534 +++++++++++------ crates/gpui/src/window.rs | 8 +- crates/gpui_macros/src/derive_into_element.rs | 4 +- crates/gpui_macros/src/gpui_macros.rs | 4 +- crates/workspace/src/workspace.rs | 2 +- 12 files changed, 1464 insertions(+), 298 deletions(-) create mode 100644 crates/gpui/examples/view_example/example_editor.rs create mode 100644 crates/gpui/examples/view_example/example_input.rs create mode 100644 crates/gpui/examples/view_example/example_tests.rs create mode 100644 crates/gpui/examples/view_example/example_text_area.rs create mode 100644 crates/gpui/examples/view_example/view_example_main.rs diff --git a/crates/gpui/Cargo.toml b/crates/gpui/Cargo.toml index fa6fcace40f..bad1feabb3b 100644 --- a/crates/gpui/Cargo.toml +++ b/crates/gpui/Cargo.toml @@ -256,3 +256,7 @@ path = "examples/mouse_pressure.rs" [[example]] name = "a11y" path = "examples/a11y.rs" + +[[example]] +name = "view_example" +path = "examples/view_example/view_example_main.rs" diff --git a/crates/gpui/examples/view_example/example_editor.rs b/crates/gpui/examples/view_example/example_editor.rs new file mode 100644 index 00000000000..2064d7cacef --- /dev/null +++ b/crates/gpui/examples/view_example/example_editor.rs @@ -0,0 +1,549 @@ +//! `Editor` — the workhorse entity. It owns the cursor, blink, focus, keyboard +//! handling, and the specialized text-shaping renderer. The *text itself* lives +//! in a shared `Entity` it's handed at construction, so the value is +//! readable/writable from outside while the editing machinery stays in here. +//! +//! This is the piece that proves the point: a text input is genuinely +//! complicated, and `View` lets all of that complexity live in one entity that +//! anything can embed. + +use std::ops::Range; +use std::time::Duration; + +use gpui::{ + App, Bounds, Context, ElementInputHandler, Entity, EntityInputHandler, FocusHandle, Focusable, + InteractiveElement, LayoutId, PaintQuad, Pixels, ShapedLine, SharedString, Subscription, Task, + TextRun, UTF16Selection, Window, fill, hsla, point, prelude::*, px, relative, size, +}; +use unicode_segmentation::*; + +use crate::{Backspace, Delete, End, Home, Left, Right}; + +pub struct Editor { + pub value: Entity, + pub focus_handle: FocusHandle, + pub cursor: usize, + pub cursor_visible: bool, + _blink_task: Task<()>, + _subscriptions: Vec, +} + +impl Editor { + /// An editor that owns its own string internally, seeded with `text`. + /// Nothing to allocate or wire up at the call site. + pub fn new(text: impl Into, window: &mut Window, cx: &mut Context) -> Self { + let value = cx.new(|_| text.into()); + Self::over(value, window, cx) + } + + /// An editor over a string *you* own, so the value is shared in and out. + pub fn over(value: Entity, window: &mut Window, cx: &mut Context) -> Self { + let focus_handle = cx.focus_handle(); + + let focus_sub = cx.on_focus(&focus_handle, window, |this, _window, cx| { + this.start_blink(cx); + }); + let blur_sub = cx.on_blur(&focus_handle, window, |this, _window, cx| { + this.stop_blink(cx); + }); + + // The value is shared: anything can write it while we hold a cursor into + // it. Observe it so external writes (a) clamp the cursor back onto a char + // boundary before the next IME round-trip can slice out of bounds, and + // (b) notify us, so an `editor.cached(..)` subtree re-renders — the cache + // is keyed on *our* notify, not the value's. + let value_sub = cx.observe(&value, |this, value, cx| { + let content = value.read(cx); + let mut cursor = this.cursor.min(content.len()); + while cursor > 0 && !content.is_char_boundary(cursor) { + cursor -= 1; + } + this.cursor = cursor; + cx.notify(); + }); + + Self { + value, + focus_handle, + cursor: 0, + cursor_visible: false, + _blink_task: Task::ready(()), + _subscriptions: vec![focus_sub, blur_sub, value_sub], + } + } + + /// The current text. Read this from anywhere to get the value out. + pub fn text(&self, cx: &App) -> String { + self.value.read(cx).clone() + } + + fn start_blink(&mut self, cx: &mut Context) { + self.cursor_visible = true; + self._blink_task = Self::spawn_blink_task(cx); + } + + fn stop_blink(&mut self, cx: &mut Context) { + self.cursor_visible = false; + self._blink_task = Task::ready(()); + cx.notify(); + } + + fn spawn_blink_task(cx: &mut Context) -> Task<()> { + cx.spawn(async move |this, cx| { + loop { + cx.background_executor() + .timer(Duration::from_millis(500)) + .await; + let result = this.update(cx, |editor, cx| { + editor.cursor_visible = !editor.cursor_visible; + cx.notify(); + }); + if result.is_err() { + break; + } + } + }) + } + + fn reset_blink(&mut self, cx: &mut Context) { + self.cursor_visible = true; + self._blink_task = Self::spawn_blink_task(cx); + } + + pub fn left(&mut self, _: &Left, _: &mut Window, cx: &mut Context) { + let content = self.text(cx); + if self.cursor > 0 { + self.cursor = previous_boundary(&content, self.cursor); + } + self.reset_blink(cx); + cx.notify(); + } + + pub fn right(&mut self, _: &Right, _: &mut Window, cx: &mut Context) { + let content = self.text(cx); + if self.cursor < content.len() { + self.cursor = next_boundary(&content, self.cursor); + } + self.reset_blink(cx); + cx.notify(); + } + + pub fn home(&mut self, _: &Home, _: &mut Window, cx: &mut Context) { + self.cursor = 0; + self.reset_blink(cx); + cx.notify(); + } + + pub fn end(&mut self, _: &End, _: &mut Window, cx: &mut Context) { + self.cursor = self.text(cx).len(); + self.reset_blink(cx); + cx.notify(); + } + + pub fn backspace(&mut self, _: &Backspace, _: &mut Window, cx: &mut Context) { + let content = self.text(cx); + if self.cursor > 0 { + let prev = previous_boundary(&content, self.cursor); + let cursor = self.cursor; + self.value.update(cx, |s, cx| { + s.drain(prev..cursor); + cx.notify(); + }); + self.cursor = prev; + } + self.reset_blink(cx); + cx.notify(); + } + + pub fn delete(&mut self, _: &Delete, _: &mut Window, cx: &mut Context) { + let content = self.text(cx); + if self.cursor < content.len() { + let next = next_boundary(&content, self.cursor); + let cursor = self.cursor; + self.value.update(cx, |s, cx| { + s.drain(cursor..next); + cx.notify(); + }); + } + self.reset_blink(cx); + cx.notify(); + } + + pub fn insert_newline(&mut self, cx: &mut Context) { + let cursor = self.cursor; + self.value.update(cx, |s, cx| { + s.insert(cursor, '\n'); + cx.notify(); + }); + self.cursor += 1; + self.reset_blink(cx); + cx.notify(); + } +} + +fn previous_boundary(content: &str, offset: usize) -> usize { + content + .grapheme_indices(true) + .rev() + .find_map(|(idx, _)| (idx < offset).then_some(idx)) + .unwrap_or(0) +} + +fn next_boundary(content: &str, offset: usize) -> usize { + content + .grapheme_indices(true) + .find_map(|(idx, _)| (idx > offset).then_some(idx)) + .unwrap_or(content.len()) +} + +fn offset_from_utf16(content: &str, offset: usize) -> usize { + let mut utf8_offset = 0; + let mut utf16_count = 0; + for ch in content.chars() { + if utf16_count >= offset { + break; + } + utf16_count += ch.len_utf16(); + utf8_offset += ch.len_utf8(); + } + utf8_offset +} + +fn offset_to_utf16(content: &str, offset: usize) -> usize { + let mut utf16_offset = 0; + let mut utf8_count = 0; + for ch in content.chars() { + if utf8_count >= offset { + break; + } + utf8_count += ch.len_utf8(); + utf16_offset += ch.len_utf16(); + } + utf16_offset +} + +fn range_to_utf16(content: &str, range: &Range) -> Range { + offset_to_utf16(content, range.start)..offset_to_utf16(content, range.end) +} + +fn range_from_utf16(content: &str, range_utf16: &Range) -> Range { + offset_from_utf16(content, range_utf16.start)..offset_from_utf16(content, range_utf16.end) +} + +impl Focusable for Editor { + fn focus_handle(&self, _cx: &App) -> FocusHandle { + self.focus_handle.clone() + } +} + +impl EntityInputHandler for Editor { + fn text_for_range( + &mut self, + range_utf16: Range, + actual_range: &mut Option>, + _window: &mut Window, + cx: &mut Context, + ) -> Option { + let content = self.text(cx); + let range = range_from_utf16(&content, &range_utf16); + actual_range.replace(range_to_utf16(&content, &range)); + Some(content[range].to_string()) + } + + fn selected_text_range( + &mut self, + _ignore_disabled_input: bool, + _window: &mut Window, + cx: &mut Context, + ) -> Option { + let content = self.text(cx); + let utf16_cursor = offset_to_utf16(&content, self.cursor); + Some(UTF16Selection { + range: utf16_cursor..utf16_cursor, + reversed: false, + }) + } + + fn marked_text_range( + &self, + _window: &mut Window, + _cx: &mut Context, + ) -> Option> { + None + } + + fn unmark_text(&mut self, _window: &mut Window, _cx: &mut Context) {} + + fn replace_text_in_range( + &mut self, + range_utf16: Option>, + new_text: &str, + _window: &mut Window, + cx: &mut Context, + ) { + let content = self.text(cx); + let range = range_utf16 + .as_ref() + .map(|r| range_from_utf16(&content, r)) + .unwrap_or(self.cursor..self.cursor); + + let new_content = content[..range.start].to_owned() + new_text + &content[range.end..]; + self.cursor = range.start + new_text.len(); + self.value.update(cx, |s, cx| { + *s = new_content; + cx.notify(); + }); + self.reset_blink(cx); + cx.notify(); + } + + fn replace_and_mark_text_in_range( + &mut self, + range_utf16: Option>, + new_text: &str, + _new_selected_range_utf16: Option>, + window: &mut Window, + cx: &mut Context, + ) { + self.replace_text_in_range(range_utf16, new_text, window, cx); + } + + fn bounds_for_range( + &mut self, + _range_utf16: Range, + _bounds: Bounds, + _window: &mut Window, + _cx: &mut Context, + ) -> Option> { + None + } + + fn character_index_for_point( + &mut self, + _point: gpui::Point, + _window: &mut Window, + _cx: &mut Context, + ) -> Option { + None + } +} + +impl gpui::Render for Editor { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + EditorText { + editor: cx.entity(), + } + } +} + +// --------------------------------------------------------------------------- +// EditorText — the specialized renderer: shapes the text and paints the cursor. +// --------------------------------------------------------------------------- + +struct EditorText { + editor: Entity, +} + +struct EditorTextPrepaint { + lines: Vec, + cursor: Option, +} + +impl IntoElement for EditorText { + type Element = Self; + + fn into_element(self) -> Self::Element { + self + } +} + +impl Element for EditorText { + type RequestLayoutState = (); + type PrepaintState = EditorTextPrepaint; + + fn id(&self) -> Option { + None + } + + fn source_location(&self) -> Option<&'static core::panic::Location<'static>> { + None + } + + fn request_layout( + &mut self, + _id: Option<&gpui::GlobalElementId>, + _inspector_id: Option<&gpui::InspectorElementId>, + window: &mut Window, + cx: &mut App, + ) -> (LayoutId, Self::RequestLayoutState) { + let editor = self.editor.read(cx); + let content = editor.value.read(cx); + let line_count = content.split('\n').count().max(1); + let line_height = window.line_height(); + let mut style = gpui::Style::default(); + style.size.width = relative(1.).into(); + style.size.height = (line_height * line_count as f32).into(); + (window.request_layout(style, [], cx), ()) + } + + fn prepaint( + &mut self, + _id: Option<&gpui::GlobalElementId>, + _inspector_id: Option<&gpui::InspectorElementId>, + bounds: Bounds, + _request_layout: &mut Self::RequestLayoutState, + window: &mut Window, + cx: &mut App, + ) -> Self::PrepaintState { + let editor = self.editor.read(cx); + let content = editor.value.read(cx).clone(); + let cursor_offset = editor.cursor; + let cursor_visible = editor.cursor_visible; + let is_focused = editor.focus_handle.is_focused(window); + + let style = window.text_style(); + let text_color = style.color; + let font_size = style.font_size.to_pixels(window.rem_size()); + let line_height = window.line_height(); + + let is_placeholder = content.is_empty(); + + let lines: Vec = if is_placeholder { + let placeholder: SharedString = "Type here...".into(); + let run = TextRun { + len: placeholder.len(), + font: style.font(), + color: hsla(0., 0., 0.5, 0.5), + background_color: None, + underline: None, + strikethrough: None, + }; + vec![ + window + .text_system() + .shape_line(placeholder, font_size, &[run], None), + ] + } else { + content + .split('\n') + .map(|line_str| { + let text: SharedString = SharedString::from(line_str.to_string()); + let run = TextRun { + len: text.len(), + font: style.font(), + color: text_color, + background_color: None, + underline: None, + strikethrough: None, + }; + window + .text_system() + .shape_line(text, font_size, &[run], None) + }) + .collect() + }; + + let cursor = if is_focused && cursor_visible && !is_placeholder { + let (cursor_line, offset_in_line) = cursor_line_and_offset(&content, cursor_offset); + let cursor_line = cursor_line.min(lines.len().saturating_sub(1)); + let cursor_x = lines[cursor_line].x_for_index(offset_in_line); + Some(fill( + Bounds::new( + point( + bounds.left() + cursor_x, + bounds.top() + line_height * cursor_line as f32, + ), + size(px(1.5), line_height), + ), + text_color, + )) + } else if is_focused && cursor_visible && is_placeholder { + Some(fill( + Bounds::new( + point(bounds.left(), bounds.top()), + size(px(1.5), line_height), + ), + text_color, + )) + } else { + None + }; + + EditorTextPrepaint { lines, cursor } + } + + fn paint( + &mut self, + _id: Option<&gpui::GlobalElementId>, + _inspector_id: Option<&gpui::InspectorElementId>, + bounds: Bounds, + _request_layout: &mut Self::RequestLayoutState, + prepaint: &mut Self::PrepaintState, + window: &mut Window, + cx: &mut App, + ) { + let focus_handle = self.editor.read(cx).focus_handle.clone(); + window.handle_input( + &focus_handle, + ElementInputHandler::new(bounds, self.editor.clone()), + cx, + ); + + let line_height = window.line_height(); + for (i, line) in prepaint.lines.iter().enumerate() { + let origin = point(bounds.left(), bounds.top() + line_height * i as f32); + line.paint(origin, line_height, gpui::TextAlign::Left, None, window, cx) + .unwrap(); + } + + if let Some(cursor) = prepaint.cursor.take() { + window.paint_quad(cursor); + } + } +} + +fn cursor_line_and_offset(content: &str, cursor: usize) -> (usize, usize) { + let mut line_index = 0; + let mut line_start = 0; + for (i, ch) in content.char_indices() { + if i >= cursor { + break; + } + if ch == '\n' { + line_index += 1; + line_start = i + 1; + } + } + (line_index, cursor - line_start) +} + +pub fn standard_actions(editor: Entity) -> impl FnOnce(E) -> E { + move |element| { + element + .on_action({ + let editor = editor.clone(); + move |a: &Left, window, cx| editor.update(cx, |e, cx| e.left(a, window, cx)) + }) + .on_action({ + let editor = editor.clone(); + move |a: &Right, window, cx| editor.update(cx, |e, cx| e.right(a, window, cx)) + }) + .on_action({ + let editor = editor.clone(); + move |a: &Home, window, cx| editor.update(cx, |e, cx| e.home(a, window, cx)) + }) + .on_action({ + let editor = editor.clone(); + move |a: &End, window, cx| editor.update(cx, |e, cx| e.end(a, window, cx)) + }) + .on_action({ + let editor = editor.clone(); + move |a: &Backspace, window, cx| { + editor.update(cx, |e, cx| e.backspace(a, window, cx)) + } + }) + .on_action(move |a: &Delete, window, cx| { + editor.update(cx, |e, cx| e.delete(a, window, cx)) + }) + } +} diff --git a/crates/gpui/examples/view_example/example_input.rs b/crates/gpui/examples/view_example/example_input.rs new file mode 100644 index 00000000000..25d74013deb --- /dev/null +++ b/crates/gpui/examples/view_example/example_input.rs @@ -0,0 +1,121 @@ +//! `Input` — a single-line text input. The shaping layer over `Editor`. +//! +//! Construct it two ways, depending on how much state you want to own: +//! * `Input::new(value: Entity)` — you hold just the string; the input +//! allocates the `Editor` internally via `use_state`. Value readable, cursor hidden. +//! * `Input::editor(editor: Entity)` — you hold the editor; cursor/selection +//! are now yours to read and drive too. +//! +//! Either way the chrome is identical. Because the string (or editor) is the +//! input's *identity*, the internal `use_state(Editor)` is collision-safe across +//! any number of inputs. + +use gpui::{ + App, BoxShadow, CursorStyle, Entity, EntityId, Hsla, IntoElement, Pixels, StyleRefinement, + Window, div, hsla, point, prelude::*, px, white, +}; + +use crate::example_editor::{Editor, standard_actions}; + +enum Source { + Value(Entity), + Editor(Entity), +} + +#[derive(IntoElement)] +pub struct Input { + source: Source, + width: Option, + color: Option, +} + +impl Input { + /// Backed by a bare string; the editor is allocated internally. + pub fn new(value: Entity) -> Self { + Self { + source: Source::Value(value), + width: None, + color: None, + } + } + + /// Backed by an editor you own (so you can read/drive its cursor). + pub fn editor(editor: Entity) -> Self { + Self { + source: Source::Editor(editor), + width: None, + color: None, + } + } + + pub fn width(mut self, width: Pixels) -> Self { + self.width = Some(width); + self + } + + pub fn color(mut self, color: Hsla) -> Self { + self.color = Some(color); + self + } +} + +impl gpui::View for Input { + fn entity_id(&self) -> Option { + Some(match &self.source { + Source::Value(value) => value.entity_id(), + Source::Editor(editor) => editor.entity_id(), + }) + } + + fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement { + // Get the editor: use the one we were handed, or allocate it under our + // own (string-derived) identity so it persists and never collides. + let editor = match self.source { + Source::Value(value) => { + window.use_state(cx, move |window, cx| Editor::over(value, window, cx)) + } + Source::Editor(editor) => editor, + }; + + let focus_handle = editor.read(cx).focus_handle.clone(); + let is_focused = focus_handle.is_focused(window); + let text_color = self.color.unwrap_or(hsla(0., 0., 0.1, 1.)); + let box_width = self.width.unwrap_or(px(300.)); + + let border = if is_focused { + hsla(220. / 360., 0.8, 0.5, 1.) + } else { + hsla(0., 0., 0.75, 1.) + }; + + div() + .id("input") + .key_context("TextInput") + .track_focus(&focus_handle) + .cursor(CursorStyle::IBeam) + .map(standard_actions(editor.clone())) + .w(box_width) + .h(px(36.)) + .px(px(8.)) + .bg(white()) + .border_1() + .border_color(border) + .when(is_focused, |this| { + this.shadow(vec![BoxShadow { + color: hsla(220. / 360., 0.8, 0.5, 0.3), + offset: point(px(0.), px(0.)), + blur_radius: px(4.), + spread_radius: px(1.), + inset: false, + }]) + }) + .rounded(px(4.)) + .overflow_hidden() + .flex() + .items_center() + .line_height(px(20.)) + .text_size(px(14.)) + .text_color(text_color) + .child(editor.cached(StyleRefinement::default().size_full())) + } +} diff --git a/crates/gpui/examples/view_example/example_tests.rs b/crates/gpui/examples/view_example/example_tests.rs new file mode 100644 index 00000000000..a3edae8cda1 --- /dev/null +++ b/crates/gpui/examples/view_example/example_tests.rs @@ -0,0 +1,131 @@ +//! Tests for the input composition. Require the `test-support` feature: +//! +//! ```sh +//! cargo test -p gpui --example view_example --features test-support +//! ``` + +#[cfg(test)] +mod tests { + use gpui::{Context, Entity, KeyBinding, TestAppContext, Window, prelude::*}; + + use crate::example_editor::Editor; + use crate::example_input::Input; + use crate::{Backspace, Delete, End, Home, Left, Right}; + + /// Two inputs, each backed by an editor we own (so the test can focus and + /// read them). Proves data flows through the shared `String` and that + /// sibling inputs stay isolated. + struct Harness { + a: Entity, + b: Entity, + } + + impl Render for Harness { + fn render(&mut self, _window: &mut Window, _cx: &mut Context) -> impl IntoElement { + gpui::div() + .child(Input::editor(self.a.clone())) + .child(Input::editor(self.b.clone())) + } + } + + fn bind_keys(cx: &mut TestAppContext) { + cx.update(|cx| { + cx.bind_keys([ + KeyBinding::new("backspace", Backspace, None), + KeyBinding::new("delete", Delete, None), + KeyBinding::new("left", Left, None), + KeyBinding::new("right", Right, None), + KeyBinding::new("home", Home, None), + KeyBinding::new("end", End, None), + ]); + }); + } + + fn setup( + cx: &mut TestAppContext, + ) -> ( + Entity, + Entity, + Entity, + &mut gpui::VisualTestContext, + ) { + bind_keys(cx); + + let (harness, cx) = cx.add_window_view(|window, cx| { + let a_value = cx.new(|_| String::new()); + let b_value = cx.new(|_| String::new()); + let a = cx.new(|cx| Editor::over(a_value, window, cx)); + let b = cx.new(|cx| Editor::over(b_value, window, cx)); + Harness { a, b } + }); + + let a = cx.read_entity(&harness, |h, _| h.a.clone()); + let b = cx.read_entity(&harness, |h, _| h.b.clone()); + let a_value = cx.read_entity(&a, |e, _| e.value.clone()); + let b_value = cx.read_entity(&b, |e, _| e.value.clone()); + + // Focus the first input's editor. + cx.update(|window, cx| { + let focus_handle = a.read(cx).focus_handle.clone(); + window.focus(&focus_handle, cx); + }); + + (a, a_value, b_value, cx) + } + + #[gpui::test] + fn typing_updates_the_shared_string(cx: &mut TestAppContext) { + let (editor, a_value, _b_value, cx) = setup(cx); + + cx.simulate_input("hello"); + + cx.read_entity(&a_value, |value, _| assert_eq!(value, "hello")); + cx.read_entity(&editor, |editor, _| assert_eq!(editor.cursor, 5)); + } + + #[gpui::test] + fn sibling_inputs_are_isolated(cx: &mut TestAppContext) { + let (_editor, a_value, b_value, cx) = setup(cx); + + cx.simulate_input("x"); + + cx.read_entity(&a_value, |value, _| assert_eq!(value, "x")); + cx.read_entity(&b_value, |value, _| { + assert_eq!(value, "", "typing in input A must not touch input B") + }); + } + + #[gpui::test] + fn external_writes_clamp_the_cursor(cx: &mut TestAppContext) { + let (editor, a_value, _b_value, cx) = setup(cx); + + cx.simulate_input("hello"); + cx.read_entity(&editor, |editor, _| assert_eq!(editor.cursor, 5)); + + // Write the shared value from outside the editor. The old cursor (5) + // now points into the middle of a multi-byte character; the editor's + // observation must clamp it back onto a boundary. + cx.update(|_, cx| { + a_value.update(cx, |value, cx| { + *value = "日本".to_string(); + cx.notify(); + }) + }); + + cx.read_entity(&a_value, |value, _| assert_eq!(value, "日本")); + cx.read_entity(&editor, |editor, _| { + assert_eq!(editor.cursor, 3, "cursor must clamp to a char boundary"); + }); + } + + #[gpui::test] + fn arrows_move_the_cursor(cx: &mut TestAppContext) { + let (editor, _a_value, _b_value, cx) = setup(cx); + + cx.simulate_input("abc"); + cx.read_entity(&editor, |editor, _| assert_eq!(editor.cursor, 3)); + + cx.simulate_keystrokes("left left"); + cx.read_entity(&editor, |editor, _| assert_eq!(editor.cursor, 1)); + } +} diff --git a/crates/gpui/examples/view_example/example_text_area.rs b/crates/gpui/examples/view_example/example_text_area.rs new file mode 100644 index 00000000000..07640b93294 --- /dev/null +++ b/crates/gpui/examples/view_example/example_text_area.rs @@ -0,0 +1,118 @@ +//! `TextArea` — a multi-line text box. Same `Editor` workhorse, taller chrome, +//! and `Enter` inserts a newline instead of being ignored. Constructible from a +//! string or an editor, exactly like [`Input`](crate::example_input::Input). + +use gpui::{ + App, BoxShadow, CursorStyle, Entity, EntityId, Hsla, IntoElement, StyleRefinement, Window, div, + hsla, point, prelude::*, px, white, +}; + +use crate::Enter; +use crate::example_editor::{Editor, standard_actions}; + +enum Source { + Value(Entity), + Editor(Entity), +} + +#[derive(IntoElement)] +pub struct TextArea { + source: Source, + rows: usize, + color: Option, +} + +impl TextArea { + pub fn new(value: Entity, rows: usize) -> Self { + Self { + source: Source::Value(value), + rows, + color: None, + } + } + + pub fn editor(editor: Entity, rows: usize) -> Self { + Self { + source: Source::Editor(editor), + rows, + color: None, + } + } + + pub fn color(mut self, color: Hsla) -> Self { + self.color = Some(color); + self + } +} + +impl gpui::View for TextArea { + fn entity_id(&self) -> Option { + Some(match &self.source { + Source::Value(value) => value.entity_id(), + Source::Editor(editor) => editor.entity_id(), + }) + } + + fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement { + let editor = match self.source { + Source::Value(value) => { + window.use_state(cx, move |window, cx| Editor::over(value, window, cx)) + } + Source::Editor(editor) => editor, + }; + + let focus_handle = editor.read(cx).focus_handle.clone(); + let is_focused = focus_handle.is_focused(window); + let text_color = self.color.unwrap_or(hsla(0., 0., 0.1, 1.)); + let row_height = px(20.); + let box_height = row_height * self.rows as f32 + px(16.); + + let border = if is_focused { + hsla(220. / 360., 0.8, 0.5, 1.) + } else { + hsla(0., 0., 0.75, 1.) + }; + + div() + .id("text-area") + .key_context("TextInput") + .track_focus(&focus_handle) + .cursor(CursorStyle::IBeam) + .map(standard_actions(editor.clone())) + // Enter is the one binding that differs from a single-line input. + .on_action({ + let editor = editor.clone(); + move |_: &Enter, _window, cx| editor.update(cx, |e, cx| e.insert_newline(cx)) + }) + .w(px(400.)) + .h(box_height) + .p(px(8.)) + .bg(white()) + .border_1() + .border_color(border) + .when(is_focused, |this| { + this.shadow(vec![BoxShadow { + color: hsla(220. / 360., 0.8, 0.5, 0.3), + offset: point(px(0.), px(0.)), + blur_radius: px(4.), + spread_radius: px(1.), + inset: false, + }]) + }) + .rounded(px(4.)) + .overflow_hidden() + .line_height(row_height) + .text_size(px(14.)) + .text_color(text_color) + // The cache style is computed from the `rows` prop: change `rows` and + // the editor's cached bounds change, busting its cache and re-laying + // out the text. (`Input` just uses `size_full()` — nothing to vary.) + .child( + editor.cached( + StyleRefinement::default() + .w_full() + .h(row_height * self.rows as f32), + ), + ) + } +} diff --git a/crates/gpui/examples/view_example/view_example_main.rs b/crates/gpui/examples/view_example/view_example_main.rs new file mode 100644 index 00000000000..0eac8494ffc --- /dev/null +++ b/crates/gpui/examples/view_example/view_example_main.rs @@ -0,0 +1,173 @@ +#![cfg_attr(target_family = "wasm", no_main)] + +//! View example — composing a text input from the `View` primitives. +//! +//! The whole point: a text input is deceptively complicated, and `View` makes it +//! easy to compose one. Three pieces, each shown in its own section: +//! +//! * `Editor` — the workhorse entity: cursor, blink, focus, keyboard, and a +//! specialized text renderer. All the hard parts live here. +//! * `String` — the data plane. `editor.text(cx)` / `value.read(cx)` get it out. +//! * `Input` / `TextArea` — the shaping layer. Each takes a `String` (and grows +//! the editor internally) OR an `Editor` (so you can read the cursor). +//! +//! Run: `cargo run -p gpui --example view_example` + +mod example_editor; +mod example_input; +mod example_text_area; + +#[cfg(test)] +mod example_tests; + +use example_editor::Editor; +use example_input::Input; +use example_text_area::TextArea; + +use gpui::{ + App, Bounds, Context, Div, Entity, IntoElement, KeyBinding, Render, SharedString, Window, + WindowBounds, WindowOptions, actions, div, hsla, prelude::*, px, rgb, size, +}; +use gpui_platform::application; + +actions!( + view_example, + [Backspace, Delete, Left, Right, Home, End, Enter, Quit] +); + +/// A tiny stateless view that reads an editor's cursor and is composed *beside* +/// the thing editing it — two views over one entity, zero wiring. +#[derive(IntoElement)] +struct CursorReadout { + editor: Entity, +} + +impl CursorReadout { + fn new(editor: Entity) -> Self { + Self { editor } + } +} + +impl gpui::RenderOnce for CursorReadout { + fn render(self, _window: &mut Window, cx: &mut App) -> impl IntoElement { + let cursor = self.editor.read(cx).cursor; + div() + .text_sm() + .text_color(hsla(0., 0., 0.45, 1.)) + .child(SharedString::from(format!("cursor @ {cursor}"))) + } +} + +struct ViewExample; + +impl ViewExample { + fn new() -> Self { + Self + } +} + +impl Render for ViewExample { + fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { + // The data plane: plain strings, allocated at the top by the hook. + let name = window.use_state(cx, |_, _| String::new()); + let email = window.use_state(cx, |_, _| String::from("me@example.com")); + let bio = window.use_state(cx, |_, _| String::new()); + // Editors that own their own string internally — no extra wiring up top. + let notes = window.use_state(cx, |window, cx| Editor::new("multi\nline", window, cx)); + let owned = window.use_state(cx, |window, cx| Editor::new("editable", window, cx)); + + div() + .flex() + .flex_col() + .size_full() + .bg(rgb(0xf0f0f0)) + .p(px(24.)) + .gap(px(24.)) + .child( + section("Inputs — from a String (cursor stays internal)") + .child(Input::new(name).width(px(320.))) + .child( + Input::new(email) + .width(px(320.)) + .color(hsla(0., 0., 0.3, 1.)), + ), + ) + .child( + section("Input — from an Editor (read its cursor beside it)").child( + div() + .flex() + .items_center() + .gap(px(12.)) + .child(Input::editor(owned.clone()).width(px(320.))) + .child(CursorReadout::new(owned)), + ), + ) + .child( + section("Text areas — from a String, or from an Editor") + .child(TextArea::new(bio, 3)) + .child( + div() + .flex() + .items_start() + .gap(px(12.)) + .child(TextArea::editor(notes.clone(), 3).color(hsla( + 250. / 360., + 0.7, + 0.4, + 1., + ))) + .child(CursorReadout::new(notes)), + ), + ) + } +} + +/// A labeled vertical section. +fn section(title: &str) -> Div { + div().flex().flex_col().gap(px(8.)).child( + div() + .text_sm() + .text_color(hsla(0., 0., 0.3, 1.)) + .child(SharedString::from(title.to_string())), + ) +} + +fn run_example() { + application().run(|cx: &mut App| { + let bounds = Bounds::centered(None, size(px(560.0), px(480.0)), cx); + cx.bind_keys([ + KeyBinding::new("backspace", Backspace, None), + KeyBinding::new("delete", Delete, None), + KeyBinding::new("left", Left, None), + KeyBinding::new("right", Right, None), + KeyBinding::new("home", Home, None), + KeyBinding::new("end", End, None), + KeyBinding::new("enter", Enter, None), + KeyBinding::new("cmd-q", Quit, None), + ]); + + cx.open_window( + WindowOptions { + window_bounds: Some(WindowBounds::Windowed(bounds)), + ..Default::default() + }, + |_, cx| cx.new(|_| ViewExample::new()), + ) + .unwrap(); + + cx.on_action(|_: &Quit, cx| cx.quit()); + cx.activate(true); + }); +} + +#[cfg(not(target_family = "wasm"))] +fn main() { + run_example(); +} + +#[cfg(target_family = "wasm")] +#[wasm_bindgen::prelude::wasm_bindgen(start)] +pub fn start() { + gpui_platform::web_init(); + run_example(); +} diff --git a/crates/gpui/src/element.rs b/crates/gpui/src/element.rs index f212f246caa..dbd82b1c2ef 100644 --- a/crates/gpui/src/element.rs +++ b/crates/gpui/src/element.rs @@ -33,12 +33,12 @@ use crate::{ A11ySubtreeBuilder, App, ArenaBox, AvailableSpace, Bounds, Context, DispatchNodeId, ElementId, - FocusHandle, InspectorElementId, LayoutId, Pixels, Point, SharedString, Size, Style, Window, + FocusHandle, InspectorElementId, LayoutId, Pixels, Point, Size, Style, Window, util::FluentBuilder, window::with_element_arena, }; use derive_more::{Deref, DerefMut}; use std::{ - any::{Any, type_name}, + any::Any, fmt::{self, Debug, Display}, mem, panic, sync::Arc, @@ -208,116 +208,6 @@ pub trait ParentElement { } } -/// An element for rendering components. An implementation detail of the [`IntoElement`] derive macro -/// for [`RenderOnce`] -#[doc(hidden)] -pub struct Component { - component: Option, - #[cfg(debug_assertions)] - source: &'static core::panic::Location<'static>, -} - -impl Component { - /// Create a new component from the given RenderOnce type. - #[track_caller] - pub fn new(component: C) -> Self { - Component { - component: Some(component), - #[cfg(debug_assertions)] - source: core::panic::Location::caller(), - } - } -} - -fn prepaint_component( - (element, name): &mut (AnyElement, &'static str), - window: &mut Window, - cx: &mut App, -) { - window.with_id(ElementId::Name(SharedString::new_static(name)), |window| { - element.prepaint(window, cx); - }) -} - -fn paint_component( - (element, name): &mut (AnyElement, &'static str), - window: &mut Window, - cx: &mut App, -) { - window.with_id(ElementId::Name(SharedString::new_static(name)), |window| { - element.paint(window, cx); - }) -} -impl Element for Component { - type RequestLayoutState = (AnyElement, &'static str); - type PrepaintState = (); - - fn id(&self) -> Option { - None - } - - fn source_location(&self) -> Option<&'static core::panic::Location<'static>> { - #[cfg(debug_assertions)] - return Some(self.source); - - #[cfg(not(debug_assertions))] - return None; - } - - fn request_layout( - &mut self, - _id: Option<&GlobalElementId>, - _inspector_id: Option<&InspectorElementId>, - window: &mut Window, - cx: &mut App, - ) -> (LayoutId, Self::RequestLayoutState) { - window.with_id(ElementId::Name(type_name::().into()), |window| { - let mut element = self - .component - .take() - .unwrap() - .render(window, cx) - .into_any_element(); - - let layout_id = element.request_layout(window, cx); - (layout_id, (element, type_name::())) - }) - } - - fn prepaint( - &mut self, - _id: Option<&GlobalElementId>, - _inspector_id: Option<&InspectorElementId>, - _: Bounds, - state: &mut Self::RequestLayoutState, - window: &mut Window, - cx: &mut App, - ) { - prepaint_component(state, window, cx); - } - - fn paint( - &mut self, - _id: Option<&GlobalElementId>, - _inspector_id: Option<&InspectorElementId>, - _: Bounds, - state: &mut Self::RequestLayoutState, - _: &mut Self::PrepaintState, - window: &mut Window, - cx: &mut App, - ) { - paint_component(state, window, cx); - } -} - -impl IntoElement for Component { - type Element = Self; - - fn into_element(self) -> Self::Element { - self - } -} - /// A globally unique identifier for an element, used to track state across frames. #[derive(Deref, DerefMut, Clone, Default, Debug, Eq, PartialEq, Hash)] pub struct GlobalElementId(pub(crate) Arc<[ElementId]>); diff --git a/crates/gpui/src/view.rs b/crates/gpui/src/view.rs index 39b87dbb803..394f00c1082 100644 --- a/crates/gpui/src/view.rs +++ b/crates/gpui/src/view.rs @@ -1,36 +1,24 @@ use crate::{ AnyElement, AnyEntity, AnyWeakEntity, App, Bounds, ContentMask, Context, Element, ElementId, Entity, EntityId, GlobalElementId, InspectorElementId, IntoElement, LayoutId, PaintIndex, - Pixels, PrepaintStateIndex, Render, Style, StyleRefinement, TextStyle, WeakEntity, + Pixels, PrepaintStateIndex, Render, RenderOnce, Style, StyleRefinement, TextStyle, WeakEntity, }; use crate::{Empty, Window}; use anyhow::Result; use collections::FxHashSet; use refineable::Refineable; use std::mem; -use std::rc::Rc; use std::{any::TypeId, fmt, ops::Range}; -struct AnyViewState { - prepaint_range: Range, - paint_range: Range, - cache_key: ViewCacheKey, - accessed_entities: FxHashSet, -} - -#[derive(Default)] -struct ViewCacheKey { - bounds: Bounds, - content_mask: ContentMask, - text_style: TextStyle, -} - -/// A dynamically-typed handle to a view, which can be downcast to a [Entity] for a specific type. +/// A dynamically-typed view handle that can be downcast to a specific `Entity`. +/// +/// This is the type-erased counterpart to [`ViewElement`]: it holds an entity plus +/// a function pointer to its render, and is itself a [`View`], so embedding it as an +/// element goes through the same [`ViewElement`] machinery as any other view. #[derive(Clone, Debug)] pub struct AnyView { entity: AnyEntity, render: fn(&AnyView, &mut Window, &mut App) -> AnyElement, - cached_style: Option>, } impl From> for AnyView { @@ -38,18 +26,18 @@ impl From> for AnyView { AnyView { entity: value.into_any(), render: any_view::render::, - cached_style: None, } } } impl AnyView { - /// Indicate that this view should be cached when using it as an element. - /// When using this method, the view's previous layout and paint will be recycled from the previous frame if [Context::notify] has not been called since it was rendered. - /// The one exception is when [Window::refresh] is called, in which case caching is ignored. - pub fn cached(mut self, style: StyleRefinement) -> Self { - self.cached_style = Some(style.into()); - self + /// Embed this view as a cached [`ViewElement`] laid out at `style`. + /// + /// The rendered subtree is recycled from the previous frame unless + /// [Context::notify] was called on the backing entity since it was rendered + /// (or [Window::refresh] is called, which ignores caching). + pub fn cached(self, style: StyleRefinement) -> ViewElement { + ViewElement::new(self).cached(style) } /// Convert this to a weak handle. @@ -68,7 +56,6 @@ impl AnyView { Err(entity) => Err(Self { entity, render: self.render, - cached_style: self.cached_style, }), } } @@ -78,7 +65,7 @@ impl AnyView { self.entity.entity_type } - /// Gets the entity id of this handle. + /// The [`EntityId`] of this view. pub fn entity_id(&self) -> EntityId { self.entity.entity_id() } @@ -92,184 +79,48 @@ impl PartialEq for AnyView { impl Eq for AnyView {} -impl Element for AnyView { - type RequestLayoutState = Option; - type PrepaintState = Option; - - fn id(&self) -> Option { - Some(ElementId::View(self.entity_id())) +/// `AnyView` is the type-erased [`View`]: its `render` is a function pointer rather +/// than a concrete type, but it participates in the reactive graph exactly like any +/// other view via [`ViewElement`]. +impl View for AnyView { + fn entity_id(&self) -> Option { + Some(self.entity.entity_id()) } - fn source_location(&self) -> Option<&'static core::panic::Location<'static>> { - None - } - - fn request_layout( - &mut self, - _id: Option<&GlobalElementId>, - _inspector_id: Option<&InspectorElementId>, - window: &mut Window, - cx: &mut App, - ) -> (LayoutId, Self::RequestLayoutState) { - window.with_rendered_view(self.entity_id(), |window| { - // Disable caching when inspecting so that mouse_hit_test has all hitboxes. - let caching_disabled = window.is_inspector_picking(cx); - match self.cached_style.as_ref() { - Some(style) if !caching_disabled => { - let mut root_style = Style::default(); - root_style.refine(style); - let layout_id = window.request_layout(root_style, None, cx); - (layout_id, None) - } - _ => { - let mut element = (self.render)(self, window, cx); - let layout_id = element.request_layout(window, cx); - (layout_id, Some(element)) - } - } - }) - } - - fn prepaint( - &mut self, - global_id: Option<&GlobalElementId>, - _inspector_id: Option<&InspectorElementId>, - bounds: Bounds, - element: &mut Self::RequestLayoutState, - window: &mut Window, - cx: &mut App, - ) -> Option { - window.set_view_id(self.entity_id()); - window.with_rendered_view(self.entity_id(), |window| { - if let Some(mut element) = element.take() { - element.prepaint(window, cx); - return Some(element); - } - - window.with_element_state::( - global_id.unwrap(), - |element_state, window| { - let content_mask = window.content_mask(); - let text_style = window.text_style(); - - if let Some(mut element_state) = element_state - && element_state.cache_key.bounds == bounds - && element_state.cache_key.content_mask == content_mask - && element_state.cache_key.text_style == text_style - && !window.dirty_views.contains(&self.entity_id()) - && !window.refreshing - { - let prepaint_start = window.prepaint_index(); - window.reuse_prepaint(element_state.prepaint_range.clone()); - cx.entities - .extend_accessed(&element_state.accessed_entities); - let prepaint_end = window.prepaint_index(); - element_state.prepaint_range = prepaint_start..prepaint_end; - - return (None, element_state); - } - - let refreshing = mem::replace(&mut window.refreshing, true); - let prepaint_start = window.prepaint_index(); - let (mut element, accessed_entities) = cx.detect_accessed_entities(|cx| { - let mut element = (self.render)(self, window, cx); - element.layout_as_root(bounds.size.into(), window, cx); - element.prepaint_at(bounds.origin, window, cx); - element - }); - - let prepaint_end = window.prepaint_index(); - window.refreshing = refreshing; - - ( - Some(element), - AnyViewState { - accessed_entities, - prepaint_range: prepaint_start..prepaint_end, - paint_range: PaintIndex::default()..PaintIndex::default(), - cache_key: ViewCacheKey { - bounds, - content_mask, - text_style, - }, - }, - ) - }, - ) - }) - } - - fn paint( - &mut self, - global_id: Option<&GlobalElementId>, - _inspector_id: Option<&InspectorElementId>, - _bounds: Bounds, - _: &mut Self::RequestLayoutState, - element: &mut Self::PrepaintState, - window: &mut Window, - cx: &mut App, - ) { - window.with_rendered_view(self.entity_id(), |window| { - let caching_disabled = window.is_inspector_picking(cx); - if self.cached_style.is_some() && !caching_disabled { - window.with_element_state::( - global_id.unwrap(), - |element_state, window| { - let mut element_state = element_state.unwrap(); - - let paint_start = window.paint_index(); - - if let Some(element) = element { - let refreshing = mem::replace(&mut window.refreshing, true); - element.paint(window, cx); - window.refreshing = refreshing; - } else { - window.reuse_paint(element_state.paint_range.clone()); - } - - let paint_end = window.paint_index(); - element_state.paint_range = paint_start..paint_end; - - ((), element_state) - }, - ) - } else { - element.as_mut().unwrap().paint(window, cx); - } - }); + fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement { + (self.render)(&self, window, cx) } } impl IntoElement for Entity { - type Element = AnyView; + type Element = ViewElement>; fn into_element(self) -> Self::Element { - self.into() + ViewElement::new(self) } } impl IntoElement for AnyView { - type Element = Self; + type Element = ViewElement; fn into_element(self) -> Self::Element { - self + ViewElement::new(self) } } -/// A weak, dynamically-typed view handle that does not prevent the view from being released. +/// A weak, dynamically-typed view handle. pub struct AnyWeakView { entity: AnyWeakEntity, render: fn(&AnyView, &mut Window, &mut App) -> AnyElement, } impl AnyWeakView { - /// Convert to a strongly-typed handle if the referenced view has not yet been released. + /// Upgrade to a strong `AnyView` handle, if the view is still alive. pub fn upgrade(&self) -> Option { let entity = self.entity.upgrade()?; Some(AnyView { entity, render: self.render, - cached_style: None, }) } } @@ -310,6 +161,335 @@ mod any_view { } } +/// A renderable that participates in GPUI's reactive graph — the unifying model +/// behind [`Render`] and [`RenderOnce`]. +/// +/// When `entity_id()` returns `Some`, that id becomes the view's identity: it gets +/// a unique element-id space (so internal `use_state` / `.id(..)` never collide +/// across siblings) and `cx.notify()` on that entity re-renders only this view's +/// subtree. `None` behaves like a stateless component. +/// +/// You rarely implement `View` directly. `Entity` and any `T: RenderOnce` +/// get a blanket impl below; implement it by hand only when a component needs both +/// parent-supplied props *and* a backing entity for identity. +pub trait View: 'static + Sized { + /// This view's identity, if it has one. A view typically holds the backing + /// entity as a field and returns its [`EntityId`] here. + /// + /// The id becomes this view's [`ElementId`], so two views keyed on the same + /// entity must not be rendered at the same position in the element tree + /// (e.g. as siblings under the same parent): their internal element state + /// (`use_state`, scroll offsets, etc.) would silently collide. Nesting is + /// fine — the id is scoped by the parent path. + fn entity_id(&self) -> Option; + + /// Render this view into an element tree, consuming `self`. + fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement; +} + +/// A stateless component (`RenderOnce`) is a `View` with no identity. +impl View for T { + fn entity_id(&self) -> Option { + None + } + + #[inline] + fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement { + RenderOnce::render(self, window, cx) + } +} + +/// An entity that renders itself (`Render`) is a `View` keyed on its own id. +impl View for Entity { + fn entity_id(&self) -> Option { + Some(Entity::entity_id(self)) + } + + #[inline] + fn render(self, window: &mut Window, cx: &mut App) -> impl IntoElement { + self.update(cx, |this, cx| { + Render::render(this, window, cx).into_any_element() + }) + } +} + +impl Entity { + /// Embed this entity as a cached [`ViewElement`] laid out at `style`. + /// + /// The rendered subtree is reused until the entity is notified (or the + /// cached bounds / text style change). Caching requires a definite size: + /// a cached view is laid out from `style` and is *not* measured from its + /// contents. Use [`ViewElement::new`] (or `.child(entity)`) for the + /// uncached case. + #[track_caller] + pub fn cached(self, style: StyleRefinement) -> ViewElement> { + ViewElement::new(self).cached(style) + } +} + +/// The element type for [`View`] implementations. Wraps a `View` and hooks it +/// into layout, prepaint, and paint. Constructed via [`ViewElement::new`]. +#[doc(hidden)] +pub struct ViewElement { + view: Option, + entity_id: Option, + cached_style: Option, + #[cfg(debug_assertions)] + source: &'static core::panic::Location<'static>, +} + +impl ViewElement { + /// Wrap a [`View`] as an element. + #[track_caller] + pub fn new(view: V) -> Self { + let entity_id = view.entity_id(); + ViewElement { + entity_id, + cached_style: None, + view: Some(view), + #[cfg(debug_assertions)] + source: core::panic::Location::caller(), + } + } + + /// Enable caching of this view's rendered subtree, laid out at `style`. + /// The composer supplies the layout style because caching skips rendering + /// the contents to measure them. + /// + /// Crate-private on purpose: caching is only sound for entity-backed views, + /// where [`Context::notify`] is the contract that busts the cache. A stateless + /// view has no such contract, so a frozen subtree could never be invalidated. + /// Reach this through [`Entity::cached`] or [`AnyView::cached`], which are + /// entity-backed by construction. + pub(crate) fn cached(mut self, style: StyleRefinement) -> Self { + self.cached_style = Some(style); + self + } +} + +impl IntoElement for ViewElement { + type Element = Self; + + fn into_element(self) -> Self::Element { + self + } +} + +struct ViewElementState { + prepaint_range: Range, + paint_range: Range, + cache_key: ViewElementCacheKey, + accessed_entities: FxHashSet, +} + +struct ViewElementCacheKey { + bounds: Bounds, + content_mask: ContentMask, + text_style: TextStyle, +} + +impl Element for ViewElement { + type RequestLayoutState = Option; + type PrepaintState = Option; + + fn id(&self) -> Option { + self.entity_id.map(ElementId::View) + } + + fn source_location(&self) -> Option<&'static core::panic::Location<'static>> { + #[cfg(debug_assertions)] + return Some(self.source); + + #[cfg(not(debug_assertions))] + return None; + } + + fn request_layout( + &mut self, + _id: Option<&GlobalElementId>, + _inspector_id: Option<&InspectorElementId>, + window: &mut Window, + cx: &mut App, + ) -> (LayoutId, Self::RequestLayoutState) { + if let Some(entity_id) = self.entity_id { + // Stateful path: create a reactive boundary. + window.with_rendered_view(entity_id, |window| { + let caching_disabled = window.is_inspector_picking(cx); + match self.cached_style.as_ref() { + Some(style) if !caching_disabled => { + let mut root_style = Style::default(); + root_style.refine(style); + let layout_id = window.request_layout(root_style, None, cx); + (layout_id, None) + } + _ => { + let mut element = self + .view + .take() + .unwrap() + .render(window, cx) + .into_any_element(); + let layout_id = element.request_layout(window, cx); + (layout_id, Some(element)) + } + } + }) + } else { + // Stateless path: isolate subtree via type name (no entity identity). + window.with_id( + ElementId::Name(std::any::type_name::().into()), + |window| { + let mut element = self + .view + .take() + .unwrap() + .render(window, cx) + .into_any_element(); + let layout_id = element.request_layout(window, cx); + (layout_id, Some(element)) + }, + ) + } + } + + fn prepaint( + &mut self, + global_id: Option<&GlobalElementId>, + _inspector_id: Option<&InspectorElementId>, + bounds: Bounds, + element: &mut Self::RequestLayoutState, + window: &mut Window, + cx: &mut App, + ) -> Option { + if let Some(entity_id) = self.entity_id { + // Stateful path. + window.set_view_id(entity_id); + window.with_rendered_view(entity_id, |window| { + if let Some(mut element) = element.take() { + element.prepaint(window, cx); + return Some(element); + } + + window.with_element_state::( + global_id.unwrap(), + |element_state, window| { + let content_mask = window.content_mask(); + let text_style = window.text_style(); + + if let Some(mut element_state) = element_state + && element_state.cache_key.bounds == bounds + && element_state.cache_key.content_mask == content_mask + && element_state.cache_key.text_style == text_style + && !window.dirty_views.contains(&entity_id) + && !window.refreshing + { + let prepaint_start = window.prepaint_index(); + window.reuse_prepaint(element_state.prepaint_range.clone()); + cx.entities + .extend_accessed(&element_state.accessed_entities); + let prepaint_end = window.prepaint_index(); + element_state.prepaint_range = prepaint_start..prepaint_end; + + return (None, element_state); + } + + let refreshing = mem::replace(&mut window.refreshing, true); + let prepaint_start = window.prepaint_index(); + let (mut element, accessed_entities) = cx.detect_accessed_entities(|cx| { + let mut element = self + .view + .take() + .unwrap() + .render(window, cx) + .into_any_element(); + element.layout_as_root(bounds.size.into(), window, cx); + element.prepaint_at(bounds.origin, window, cx); + element + }); + + let prepaint_end = window.prepaint_index(); + window.refreshing = refreshing; + + ( + Some(element), + ViewElementState { + accessed_entities, + prepaint_range: prepaint_start..prepaint_end, + paint_range: PaintIndex::default()..PaintIndex::default(), + cache_key: ViewElementCacheKey { + bounds, + content_mask, + text_style, + }, + }, + ) + }, + ) + }) + } else { + // Stateless path: just prepaint the element. + window.with_id( + ElementId::Name(std::any::type_name::().into()), + |window| { + element.as_mut().unwrap().prepaint(window, cx); + }, + ); + Some(element.take().unwrap()) + } + } + + fn paint( + &mut self, + global_id: Option<&GlobalElementId>, + _inspector_id: Option<&InspectorElementId>, + _bounds: Bounds, + _request_layout: &mut Self::RequestLayoutState, + element: &mut Self::PrepaintState, + window: &mut Window, + cx: &mut App, + ) { + if let Some(entity_id) = self.entity_id { + // Stateful path. + window.with_rendered_view(entity_id, |window| { + let caching_disabled = window.is_inspector_picking(cx); + if self.cached_style.is_some() && !caching_disabled { + window.with_element_state::( + global_id.unwrap(), + |element_state, window| { + let mut element_state = element_state.unwrap(); + + let paint_start = window.paint_index(); + + if let Some(element) = element { + let refreshing = mem::replace(&mut window.refreshing, true); + element.paint(window, cx); + window.refreshing = refreshing; + } else { + window.reuse_paint(element_state.paint_range.clone()); + } + + let paint_end = window.paint_index(); + element_state.paint_range = paint_start..paint_end; + + ((), element_state) + }, + ) + } else { + element.as_mut().unwrap().paint(window, cx); + } + }); + } else { + // Stateless path: just paint the element. + window.with_id( + ElementId::Name(std::any::type_name::().into()), + |window| { + element.as_mut().unwrap().paint(window, cx); + }, + ); + } + } +} + /// A view that renders nothing pub struct EmptyView; diff --git a/crates/gpui/src/window.rs b/crates/gpui/src/window.rs index f4bd64ecba5..62a8011d4e6 100644 --- a/crates/gpui/src/window.rs +++ b/crates/gpui/src/window.rs @@ -2824,7 +2824,7 @@ impl Window { }; // Layout all root elements. - let mut root_element = self.root.as_ref().unwrap().clone().into_any(); + let mut root_element = self.root.as_ref().unwrap().clone().into_any_element(); root_element.prepaint_as_root(Point::default(), root_size.into(), self, cx); #[cfg(any(feature = "inspector", debug_assertions))] @@ -2836,12 +2836,12 @@ impl Window { let mut active_drag_element = None; let mut tooltip_element = None; if let Some(prompt) = self.prompt.take() { - let mut element = prompt.view.any_view().into_any(); + let mut element = prompt.view.any_view().into_any_element(); element.prepaint_as_root(Point::default(), root_size.into(), self, cx); prompt_element = Some(element); self.prompt = Some(prompt); } else if let Some(active_drag) = cx.active_drag.take() { - let mut element = active_drag.view.clone().into_any(); + let mut element = active_drag.view.clone().into_any_element(); let offset = self.mouse_position() - active_drag.cursor_offset; element.prepaint_as_root(offset, AvailableSpace::min_size(), self, cx); active_drag_element = Some(element); @@ -2905,7 +2905,7 @@ impl Window { log::error!("Unexpectedly absent TooltipRequest"); continue; }; - let mut element = tooltip_request.tooltip.view.clone().into_any(); + let mut element = tooltip_request.tooltip.view.clone().into_any_element(); let mouse_position = tooltip_request.tooltip.mouse_position; let tooltip_size = element.layout_as_root(AvailableSpace::min_size(), self, cx); diff --git a/crates/gpui_macros/src/derive_into_element.rs b/crates/gpui_macros/src/derive_into_element.rs index 89d609ae65d..51d2a8ab3f7 100644 --- a/crates/gpui_macros/src/derive_into_element.rs +++ b/crates/gpui_macros/src/derive_into_element.rs @@ -11,11 +11,11 @@ pub fn derive_into_element(input: TokenStream) -> TokenStream { impl #impl_generics gpui::IntoElement for #type_name #type_generics #where_clause { - type Element = gpui::Component; + type Element = gpui::ViewElement; #[track_caller] fn into_element(self) -> Self::Element { - gpui::Component::new(self) + gpui::ViewElement::new(self) } } }; diff --git a/crates/gpui_macros/src/gpui_macros.rs b/crates/gpui_macros/src/gpui_macros.rs index f3958bca568..50305af752c 100644 --- a/crates/gpui_macros/src/gpui_macros.rs +++ b/crates/gpui_macros/src/gpui_macros.rs @@ -29,8 +29,8 @@ pub fn register_action(ident: TokenStream) -> TokenStream { register_action::register_action(ident) } -/// #[derive(IntoElement)] is used to create a Component out of anything that implements -/// the `RenderOnce` trait. +/// #[derive(IntoElement)] generates an `IntoElement` impl for any `RenderOnce` +/// type, wrapping it in a `ViewElement` so it can be used as a child. #[proc_macro_derive(IntoElement)] pub fn derive_into_element(input: TokenStream) -> TokenStream { derive_into_element::derive_into_element(input) diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index dd88d767413..0fc345b1b7c 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -6294,7 +6294,7 @@ impl Workspace { .children( self.notifications .iter() - .map(|(_, notification)| notification.clone().into_any()), + .map(|(_, notification)| notification.clone().into_any_element()), ), ) } From d9e35975c238013d2888c5ef8231c9f3d786c993 Mon Sep 17 00:00:00 2001 From: Oleksandr Kholiavko <43780952+HalavicH@users.noreply.github.com> Date: Wed, 8 Jul 2026 17:54:36 +0200 Subject: [PATCH 04/61] csv_preview: Add row filtering feature (#60339) # Objective CSV feature needs row filtering feature by column values. This PR provides base implementation of it with barebones ui. > NOTE: Sleek UI with search & proper scrolling hanling is implemented in next PR. It's stacked on top to reduce review scope ## Solution - New `FilterEntry` / `FilterEntryState` model in `table_data_engine/filtering_by_column.rs` tracking per-column applied/candidate filter values - Filtering runs in the background (`feat: Implement background filtering`) so large CSVs don't block the UI thread while a filter is applied - Filter menu entries reflect live counts and support a configurable sort order (`FilterSortOrder`, added in `renderer/settings.rs` / `settings.rs`) - Filter/sort trigger buttons on column headers are hidden until hover, using `GradientFade` (new in `ui/src/components/gradient_fade.rs`) to fade content behind them ## Testing Filter chain tested on csv fixtures with multiple filters applied sequentially columns. ## Self-Review Checklist: (todo) - [ ] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [ ] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) (out of scope of this pr) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable ## Showcase image image --- Release Notes: - Added initial row filtering UI & logic --- crates/csv_preview/src/csv_preview.rs | 70 +++-- crates/csv_preview/src/parser.rs | 9 +- .../renderer/performance_metrics_overlay.rs | 2 +- .../csv_preview/src/renderer/preview_view.rs | 16 +- .../src/renderer/row_identifiers.rs | 1 + crates/csv_preview/src/renderer/settings.rs | 53 +++- .../csv_preview/src/renderer/table_header.rs | 199 +++++++++++++- crates/csv_preview/src/settings.rs | 14 +- crates/csv_preview/src/table_data_engine.rs | 62 +++-- .../table_data_engine/filtering_by_column.rs | 250 ++++++++++++++++++ crates/gpui/src/elements/list.rs | 38 +++ 11 files changed, 668 insertions(+), 46 deletions(-) create mode 100644 crates/csv_preview/src/table_data_engine/filtering_by_column.rs diff --git a/crates/csv_preview/src/csv_preview.rs b/crates/csv_preview/src/csv_preview.rs index 322777da042..8f8ba768754 100644 --- a/crates/csv_preview/src/csv_preview.rs +++ b/crates/csv_preview/src/csv_preview.rs @@ -8,7 +8,7 @@ use std::{ time::{Duration, Instant}, }; -use crate::table_data_engine::TableDataEngine; +use crate::table_data_engine::{DisplayToDataMapping, TableDataEngine}; use ui::{ AbsoluteLength, ResizableColumnsState, SharedString, TableInteractionState, TableResizeBehavior, prelude::*, @@ -41,6 +41,10 @@ pub struct CsvPreviewView { pub(crate) table_interaction_state: Entity, pub(crate) column_widths: ColumnWidths, pub(crate) parsing_task: Option>>, + pub(crate) is_parsing: bool, + /// Background task computing the display-to-data mapping after a filter/sort change. + /// Stored here so that a new change cancels the previous in-flight computation. + pub(crate) filter_sort_task: Option>, pub(crate) settings: CsvPreviewSettings, /// Performance metrics for debugging and monitoring CSV operations. pub(crate) performance_metrics: PerformanceMetrics, @@ -178,9 +182,11 @@ impl CsvPreviewView { table_interaction_state, column_widths: ColumnWidths::new(cx, 1), parsing_task: None, + is_parsing: false, + filter_sort_task: None, performance_metrics: PerformanceMetrics::default(), list_state: gpui::ListState::new(contents.rows.len(), ListAlignment::Top, px(1.)) - .measure_all(), + .with_uniform_item_height(px(24.)), settings: CsvPreviewSettings::default(), last_parse_end_time: None, engine: TableDataEngine::default(), @@ -194,22 +200,54 @@ impl CsvPreviewView { pub(crate) fn editor_state(&self) -> &EditorState { &self.active_editor_state } - pub(crate) fn apply_sort(&mut self) { - self.performance_metrics.record("Sort", || { - self.engine.apply_sort(); - }); + pub(crate) fn apply_sort(&mut self, cx: &mut Context) { + self.apply_filter_sort(cx); } - /// Update ordered indices when ordering or content changes - pub(crate) fn apply_filter_sort(&mut self) { - self.performance_metrics.record("Filter&sort", || { - self.engine.calculate_d2d_mapping(); - }); + pub fn clear_filters(&mut self, col: types::AnyColumn, cx: &mut Context) { + self.engine.clear_filters_for_col(col); + self.apply_filter_sort(cx); + } - // Update list state with filtered row count - let visible_rows = self.engine.d2d_mapping().visible_row_count(); - self.list_state = - gpui::ListState::new(visible_rows, ListAlignment::Top, px(100.)).measure_all(); + pub fn toggle_filter( + &mut self, + col: types::AnyColumn, + value: Option, + cx: &mut Context, + ) { + if let Err(err) = self.engine.toggle_filter(col, value) { + log::error!("Failed to toggle filter: {err}"); + return; + } + self.apply_filter_sort(cx); + } + + /// Spawns a background task to recompute the display-to-data mapping after a filter or sort + /// change. Storing the task cancels any previous in-flight computation automatically. + pub(crate) fn apply_filter_sort(&mut self, cx: &mut Context) { + let contents = self.engine.contents.clone(); + let filter_stack = self.engine.filter_stack.clone(); + let sorting = self.engine.applied_sorting; + + self.filter_sort_task = Some(cx.spawn(async move |this, cx| { + let mapping = cx + .background_spawn(async move { + DisplayToDataMapping::compute(&contents, &filter_stack, sorting) + }) + .await; + + this.update(cx, |view, cx| { + view.engine.set_d2d_mapping(mapping); + let visible_rows = view.engine.d2d_mapping().visible_row_count(); + // Approximation of single csv table row height. Will be re-measured on scrolling. + // This cheap solution allow to render scrollbar with fraction of a cost compared to `.measure_all()` call + let approximate_height = px(24.); + view.list_state + .reset_with_uniform_height(visible_rows, approximate_height); + cx.notify(); + }) + .ok(); + })); } pub fn resolve_active_item_as_csv_editor( @@ -301,7 +339,7 @@ impl PerformanceMetrics { .map(|(name, (duration, time))| { let took = duration.as_secs_f32() * 1000.; let ago = time.elapsed().as_secs(); - format!("{name}: {took:.2}ms {ago}s ago") + format!("{name}: {took:.3}ms {ago}s ago") }) .collect::>() .join("\n") diff --git a/crates/csv_preview/src/parser.rs b/crates/csv_preview/src/parser.rs index efa3573d7aa..116c8912a38 100644 --- a/crates/csv_preview/src/parser.rs +++ b/crates/csv_preview/src/parser.rs @@ -1,3 +1,5 @@ +use std::sync::Arc; + use crate::{ CsvPreviewView, types::TableLikeContent, @@ -23,6 +25,7 @@ impl CsvPreviewView { cx: &mut Context, ) { let editor = self.active_editor_state.editor.clone(); + self.is_parsing = true; self.parsing_task = Some(self.parse_csv_in_background(wait_for_debounce, editor, cx)); } @@ -80,11 +83,13 @@ impl CsvPreviewView { .insert("Parsing", (parse_duration, Instant::now())); log::debug!("Parsed {} rows", parsed_csv.rows.len()); - view.engine.contents = parsed_csv; + view.engine.contents = Arc::new(parsed_csv); + view.engine.calculate_available_filters(); view.sync_column_widths(cx); view.last_parse_end_time = Some(parse_end_time); - view.apply_filter_sort(); + view.is_parsing = false; + view.apply_filter_sort(cx); cx.notify(); }) }) diff --git a/crates/csv_preview/src/renderer/performance_metrics_overlay.rs b/crates/csv_preview/src/renderer/performance_metrics_overlay.rs index 3d0cf50cf1d..d9e7ce6ad9b 100644 --- a/crates/csv_preview/src/renderer/performance_metrics_overlay.rs +++ b/crates/csv_preview/src/renderer/performance_metrics_overlay.rs @@ -20,7 +20,7 @@ impl CsvPreviewView { let children = div() .absolute() - .top_24() + .bottom_8() .right_4() .px_3() .py_2() diff --git a/crates/csv_preview/src/renderer/preview_view.rs b/crates/csv_preview/src/renderer/preview_view.rs index 90500d53d06..335633656af 100644 --- a/crates/csv_preview/src/renderer/preview_view.rs +++ b/crates/csv_preview/src/renderer/preview_view.rs @@ -1,6 +1,6 @@ use std::time::Instant; -use ui::{div, prelude::*}; +use ui::{SpinnerLabel, div, prelude::*}; use crate::CsvPreviewView; @@ -11,12 +11,12 @@ impl Render for CsvPreviewView { let render_prep_start = Instant::now(); let table_with_settings = v_flex() .size_full() - .p_4() .bg(theme.colors().editor_background) .track_focus(&self.focus_handle) .child(self.render_settings_panel(window, cx)) .child({ - if self.engine.contents.number_of_cols == 0 { + let is_parsing = self.is_parsing; + if is_parsing || self.engine.contents.number_of_cols == 0 { div() .flex() .items_center() @@ -25,7 +25,15 @@ impl Render for CsvPreviewView { .text_ui(cx) .font_buffer(cx) .text_color(cx.theme().colors().text_muted) - .child("No CSV content to display") + .when(is_parsing, |div| { + div.child( + h_flex() + .gap_2() + .child(SpinnerLabel::new()) + .child("Loading…"), + ) + }) + .when(!is_parsing, |div| div.child("No CSV content to display")) .into_any_element() } else { self.create_table(&self.column_widths.widths, cx) diff --git a/crates/csv_preview/src/renderer/row_identifiers.rs b/crates/csv_preview/src/renderer/row_identifiers.rs index 06a26e4696e..e24441d3885 100644 --- a/crates/csv_preview/src/renderer/row_identifiers.rs +++ b/crates/csv_preview/src/renderer/row_identifiers.rs @@ -171,6 +171,7 @@ impl CsvPreviewView { .px_1() .border_b_1() .border_color(cx.theme().colors().border_variant) + .bg(cx.theme().colors().panel_background) .h_full() .text_ui(cx) // Row identifiers are always centered diff --git a/crates/csv_preview/src/renderer/settings.rs b/crates/csv_preview/src/renderer/settings.rs index cafa2a4c1bd..18c185bb0f3 100644 --- a/crates/csv_preview/src/renderer/settings.rs +++ b/crates/csv_preview/src/renderer/settings.rs @@ -3,7 +3,10 @@ use ui::{ IntoElement as _, ParentElement as _, Styled as _, Tooltip, Window, div, h_flex, }; -use crate::{CsvPreviewView, settings::VerticalAlignment}; +use crate::{ + CsvPreviewView, + settings::{FilterSortOrder, VerticalAlignment}, +}; ///// Settings related ///// impl CsvPreviewView { @@ -18,6 +21,11 @@ impl CsvPreviewView { VerticalAlignment::Center => "Center", }; + let current_filter_sort_text = match self.settings.filter_sort_order { + FilterSortOrder::AlphaThenCount => "A-Z, then Count", + FilterSortOrder::CountThenAlpha => "Count, then A-Z", + }; + let view = cx.entity(); let alignment_dropdown_menu = ContextMenu::build(window, cx, |menu, _window, _cx| { menu.entry("Top", None, { @@ -40,6 +48,27 @@ impl CsvPreviewView { }) }); + let filter_sort_dropdown_menu = ContextMenu::build(window, cx, |menu, _window, _cx| { + menu.entry("A-Z, then Count", None, { + let view = view.clone(); + move |_window, cx| { + view.update(cx, |this, cx| { + this.settings.filter_sort_order = FilterSortOrder::AlphaThenCount; + cx.notify(); + }); + } + }) + .entry("Count, then A-Z", None, { + let view = view.clone(); + move |_window, cx| { + view.update(cx, |this, cx| { + this.settings.filter_sort_order = FilterSortOrder::CountThenAlpha; + cx.notify(); + }); + } + }) + }); + let panel = h_flex() .gap_4() .p_2() @@ -68,6 +97,28 @@ impl CsvPreviewView { "Choose vertical text alignment within cells", )), ), + ) + .child( + h_flex() + .gap_2() + .items_center() + .child( + div() + .text_sm() + .text_color(cx.theme().colors().text_muted) + .child("Filter Sort:"), + ) + .child( + DropdownMenu::new( + ElementId::Name("filter-sort-order-dropdown".into()), + current_filter_sort_text, + filter_sort_dropdown_menu, + ) + .trigger_size(ButtonSize::Compact) + .trigger_tooltip(Tooltip::text( + "Choose how filter values are sorted in the filter menu", + )), + ), ); #[cfg(feature = "dev-tools")] diff --git a/crates/csv_preview/src/renderer/table_header.rs b/crates/csv_preview/src/renderer/table_header.rs index 05652b49a48..1ce8d34d22b 100644 --- a/crates/csv_preview/src/renderer/table_header.rs +++ b/crates/csv_preview/src/renderer/table_header.rs @@ -1,9 +1,13 @@ use gpui::ElementId; -use ui::{Tooltip, prelude::*}; +use ui::{ContextMenu, PopoverMenu, Tooltip, prelude::*}; use crate::{ CsvPreviewView, - table_data_engine::sorting_by_column::{AppliedSorting, SortDirection}, + settings::FilterSortOrder, + table_data_engine::{ + filtering_by_column::{FilterEntry, FilterEntryState}, + sorting_by_column::{AppliedSorting, SortDirection}, + }, types::AnyColumn, }; @@ -22,7 +26,12 @@ impl CsvPreviewView { .w_full() .font_buffer(cx) .child(div().child(header_text)) - .child(h_flex().gap_1().child(self.create_sort_button(cx, col_idx))) + .child( + h_flex() + .gap_1() + .child(self.create_filter_button(cx, col_idx)) + .child(self.create_sort_button(cx, col_idx)), + ) .into_any_element() } @@ -82,9 +91,191 @@ impl CsvPreviewView { }; this.engine.applied_sorting = new_sorting; - this.apply_sort(); + this.apply_sort(cx); cx.notify(); })); sort_btn } + + fn create_filter_button( + &self, + cx: &mut Context<'_, CsvPreviewView>, + col: AnyColumn, + ) -> PopoverMenu { + let has_active_filters = self.engine.has_active_filters(col); + + PopoverMenu::new(ElementId::NamedInteger( + "filter-menu".into(), + col.get() as u64, + )) + .trigger_with_tooltip( + Button::new( + ElementId::NamedInteger("filter-button".into(), col.get() as u64), + if has_active_filters { "⛊" } else { "⛉" }, + ) + .size(ButtonSize::Compact) + .style(if has_active_filters { + ButtonStyle::Filled + } else { + ButtonStyle::Subtle + }), + Tooltip::text(if has_active_filters { + "Column has active filters. Click to manage" + } else { + "No filters applied. Click to add filters" + }), + ) + .menu({ + let view_entity = cx.entity(); + move |window, cx| { + let view = view_entity.read(cx); + let column_filters = match view.engine.get_filters_for_column(col) { + Ok(filters) => filters, + Err(err) => { + log::error!("Failed to get filters for column: {err}"); + return None; + } + }; + let filter_sort_order = view.settings.filter_sort_order; + let filter_menu = Self::create_filter_menu( + window, + cx, + view_entity.clone(), + col, + &column_filters, + has_active_filters, + filter_sort_order, + ); + Some(filter_menu) + } + }) + } + + fn create_filter_menu( + window: &mut ui::Window, + cx: &mut ui::App, + view_entity: gpui::Entity, + col: AnyColumn, + column_filters: &[(FilterEntry, FilterEntryState)], + has_active_filters: bool, + sort_order: FilterSortOrder, + ) -> gpui::Entity { + let mut available: Vec<&FilterEntry> = column_filters + .iter() + .filter_map(|(entry, state)| { + matches!(state, FilterEntryState::Available { .. }).then_some(entry) + }) + .collect(); + + match sort_order { + FilterSortOrder::AlphaThenCount => available.sort_by(|a, b| { + a.content + .cmp(&b.content) + .then_with(|| b.occurred_times().cmp(&a.occurred_times())) + }), + FilterSortOrder::CountThenAlpha => available.sort_by(|a, b| { + b.occurred_times() + .cmp(&a.occurred_times()) + .then_with(|| a.content.cmp(&b.content)) + }), + } + + let unavailable: Vec<(&FilterEntry, AnyColumn)> = column_filters + .iter() + .filter_map(|(entry, state)| { + if let FilterEntryState::Unavailable { blocked_by } = state { + Some((entry, *blocked_by)) + } else { + None + } + }) + .collect(); + + // Pre-build applied-state lookup before moving into the closure + let applied_states: Vec<(FilterEntry, bool)> = column_filters + .iter() + .filter_map(|(entry, state)| { + if let FilterEntryState::Available { is_applied } = state { + Some((entry.clone(), *is_applied)) + } else { + None + } + }) + .collect(); + + let available_cloned: Vec = available.iter().map(|e| (*e).clone()).collect(); + let unavailable_cloned: Vec<(FilterEntry, AnyColumn)> = unavailable + .into_iter() + .map(|(e, col)| (e.clone(), col)) + .collect(); + + ContextMenu::build(window, cx, move |menu, _, _| { + let mut menu = menu; + + if has_active_filters { + menu = menu + .toggleable_entry("Clear all", false, ui::IconPosition::Start, None, { + let view_entity = view_entity.clone(); + move |_window, cx| { + view_entity.update(cx, |view, cx| { + view.clear_filters(col, cx); + cx.notify(); + }); + } + }) + .separator(); + } + + for entry in &available_cloned { + let is_applied = applied_states + .iter() + .find(|(e, _)| e.content == entry.content) + .map_or(false, |(_, applied)| *applied); + + let label: SharedString = + format_filter_label(entry.content.as_ref(), entry.occurred_times()).into(); + let entry_value = entry.content.clone(); + + menu = menu.toggleable_entry(&label, is_applied, ui::IconPosition::Start, None, { + let view_entity = view_entity.clone(); + move |_window, cx| { + view_entity.update(cx, |view, cx| { + view.toggle_filter(col, entry_value.clone(), cx); + cx.notify(); + }); + } + }); + } + + if !unavailable_cloned.is_empty() { + menu = menu.separator().header("Hidden by other filters"); + for (entry, _blocked_by) in &unavailable_cloned { + let label: SharedString = + format_filter_label(entry.content.as_ref(), entry.occurred_times()).into(); + menu = menu.custom_entry( + { + let label = label.clone(); + move |_window, cx| { + div() + .px_2() + .text_color(cx.theme().colors().text_muted) + .child(label.clone()) + .into_any_element() + } + }, + |_, _| {}, + ); + } + } + + menu + }) + } +} + +fn format_filter_label(content: Option<&SharedString>, count: usize) -> String { + match content { + Some(s) => format!("{s} ({count})"), + None => format!(" ({count})"), + } } diff --git a/crates/csv_preview/src/settings.rs b/crates/csv_preview/src/settings.rs index 215d681c28f..2d5cc78c5f5 100644 --- a/crates/csv_preview/src/settings.rs +++ b/crates/csv_preview/src/settings.rs @@ -1,10 +1,10 @@ #[derive(Default, Clone, Copy, PartialEq)] pub enum RowRenderMechanism { /// More correct for multiline content, but slower. - #[allow(dead_code)] // Will be used when settings ui is added + #[default] VariableList, /// Default behaviour for now while resizable columns are being stabilized. - #[default] + #[allow(dead_code)] // Will be used when settings ui is added UniformList, } @@ -26,11 +26,21 @@ pub enum RowIdentifiers { RowNum, } +#[derive(Default, Clone, Copy, PartialEq)] +pub enum FilterSortOrder { + /// Sort alphabetically (A→Z), then by number of occurrences descending within ties + #[default] + AlphaThenCount, + /// Sort by number of occurrences descending, then alphabetically within ties + CountThenAlpha, +} + #[derive(Clone, Default)] pub(crate) struct CsvPreviewSettings { pub(crate) rendering_with: RowRenderMechanism, pub(crate) vertical_alignment: VerticalAlignment, pub(crate) numbering_type: RowIdentifiers, + pub(crate) filter_sort_order: FilterSortOrder, pub(crate) show_debug_info: bool, #[cfg(feature = "dev-tools")] pub(crate) show_perf_metrics_overlay: bool, diff --git a/crates/csv_preview/src/table_data_engine.rs b/crates/csv_preview/src/table_data_engine.rs index 382b41a2850..f2613215fa5 100644 --- a/crates/csv_preview/src/table_data_engine.rs +++ b/crates/csv_preview/src/table_data_engine.rs @@ -5,22 +5,32 @@ //! //! It's designed to contain core logic of operations without relying on `CsvPreviewView`, context or window handles. -use std::{collections::HashMap, sync::Arc}; +use std::{ + collections::{HashMap, HashSet}, + sync::Arc, +}; use ui::table_row::TableRow; use crate::{ - table_data_engine::sorting_by_column::{AppliedSorting, sort_data_rows}, - types::{DataRow, DisplayRow, TableCell, TableLikeContent}, + table_data_engine::{ + filtering_by_column::{FilterEntry, FilterStack, calculate_available_filters, retain_rows}, + sorting_by_column::{AppliedSorting, sort_data_rows}, + }, + types::{AnyColumn, DataRow, DisplayRow, TableCell, TableLikeContent}, }; +pub mod filtering_by_column; pub mod sorting_by_column; #[derive(Default)] pub(crate) struct TableDataEngine { + pub filter_stack: FilterStack, + /// Pre-computed unique values per column, used to populate filter menus + all_filters: HashMap>, pub applied_sorting: Option, d2d_mapping: DisplayToDataMapping, - pub contents: TableLikeContent, + pub contents: Arc, } impl TableDataEngine { @@ -28,32 +38,47 @@ impl TableDataEngine { &self.d2d_mapping } - pub(crate) fn apply_sort(&mut self) { - self.d2d_mapping - .apply_sorting(self.applied_sorting, &self.contents.rows); - self.d2d_mapping.merge_mappings(); + pub(crate) fn set_d2d_mapping(&mut self, mapping: DisplayToDataMapping) { + self.d2d_mapping = mapping; } - /// Applies sorting and filtering to the data and produces display to data mapping - pub(crate) fn calculate_d2d_mapping(&mut self) { - self.d2d_mapping - .apply_sorting(self.applied_sorting, &self.contents.rows); - self.d2d_mapping.merge_mappings(); + /// Recomputes the unique filter entries for every column from the current table data. + /// Must be called after content changes (e.g. after parsing). + pub fn calculate_available_filters(&mut self) { + self.all_filters = + calculate_available_filters(&self.contents.rows, self.contents.number_of_cols); } } /// Relation of Display (rendered) rows to Data (src) rows with applied transformations /// Transformations applied: /// - sorting by column +/// - filtering by column values #[derive(Debug, Default)] pub struct DisplayToDataMapping { - /// All rows sorted, regardless of applied filtering. Applied every time sorting changes + /// All rows sorted, regardless of applied filtering. Recomputed every time sorting changes pub sorted_rows: Vec, - /// Filtered and sorted rows. Computed cheaply from `sorted_mapping` and `filtered_out_rows` + /// Rows that survive the active filters. Recomputed every time filters change + pub retained_rows: HashSet, + /// Merged result: sorted rows intersected with retained rows pub mapping: Arc>, } impl DisplayToDataMapping { + /// Computes the full display-to-data mapping from owned inputs. + /// Intended to be called from a background thread. + pub(crate) fn compute( + contents: &Arc, + filter_stack: &FilterStack, + sorting: Option, + ) -> Self { + let mut mapping = Self::default(); + mapping.apply_sorting(sorting, &contents.rows); + mapping.apply_filtering(filter_stack, &contents.rows); + mapping.merge_mappings(); + mapping + } + /// Get the data row for a given display row pub fn get_data_row(&self, display_row: DisplayRow) -> Option { self.mapping.get(&display_row).copied() @@ -77,11 +102,16 @@ impl DisplayToDataMapping { self.sorted_rows = sorted_rows; } - /// Take pre-computed sorting and filtering results, and apply them to the mapping + fn apply_filtering(&mut self, filter_stack: &FilterStack, rows: &[TableRow]) { + self.retained_rows = retain_rows(rows, filter_stack); + } + + /// Merges pre-computed sorting and filtering into the final display mapping fn merge_mappings(&mut self) { self.mapping = Arc::new( self.sorted_rows .iter() + .filter(|data_row| self.retained_rows.contains(data_row)) .enumerate() .map(|(display, data)| (DisplayRow(display), *data)) .collect(), diff --git a/crates/csv_preview/src/table_data_engine/filtering_by_column.rs b/crates/csv_preview/src/table_data_engine/filtering_by_column.rs new file mode 100644 index 00000000000..b2b8a78c05a --- /dev/null +++ b/crates/csv_preview/src/table_data_engine/filtering_by_column.rs @@ -0,0 +1,250 @@ +use std::{ + collections::{HashMap, HashSet}, + sync::Arc, +}; + +use ui::{SharedString, table_row::TableRow}; + +use crate::{ + table_data_engine::TableDataEngine, + types::{AnyColumn, DataRow, TableCell}, +}; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +pub enum FilterEntryState { + Available { is_applied: bool }, + Unavailable { blocked_by: AnyColumn }, +} + +#[derive(Debug, Clone)] +pub struct FilterEntry { + /// Content to display. None if cell is virtual + pub content: Option, + /// List of rows in which this value occurs + pub rows: Vec, +} + +impl FilterEntry { + pub(crate) fn occurred_times(&self) -> usize { + self.rows.len() + } +} + +#[derive(Debug, Default, Clone)] +pub(crate) struct FilterStack { + /// Columns in the order their first filter was applied, used to compute cascade availability + activation_order: Vec, + /// Which cell values are currently allowed for each filtered column + retention_config: HashMap>>, +} + +impl TableDataEngine { + pub(crate) fn has_active_filters(&self, col: AnyColumn) -> bool { + self.filter_stack.retention_config.contains_key(&col) + } + + /// Get available filters for a specific column with cascade behavior. + /// + /// A filter entry is "unavailable" when all of its rows are hidden by a + /// filter on an earlier-activated column, meaning selecting it would show + /// zero rows. The cascade walk stops at `column` so that the column's own + /// current filter does not affect its own entry availability. + pub(crate) fn get_filters_for_column( + &self, + column: AnyColumn, + ) -> anyhow::Result>> { + let all_column_entries = self + .all_filters + .get(&column) + .ok_or_else(|| anyhow::anyhow!("Expected {column:?} to have filter entries"))?; + + let mut unavailable_entries: HashMap, AnyColumn> = HashMap::new(); + + for &column_applied_previously in &self.filter_stack.activation_order { + if column_applied_previously == column { + break; + } + + let retained_values = self + .filter_stack + .retention_config + .get(&column_applied_previously) + .ok_or_else(|| { + anyhow::anyhow!( + "Expected {column_applied_previously:?} to have retained entries \ + as it is present in the filter stack" + ) + })?; + + // Rows that survive the filter on `column_applied_previously` + let retained_rows: HashSet = self + .contents + .rows + .iter() + .enumerate() + .filter(|(_, row)| { + let cell_value = row + .get(column_applied_previously) + .and_then(|cell| cell.display_value().cloned()); + retained_values.contains(&cell_value) + }) + .map(|(index, _)| DataRow(index)) + .collect(); + + // An entry is unavailable when none of its rows survive the parent filter + for entry in all_column_entries { + if !entry.rows.iter().any(|row| retained_rows.contains(row)) { + unavailable_entries.insert(entry.content.clone(), column_applied_previously); + } + } + } + + let empty = HashSet::new(); + let active_column_filters = self + .filter_stack + .retention_config + .get(&column) + .unwrap_or(&empty); + + Ok(Arc::new( + all_column_entries + .iter() + .map(|entry| { + let state = if let Some(&blocked_by) = unavailable_entries.get(&entry.content) { + FilterEntryState::Unavailable { blocked_by } + } else { + FilterEntryState::Available { + is_applied: active_column_filters.contains(&entry.content), + } + }; + (entry.clone(), state) + }) + .collect(), + )) + } + + pub(crate) fn clear_filters_for_col(&mut self, col: AnyColumn) { + self.filter_stack + .activation_order + .retain(|&entry| entry != col); + self.filter_stack.retention_config.remove(&col); + } + + /// Toggle a filter value for a column. Returns `true` if the filter was + /// added, `false` if it was removed. + pub(crate) fn toggle_filter( + &mut self, + column: AnyColumn, + value: Option, + ) -> anyhow::Result { + let is_currently_applied = self + .filter_stack + .retention_config + .get(&column) + .is_some_and(|filters| filters.contains(&value)); + + if is_currently_applied { + self.remove_filter(column, value)?; + Ok(false) + } else { + self.apply_filter(column, value); + Ok(true) + } + } + + fn remove_filter( + &mut self, + column: AnyColumn, + value: Option, + ) -> anyhow::Result<()> { + let entries = self + .filter_stack + .retention_config + .get_mut(&column) + .ok_or_else(|| { + anyhow::anyhow!("Expected {column:?} to be present in active filters") + })?; + + debug_assert!( + entries.contains(&value), + "Expected value to be present in {column:?} active filters" + ); + + if entries.len() == 1 { + self.filter_stack.retention_config.remove(&column); + self.filter_stack + .activation_order + .retain(|&entry| entry != column); + } else { + entries.remove(&value); + } + Ok(()) + } + + fn apply_filter(&mut self, column: AnyColumn, value: Option) { + // Track the column only on its first activation to preserve cascade order + if !self.filter_stack.activation_order.contains(&column) { + self.filter_stack.activation_order.push(column); + } + self.filter_stack + .retention_config + .entry(column) + .or_default() + .insert(value); + } +} + +/// Calculate available filter entries for each column from the table data. +pub fn calculate_available_filters( + content_rows: &[TableRow], + number_of_cols: usize, +) -> HashMap> { + let mut available_filters = HashMap::new(); + + for col_idx in 0..number_of_cols { + let column = AnyColumn::new(col_idx); + let mut value_to_rows: HashMap, Vec> = HashMap::new(); + + for (row_index, row) in content_rows.iter().enumerate() { + let cell_value = row + .get(column) + .and_then(|cell| cell.display_value().cloned()); + value_to_rows + .entry(cell_value) + .or_default() + .push(DataRow(row_index)); + } + + let filter_entries: Vec = value_to_rows + .into_iter() + .map(|(content, rows)| FilterEntry { content, rows }) + .collect(); + + available_filters.insert(column, filter_entries); + } + + available_filters +} + +/// Returns the set of data rows that survive all active filters in the stack. +pub fn retain_rows( + content_rows: &[TableRow], + filter_stack: &FilterStack, +) -> HashSet { + let config = &filter_stack.retention_config; + if config.is_empty() { + return (0..content_rows.len()).map(DataRow).collect(); + } + + content_rows + .iter() + .enumerate() + .filter(|(_, row)| { + config.iter().all(|(col, allowed_values)| { + let cell_value = row.get(*col).and_then(|cell| cell.display_value().cloned()); + allowed_values.contains(&cell_value) + }) + }) + .map(|(index, _)| DataRow(index)) + .collect() +} diff --git a/crates/gpui/src/elements/list.rs b/crates/gpui/src/elements/list.rs index 85b38d5234e..62b481bc530 100644 --- a/crates/gpui/src/elements/list.rs +++ b/crates/gpui/src/elements/list.rs @@ -338,6 +338,17 @@ impl ListState { self } + /// Pre-populate every unmeasured item with a uniform height hint so the scrollbar thumb + /// is correctly sized from the first frame, without measuring all items up front. + /// + /// As items are actually rendered their real heights replace the hint, so the scrollbar + /// converges to the exact size over time. This is a cheaper alternative to [`Self::measure_all`] + /// for lists where items have roughly uniform heights (e.g. table rows). + pub fn with_uniform_item_height(self, height: Pixels) -> Self { + self.apply_uniform_item_height(height); + self + } + /// Reset this instantiation of the list state. /// /// Note that this will cause scroll events to be dropped until the next paint. @@ -355,6 +366,33 @@ impl ListState { self.splice(0..old_count, element_count); } + /// Reset the list to `element_count` items, pre-populating every item with a + /// uniform height hint so the scrollbar thumb is correctly sized from the first + /// frame even for off-screen items. + pub fn reset_with_uniform_height(&self, element_count: usize, height: Pixels) { + self.reset(element_count); + self.apply_uniform_item_height(height); + } + + fn apply_uniform_item_height(&self, height: Pixels) { + let size_hint = Size { + width: px(0.), + height, + }; + let mut state = self.0.borrow_mut(); + let new_items = state + .items + .iter() + .map(|item| ListItem::Unmeasured { + size_hint: Some(item.size_hint().unwrap_or(size_hint)), + focus_handle: item.focus_handle(), + }) + .collect::>(); + let mut tree = SumTree::default(); + tree.extend(new_items, ()); + state.items = tree; + } + /// Remeasure all items while preserving proportional scroll position. /// /// Use this when item heights may have changed (e.g., font size changes) From 6979d7281f261004e72c15a14f7c31da89d16e63 Mon Sep 17 00:00:00 2001 From: "zed-zippy[bot]" <234243425+zed-zippy[bot]@users.noreply.github.com> Date: Wed, 8 Jul 2026 18:06:44 +0200 Subject: [PATCH 05/61] Bump Zed to v1.12.0 (#60600) Release Notes: - N/A Co-authored-by: zed-zippy[bot] <234243425+zed-zippy[bot]@users.noreply.github.com> --- Cargo.lock | 2 +- crates/zed/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1d5fe8fd9bd..b9b4ea96158 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -22994,7 +22994,7 @@ dependencies = [ [[package]] name = "zed" -version = "1.11.0" +version = "1.12.0" dependencies = [ "acp_thread", "acp_tools", diff --git a/crates/zed/Cargo.toml b/crates/zed/Cargo.toml index 2b67736bf0e..213147207c3 100644 --- a/crates/zed/Cargo.toml +++ b/crates/zed/Cargo.toml @@ -2,7 +2,7 @@ description = "The fast, collaborative code editor." edition.workspace = true name = "zed" -version = "1.11.0" +version = "1.12.0" publish.workspace = true license = "GPL-3.0-or-later" authors = ["Zed Team "] From 7f5cf583dc72035efe68cb3ef086857cda0c7a47 Mon Sep 17 00:00:00 2001 From: Jiby Jose Date: Wed, 8 Jul 2026 18:37:02 +0200 Subject: [PATCH 06/61] Fix worktree entry IDs for symlinked files (#57846) 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 #55792 Release Notes: - Fixed files in pnpm workspaces moving to symlinked `node_modules` paths after saving. --------- Co-authored-by: Kirill Bulatov --- crates/worktree/src/worktree.rs | 122 ++++++++++++------ .../tests/integration/worktree_tests.rs | 92 +++++++++++++ 2 files changed, 174 insertions(+), 40 deletions(-) diff --git a/crates/worktree/src/worktree.rs b/crates/worktree/src/worktree.rs index c31385f4553..6c90de189b5 100644 --- a/crates/worktree/src/worktree.rs +++ b/crates/worktree/src/worktree.rs @@ -272,16 +272,65 @@ struct BackgroundScannerState { watched_dir_abs_paths_by_entry_id: HashMap>, path_prefixes_to_scan: HashSet>, paths_to_scan: HashSet>, - /// The ids of all of the entries that were removed from the snapshot - /// as part of the current update. These entry ids may be re-used - /// if the same inode is discovered at a new path, or if the given - /// path is re-created after being deleted. - removed_entries: HashMap, + removed_entries: RemovedEntries, changed_paths: Vec>, prev_snapshot: Snapshot, scanning_enabled: bool, } +/// The entries that were removed from the snapshot as part of the current +/// update. Their entry ids may be re-used if the same inode is discovered +/// at a new path, or if the given path is re-created after being deleted. +/// +/// Symlink aliases inside the worktree share their inode (and usually mtime) +/// with the symlink target, so an inode may correspond to several entries. +/// The path index allows an exact match to take precedence over the +/// inode-based rename heuristics in that case. +#[derive(Default)] +struct RemovedEntries { + by_inode: HashMap, + by_path: HashMap, Entry>, +} + +impl RemovedEntries { + fn insert(&mut self, entry: &Entry) { + self.by_path.insert(entry.path.clone(), entry.clone()); + match self.by_inode.entry(entry.inode) { + hash_map::Entry::Occupied(mut o) => { + if entry.id > o.get().id { + o.insert(entry.clone()); + } + } + hash_map::Entry::Vacant(v) => { + v.insert(entry.clone()); + } + } + } + + fn take_by_path(&mut self, path: &RelPath, inode: u64) -> Option { + if self.by_path.get(path)?.inode != inode { + return None; + } + let removed = self.by_path.remove(path)?; + if let hash_map::Entry::Occupied(o) = self.by_inode.entry(removed.inode) + && o.get().id == removed.id + { + o.remove(); + } + Some(removed) + } + + fn take_by_inode(&mut self, inode: u64) -> Option { + let removed = self.by_inode.remove(&inode)?; + if let hash_map::Entry::Occupied(o) = self.by_path.entry(removed.path.clone()) + && o.get().id == removed.id + { + o.remove(); + } + Some(removed) + } +} + #[derive(Clone, Debug, Eq, PartialEq)] struct EventRoot { path: Arc, @@ -1229,7 +1278,7 @@ impl LocalWorktree { scanning_enabled, path_prefixes_to_scan: Default::default(), paths_to_scan: Default::default(), - removed_entries: Default::default(), + removed_entries: RemovedEntries::default(), changed_paths: Default::default(), }), phase: BackgroundScannerPhase::InitialScan, @@ -3086,21 +3135,11 @@ impl BackgroundScannerState { } fn reuse_entry_id(&mut self, entry: &mut Entry) { - if let Some(mtime) = entry.mtime { - // If an entry with the same inode was removed from the worktree during this scan, - // then it *might* represent the same file or directory. But the OS might also have - // re-used the inode for a completely different file or directory. - // - // Conditionally reuse the old entry's id: - // * if the mtime is the same, the file was probably been renamed. - // * if the path is the same, the file may just have been updated - if let Some(removed_entry) = self.removed_entries.remove(&entry.inode) { - if removed_entry.mtime == Some(mtime) || removed_entry.path == entry.path { - entry.id = removed_entry.id; - } - } else if let Some(existing_entry) = self.snapshot.entry_for_path(&entry.path) { - entry.id = existing_entry.id; - } + let Some(mtime) = entry.mtime else { + return; + }; + if let Some(entry_id) = self.reused_entry_id(&entry.path, entry.inode, mtime) { + entry.id = entry_id; } } @@ -3110,6 +3149,20 @@ impl BackgroundScannerState { path: &RelPath, metadata: &fs::Metadata, ) -> ProjectEntryId { + self.reused_entry_id(path, metadata.inode, metadata.mtime) + .unwrap_or_else(|| ProjectEntryId::new(next_entry_id)) + } + + fn reused_entry_id( + &mut self, + path: &RelPath, + inode: u64, + mtime: MTime, + ) -> Option { + if let Some(removed_entry) = self.removed_entries.take_by_path(path, inode) { + return Some(removed_entry.id); + } + // If an entry with the same inode was removed from the worktree during this scan, // then it *might* represent the same file or directory. But the OS might also have // re-used the inode for a completely different file or directory. @@ -3117,14 +3170,12 @@ impl BackgroundScannerState { // Conditionally reuse the old entry's id: // * if the mtime is the same, the file was probably been renamed. // * if the path is the same, the file may just have been updated - if let Some(removed_entry) = self.removed_entries.remove(&metadata.inode) { - if removed_entry.mtime == Some(metadata.mtime) || *removed_entry.path == *path { - return removed_entry.id; - } - } else if let Some(existing_entry) = self.snapshot.entry_for_path(path) { - return existing_entry.id; + if let Some(removed_entry) = self.removed_entries.take_by_inode(inode) { + (removed_entry.mtime == Some(mtime) || *removed_entry.path == *path) + .then_some(removed_entry.id) + } else { + Some(self.snapshot.entry_for_path(path)?.id) } - ProjectEntryId::new(next_entry_id) } async fn insert_entry(&mut self, entry: Entry, fs: &dyn Fs, watcher: &dyn Watcher) -> Entry { @@ -3292,17 +3343,7 @@ impl BackgroundScannerState { removed_dir_abs_paths.push(watch_path); } - match self.removed_entries.entry(entry.inode) { - hash_map::Entry::Occupied(mut e) => { - let prev_removed_entry = e.get_mut(); - if entry.id > prev_removed_entry.id { - *prev_removed_entry = entry.clone(); - } - } - hash_map::Entry::Vacant(e) => { - e.insert(entry.clone()); - } - } + self.removed_entries.insert(entry); if entry.path.file_name() == Some(GITIGNORE) { let abs_parent_path = self.snapshot.absolutize(&entry.path.parent().unwrap()); @@ -4800,7 +4841,8 @@ impl BackgroundScanner { { let mut state = self.state.lock().await; state.snapshot.completed_scan_id = state.snapshot.scan_id; - for (_, entry) in mem::take(&mut state.removed_entries) { + let RemovedEntries { by_inode, by_path } = mem::take(&mut state.removed_entries); + for entry in by_inode.into_values().chain(by_path.into_values()) { state.scanned_dirs.remove(&entry.id); } } diff --git a/crates/worktree/tests/integration/worktree_tests.rs b/crates/worktree/tests/integration/worktree_tests.rs index 1fd7ed5b557..f63bf7d1085 100644 --- a/crates/worktree/tests/integration/worktree_tests.rs +++ b/crates/worktree/tests/integration/worktree_tests.rs @@ -1198,6 +1198,98 @@ async fn test_real_fs_scan_symlinks_expanded(cx: &mut TestAppContext) { }); } +#[gpui::test] +async fn test_internal_symlink_updates_preserve_entry_ids(cx: &mut TestAppContext) { + init_test(cx); + let fs = FakeFs::new(cx.background_executor.clone()); + + fs.insert_tree( + "/root", + json!({ + "project": { + "real-dir": { + "existing.rs": "old", + }, + "links": {} + } + }), + ) + .await; + + fs.create_symlink( + "/root/project/links/internal".as_ref(), + "../real-dir".into(), + ) + .await + .unwrap(); + + let tree = Worktree::local( + Path::new("/root/project"), + true, + fs.clone(), + Default::default(), + true, + WorktreeId::from_proto(0), + &mut cx.to_async(), + ) + .await + .unwrap(); + + cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete()) + .await; + + let (real_entry_id, symlink_entry_id, old_mtime) = tree.read_with(cx, |tree, _| { + let real_entry = tree + .entry_for_path(rel_path("real-dir/existing.rs")) + .unwrap(); + let symlink_entry = tree + .entry_for_path(rel_path("links/internal/existing.rs")) + .unwrap(); + assert_eq!(real_entry.inode, symlink_entry.inode); + assert_eq!(real_entry.mtime, symlink_entry.mtime); + assert_ne!(real_entry.id, symlink_entry.id); + (real_entry.id, symlink_entry.id, real_entry.mtime) + }); + + fs.write(Path::new("/root/project/real-dir/existing.rs"), b"new") + .await + .unwrap(); + + wait_for_condition(cx, |cx| { + tree.read_with(cx, |tree, _| { + let real_entry = tree + .entry_for_path(rel_path("real-dir/existing.rs")) + .unwrap(); + let symlink_entry = tree + .entry_for_path(rel_path("links/internal/existing.rs")) + .unwrap(); + real_entry.mtime != old_mtime && symlink_entry.mtime != old_mtime + }) + }) + .await; + + tree.read_with(cx, |tree, _| { + let real_entry = tree + .entry_for_path(rel_path("real-dir/existing.rs")) + .unwrap(); + let symlink_entry = tree + .entry_for_path(rel_path("links/internal/existing.rs")) + .unwrap(); + + assert_eq!(real_entry.inode, symlink_entry.inode); + assert_eq!(real_entry.id, real_entry_id); + assert_eq!( + tree.entry_for_id(real_entry_id).unwrap().path.as_ref(), + rel_path("real-dir/existing.rs") + ); + assert_eq!(symlink_entry.id, symlink_entry_id); + assert_eq!( + tree.entry_for_id(symlink_entry_id).unwrap().path.as_ref(), + rel_path("links/internal/existing.rs") + ); + }); +} + #[cfg(target_os = "macos")] #[gpui::test] async fn test_renaming_case_only(cx: &mut TestAppContext) { From 029bf2f284b4e59f20175d78443e630468f3a3e5 Mon Sep 17 00:00:00 2001 From: TwoClocks <5883156+TwoClocks@users.noreply.github.com> Date: Wed, 8 Jul 2026 09:37:08 -0700 Subject: [PATCH 07/61] Make editor::LineUp & editor::LineDown honor vertical_scroll_margin like vim::LineUp & vim::LineDown (#52057) ## Context This is an implementation of this feature: https://github.com/zed-industries/zed/discussions/49821 Although, I'd argue it's a bug fix, not a feature. Either way : This copies the window scroll logic from the `vim` mode versions with out all the extra logic for visual mode. Could refactor the common logic out of the vim code and make it common. But that seems like a bigger PR. Happy to take a stab at it if that's what you prefer. Could also add new commands for the new behavior if you prefer. I didn't do that, because it seems like more clutter in the commands, and my belief that the existing behavior is a bug. But happy to do that if you prefer. ## How to Review creates new function `scroll_screen_with_cursor_margin` in `scroll.rs` wires up editor::LineUp/LineDown to new function in `elements.rs` adds a test in `editor_tests.rs` ## 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: - editor::LineUp/LineDown commands now honor vertical_scroll_margin (same as vim::LineUp/LineDown) --------- Co-authored-by: Kirill Bulatov --- crates/editor/src/editor_tests.rs | 84 +++++++++++++++++++++++++++++++ crates/editor/src/element.rs | 12 ++--- crates/editor/src/scroll.rs | 59 +++++++++++++++++++++- 3 files changed, 148 insertions(+), 7 deletions(-) diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 2fa319300e5..ca5cc9a6a2f 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -2958,6 +2958,90 @@ async fn test_scroll_page_up_page_down(cx: &mut TestAppContext) { }); } +#[gpui::test] +async fn test_scroll_line_up_down_cursor_margin(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + let mut cx = EditorTestContext::new(cx).await; + let line_height = cx.update_editor(|editor, window, cx| { + editor.set_vertical_scroll_margin(2, cx); + editor + .style(cx) + .text + .line_height_in_pixels(window.rem_size()) + }); + let window = cx.window; + // 5 visible lines with margin=2: valid cursor rows are [top+2 .. top+2] (only the middle row) + cx.simulate_window_resize(window, size(px(1000.), 5. * line_height)); + + // Cursor at row 0 — autoscroll leaves viewport at y=0. + cx.set_state(indoc! {" + ˇone + two + three + four + five + six + seven + eight + nine + ten + "}); + + cx.update_editor(|editor, window, cx| { + assert_eq!(editor.snapshot(window, cx).scroll_position().y, 0.); + + // Scroll down 1: top=1, visible rows 1-5, margin=2 → min_row=3, max_row=3. + // Cursor at row 0 < min_row=3 → clamped to row 3. + editor.scroll_screen_with_cursor_margin(&ScrollAmount::Line(1.), window, cx); + assert_eq!(editor.snapshot(window, cx).scroll_position().y, 1.); + let snapshot = editor.display_snapshot(cx); + assert_eq!( + editor.selections.newest_display(&snapshot).head().row().0, + 3, + "cursor clamped to min_row after scrolling down" + ); + }); + + cx.update_editor(|editor, window, cx| { + // Scroll up 1: top=0, visible rows 0-4, margin=2 (top==0 special case) → min_row=0, max_row=2. + // Cursor at row 3 > max_row=2 → clamped to row 2. + editor.scroll_screen_with_cursor_margin(&ScrollAmount::Line(-1.), window, cx); + assert_eq!(editor.snapshot(window, cx).scroll_position().y, 0.); + let snapshot = editor.display_snapshot(cx); + assert_eq!( + editor.selections.newest_display(&snapshot).head().row().0, + 2, + "cursor clamped to max_row after scrolling up" + ); + }); + + cx.update_editor(|editor, window, cx| { + // Half page down (2 lines): top=2, visible rows 2-6, margin=2 → min_row=4, max_row=4. + // Cursor at row 2 < min_row=4 → clamped to row 4. + editor.scroll_screen_with_cursor_margin(&ScrollAmount::Page(0.5), window, cx); + assert_eq!(editor.snapshot(window, cx).scroll_position().y, 2.); + let snapshot = editor.display_snapshot(cx); + assert_eq!( + editor.selections.newest_display(&snapshot).head().row().0, + 4, + "cursor clamped to min_row after scrolling half page down" + ); + }); + + cx.update_editor(|editor, window, cx| { + // Half page up (2 lines): top=0, visible rows 0-4, margin=2 (top==0 special case) → min_row=0, max_row=2. + // Cursor at row 4 > max_row=2 → clamped to row 2. + editor.scroll_screen_with_cursor_margin(&ScrollAmount::Page(-0.5), window, cx); + assert_eq!(editor.snapshot(window, cx).scroll_position().y, 0.); + let snapshot = editor.display_snapshot(cx); + assert_eq!( + editor.selections.newest_display(&snapshot).head().row().0, + 2, + "cursor clamped to max_row after scrolling half page up" + ); + }); +} + #[gpui::test] async fn test_autoscroll(cx: &mut TestAppContext) { init_test(cx, |_| {}); diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index a82b3ea4488..4ce36632f25 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -287,22 +287,22 @@ impl EditorElement { register_action(editor, window, Editor::scroll_cursor_bottom); register_action(editor, window, Editor::scroll_cursor_center_top_bottom); register_action(editor, window, |editor, _: &LineDown, window, cx| { - editor.scroll_screen(&ScrollAmount::Line(1.), window, cx) + editor.scroll_screen_with_cursor_margin(&ScrollAmount::Line(1.), window, cx) }); register_action(editor, window, |editor, _: &LineUp, window, cx| { - editor.scroll_screen(&ScrollAmount::Line(-1.), window, cx) + editor.scroll_screen_with_cursor_margin(&ScrollAmount::Line(-1.), window, cx) }); register_action(editor, window, |editor, _: &HalfPageDown, window, cx| { - editor.scroll_screen(&ScrollAmount::Page(0.5), window, cx) + editor.scroll_screen_with_cursor_margin(&ScrollAmount::Page(0.5), window, cx) }); register_action(editor, window, |editor, _: &HalfPageUp, window, cx| { - editor.scroll_screen(&ScrollAmount::Page(-0.5), window, cx) + editor.scroll_screen_with_cursor_margin(&ScrollAmount::Page(-0.5), window, cx) }); register_action(editor, window, |editor, _: &PageDown, window, cx| { - editor.scroll_screen(&ScrollAmount::Page(1.), window, cx) + editor.scroll_screen_with_cursor_margin(&ScrollAmount::Page(1.), window, cx) }); register_action(editor, window, |editor, _: &PageUp, window, cx| { - editor.scroll_screen(&ScrollAmount::Page(-1.), window, cx) + editor.scroll_screen_with_cursor_margin(&ScrollAmount::Page(-1.), window, cx) }); register_action(editor, window, Editor::move_to_previous_word_start); register_action(editor, window, Editor::move_to_previous_subword_start); diff --git a/crates/editor/src/scroll.rs b/crates/editor/src/scroll.rs index ec7f9036c4a..dcd96c675c5 100644 --- a/crates/editor/src/scroll.rs +++ b/crates/editor/src/scroll.rs @@ -5,7 +5,7 @@ pub(crate) mod scroll_amount; use crate::editor_settings::ScrollBeyondLastLine; use crate::{ Anchor, DisplayPoint, DisplayRow, Editor, EditorEvent, EditorMode, EditorSettings, - MultiBufferSnapshot, RowExt, SizingBehavior, ToPoint, + MultiBufferSnapshot, RowExt, SelectionEffects, SizingBehavior, ToPoint, display_map::{DisplaySnapshot, ToDisplayPoint}, hover_popover::hide_hover, persistence::EditorDb, @@ -945,6 +945,63 @@ impl Editor { self.set_scroll_position(new_position, window, cx); } + pub fn scroll_screen_with_cursor_margin( + &mut self, + amount: &ScrollAmount, + window: &mut Window, + cx: &mut Context, + ) { + self.scroll_screen(amount, window, cx); + + let Some(visible_line_count) = self.visible_line_count() else { + return; + }; + let display_snapshot = self.display_map.update(cx, |map, cx| map.snapshot(cx)); + let top = self + .scroll_manager + .scroll_top_display_point(&display_snapshot, cx); + let vertical_scroll_margin = + (self.vertical_scroll_margin() as u32).min(visible_line_count as u32 / 2); + + let max_point = display_snapshot.max_point(); + let min_row = if top.row().0 == 0 { + DisplayRow(0) + } else { + DisplayRow(top.row().0 + vertical_scroll_margin) + }; + let max_row = if top.row().0 + visible_line_count as u32 >= max_point.row().0 { + max_point.row() + } else { + DisplayRow( + (top.row().0 + visible_line_count as u32) + .saturating_sub(1 + vertical_scroll_margin), + ) + }; + + self.change_selections( + SelectionEffects::no_scroll().nav_history(false), + window, + cx, + |s| { + s.move_with(&mut |map, selection| { + let head = selection.head(); + let new_row = if head.row() < min_row { + min_row + } else if head.row() > max_row { + max_row + } else { + head.row() + }; + if new_row != head.row() { + let new_head = + map.clip_point(DisplayPoint::new(new_row, head.column()), Bias::Left); + selection.collapse_to(new_head, selection.goal); + } + }) + }, + ); + } + /// Returns an ordering. The newest selection is: /// Ordering::Equal => on screen /// Ordering::Less => above or to the left of the screen From 7dc634124c249c5625da643d94f9fccefad80c56 Mon Sep 17 00:00:00 2001 From: Lena <241371603+zelenenka@users.noreply.github.com> Date: Wed, 8 Jul 2026 18:52:33 +0200 Subject: [PATCH 08/61] Switch Guild board automation to role checks (#60606) Outside collaborators can't be added to GitHub teams, team membership is restricted to org members. https://docs.github.com/en/organizations/managing-user-access-to-your-organizations-repositories/managing-outside-collaborators/adding-outside-collaborators-to-repositories-in-your-organization#:~:text=Outside%20collaborators%20cannot%20be%20added%20to%20a%20team%2C%20team%20membership%20is%20restricted%20to%20members%20of%20the%20organization Release Notes: - N/A --- .github/workflows/guild_new_pr_notify.yml | 15 +++++++++----- .github/workflows/pr_issue_labeler.yml | 24 +++++++++++++++++++++-- script/github-guild-board.py | 15 ++++++++++---- 3 files changed, 43 insertions(+), 11 deletions(-) diff --git a/.github/workflows/guild_new_pr_notify.yml b/.github/workflows/guild_new_pr_notify.yml index babab1c8b9c..8aad061e5f8 100644 --- a/.github/workflows/guild_new_pr_notify.yml +++ b/.github/workflows/guild_new_pr_notify.yml @@ -40,18 +40,23 @@ jobs: with: github-token: ${{ steps.app-token.outputs.token }} script: | - const GUILD_TEAM_SLUG = 'guild-cohort-2'; + // Guild cohort members are outside collaborators holding this custom + // repository role, not members of an org team. + const GUILD_ROLE_NAME = 'Guild Assign issues/PRs'; const pr = context.payload.pull_request; const author = pr.user.login; const isGuildMember = async (username) => { try { - const response = await github.rest.teams.getMembershipForUserInOrg({ - org: 'zed-industries', - team_slug: GUILD_TEAM_SLUG, + const response = await github.rest.repos.getCollaboratorPermissionLevel({ + owner: 'zed-industries', + repo: 'zed', username }); - return response.data.state === 'active'; + // role_name is the effective (highest) role; for cohort outside + // collaborators that is the custom role. Built-in roles come back + // lowercased and won't match. + return (response.data.role_name || '').toLowerCase() === GUILD_ROLE_NAME.toLowerCase(); } catch (error) { if (error.status === 404) { return false; diff --git a/.github/workflows/pr_issue_labeler.yml b/.github/workflows/pr_issue_labeler.yml index f295dcb4f9a..524d301aecc 100644 --- a/.github/workflows/pr_issue_labeler.yml +++ b/.github/workflows/pr_issue_labeler.yml @@ -41,7 +41,9 @@ jobs: const STAFF_TEAM_SLUG = 'staff'; const FIRST_CONTRIBUTION_LABEL = 'first contribution'; const GUILD_LABEL = 'guild'; - const GUILD_TEAM_SLUG = 'guild-cohort-2'; + // Guild cohort members are outside collaborators holding this custom + // repository role, not members of an org team. + const GUILD_ROLE_NAME = 'Guild Assign issues/PRs'; const COMMUNITY_CHAMPION_LABEL = 'community champion'; const COMMUNITY_CHAMPIONS = [ '0x2CA', @@ -138,7 +140,25 @@ jobs: }; const isStaffMember = (author) => isTeamMember(STAFF_TEAM_SLUG, author); - const isGuildMember = (author) => isTeamMember(GUILD_TEAM_SLUG, author); + + const isGuildMember = async (author) => { + try { + const response = await github.rest.repos.getCollaboratorPermissionLevel({ + owner: 'zed-industries', + repo: 'zed', + username: author + }); + // role_name is the effective (highest) role; for cohort outside + // collaborators that is the custom role. Built-in roles come back + // lowercased and won't match. + return (response.data.role_name || '').toLowerCase() === GUILD_ROLE_NAME.toLowerCase(); + } catch (error) { + if (error.status !== 404) { + throw error; + } + return false; + } + }; const getIssueLabels = () => { if (listIncludesAuthor(COMMUNITY_CHAMPIONS, author)) { diff --git a/script/github-guild-board.py b/script/github-guild-board.py index 1fe4c34baaf..7bdd8ab3a2a 100644 --- a/script/github-guild-board.py +++ b/script/github-guild-board.py @@ -38,7 +38,10 @@ RETRY_DELAY_SECONDS = 5 GITHUB_API_URL = "https://api.github.com" REPO_OWNER = "zed-industries" REPO_NAME = "zed" -GUILD_TEAM_SLUG = "guild-cohort-2" +# Cohort members are outside collaborators on the repo holding this custom +# repository role, rather than members of an org team. Rotating the cohort is +# then just adding/removing collaborators, with no org seats involved. +GUILD_ROLE_NAME = "Guild Assign issues/PRs" STATUS_FIELD = "Status" STATUS_IN_PROGRESS = "In Progress" @@ -137,15 +140,19 @@ def github_rest_get_paginated(path): @lru_cache(maxsize=None) def is_guild_member(username): response = requests.get( - f"{GITHUB_API_URL}/orgs/{REPO_OWNER}/teams/{GUILD_TEAM_SLUG}/memberships/{username}", + f"{GITHUB_API_URL}/repos/{REPO_OWNER}/{REPO_NAME}/collaborators/{username}/permission", headers=GITHUB_HEADERS, timeout=30, ) + # 404 means the user isn't a collaborator on the repo at all. if response.status_code == 404: return False response.raise_for_status() - # A pending invitation reports state "pending"; only active members count. - return response.json().get("state") == "active" + # role_name is the effective (highest) role for the user. For a cohort of + # outside collaborators whose only grant is this custom role, that is the + # custom role's name; built-in roles come back lowercased and won't match. + role_name = response.json().get("role_name") or "" + return role_name.lower() == GUILD_ROLE_NAME.lower() def issue_comments(issue_number): From b9f3396b68c61e27f2aab38fbe4bcd274968befa Mon Sep 17 00:00:00 2001 From: G36maid <53391375+G36maid@users.noreply.github.com> Date: Thu, 9 Jul 2026 00:53:11 +0800 Subject: [PATCH 09/61] Add support for `format_on_save` only changed lines (#49964) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Introduces `format_on_save` variants that format only modified lines, limiting formatting to lines changed since the last commit. This prevents massive diffs when editing legacy codebases. Aligns with VS Code's `editor.formatOnSaveMode` naming: - `"modifications"` — formats only git-diffed lines. Requires source control; skips formatting if no diff is available. - `"modifications_if_available"` — formats only git-diffed lines, falling back to full-file formatting for untracked files, when source control is unavailable, or when the LSP does not support range formatting. Also supports importing equivalent VS Code settings (`formatOnSaveMode`). This PR uses the range-based whitespace/newline infrastructure from the dependency PR above. ## Changes
New settings, modified ranges computation, VS Code import ### New settings (`language.rs`, `default.json`, `all-settings.md`) Two new `FormatOnSave` variants: `Modifications` and `ModificationsIfAvailable`. ### Modified ranges computation (`items.rs`) - `compute_format_decision()` — reads `format_on_save` setting across all buffers, determines whether to use ranged formatting (`Ranges`), full formatting (`Full`), or skip (`Skip`). - `compute_modified_ranges()` — extracts modified line ranges from git diff hunks via `BufferDiffSnapshot`. - `is_empty_range()` — helper to detect deletion-only hunks that produce no formatable content. The `save()` method calls `compute_format_decision()` and dispatches accordingly. ### Modifications mode in `lsp_store.rs` - Adds `FormatOnSave::Modifications | FormatOnSave::ModificationsIfAvailable` to the formatter selection match arm. - `ModificationsIfAvailable` falls back to full-file formatting when ranged formatting produces no results. ### VS Code settings import (`vscode_import.rs`, `settings_store.rs`) Imports `editor.formatOnSaveMode` mapping: - `"modifications"` → `FormatOnSave::Modifications` - `"modificationsIfAvailable"` → `FormatOnSave::ModificationsIfAvailable` - `"file"` → `FormatOnSave::On`
## Tests - `test_modifications_format_on_save` — basic modifications mode formatting with dirty buffer - `test_modifications_format_no_changes` — clean buffer triggers no formatting - `test_modifications_format_lsp_no_range_support` — LSP without range formatting skips entirely for `Modifications` - `test_modifications_format_lsp_returns_empty_edits` — empty edits handled gracefully - `test_modifications_format_multiple_hunks` — two non-adjacent edits produce two separate range formatting requests --- 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 #16509 Depends on #53942 Release Notes: - Added `modifications` and `modifications_if_available` options to `format_on_save`, which format only git-changed lines instead of the entire file - When using modifications mode, `remove_trailing_whitespace_on_save` and `ensure_final_newline_on_save` are also scoped to changed lines, preventing unwanted diff jitter in legacy codebases - Added support for importing VS Code's `editor.formatOnSaveMode` setting --------- Co-authored-by: Kirill Bulatov --- assets/settings/default.json | 8 +- crates/editor/src/editor_tests.rs | 1020 ++++++++++++++++- crates/editor/src/items.rs | 258 ++++- .../src/migrations/m_2025_10_02/settings.rs | 6 +- crates/project/src/lsp_store.rs | 57 +- crates/settings/src/settings_store.rs | 142 +++ crates/settings/src/vscode_import.rs | 12 +- crates/settings_content/src/language.rs | 12 +- crates/settings_ui/src/page_data.rs | 2 +- docs/src/reference/all-settings.md | 24 +- 10 files changed, 1507 insertions(+), 34 deletions(-) diff --git a/assets/settings/default.json b/assets/settings/default.json index 983784e8951..0e42d27e980 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -1458,7 +1458,13 @@ // The EditorConfig `end_of_line` property overrides this setting and behaves // like `enforce_lf` or `enforce_crlf`. "line_ending": "detect", - // Whether or not to perform a buffer format before saving: [on, off] + // Whether or not to perform a buffer format before saving: + // "on" — format the whole buffer + // "off" — do not format + // "modifications" — format only lines with unstaged changes; skips formatting + // when no git diff is available or the language server lacks range formatting + // "modifications_if_available" — same, but falls back to formatting the whole + // buffer when range formatting cannot be used // Keep in mind, if the autosave with delay is enabled, format_on_save will be ignored "format_on_save": "off", // How to perform a buffer format. This setting can take multiple values: diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index ca5cc9a6a2f..9b78a9a5c0c 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -29,7 +29,8 @@ use language::{ LanguageConfig, LanguageConfigOverride, LanguageMatcher, LanguageName, LanguageQueries, LanguageToolchainStore, Override, Point, language_settings::{ - CompletionSettingsContent, FormatterList, LanguageSettingsContent, LspInsertMode, + CompletionSettingsContent, FormatOnSave, FormatterList, LanguageSettingsContent, + LspInsertMode, }, tree_sitter_python, }; @@ -15171,6 +15172,89 @@ async fn setup_range_format_test( .await } +/// Like `setup_range_format_test`, but backs the buffer with a FakeFs git +/// repository so that `GitStore::get_unstaged_diff` returns a real diff. +/// `head_content` sets the HEAD base, `index_content` sets the staged base. +/// The buffer starts empty; the caller must `editor.set_text(...)` to set the +/// working-tree content (the diff recomputes from buffer changes). +async fn setup_range_format_test_with_git<'a>( + cx: &'a mut TestAppContext, + head_content: &str, + index_content: &str, +) -> ( + Entity, + Entity, + &'a mut gpui::VisualTestContext, + lsp::FakeLanguageServer, +) { + init_test(cx, |_| {}); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + path!("/project"), + json!({ + ".git": {}, + "file.rs": "", + }), + ) + .await; + + fs.set_head_for_repo( + std::path::Path::new(path!("/project/.git")), + &[("file.rs", head_content.to_string())], + "deadbeef", + ); + fs.set_index_for_repo( + std::path::Path::new(path!("/project/.git")), + &[("file.rs", index_content.to_string())], + ); + + let project = Project::test(fs, [path!("/project").as_ref()], cx).await; + + let language_registry = project.read_with(cx, |project, _| project.languages().clone()); + language_registry.add(rust_lang()); + let mut fake_servers = language_registry.register_fake_lsp( + "Rust", + FakeLspAdapter { + capabilities: lsp::ServerCapabilities { + document_range_formatting_provider: Some(lsp::OneOf::Left(true)), + document_formatting_provider: Some(lsp::OneOf::Left(true)), + ..lsp::ServerCapabilities::default() + }, + ..FakeLspAdapter::default() + }, + ); + + let buffer = project + .update(cx, |project, cx| { + project.open_local_buffer(path!("/project/file.rs"), cx) + }) + .await + .unwrap(); + + // Open the unstaged diff so GitStore tracks this buffer. Without this, + // `get_unstaged_diff` returns None and compute_format_target cannot + // produce range-based FormatTarget. + project + .update(cx, |project, cx| { + project.open_unstaged_diff(buffer.clone(), cx) + }) + .await + .unwrap(); + + let buffer = cx.new(|cx| MultiBuffer::singleton(buffer, cx)); + let (editor, cx) = cx.add_window_view(|window, cx| { + build_editor_with_project(project.clone(), buffer, window, cx) + }); + editor.update_in(cx, |editor, window, cx| { + window.focus(&editor.focus_handle(cx), cx); + }); + + let fake_server = fake_servers.next().await.unwrap(); + + (project, editor, cx, fake_server) +} + fn refresh_editor_actions(cx: &mut VisualTestContext) { cx.executor().run_until_parked(); cx.update(|window, cx| { @@ -15383,6 +15467,940 @@ async fn test_range_format_on_save_timeout(cx: &mut TestAppContext) { assert!(!cx.read(|cx| editor.is_dirty(cx))); } +#[gpui::test] +async fn test_modifications_format_on_save(cx: &mut TestAppContext) { + let (project, editor, cx, fake_server) = setup_range_format_test_with_git(cx, "", "").await; + + update_test_language_settings(cx, &|settings| { + settings.defaults.format_on_save = Some(FormatOnSave::Modifications); + }); + + editor.update_in(cx, |editor, window, cx| { + editor.set_text("one\ntwo\nthree\n", window, cx) + }); + cx.run_until_parked(); + assert!(cx.read(|cx| editor.is_dirty(cx))); + + let save = editor + .update_in(cx, |editor, window, cx| { + editor.save( + SaveOptions { + format: true, + autosave: false, + force_format: false, + }, + project.clone(), + window, + cx, + ) + }) + .unwrap(); + fake_server + .set_request_handler::(move |params, _| async move { + assert_eq!( + params.text_document.uri, + lsp::Uri::from_file_path(path!("/project/file.rs")).unwrap() + ); + Ok(Some(vec![lsp::TextEdit::new( + lsp::Range::new(lsp::Position::new(0, 3), lsp::Position::new(1, 0)), + ", ".to_string(), + )])) + }) + .next() + .await; + save.await; + assert_eq!( + editor.update(cx, |editor, cx| editor.text(cx)), + "one, two\nthree\n" + ); + assert!(!cx.read(|cx| editor.is_dirty(cx))); +} + +#[gpui::test] +async fn test_modifications_format_skips_without_git_diff(cx: &mut TestAppContext) { + let (project, editor, cx, fake_server) = setup_range_format_test(cx).await; + + update_test_language_settings(cx, &|settings| { + settings.defaults.format_on_save = Some(FormatOnSave::Modifications); + }); + + editor.update_in(cx, |editor, window, cx| { + editor.set_text("one\ntwo\nthree\n", window, cx) + }); + assert!( + cx.read(|cx| editor.is_dirty(cx)), + "editor should be dirty before save" + ); + + let _no_range_format_handler = fake_server + .set_request_handler::(move |_, _| async move { + panic!("rangeFormatting must not be called when no git diff is available"); + }) + .next(); + let _no_format_handler = fake_server + .set_request_handler::(move |_, _| async move { + panic!("full formatting must not be called for Modifications without a git diff"); + }) + .next(); + + let save = editor + .update_in(cx, |editor, window, cx| { + editor.save( + SaveOptions { + format: true, + autosave: false, + force_format: false, + }, + project.clone(), + window, + cx, + ) + }) + .unwrap(); + save.await; + cx.run_until_parked(); + assert!( + !cx.read(|cx| editor.is_dirty(cx)), + "the buffer should be saved despite the skipped formatting" + ); + assert_eq!( + editor.update(cx, |editor, cx| editor.text(cx)), + "one\ntwo\nthree\n" + ); +} + +#[gpui::test] +async fn test_modifications_format_lsp_no_range_support(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + path!("/project"), + json!({ + ".git": {}, + "file.rs": "", + }), + ) + .await; + + let head_content = "one\ntwo\nthree\n"; + fs.set_head_and_index_for_repo( + std::path::Path::new(path!("/project/.git")), + &[("file.rs", head_content.to_string())], + ); + + let project = Project::test(fs, [path!("/project").as_ref()], cx).await; + + let language_registry = project.read_with(cx, |project, _| project.languages().clone()); + language_registry.add(rust_lang()); + let mut fake_servers = language_registry.register_fake_lsp( + "Rust", + FakeLspAdapter { + capabilities: lsp::ServerCapabilities { + document_range_formatting_provider: Some(lsp::OneOf::Left(false)), + document_formatting_provider: Some(lsp::OneOf::Left(true)), + ..lsp::ServerCapabilities::default() + }, + ..FakeLspAdapter::default() + }, + ); + + let buffer = project + .update(cx, |project, cx| { + project.open_local_buffer(path!("/project/file.rs"), cx) + }) + .await + .unwrap(); + project + .update(cx, |project, cx| { + project.open_unstaged_diff(buffer.clone(), cx) + }) + .await + .unwrap(); + + let buffer = cx.new(|cx| MultiBuffer::singleton(buffer, cx)); + let (editor, cx) = cx.add_window_view(|window, cx| { + build_editor_with_project(project.clone(), buffer, window, cx) + }); + + let fake_server = fake_servers.next().await.unwrap(); + + update_test_language_settings(cx, &|settings| { + settings.defaults.format_on_save = Some(FormatOnSave::Modifications); + }); + + editor.update_in(cx, |editor, window, cx| { + editor.set_text("one\nTWO\nthree\n", window, cx) + }); + cx.run_until_parked(); + assert!(cx.read(|cx| editor.is_dirty(cx))); + + let _no_range_format_handler = fake_server + .set_request_handler::(move |_, _| async move { + panic!("rangeFormatting must not be called when range formatting is unsupported"); + }) + .next(); + let _no_format_handler = fake_server + .set_request_handler::(move |_, _| async move { + panic!("full formatting must not be called for Modifications when range formatting is unsupported"); + }) + .next(); + + let save = editor + .update_in(cx, |editor, window, cx| { + editor.save( + SaveOptions { + format: true, + autosave: false, + force_format: false, + }, + project.clone(), + window, + cx, + ) + }) + .unwrap(); + save.await; + cx.run_until_parked(); + assert!(!cx.read(|cx| editor.is_dirty(cx))); + assert_eq!( + editor.update(cx, |editor, cx| editor.text(cx)), + "one\nTWO\nthree\n" + ); +} + +#[gpui::test] +async fn test_modifications_format_lsp_returns_empty_edits(cx: &mut TestAppContext) { + let (project, editor, cx, fake_server) = setup_range_format_test_with_git(cx, "", "").await; + + update_test_language_settings(cx, &|settings| { + settings.defaults.format_on_save = Some(FormatOnSave::Modifications); + }); + + editor.update_in(cx, |editor, window, cx| { + editor.set_text("aaa\nbbb\nccc\nddd\neee\n", window, cx) + }); + cx.run_until_parked(); + assert!(cx.read(|cx| editor.is_dirty(cx))); + + let save = editor + .update_in(cx, |editor, window, cx| { + editor.save( + SaveOptions { + format: true, + autosave: false, + force_format: false, + }, + project.clone(), + window, + cx, + ) + }) + .unwrap(); + fake_server + .set_request_handler::(move |params, _| async move { + assert_eq!( + params.text_document.uri, + lsp::Uri::from_file_path(path!("/project/file.rs")).unwrap() + ); + Ok(Some(Vec::new())) + }) + .next() + .await; + save.await; + assert!(!cx.read(|cx| editor.is_dirty(cx))); + assert_eq!( + editor.update(cx, |editor, cx| editor.text(cx)), + "aaa\nbbb\nccc\nddd\neee\n" + ); +} + +#[gpui::test] +async fn test_modifications_format_multiple_hunks(cx: &mut TestAppContext) { + let head_content = "line0\nline1\nline2\nline3\nline4\nline5\n"; + let (project, editor, cx, fake_server) = + setup_range_format_test_with_git(cx, head_content, head_content).await; + + update_test_language_settings(cx, &|settings| { + settings.defaults.format_on_save = Some(FormatOnSave::Modifications); + }); + + editor.update_in(cx, |editor, window, cx| { + editor.set_text("line0\nLINE1\nline2\nline3\nLINE4\nline5\n", window, cx); + }); + cx.run_until_parked(); + assert!(cx.read(|cx| editor.is_dirty(cx))); + + let request_count = Arc::new(AtomicUsize::new(0)); + let request_count_clone = request_count.clone(); + let mut responded_rx = + fake_server.set_request_handler::(move |params, _| { + let count = request_count_clone.fetch_add(1, atomic::Ordering::SeqCst); + async move { + assert_eq!( + params.text_document.uri, + lsp::Uri::from_file_path(path!("/project/file.rs")).unwrap() + ); + match count { + 0 => Ok(Some(vec![lsp::TextEdit::new( + lsp::Range::new(lsp::Position::new(1, 5), lsp::Position::new(1, 5)), + "!".to_string(), + )])), + 1 => Ok(Some(vec![lsp::TextEdit::new( + lsp::Range::new(lsp::Position::new(4, 5), lsp::Position::new(4, 5)), + "!".to_string(), + )])), + _ => panic!("unexpected third range formatting request"), + } + } + }); + + let save = editor + .update_in(cx, |editor, window, cx| { + editor.save( + SaveOptions { + format: true, + autosave: false, + force_format: false, + }, + project.clone(), + window, + cx, + ) + }) + .unwrap(); + responded_rx.next().await; + responded_rx.next().await; + save.await; + + assert_eq!(request_count.load(atomic::Ordering::SeqCst), 2); + assert_eq!( + editor.update(cx, |editor, cx| editor.text(cx)), + "line0\nLINE1!\nline2\nline3\nLINE4!\nline5\n" + ); + assert!(!cx.read(|cx| editor.is_dirty(cx))); +} + +#[gpui::test] +async fn test_modifications_format_excludes_staged_changes(cx: &mut TestAppContext) { + let head_content = "line0\nline1\nline2\nline3\nline4\n"; + let staged_content = "line0\nLINE1\nline2\nline3\nline4\n"; + let (project, editor, cx, fake_server) = + setup_range_format_test_with_git(cx, head_content, staged_content).await; + + update_test_language_settings(cx, &|settings| { + settings.defaults.format_on_save = Some(FormatOnSave::Modifications); + }); + + let working_content = "line0\nLINE1\nline2\nline3\nLINE4\n"; + editor.update_in(cx, |editor, window, cx| { + editor.set_text(working_content, window, cx) + }); + cx.run_until_parked(); + + let request_count = Arc::new(AtomicUsize::new(0)); + let request_count_clone = request_count.clone(); + let mut responded_rx = + fake_server.set_request_handler::(move |params, _| { + request_count_clone.fetch_add(1, atomic::Ordering::SeqCst); + assert_eq!( + params.range.start.line, 4, + "only the unstaged hunk (LINE4) should be formatted" + ); + async move { Ok(Some(Vec::new())) } + }); + + let save = editor + .update_in(cx, |editor, window, cx| { + editor.save( + SaveOptions { + format: true, + autosave: false, + force_format: false, + }, + project.clone(), + window, + cx, + ) + }) + .unwrap(); + responded_rx.next().await; + save.await; + + assert_eq!( + request_count.load(atomic::Ordering::SeqCst), + 1, + "staged hunk (LINE1) must not be formatted, only the unstaged hunk (LINE4)" + ); +} + +#[gpui::test] +async fn test_modifications_format_range_excludes_staged_hunk(cx: &mut TestAppContext) { + let head_content = "a\nb\nc\n"; + let staged_content = "A\nb\nc\n"; + let (project, editor, cx, fake_server) = + setup_range_format_test_with_git(cx, head_content, staged_content).await; + + update_test_language_settings(cx, &|settings| { + settings.defaults.format_on_save = Some(FormatOnSave::Modifications); + }); + + editor.update_in(cx, |editor, window, cx| { + editor.set_text("A\nB\nc\n", window, cx) + }); + cx.run_until_parked(); + + let captured_start_line = Arc::new(AtomicUsize::new(usize::MAX)); + let captured_start_line_clone = captured_start_line.clone(); + let mut responded_rx = + fake_server.set_request_handler::(move |params, _| { + captured_start_line_clone + .store(params.range.start.line as usize, atomic::Ordering::SeqCst); + async move { Ok(Some(Vec::new())) } + }); + + let save = editor + .update_in(cx, |editor, window, cx| { + editor.save( + SaveOptions { + format: true, + autosave: false, + force_format: false, + }, + project.clone(), + window, + cx, + ) + }) + .unwrap(); + responded_rx.next().await; + save.await; + + let start_line = captured_start_line.load(atomic::Ordering::SeqCst); + assert_ne!( + start_line, + usize::MAX, + "expected at least one range formatting request" + ); + assert_eq!( + start_line, 1, + "format range must exclude the staged row and start at the unstaged row" + ); +} + +#[gpui::test] +async fn test_modifications_format_pure_unstaged_with_git(cx: &mut TestAppContext) { + let head_content = "line0\nline1\nline2\nline3\nline4\n"; + let (project, editor, cx, fake_server) = + setup_range_format_test_with_git(cx, head_content, head_content).await; + + update_test_language_settings(cx, &|settings| { + settings.defaults.format_on_save = Some(FormatOnSave::Modifications); + }); + + let working_content = "line0\nLINE1\nline2\nline3\nLINE4\n"; + editor.update_in(cx, |editor, window, cx| { + editor.set_text(working_content, window, cx) + }); + cx.run_until_parked(); + + let request_count = Arc::new(AtomicUsize::new(0)); + let request_count_clone = request_count.clone(); + let mut responded_rx = fake_server.set_request_handler::( + move |_params, _| { + request_count_clone.fetch_add(1, atomic::Ordering::SeqCst); + async move { Ok(Some(Vec::new())) } + }, + ); + + let save = editor + .update_in(cx, |editor, window, cx| { + editor.save( + SaveOptions { + format: true, + autosave: false, + force_format: false, + }, + project.clone(), + window, + cx, + ) + }) + .unwrap(); + responded_rx.next().await; + responded_rx.next().await; + save.await; + + assert_eq!( + request_count.load(atomic::Ordering::SeqCst), + 2, + "unstaged hunks (LINE1, LINE4) must be formatted via the git diff path" + ); +} + +#[gpui::test] +async fn test_modifications_format_no_unstaged_changes_with_git(cx: &mut TestAppContext) { + let head_content = "line0\nline1\nline2\nline3\nline4\n"; + let staged_content = "line0\nLINE1\nline2\nline3\nLINE4\n"; + let (project, editor, cx, fake_server) = + setup_range_format_test_with_git(cx, head_content, staged_content).await; + + update_test_language_settings(cx, &|settings| { + settings.defaults.format_on_save = Some(FormatOnSave::Modifications); + }); + + editor.update_in(cx, |editor, window, cx| { + editor.set_text(staged_content, window, cx) + }); + cx.run_until_parked(); + + let _no_format_handler = fake_server + .set_request_handler::(move |_, _| async move { + panic!("range formatting must not be called when all changes are staged"); + }) + .next(); + + let save = editor + .update_in(cx, |editor, window, cx| { + editor.save( + SaveOptions { + format: true, + autosave: false, + force_format: false, + }, + project.clone(), + window, + cx, + ) + }) + .unwrap(); + save.await; + cx.run_until_parked(); +} + +#[gpui::test] +async fn test_modifications_format_no_changes_with_git(cx: &mut TestAppContext) { + let head_content = "line0\nline1\nline2\n"; + let (project, editor, cx, fake_server) = + setup_range_format_test_with_git(cx, head_content, head_content).await; + + update_test_language_settings(cx, &|settings| { + settings.defaults.format_on_save = Some(FormatOnSave::Modifications); + }); + + editor.update_in(cx, |editor, window, cx| { + editor.set_text(head_content, window, cx) + }); + cx.run_until_parked(); + + let _no_format_handler = fake_server + .set_request_handler::(move |_, _| async move { + panic!("range formatting must not be called when buffer matches HEAD"); + }) + .next(); + + let save = editor + .update_in(cx, |editor, window, cx| { + editor.save( + SaveOptions { + format: true, + autosave: false, + force_format: false, + }, + project.clone(), + window, + cx, + ) + }) + .unwrap(); + save.await; + cx.run_until_parked(); +} + +#[gpui::test] +async fn test_modifications_format_crlf_line_endings(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + path!("/project"), + json!({ + ".git": {}, + "file.rs": "line0\r\nline1\r\nline2\r\nline3\r\nline4\r\n", + }), + ) + .await; + + let head_content = "line0\nline1\nline2\nline3\nline4\n"; + fs.set_head_for_repo( + std::path::Path::new(path!("/project/.git")), + &[("file.rs", head_content.to_string())], + "deadbeef", + ); + fs.set_index_for_repo( + std::path::Path::new(path!("/project/.git")), + &[("file.rs", head_content.to_string())], + ); + + let project = Project::test(fs, [path!("/project").as_ref()], cx).await; + + let language_registry = project.read_with(cx, |project, _| project.languages().clone()); + language_registry.add(rust_lang()); + let mut fake_servers = language_registry.register_fake_lsp( + "Rust", + FakeLspAdapter { + capabilities: lsp::ServerCapabilities { + document_range_formatting_provider: Some(lsp::OneOf::Left(true)), + ..lsp::ServerCapabilities::default() + }, + ..FakeLspAdapter::default() + }, + ); + + let buffer = project + .update(cx, |project, cx| { + project.open_local_buffer(path!("/project/file.rs"), cx) + }) + .await + .unwrap(); + + buffer.read_with(cx, |buffer, _| { + assert_eq!( + buffer.line_ending(), + language::LineEnding::Windows, + "buffer should detect CRLF line endings from the working tree file" + ); + }); + + project + .update(cx, |project, cx| { + project.open_unstaged_diff(buffer.clone(), cx) + }) + .await + .unwrap(); + + let buffer = cx.new(|cx| MultiBuffer::singleton(buffer, cx)); + let (editor, cx) = cx.add_window_view(|window, cx| { + build_editor_with_project(project.clone(), buffer, window, cx) + }); + editor.update_in(cx, |editor, window, cx| { + window.focus(&editor.focus_handle(cx), cx); + }); + + let fake_server = fake_servers.next().await.unwrap(); + + update_test_language_settings(cx, &|settings| { + settings.defaults.format_on_save = Some(FormatOnSave::Modifications); + }); + + editor.update_in(cx, |editor, window, cx| { + editor.set_text("line0\nLINE1\nline2\nline3\nLINE4\n", window, cx) + }); + cx.run_until_parked(); + + let request_count = Arc::new(AtomicUsize::new(0)); + let request_count_clone = request_count.clone(); + let mut responded_rx = fake_server.set_request_handler::( + move |_params, _| { + request_count_clone.fetch_add(1, atomic::Ordering::SeqCst); + async move { Ok(Some(Vec::new())) } + }, + ); + + let save = editor + .update_in(cx, |editor, window, cx| { + editor.save( + SaveOptions { + format: true, + autosave: false, + force_format: false, + }, + project.clone(), + window, + cx, + ) + }) + .unwrap(); + responded_rx.next().await; + responded_rx.next().await; + save.await; + + assert_eq!( + request_count.load(atomic::Ordering::SeqCst), + 2, + "CRLF line endings must not cause spurious diff hunks" + ); +} + +#[gpui::test] +async fn test_modifications_format_merge_boundary_one_row_gap(cx: &mut TestAppContext) { + let head_content = "line0\nline1\nline2\nline3\nline4\n"; + let (project, editor, cx, fake_server) = + setup_range_format_test_with_git(cx, head_content, head_content).await; + + update_test_language_settings(cx, &|settings| { + settings.defaults.format_on_save = Some(FormatOnSave::Modifications); + }); + + let working_content = "line0\nLINE1\nline2\nLINE3\nline4\n"; + editor.update_in(cx, |editor, window, cx| { + editor.set_text(working_content, window, cx) + }); + cx.run_until_parked(); + + let request_count = Arc::new(AtomicUsize::new(0)); + let request_count_clone = request_count.clone(); + let mut responded_rx = fake_server.set_request_handler::( + move |_params, _| { + request_count_clone.fetch_add(1, atomic::Ordering::SeqCst); + async move { Ok(Some(Vec::new())) } + }, + ); + + let save = editor + .update_in(cx, |editor, window, cx| { + editor.save( + SaveOptions { + format: true, + autosave: false, + force_format: false, + }, + project.clone(), + window, + cx, + ) + }) + .unwrap(); + responded_rx.next().await; + responded_rx.next().await; + save.await; + + assert_eq!( + request_count.load(atomic::Ordering::SeqCst), + 2, + "hunks separated by exactly one unchanged row must not merge" + ); +} + +#[gpui::test] +async fn test_modifications_if_available_empty_diff_skips_formatting(cx: &mut TestAppContext) { + let head_content = "line0\nline1\nline2\n"; + let (project, editor, cx, fake_server) = + setup_range_format_test_with_git(cx, head_content, head_content).await; + + update_test_language_settings(cx, &|settings| { + settings.defaults.format_on_save = Some(FormatOnSave::ModificationsIfAvailable); + }); + + editor.update_in(cx, |editor, window, cx| { + editor.set_text(head_content, window, cx) + }); + cx.run_until_parked(); + + let _no_range_format = fake_server + .set_request_handler::(move |_, _| async move { + panic!("range formatting must not be called when diff is empty"); + }) + .next(); + let _no_format = fake_server + .set_request_handler::(move |_, _| async move { + panic!("full formatting must not be called when a diff is available but empty"); + }) + .next(); + + let save = editor + .update_in(cx, |editor, window, cx| { + editor.save( + SaveOptions { + format: true, + autosave: false, + force_format: false, + }, + project.clone(), + window, + cx, + ) + }) + .unwrap(); + save.await; + cx.run_until_parked(); + assert!( + !cx.read(|cx| editor.is_dirty(cx)), + "the buffer should be saved despite the skipped formatting" + ); +} + +#[gpui::test] +async fn test_modifications_if_available_no_git_falls_back_to_full_format(cx: &mut TestAppContext) { + let (project, editor, cx, fake_server) = setup_range_format_test_with_capabilities( + cx, + lsp::ServerCapabilities { + document_range_formatting_provider: Some(lsp::OneOf::Left(true)), + document_formatting_provider: Some(lsp::OneOf::Left(true)), + ..lsp::ServerCapabilities::default() + }, + ) + .await; + + update_test_language_settings(cx, &|settings| { + settings.defaults.format_on_save = Some(FormatOnSave::ModificationsIfAvailable); + }); + + editor.update_in(cx, |editor, window, cx| { + editor.set_text("one\ntwo\nthree\n", window, cx) + }); + assert!(cx.read(|cx| editor.is_dirty(cx))); + + let _no_range_format = fake_server + .set_request_handler::(move |_, _| async move { + panic!("range formatting must not be called when no git diff is available"); + }) + .next(); + + let formatting_called = Arc::new(AtomicBool::new(false)); + let formatting_called_clone = formatting_called.clone(); + let mut formatting_rx = + fake_server.set_request_handler::(move |_, _| { + formatting_called_clone.store(true, atomic::Ordering::SeqCst); + async move { Ok(Some(Vec::new())) } + }); + + let save = editor + .update_in(cx, |editor, window, cx| { + editor.save( + SaveOptions { + format: true, + autosave: false, + force_format: false, + }, + project.clone(), + window, + cx, + ) + }) + .unwrap(); + formatting_rx.next().await; + save.await; + + assert!( + formatting_called.load(atomic::Ordering::SeqCst), + "ModificationsIfAvailable must fall back to full-buffer formatting when no git diff is available" + ); +} + +#[gpui::test] +async fn test_modifications_if_available_lsp_no_range_support_falls_back_to_full_format( + cx: &mut TestAppContext, +) { + init_test(cx, |_| {}); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + path!("/project"), + json!({ + ".git": {}, + "file.rs": "", + }), + ) + .await; + + let head_content = "line0\nline1\nline2\n"; + fs.set_head_for_repo( + std::path::Path::new(path!("/project/.git")), + &[("file.rs", head_content.to_string())], + "deadbeef", + ); + fs.set_index_for_repo( + std::path::Path::new(path!("/project/.git")), + &[("file.rs", head_content.to_string())], + ); + + let project = Project::test(fs, [path!("/project").as_ref()], cx).await; + + let language_registry = project.read_with(cx, |project, _| project.languages().clone()); + language_registry.add(rust_lang()); + let mut fake_servers = language_registry.register_fake_lsp( + "Rust", + FakeLspAdapter { + capabilities: lsp::ServerCapabilities { + document_range_formatting_provider: Some(lsp::OneOf::Left(false)), + document_formatting_provider: Some(lsp::OneOf::Left(true)), + ..lsp::ServerCapabilities::default() + }, + ..FakeLspAdapter::default() + }, + ); + + let buffer = project + .update(cx, |project, cx| { + project.open_local_buffer(path!("/project/file.rs"), cx) + }) + .await + .unwrap(); + project + .update(cx, |project, cx| { + project.open_unstaged_diff(buffer.clone(), cx) + }) + .await + .unwrap(); + + let buffer = cx.new(|cx| MultiBuffer::singleton(buffer, cx)); + let (editor, cx) = cx.add_window_view(|window, cx| { + build_editor_with_project(project.clone(), buffer, window, cx) + }); + editor.update_in(cx, |editor, window, cx| { + window.focus(&editor.focus_handle(cx), cx); + }); + + let fake_server = fake_servers.next().await.unwrap(); + + update_test_language_settings(cx, &|settings| { + settings.defaults.format_on_save = Some(FormatOnSave::ModificationsIfAvailable); + }); + + editor.update_in(cx, |editor, window, cx| { + editor.set_text("line0\nLINE1\nline2\n", window, cx); + }); + cx.run_until_parked(); + assert!(cx.read(|cx| editor.is_dirty(cx))); + + let _no_range_format = fake_server + .set_request_handler::(move |_, _| async move { + panic!("rangeFormatting must not be called when LSP lacks range support"); + }) + .next(); + + let formatting_called = Arc::new(AtomicBool::new(false)); + let formatting_called_clone = formatting_called.clone(); + let mut formatting_rx = + fake_server.set_request_handler::(move |_, _| { + formatting_called_clone.store(true, atomic::Ordering::SeqCst); + async move { Ok(Some(Vec::new())) } + }); + + let save = editor + .update_in(cx, |editor, window, cx| { + editor.save( + SaveOptions { + format: true, + autosave: false, + force_format: false, + }, + project.clone(), + window, + cx, + ) + }) + .unwrap(); + formatting_rx.next().await; + save.await; + + assert!( + formatting_called.load(atomic::Ordering::SeqCst), + "ModificationsIfAvailable must fall back to full-file formatting when the LSP lacks range support" + ); +} + #[gpui::test] async fn test_range_format_not_called_for_clean_buffer(cx: &mut TestAppContext) { let (project, editor, cx, fake_server) = setup_range_format_test(cx).await; diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index 4e97c8c3fe3..4acdb10ff75 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -19,12 +19,14 @@ use gpui::{ }; use language::{ Bias, Buffer, BufferRow, CharKind, CharScopeContext, HighlightedText, LocalFile, PLAIN_TEXT, - Point, SelectionGoal, proto::serialize_anchor as serialize_text_anchor, + Point, SelectionGoal, + language_settings::{FormatOnSave, LanguageSettings}, + proto::serialize_anchor as serialize_text_anchor, }; use lsp::DiagnosticSeverity; use multi_buffer::{BufferOffset, MultiBufferOffset, PathKey}; use project::{ - File, Project, ProjectItem as _, ProjectPath, lsp_store::FormatTrigger, + File, Project, ProjectItem as _, ProjectPath, git_store::GitStore, lsp_store::FormatTrigger, project_settings::ProjectSettings, search::SearchQuery, }; use rope::TextSummary; @@ -39,7 +41,7 @@ use std::{ path::{Path, PathBuf}, sync::Arc, }; -use text::{BufferId, BufferSnapshot, OffsetRangeExt, Selection}; +use text::{BufferId, BufferSnapshot, OffsetRangeExt, Selection, ToPoint as _}; use ui::{IconDecorationKind, prelude::*}; use util::{ResultExt, TryFutureExt, paths::PathExt, rel_path::RelPath}; use workspace::item::{Dedup, ItemSettings, SerializableItem, TabContentParams}; @@ -974,16 +976,21 @@ impl Item for Editor { cx.spawn_in(window, async move |this, cx| { if options.format { - this.update_in(cx, |editor, window, cx| { - editor.perform_format( - project.clone(), + let format_task = this.update_in(cx, |editor, window, cx| { + let format_target = compute_format_target( + &buffers_to_save, format_trigger, - FormatTarget::Buffers(buffers_to_save.clone()), - window, + editor.buffer(), + project.read(cx).git_store(), cx, - ) - })? - .await?; + ); + format_target.map(|target| { + editor.perform_format(project.clone(), format_trigger, target, window, cx) + }) + })?; + if let Some(format_task) = format_task { + format_task.await?; + } } if !buffers_to_save.is_empty() { @@ -2267,6 +2274,115 @@ fn chunk_search_range( })) } +/// Decides what to format based on the `format_on_save` settings of the saved buffers. +/// +/// In the modifications modes, only lines with unstaged changes are formatted. +/// When no git diff is available for a buffer, `modifications` skips formatting while `modifications_if_available` +/// falls back to formatting entire buffers. +/// When a diff is available but empty, nothing is formatted in either mode. +fn compute_format_target( + buffers: &HashSet>, + trigger: FormatTrigger, + multi_buffer: &Entity, + git_store: &Entity, + cx: &App, +) -> Option { + if trigger == FormatTrigger::Manual { + return Some(FormatTarget::Buffers(buffers.clone())); + } + + let multi_buffer_snapshot = multi_buffer.read(cx).snapshot(cx); + let git_store = git_store.read(cx); + + let mut fall_back_to_full_format = false; + let mut modified_ranges: Vec> = Vec::new(); + + for buffer_entity in buffers.iter() { + let buffer = buffer_entity.read(cx); + let settings = LanguageSettings::for_buffer(buffer, cx); + match settings.format_on_save { + FormatOnSave::On | FormatOnSave::Off => { + return Some(FormatTarget::Buffers(buffers.clone())); + } + FormatOnSave::Modifications | FormatOnSave::ModificationsIfAvailable => {} + } + + let Some(diff_snapshot) = git_store + .get_unstaged_diff(buffer.remote_id(), cx) + .map(|diff| diff.read(cx).snapshot(cx)) + else { + if settings.format_on_save == FormatOnSave::ModificationsIfAvailable { + fall_back_to_full_format = true; + } + continue; + }; + + let anchor_ranges = compute_modified_ranges(&buffer.snapshot(), &diff_snapshot); + let flat_anchors = anchor_ranges + .iter() + .flat_map(|range| [range.start, range.end]) + .collect::>(); + let multi_buffer_anchors = + multi_buffer_snapshot.text_anchors_to_visible_anchors(flat_anchors); + for pair in multi_buffer_anchors.chunks_exact(2) { + let (Some(start), Some(end)) = (&pair[0], &pair[1]) else { + continue; + }; + modified_ranges + .push(start.to_point(&multi_buffer_snapshot)..end.to_point(&multi_buffer_snapshot)); + } + } + + if fall_back_to_full_format { + Some(FormatTarget::Buffers(buffers.clone())) + } else if modified_ranges.is_empty() { + None + } else { + Some(FormatTarget::Ranges(modified_ranges)) + } +} + +/// Computes the buffer ranges that have unstaged changes, expanded to full lines and +/// with adjacent hunks merged, for use with format-on-save. An empty result means the +/// buffer has no formatable modifications. +fn compute_modified_ranges( + buffer_snapshot: &language::BufferSnapshot, + diff_snapshot: &buffer_diff::BufferDiffSnapshot, +) -> Vec> { + let mut merged: Vec> = Vec::new(); + for hunk in diff_snapshot.hunks(buffer_snapshot) { + let range = hunk.buffer_range; + if range.start.cmp(&range.end, buffer_snapshot).is_eq() { + // Deletion-only hunks produce no buffer content to format. + continue; + } + let start_point = range.start.to_point(buffer_snapshot); + let end_point = range.end.to_point(buffer_snapshot); + let start_row = start_point.row; + let end_row = if end_point.column == 0 && end_point.row > start_point.row { + end_point.row - 1 + } else { + end_point.row + }; + let line_start = text::Point::new(start_row, 0); + let line_end = text::Point::new(end_row, buffer_snapshot.line_len(end_row)); + let expanded = + buffer_snapshot.anchor_before(line_start)..buffer_snapshot.anchor_after(line_end); + + if let Some(last) = merged.last_mut() { + let last_end_point = last.end.to_point(buffer_snapshot); + if start_row <= last_end_point.row + 1 { + if expanded.end.to_point(buffer_snapshot) > last_end_point { + last.end = expanded.end; + } + continue; + } + } + merged.push(expanded); + } + merged +} + #[cfg(test)] mod tests { use crate::editor_tests::init_test; @@ -2921,4 +3037,124 @@ mod tests { "Editor::deserialize should not add items to panes as a side effect" ); } + + #[gpui::test] + fn test_compute_modified_ranges_git_diff(cx: &mut gpui::TestAppContext) { + let base_text = "line0\nline1\nline2\nline3\nline4\nline5\nline6\n"; + // Modify line1 and line5 to create two non-adjacent hunks. + let buffer_text = "line0\nMOD1\nline2\nline3\nline4\nMOD5\nline6\n"; + + let buffer = cx.new(|cx| language::Buffer::local(buffer_text, cx)); + let diff_snapshot = buffer.update(cx, |buffer, cx| { + let diff = cx.new(|cx| { + buffer_diff::BufferDiff::new_with_base_text(base_text, &buffer.text_snapshot(), cx) + }); + diff.read(cx).snapshot(cx) + }); + + let ranges = buffer.update(cx, |buffer, _cx| { + compute_modified_ranges(&buffer.snapshot(), &diff_snapshot) + }); + + assert_eq!(ranges.len(), 2, "expected 2 non-adjacent ranges"); + + buffer.update(cx, |buffer, _cx| { + let text_snapshot: &text::BufferSnapshot = buffer; + let r0 = ranges[0].start.to_point(text_snapshot)..ranges[0].end.to_point(text_snapshot); + let r1 = ranges[1].start.to_point(text_snapshot)..ranges[1].end.to_point(text_snapshot); + assert_eq!(r0.start.row, 1, "first hunk should start at row 1"); + assert_eq!(r0.end.row, 1, "first hunk should end at row 1"); + assert_eq!(r1.start.row, 5, "second hunk should start at row 5"); + assert_eq!(r1.end.row, 5, "second hunk should end at row 5"); + }); + } + + #[gpui::test] + fn test_compute_modified_ranges_unchanged_buffer(cx: &mut gpui::TestAppContext) { + let buffer_text = "line0\nline1\nline2\n"; + let buffer = cx.new(|cx| language::Buffer::local(buffer_text, cx)); + let diff_snapshot = buffer.update(cx, |buffer, cx| { + let diff = cx.new(|cx| { + buffer_diff::BufferDiff::new_with_base_text( + buffer_text, + &buffer.text_snapshot(), + cx, + ) + }); + diff.read(cx).snapshot(cx) + }); + + let ranges = buffer.update(cx, |buffer, _cx| { + compute_modified_ranges(&buffer.snapshot(), &diff_snapshot) + }); + + assert_eq!( + ranges, + Vec::new(), + "buffer that matches its diff base should produce no modified ranges" + ); + } + + #[gpui::test] + fn test_compute_modified_ranges_deletion_only(cx: &mut gpui::TestAppContext) { + let base_text = "line0\nline1\nline2\n"; + // Buffer has line1 deleted (pure deletion). + let buffer_text = "line0\nline2\n"; + + let buffer = cx.new(|cx| language::Buffer::local(buffer_text, cx)); + let diff_snapshot = buffer.update(cx, |buffer, cx| { + let diff = cx.new(|cx| { + buffer_diff::BufferDiff::new_with_base_text(base_text, &buffer.text_snapshot(), cx) + }); + diff.read(cx).snapshot(cx) + }); + + // Verify the diff has a deletion hunk. + let hunk_count = buffer.update(cx, |buffer, _cx| { + let text_snapshot: &text::BufferSnapshot = buffer; + diff_snapshot.hunks(text_snapshot).count() + }); + assert!(hunk_count > 0, "diff should have hunks"); + + let ranges = buffer.update(cx, |buffer, _cx| { + compute_modified_ranges(&buffer.snapshot(), &diff_snapshot) + }); + + assert_eq!( + ranges, + Vec::new(), + "deletion-only hunks should be skipped, leaving no ranges" + ); + } + + #[gpui::test] + fn test_compute_modified_ranges_adjacent_hunks(cx: &mut gpui::TestAppContext) { + let base_text = "line0\nline1\nline2\nline3\nline4\n"; + // Modify lines 2 and 3 which are adjacent; they should merge into one range. + let buffer_text = "line0\nline1\nMOD2\nMOD3\nline4\n"; + + let buffer = cx.new(|cx| language::Buffer::local(buffer_text, cx)); + let diff_snapshot = buffer.update(cx, |buffer, cx| { + let diff = cx.new(|cx| { + buffer_diff::BufferDiff::new_with_base_text(base_text, &buffer.text_snapshot(), cx) + }); + diff.read(cx).snapshot(cx) + }); + + let ranges = buffer.update(cx, |buffer, _cx| { + compute_modified_ranges(&buffer.snapshot(), &diff_snapshot) + }); + + assert_eq!( + ranges.len(), + 1, + "adjacent hunks (rows 2 and 3) should be merged into one range" + ); + buffer.update(cx, |buffer, _cx| { + let text_snapshot: &text::BufferSnapshot = buffer; + let r = ranges[0].start.to_point(text_snapshot)..ranges[0].end.to_point(text_snapshot); + assert_eq!(r.start.row, 2, "merged range should start at row 2"); + assert_eq!(r.end.row, 3, "merged range should end at row 3"); + }); + } } diff --git a/crates/migrator/src/migrations/m_2025_10_02/settings.rs b/crates/migrator/src/migrations/m_2025_10_02/settings.rs index 8942008e632..bb2a5bbb299 100644 --- a/crates/migrator/src/migrations/m_2025_10_02/settings.rs +++ b/crates/migrator/src/migrations/m_2025_10_02/settings.rs @@ -14,9 +14,9 @@ fn remove_formatters_on_save_inner(value: &mut Value, path: &[&str]) -> Result<( let Some(format_on_save) = obj.get("format_on_save").cloned() else { return Ok(()); }; - let is_format_on_save_set_to_formatter = format_on_save - .as_str() - .map_or(true, |s| s != "on" && s != "off"); + let is_format_on_save_set_to_formatter = format_on_save.as_str().map_or(true, |s| { + s != "on" && s != "off" && s != "modifications" && s != "modifications_if_available" + }); if !is_format_on_save_set_to_formatter { return Ok(()); } diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 1555a25dd21..f9e563ccdb3 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -1733,9 +1733,13 @@ impl LocalLspStore { let formatters = match (trigger, &settings.format_on_save) { (FormatTrigger::Save, FormatOnSave::Off) => &[], - (FormatTrigger::Manual, _) | (FormatTrigger::Save, FormatOnSave::On) => { - settings.formatter.as_ref() - } + (FormatTrigger::Manual, _) + | ( + FormatTrigger::Save, + FormatOnSave::On + | FormatOnSave::Modifications + | FormatOnSave::ModificationsIfAvailable, + ) => settings.formatter.as_ref(), }; let formatters = code_actions_on_format_formatters @@ -1763,6 +1767,7 @@ impl LocalLspStore { &adapters_and_servers, &settings, request_timeout, + trigger, logger, cx, ) @@ -1783,6 +1788,7 @@ impl LocalLspStore { adapters_and_servers: &[(Arc, Arc)], settings: &LanguageSettings, request_timeout: Duration, + trigger: FormatTrigger, logger: zlog::Logger, cx: &mut AsyncApp, ) -> anyhow::Result<()> { @@ -1925,23 +1931,22 @@ impl LocalLspStore { }; let Some(language_server) = language_server else { - log::debug!( - "No language server found to format buffer '{:?}'. Skipping", - buffer_path_abs.as_path().to_string_lossy() + zlog::debug!( + logger => + "No language server found to format buffer {buffer_path_abs:?}. Skipping", ); return Ok(()); }; zlog::trace!( logger => - "Formatting buffer '{:?}' using language server '{:?}'", - buffer_path_abs.as_path().to_string_lossy(), + "Formatting buffer {buffer_path_abs:?} using language server {:?}", language_server.name() ); let edits = if let Some(ranges) = buffer.ranges.as_ref() { zlog::trace!(logger => "formatting ranges"); - Self::format_ranges_via_lsp( + let range_edits = Self::format_ranges_via_lsp( &lsp_store, &buffer.handle, ranges, @@ -1951,8 +1956,38 @@ impl LocalLspStore { cx, ) .await - .context("Failed to format ranges via language server")? - .unwrap_or_default() + .context("Failed to format ranges via language server")?; + + match range_edits { + Some(edits) => edits, + None => { + if trigger == FormatTrigger::Save + && settings.format_on_save == FormatOnSave::ModificationsIfAvailable + { + zlog::debug!( + logger => + "Falling back to full format - LSP does not support range formatting" + ); + Self::format_via_lsp( + &lsp_store, + &buffer.handle, + buffer_path_abs, + &language_server, + &settings, + cx, + ) + .await + .context("failed to format via language server")? + } else { + zlog::debug!( + logger => + "Skipping range format - language server {:?} does not support range formatting", + language_server.name() + ); + Vec::new() + } + } + } } else { zlog::trace!(logger => "formatting full"); Self::format_via_lsp( diff --git a/crates/settings/src/settings_store.rs b/crates/settings/src/settings_store.rs index 29d9f91dc6b..ae63dbe75fc 100644 --- a/crates/settings/src/settings_store.rs +++ b/crates/settings/src/settings_store.rs @@ -2461,6 +2461,148 @@ mod tests { .unindent(), cx, ); + + // formatOnSave: true with formatOnSaveMode: modificationsIfAvailable + check_vscode_import( + &mut store, + r#"{ + } + "# + .unindent(), + r#"{ "editor.formatOnSave": true, "editor.formatOnSaveMode": "modificationsIfAvailable" }"# + .to_owned(), + r#"{ + "base_keymap": "VSCode", + "minimap": { + "show": "always" + }, + "format_on_save": "modifications_if_available" + } + "# + .unindent(), + cx, + ); + + // formatOnSave: true with formatOnSaveMode: modifications + check_vscode_import( + &mut store, + r#"{ + } + "# + .unindent(), + r#"{ "editor.formatOnSave": true, "editor.formatOnSaveMode": "modifications" }"# + .to_owned(), + r#"{ + "base_keymap": "VSCode", + "minimap": { + "show": "always" + }, + "format_on_save": "modifications" + } + "# + .unindent(), + cx, + ); + + // formatOnSave: true with formatOnSaveMode: file + check_vscode_import( + &mut store, + r#"{ + } + "# + .unindent(), + r#"{ "editor.formatOnSave": true, "editor.formatOnSaveMode": "file" }"#.to_owned(), + r#"{ + "base_keymap": "VSCode", + "minimap": { + "show": "always" + }, + "format_on_save": "on" + } + "# + .unindent(), + cx, + ); + + // formatOnSaveMode is ignored when formatOnSave is disabled, as in VS Code + check_vscode_import( + &mut store, + r#"{ + } + "# + .unindent(), + r#"{ "editor.formatOnSave": false, "editor.formatOnSaveMode": "modifications" }"# + .to_owned(), + r#"{ + "base_keymap": "VSCode", + "minimap": { + "show": "always" + }, + "format_on_save": "off" + } + "# + .unindent(), + cx, + ); + + // formatOnSaveMode alone does nothing, as formatOnSave defaults to false in VS Code + check_vscode_import( + &mut store, + r#"{ + } + "# + .unindent(), + r#"{ "editor.formatOnSaveMode": "modifications" }"#.to_owned(), + r#"{ + "base_keymap": "VSCode", + "minimap": { + "show": "always" + } + } + "# + .unindent(), + cx, + ); + + // formatOnSaveMode not set, formatOnSave: true + check_vscode_import( + &mut store, + r#"{ + } + "# + .unindent(), + r#"{ "editor.formatOnSave": true }"#.to_owned(), + r#"{ + "base_keymap": "VSCode", + "minimap": { + "show": "always" + }, + "format_on_save": "on" + } + "# + .unindent(), + cx, + ); + + // formatOnSaveMode not set, formatOnSave: false + check_vscode_import( + &mut store, + r#"{ + } + "# + .unindent(), + r#"{ "editor.formatOnSave": false }"#.to_owned(), + r#"{ + "base_keymap": "VSCode", + "minimap": { + "show": "always" + }, + "format_on_save": "off" + } + "# + .unindent(), + cx, + ); } #[track_caller] diff --git a/crates/settings/src/vscode_import.rs b/crates/settings/src/vscode_import.rs index 9b8584c4e6b..0d2f5511430 100644 --- a/crates/settings/src/vscode_import.rs +++ b/crates/settings/src/vscode_import.rs @@ -554,9 +554,15 @@ impl VsCodeSettings { extend_comment_on_newline: None, extend_list_on_newline: None, indent_list_on_tab: None, - format_on_save: self.read_bool("editor.guides.formatOnSave").map(|b| { - if b { - FormatOnSave::On + // In VS Code, `editor.formatOnSaveMode` only applies when `editor.formatOnSave` is enabled. + format_on_save: self.read_bool("editor.formatOnSave").map(|enabled| { + if enabled { + self.read_enum("editor.formatOnSaveMode", |s| match s { + "modificationsIfAvailable" => Some(FormatOnSave::ModificationsIfAvailable), + "modifications" => Some(FormatOnSave::Modifications), + _ => None, + }) + .unwrap_or(FormatOnSave::On) } else { FormatOnSave::Off } diff --git a/crates/settings_content/src/language.rs b/crates/settings_content/src/language.rs index f31ef08e1d2..71beec78f7d 100644 --- a/crates/settings_content/src/language.rs +++ b/crates/settings_content/src/language.rs @@ -923,12 +923,22 @@ pub struct PrettierSettingsContent { strum::VariantArray, strum::VariantNames, )] -#[serde(rename_all = "lowercase")] +#[serde(rename_all = "snake_case")] pub enum FormatOnSave { /// Files should be formatted on save. On, /// Files should not be formatted on save. Off, + /// Only lines with unstaged changes are formatted on save. + /// Requires source control and LSP range formatting support. + /// If no git diff is available or if the LSP doesn't support + /// range formatting, formatting is skipped. + Modifications, + /// Only lines with unstaged changes are formatted on save. + /// If no git diff is available (e.g., when source control is + /// unavailable) or if the LSP doesn't support range formatting, + /// falls back to formatting the whole file. + ModificationsIfAvailable, } /// Controls how line endings are normalized when a buffer is saved. diff --git a/crates/settings_ui/src/page_data.rs b/crates/settings_ui/src/page_data.rs index ba35c4875ef..2f6ce0beec5 100644 --- a/crates/settings_ui/src/page_data.rs +++ b/crates/settings_ui/src/page_data.rs @@ -8934,7 +8934,7 @@ fn language_settings_data() -> Box<[SettingsPageItem]> { SettingsPageItem::SectionHeader("Formatting"), SettingsPageItem::SettingItem(SettingItem { title: "Format On Save", - description: "Whether or not to perform a buffer format before saving.", + description: "On: format the whole buffer.\nOff: do not format.\nModifications: format only lines with unstaged changes; skips formatting when a git diff or LSP range formatting is unavailable.\nModifications If Available: same, but falls back to formatting the whole buffer.", field: Box::new( // TODO(settings_ui): this setting should just be a bool SettingField { diff --git a/docs/src/reference/all-settings.md b/docs/src/reference/all-settings.md index 1450b58d6fc..2ded300ce5d 100644 --- a/docs/src/reference/all-settings.md +++ b/docs/src/reference/all-settings.md @@ -1905,7 +1905,7 @@ While other options may be changed at a runtime and should be placed under `sett } ``` -## Format On Save +## Format On Save {#format-on-save} - Description: Whether or not to perform a buffer format before saving. - Setting: `format_on_save` @@ -1929,6 +1929,26 @@ While other options may be changed at a runtime and should be placed under `sett } ``` +3. `modifications`, formats only lines with unstaged changes: + +```json [settings] +{ + "format_on_save": "modifications" +} +``` + +This mode requires source control and LSP range formatting support. If no git diff is available or if the LSP doesn't support range formatting, formatting is skipped. This is useful for editing legacy codebases where you want to avoid formatting changes in unrelated code. + +4. `modifications_if_available`, formats only modified lines with fallback to full file formatting: + +```json [settings] +{ + "format_on_save": "modifications_if_available" +} +``` + +Similar to `modifications`, but behaves like `on` when range formatting cannot be applied: when no git diff is available (e.g., when source control is unavailable) or when the language server does not support range formatting. When a git diff is available but contains no unstaged changes, nothing is formatted. + ## Formatter - Description: How to perform a buffer format. @@ -3279,7 +3299,7 @@ Examples: - Description: Preview tabs allow you to open files in preview mode, where they close automatically when you switch to another file unless you explicitly pin them. This is useful for quickly viewing files without cluttering your workspace. Preview tabs display their file names in italics. \ - There are several ways to convert a preview tab into a regular tab: + There are several ways to convert a preview tab into a regular tab: - Double-clicking on the file - Double-clicking on the tab header From 6b733d105896a20924bd4aba87bd7baa20b62ac6 Mon Sep 17 00:00:00 2001 From: Keith Hall Date: Wed, 8 Jul 2026 19:56:45 +0300 Subject: [PATCH 10/61] search: Bump fancy-regex dependency and enable CRLF mode (#55471) 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 #43396 Release Notes: - Project search now supports CRLF line endings correctly, as well as other regex features like subroutine calls --- Cargo.lock | 10 ++++----- Cargo.toml | 2 +- crates/project/src/search.rs | 1 + crates/project/tests/integration/search.rs | 26 +++++++++++++++++++++- 4 files changed, 32 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b9b4ea96158..3f71a7000d7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6465,9 +6465,9 @@ dependencies = [ [[package]] name = "fancy-regex" -version = "0.17.0" +version = "0.18.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72cf461f865c862bb7dc573f643dd6a2b6842f7c30b07882b56bd148cc2761b8" +checksum = "e1e1dacd0d2082dfcf1351c4bdd566bbe89a2b263235a2b50058f1e130a47277" dependencies = [ "bit-set 0.8.0", "regex-automata", @@ -14003,7 +14003,7 @@ dependencies = [ "dap", "encoding_rs", "extension", - "fancy-regex 0.17.0", + "fancy-regex 0.18.0", "fs", "futures 0.3.32", "fuzzy", @@ -15066,9 +15066,9 @@ dependencies = [ [[package]] name = "regex-automata" -version = "0.4.13" +version = "0.4.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5276caf25ac86c8d810222b3dbb938e512c55c6831a10f3e6ed1c93b84041f1c" +checksum = "6e1dd4122fc1595e8162618945476892eefca7b88c52820e74af6262213cae8f" dependencies = [ "aho-corasick", "memchr", diff --git a/Cargo.toml b/Cargo.toml index 7a7003ec5eb..916cb147fa5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -595,7 +595,7 @@ emojis = "0.6.1" env_logger = "0.11" encoding_rs = "0.8" exec = "0.3.1" -fancy-regex = "0.17.0" +fancy-regex = "0.18.0" fork = "0.4.0" futures = "0.3.32" futures-concurrency = "7.7.1" diff --git a/crates/project/src/search.rs b/crates/project/src/search.rs index c2804f853a3..ccb38746a2b 100644 --- a/crates/project/src/search.rs +++ b/crates/project/src/search.rs @@ -251,6 +251,7 @@ impl SearchQuery { let regex = RegexBuilder::new(&pattern) .case_insensitive(!case_sensitive) + .crlf(true) .build()?; Ok(Self::Regex { regex, diff --git a/crates/project/tests/integration/search.rs b/crates/project/tests/integration/search.rs index 79266405084..98cf7dc90ab 100644 --- a/crates/project/tests/integration/search.rs +++ b/crates/project/tests/integration/search.rs @@ -1,3 +1,4 @@ +use language::Buffer; use project::search::SearchQuery; use text::Rope; use util::{ @@ -130,6 +131,30 @@ fn test_case_sensitive_pattern_items() { ); } +#[gpui::test] +async fn test_multiline_regex_crlf(cx: &mut gpui::TestAppContext) { + let search_query = SearchQuery::regex( + "^hello$\r?\n", + false, + false, + false, + false, + Default::default(), + Default::default(), + false, + None, + ) + .expect("Should be able to create a regex SearchQuery"); + + let text = Rope::from("hello\r\nworld\r\nhello\r\nworld"); + let snapshot = cx + .update(|app| Buffer::build_snapshot(text, None, None, None, app)) + .await; + + let results = search_query.search(&snapshot, None).await; + assert_eq!(results, vec![0..7, 14..21]); +} + #[gpui::test] async fn test_multiline_regex(cx: &mut gpui::TestAppContext) { let search_query = SearchQuery::regex( @@ -145,7 +170,6 @@ async fn test_multiline_regex(cx: &mut gpui::TestAppContext) { ) .expect("Should be able to create a regex SearchQuery"); - use language::Buffer; let text = Rope::from("hello\nworld\nhello\nworld"); let snapshot = cx .update(|app| Buffer::build_snapshot(text, None, None, None, app)) From 546a16d64fa589c737a9e33dff02a129a673b21d Mon Sep 17 00:00:00 2001 From: Philipp Schaffrath Date: Wed, 8 Jul 2026 21:22:23 +0200 Subject: [PATCH 11/61] gpui: Add parent-anchored native popup windows (with wayland xdg_popup implementation only so far) (#60232) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Objective gpui can't show UI that extends past the window it belongs to. Menus, dropdowns and tooltips are drawn as elements inside the window, so they clip at its edges. This PR adds a window kind for platform-native popups anchored to a parent window, as groundwork for real native menus, dropdowns and tooltips. ## Solution `WindowKind::AnchoredPopup(PopupOptions)` opens a popup positioned relative to a parent window. Instead of giving the popup an absolute position, you describe where it should go and the platform figures out the rest: - `parent`: the window to anchor to - `anchor_rect`: a rectangle in the parent, e.g. the button that opened the menu - `anchor` and `gravity`: which point of that rect to attach to, and which direction to grow - `constraint_adjustment`: what the platform may do if the popup would leave the screen (slide, flip, resize) - `grab`: menu behavior, the popup takes focus and is dismissed when clicking outside the app The popup's size comes from `WindowOptions::window_bounds`. This model mirrors Wayland's `xdg_positioner`, where the compositor owns positioning and the client can only describe intent. Since that's the most restrictive case, the other platforms can implement the same description later with simple math against screen bounds. Only Wayland is implemented so far, via `xdg_popup` on top of the existing surface implementation. Popups can be parented to toplevels, layer-shell surfaces (a menu opened from a panel) and other popups (nested menus). macOS, Windows, X11 and web reject the kind with `PopupNotSupportedError`, so callers can detect that and fall back to in-window popovers. Some Wayland details that might help during review: - Anchor rects are translated from gpui coordinates into the parent's window geometry space and clamped to it. A rect outside the geometry, or with zero size, is a fatal protocol error - Resizing a mapped popup goes through `xdg_popup.reposition` - Mouse press serials are now recorded on press only, not release. Compositors decline grabs and interactive moves that reference a release serial ## Testing Tested manually on Wayland with an example app: the menu opens anchored below its button, extends past the parent window, flips above the button near the bottom of the screen, and a grabbing popup is dismissed when clicking into another application. Nested menus were tested in one of my projects (ignore that they are ugly, that's just a prototype :stuck_out_tongue:): https://github.com/user-attachments/assets/2cd3e2e9-87f7-4b02-986f-48e5633e205c I also have a complete runnable example demonstrating it. I did not add it to the PR, because this might give the impression that `WindowKind::AnchoredPopup` are a complete implementation, despite only working on wayland so far:
Click to view example ```rust //! Example and manual test for platform-native popups (`WindowKind::AnchoredPopup`). //! //! A native popup is a real, parent-anchored window that can extend beyond its parent onto the //! screen, unlike gpui's in-window popovers. Run it, open the menu, and confirm the points listed //! in the window. On a platform without an implementation the button reports that popups are not //! supported instead of opening anything. //! //! Run with: cargo run -p gpui --example popup #![cfg_attr(target_family = "wasm", no_main)] use gpui::{ AnyWindowHandle, App, Bounds, Context, MouseButton, SharedString, Window, WindowBounds, WindowHandle, WindowKind, WindowOptions, div, point, popup::*, prelude::*, px, rgb, size, }; use gpui_platform::application; /// The trigger button, at a fixed position so the popup can anchor to a known rectangle. Real code /// would anchor to the measured bounds of whatever element opens the popup. const BUTTON_BOUNDS: Bounds = Bounds { origin: point(px(24.), px(24.)), size: size(px(200.), px(32.)), }; const POPUP_SIZE: gpui::Size = size(px(260.), px(320.)); struct Menu; impl Render for Menu { fn render(&mut self, _window: &mut Window, _cx: &mut Context) -> impl IntoElement { let item = |label: &str| { div() .id(label.to_string()) .px_3() .py_1() .rounded_sm() .hover(|this| this.bg(rgb(0x3a3a3a))) .cursor_pointer() .child(label.to_string()) .on_click(|_, window, _| window.remove_window()) }; div() .id("menu-root") .size_full() .p_1() .flex() .flex_col() .gap_0p5() .bg(rgb(0x2a2a2a)) .text_color(gpui::white()) .rounded_md() .border_1() .border_color(rgb(0x454545)) .child(item("Foo")) .child(item("Bar")) .child(item("Baz")) .child(item("Qux")) .child(item("Alice")) .child(item("Bob")) } } struct PopupExample { menu: Option>, status: SharedString, } impl Default for PopupExample { fn default() -> Self { Self { menu: None, status: "Click \"Open menu\" to open a native popup.".into(), } } } impl PopupExample { /// Closes the menu if it is open. Returns true if a menu was actually open. fn close_menu(&mut self, cx: &mut App) -> bool { match self.menu.take() { Some(menu) => menu .update(cx, |_, window, _| window.remove_window()) .is_ok(), None => false, } } fn toggle_menu(&mut self, parent: AnyWindowHandle, cx: &mut App) { if self.close_menu(cx) { return; } match open_menu(parent, cx) { Ok(menu) => { self.menu = Some(menu); self.status = "Menu open. Dismiss it by selecting an item, clicking elsewhere in \ this window, or clicking another application." .into(); } // A real application would fall back to an in-window popover here. Err(error) => { self.status = format!("Failed to open a native popup: {error}").into(); log::error!("failed to open popup: {error}"); } } } } impl Render for PopupExample { fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { let bullet = |text: &str| div().child(format!("• {text}")); div() .id("root") .size_full() .bg(rgb(0xf7f7f7)) .text_color(rgb(0x222222)) // Same-app clicks don't auto-dismiss a grabbing popup (see `PopupOptions::grab`). .on_mouse_down( MouseButton::Left, cx.listener(|this, _, _window, cx| { this.close_menu(cx); }), ) .child( div() .size_full() .p_5() .pt(px(76.)) .flex() .flex_col() .gap_3() .child(div().text_xl().child("Native popup test")) .child(div().text_sm().child( "WindowKind::AnchoredPopup opens a real, parent-anchored window that can \ extend past this window onto the screen. Only some platforms implement \ it so far.", )) .child( div() .flex() .flex_col() .gap_1() .text_sm() .text_color(rgb(0x555555)) .child(div().child("Verify:")) .child(bullet("The menu opens anchored below the button.")) .child(bullet( "The menu extends past the bottom edge of this window.", )) .child(bullet( "Near the bottom of the screen, the menu flips above the button.", )) .child(bullet("Clicking another application dismisses the menu.")) .child(bullet( "Selecting an item or clicking in this window dismisses it.", )), ) .child( div() .text_sm() .text_color(rgb(0x333333)) .child(self.status.clone()), ), ) .child( div() .absolute() .left(BUTTON_BOUNDS.origin.x) .top(BUTTON_BOUNDS.origin.y) .w(BUTTON_BOUNDS.size.width) .h(BUTTON_BOUNDS.size.height) .flex() .items_center() .justify_center() .bg(rgb(0xffffff)) .border_1() .border_color(rgb(0xd0d0d0)) .rounded_md() .cursor_pointer() .id("open-menu") .active(|this| this.bg(rgb(0xeeeeee))) .child("Open menu ▾") // Open on mouse-down, not on click, so the grab is taken while the button is still held. .on_mouse_down( MouseButton::Left, cx.listener(|this, _, window, cx| { // Don't let the window handler above close the menu we are opening. cx.stop_propagation(); this.toggle_menu(window.window_handle(), cx); }), ), ) } } fn open_menu(parent: AnyWindowHandle, cx: &mut App) -> anyhow::Result> { cx.open_window( WindowOptions { titlebar: None, // Sizes the popup. The platform decides the position, so the origin is ignored. window_bounds: Some(WindowBounds::Windowed(Bounds { origin: point(px(0.), px(0.)), size: POPUP_SIZE, })), kind: WindowKind::AnchoredPopup(PopupOptions { parent, anchor_rect: BUTTON_BOUNDS, // Anchor to the button's bottom-left and grow down-right so the menu drops beneath it. anchor: PopupAnchor::BottomLeft, gravity: PopupGravity::BottomRight, // Slide horizontally and flip vertically if the menu would leave the screen. constraint_adjustment: PopupConstraintAdjustment::SLIDE_X | PopupConstraintAdjustment::FLIP_Y, offset: point(px(0.), px(4.)), // Grab input so the compositor dismisses the popup on clicks into other applications. grab: true, }), ..Default::default() }, |_, cx| cx.new(|_| Menu), ) } fn run_example() { application().run(|cx: &mut App| { cx.open_window( WindowOptions { window_bounds: Some(WindowBounds::Windowed(Bounds { origin: point(px(100.), px(100.)), size: size(px(420.), px(300.)), })), ..Default::default() }, |_, cx| cx.new(|_| PopupExample::default()), ) .unwrap(); cx.activate(true); }); } #[cfg(not(target_family = "wasm"))] fn main() { run_example(); } #[cfg(target_family = "wasm")] #[wasm_bindgen::prelude::wasm_bindgen(start)] pub fn start() { gpui_platform::web_init(); run_example(); } ```
## 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 adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [ ] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable --- Release Notes: - N/A --- crates/gpui/Cargo.toml | 6 +- crates/gpui/src/platform.rs | 11 + crates/gpui/src/platform/popup.rs | 134 +++++++++ crates/gpui_linux/src/linux/wayland.rs | 1 + crates/gpui_linux/src/linux/wayland/client.rs | 65 +++- crates/gpui_linux/src/linux/wayland/popup.rs | 38 +++ crates/gpui_linux/src/linux/wayland/window.rs | 278 +++++++++++++++++- crates/gpui_linux/src/linux/x11/window.rs | 8 +- crates/gpui_macos/src/platform.rs | 9 +- crates/gpui_macos/src/window.rs | 8 +- crates/gpui_web/src/platform.rs | 8 +- crates/gpui_windows/src/window.rs | 6 + 12 files changed, 544 insertions(+), 28 deletions(-) create mode 100644 crates/gpui/src/platform/popup.rs create mode 100644 crates/gpui_linux/src/linux/wayland/popup.rs diff --git a/crates/gpui/Cargo.toml b/crates/gpui/Cargo.toml index bad1feabb3b..eab072e470b 100644 --- a/crates/gpui/Cargo.toml +++ b/crates/gpui/Cargo.toml @@ -29,9 +29,7 @@ test-support = [ bench = ["test-support", "dep:criterion", "dep:hdrhistogram"] inspector = ["gpui_macros/inspector"] leak-detection = ["backtrace"] -wayland = [ - "bitflags", -] +wayland = [] x11 = [ "scap?/x11", ] @@ -51,7 +49,7 @@ accesskit.workspace = true anyhow.workspace = true async-task = "4.7" backtrace = { workspace = true, optional = true } -bitflags = { workspace = true, optional = true } +bitflags.workspace = true collections.workspace = true criterion = { workspace = true, optional = true } diff --git a/crates/gpui/src/platform.rs b/crates/gpui/src/platform.rs index 13fcbcee7a7..2a37d0b19b2 100644 --- a/crates/gpui/src/platform.rs +++ b/crates/gpui/src/platform.rs @@ -6,6 +6,9 @@ mod keystroke; #[expect(missing_docs)] pub mod layer_shell; +/// Types for configuring parent-anchored popup windows such as menus, dropdowns and tooltips. +pub mod popup; + #[cfg(any(test, feature = "bench"))] mod bench_dispatcher; @@ -1677,6 +1680,14 @@ pub enum WindowKind { /// use sparingly! PopUp, + /// A parent-anchored, platform-native popup window for menus, comboboxes, context menus and + /// tooltips. Unlike [`WindowKind::PopUp`], it is positioned relative to a parent window. + /// + /// The popup's size comes from [`WindowOptions::window_bounds`], whose origin is ignored. + /// See [`popup::PopupOptions`] for the placement options. Platforms without a native + /// implementation reject it with [`popup::PopupNotSupportedError`]. + AnchoredPopup(popup::PopupOptions), + /// A floating window that appears on top of its parent window Floating, diff --git a/crates/gpui/src/platform/popup.rs b/crates/gpui/src/platform/popup.rs new file mode 100644 index 00000000000..a1f8d7ea0d5 --- /dev/null +++ b/crates/gpui/src/platform/popup.rs @@ -0,0 +1,134 @@ +use bitflags::bitflags; +use thiserror::Error; + +use crate::{AnyWindowHandle, Bounds, Pixels, Point}; + +/// Options for a parent-anchored popup window such as a menu, dropdown, context menu or tooltip. +/// +/// A popup is placed relative to an anchor rectangle on its parent window rather than at an +/// absolute screen position. The platform resolves the final position, so this works both on +/// systems where the compositor owns window placement (Wayland) and on platforms with absolute +/// coordinates. +/// +/// The popup's size comes from [`WindowOptions::window_bounds`](crate::WindowOptions), whose +/// origin is ignored. All coordinates are in logical pixels. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct PopupOptions { + /// The window the popup is anchored to. + pub parent: AnyWindowHandle, + + /// The rectangle the popup is positioned relative to, in the parent window's coordinate + /// space (the same space element bounds are in). For example, a dropdown menu uses the + /// bounds of the button that opened it. + pub anchor_rect: Bounds, + + /// Which point of [`Self::anchor_rect`] the popup is anchored to. + pub anchor: PopupAnchor, + + /// The direction in which the popup extends away from the anchor point. A dropdown that + /// drops below its button anchors to [`PopupAnchor::BottomLeft`] with a gravity of + /// [`PopupGravity::BottomRight`] so it grows down and to the right. + pub gravity: PopupGravity, + + /// How the platform may adjust the popup if the requested placement would put it off-screen. + pub constraint_adjustment: PopupConstraintAdjustment, + + /// An additional offset applied to the popup after anchoring. + pub offset: Point, + + /// Whether the popup should take an explicit input grab. + /// + /// Grabbing popups behave like menus: they take keyboard focus and are dismissed when the + /// user clicks outside of them or presses a dismissing key. Use it for menus and comboboxes, + /// not for tooltips or other passive popups. + /// + /// A grab must be requested while the triggering input is still active, in practice the + /// press of the mouse button that opens the popup. Open grabbing popups from a mouse-down + /// handler rather than a click handler, otherwise the grab is refused. + /// + /// Automatic dismissal only covers input aimed at other applications. A click elsewhere in + /// your own application still reaches it as usual, so closing the popup in that case is up + /// to you. Nested grabbing popups must be closed in the reverse order they were opened. + pub grab: bool, +} + +/// The point of the anchor rectangle that a popup is anchored to. +#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] +pub enum PopupAnchor { + /// Anchor to the center of the anchor rectangle. + #[default] + Center, + /// Anchor to the center of the top edge. + Top, + /// Anchor to the center of the bottom edge. + Bottom, + /// Anchor to the center of the left edge. + Left, + /// Anchor to the center of the right edge. + Right, + /// Anchor to the top-left corner. + TopLeft, + /// Anchor to the bottom-left corner. + BottomLeft, + /// Anchor to the top-right corner. + TopRight, + /// Anchor to the bottom-right corner. + BottomRight, +} + +/// The direction in which a popup extends away from its anchor point. +/// +/// For instance, a gravity of [`PopupGravity::BottomRight`] places the popup below and to the +/// right of the anchor point. +#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] +pub enum PopupGravity { + /// The popup is centered over the anchor point. + #[default] + Center, + /// The popup extends upwards from the anchor point. + Top, + /// The popup extends downwards from the anchor point. + Bottom, + /// The popup extends to the left of the anchor point. + Left, + /// The popup extends to the right of the anchor point. + Right, + /// The popup extends up and to the left of the anchor point. + TopLeft, + /// The popup extends down and to the left of the anchor point. + BottomLeft, + /// The popup extends up and to the right of the anchor point. + TopRight, + /// The popup extends down and to the right of the anchor point. + BottomRight, +} + +bitflags! { + /// How a popup may be adjusted by the platform if the requested placement would put it + /// off-screen. If no flags are set, the popup is placed exactly as requested and may be + /// clipped. + #[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] + pub struct PopupConstraintAdjustment: u32 { + /// The popup may be slid horizontally to stay on-screen. + const SLIDE_X = 1; + /// The popup may be slid vertically to stay on-screen. + const SLIDE_Y = 2; + /// The popup's anchor and gravity may be flipped horizontally to stay on-screen. + const FLIP_X = 4; + /// The popup's anchor and gravity may be flipped vertically to stay on-screen. + const FLIP_Y = 8; + /// The popup may be shrunk horizontally to stay on-screen. + const RESIZE_X = 16; + /// The popup may be shrunk vertically to stay on-screen. + const RESIZE_Y = 32; + } +} + +/// Returned when the current platform has no native popup implementation yet. +/// +/// Native popups are separate from gpui's in-window popovers, which are drawn as elements inside +/// an existing window. A caller that wants a popup on every platform should treat this error as +/// a cue to fall back to that in-window rendering. +#[derive(Debug, Error)] +#[error("popups are not supported on this platform")] +pub struct PopupNotSupportedError; diff --git a/crates/gpui_linux/src/linux/wayland.rs b/crates/gpui_linux/src/linux/wayland.rs index 3e90688d1bd..cbc962bcffe 100644 --- a/crates/gpui_linux/src/linux/wayland.rs +++ b/crates/gpui_linux/src/linux/wayland.rs @@ -2,6 +2,7 @@ mod client; mod clipboard; mod cursor; mod display; +mod popup; mod serial; mod window; diff --git a/crates/gpui_linux/src/linux/wayland/client.rs b/crates/gpui_linux/src/linux/wayland/client.rs index 256875bed1e..9522ebb8f0d 100644 --- a/crates/gpui_linux/src/linux/wayland/client.rs +++ b/crates/gpui_linux/src/linux/wayland/client.rs @@ -57,7 +57,9 @@ use wayland_protocols::xdg::activation::v1::client::{xdg_activation_token_v1, xd use wayland_protocols::xdg::decoration::zv1::client::{ zxdg_decoration_manager_v1, zxdg_toplevel_decoration_v1, }; -use wayland_protocols::xdg::shell::client::{xdg_surface, xdg_toplevel, xdg_wm_base}; +use wayland_protocols::xdg::shell::client::{ + xdg_popup, xdg_positioner, xdg_surface, xdg_toplevel, xdg_wm_base, +}; use wayland_protocols::xdg::system_bell::v1::client::xdg_system_bell_v1; use wayland_protocols::{ wp::cursor_shape::v1::client::{wp_cursor_shape_device_v1, wp_cursor_shape_manager_v1}, @@ -96,7 +98,7 @@ use gpui::{ ForegroundExecutor, KeyDownEvent, KeyUpEvent, Keystroke, Modifiers, ModifiersChangedEvent, MouseButton, MouseDownEvent, MouseExitEvent, MouseMoveEvent, MouseUpEvent, NavigationDirection, Pixels, PlatformDisplay, PlatformInput, PlatformKeyboardLayout, PlatformWindow, Point, - ScrollDelta, ScrollWheelEvent, SharedString, Size, TouchPhase, WindowButtonLayout, + ScrollDelta, ScrollWheelEvent, SharedString, Size, TouchPhase, WindowButtonLayout, WindowKind, WindowParams, point, profiler, px, size, }; use gpui_wgpu::{CompositorGpuHint, GpuContext}; @@ -846,7 +848,29 @@ impl LinuxClient for WaylandClient { ) -> anyhow::Result> { let mut state = self.0.borrow_mut(); - let parent = state.keyboard_focused_window.clone(); + // Popups name their parent explicitly. Other kinds are parented to the focused window. + let (parent, popup_grab) = match ¶ms.kind { + WindowKind::AnchoredPopup(options) => { + let parent = state + .windows + .values() + .find(|window| window.handle() == options.parent) + .cloned() + .ok_or_else(|| anyhow::anyhow!("popup parent window not found"))?; + // A popup grab must reference a press event or the compositor declines it and + // immediately dismisses the popup, so use the most recent press serial, or no + // grab before any press. + let popup_grab = options.grab.then(|| { + let serial = state + .serial_tracker + .get(SerialKind::MousePress) + .max(state.serial_tracker.get(SerialKind::KeyPress)); + (serial != 0).then(|| (serial, state.wl_seat.clone())) + }); + (Some(parent), popup_grab.flatten()) + } + _ => (state.keyboard_focused_window.clone(), None), + }; let target_output = params.display_id.and_then(|display_id| { let target_protocol_id: u64 = display_id.into(); @@ -859,6 +883,7 @@ impl LinuxClient for WaylandClient { let appearance = state.common.appearance; let compositor_gpu = state.compositor_gpu.take(); + let (window, surface_id) = WaylandWindow::new( handle, state.globals.clone(), @@ -868,8 +893,10 @@ impl LinuxClient for WaylandClient { params, appearance, parent, + popup_grab, target_output, )?; + if window.0.toplevel().is_some() { state.consume_startup_activation_token(&window.0.surface()); } @@ -1196,6 +1223,7 @@ delegate_noop!(WaylandClientStatePtr: ignore wl_region::WlRegion); delegate_noop!(WaylandClientStatePtr: ignore wp_fractional_scale_manager_v1::WpFractionalScaleManagerV1); delegate_noop!(WaylandClientStatePtr: ignore zxdg_decoration_manager_v1::ZxdgDecorationManagerV1); delegate_noop!(WaylandClientStatePtr: ignore zwlr_layer_shell_v1::ZwlrLayerShellV1); +delegate_noop!(WaylandClientStatePtr: ignore xdg_positioner::XdgPositioner); delegate_noop!(WaylandClientStatePtr: ignore org_kde_kwin_blur_manager::OrgKdeKwinBlurManager); delegate_noop!(WaylandClientStatePtr: ignore zwp_text_input_manager_v3::ZwpTextInputManagerV3); delegate_noop!(WaylandClientStatePtr: ignore org_kde_kwin_blur::OrgKdeKwinBlur); @@ -1359,6 +1387,31 @@ impl Dispatch for WaylandCl drop(state); let should_close = window.handle_layersurface_event(event); + if should_close { + // Close logic will be handled in drop_window() + window.close(); + } + } +} + +impl Dispatch for WaylandClientStatePtr { + fn event( + this: &mut Self, + _: &xdg_popup::XdgPopup, + event: ::Event, + surface_id: &ObjectId, + _: &Connection, + _: &QueueHandle, + ) { + let client = this.get_client(); + let mut state = client.borrow_mut(); + let Some(window) = get_window(&mut state, surface_id) else { + return; + }; + + drop(state); + let should_close = window.handle_popup_event(event); + if should_close { // The close logic will be handled in drop_window() window.close(); @@ -1934,7 +1987,11 @@ impl Dispatch for WaylandClientStatePtr { state: WEnum::Value(button_state), .. } => { - state.serial_tracker.update(SerialKind::MousePress, serial); + // Record presses only. Requests referencing this serial (popup grabs, + // interactive moves) are declined when given a release serial. + if button_state == wl_pointer::ButtonState::Pressed { + state.serial_tracker.update(SerialKind::MousePress, serial); + } let button = linux_button_to_gpui(button); let Some(button) = button else { return }; if state.mouse_focused_window.is_none() { diff --git a/crates/gpui_linux/src/linux/wayland/popup.rs b/crates/gpui_linux/src/linux/wayland/popup.rs new file mode 100644 index 00000000000..4a15e78211b --- /dev/null +++ b/crates/gpui_linux/src/linux/wayland/popup.rs @@ -0,0 +1,38 @@ +pub use gpui::popup::*; + +use wayland_protocols::xdg::shell::client::xdg_positioner; + +pub(crate) fn wayland_anchor(anchor: PopupAnchor) -> xdg_positioner::Anchor { + match anchor { + PopupAnchor::Center => xdg_positioner::Anchor::None, + PopupAnchor::Top => xdg_positioner::Anchor::Top, + PopupAnchor::Bottom => xdg_positioner::Anchor::Bottom, + PopupAnchor::Left => xdg_positioner::Anchor::Left, + PopupAnchor::Right => xdg_positioner::Anchor::Right, + PopupAnchor::TopLeft => xdg_positioner::Anchor::TopLeft, + PopupAnchor::BottomLeft => xdg_positioner::Anchor::BottomLeft, + PopupAnchor::TopRight => xdg_positioner::Anchor::TopRight, + PopupAnchor::BottomRight => xdg_positioner::Anchor::BottomRight, + } +} + +pub(crate) fn wayland_gravity(gravity: PopupGravity) -> xdg_positioner::Gravity { + match gravity { + PopupGravity::Center => xdg_positioner::Gravity::None, + PopupGravity::Top => xdg_positioner::Gravity::Top, + PopupGravity::Bottom => xdg_positioner::Gravity::Bottom, + PopupGravity::Left => xdg_positioner::Gravity::Left, + PopupGravity::Right => xdg_positioner::Gravity::Right, + PopupGravity::TopLeft => xdg_positioner::Gravity::TopLeft, + PopupGravity::BottomLeft => xdg_positioner::Gravity::BottomLeft, + PopupGravity::TopRight => xdg_positioner::Gravity::TopRight, + PopupGravity::BottomRight => xdg_positioner::Gravity::BottomRight, + } +} + +pub(crate) fn wayland_constraint_adjustment( + adjustment: PopupConstraintAdjustment, +) -> xdg_positioner::ConstraintAdjustment { + // The flag values match the protocol bitfield, so the bits map across directly. + xdg_positioner::ConstraintAdjustment::from_bits_truncate(adjustment.bits()) +} diff --git a/crates/gpui_linux/src/linux/wayland/window.rs b/crates/gpui_linux/src/linux/wayland/window.rs index 731569a025f..675602082bb 100644 --- a/crates/gpui_linux/src/linux/wayland/window.rs +++ b/crates/gpui_linux/src/linux/wayland/window.rs @@ -1,12 +1,12 @@ use std::{ - cell::{Ref, RefCell, RefMut}, + cell::{Cell, Ref, RefCell, RefMut}, ffi::c_void, ptr::NonNull, rc::Rc, sync::Arc, }; -use collections::{FxHashSet, HashMap}; +use collections::{FxHashMap, HashMap}; use futures::channel::oneshot::Receiver; use raw_window_handle as rwh; @@ -14,10 +14,12 @@ use wayland_backend::client::ObjectId; use wayland_client::WEnum; use wayland_client::{ Proxy, - protocol::{wl_output, wl_surface}, + protocol::{wl_output, wl_seat, wl_surface}, }; use wayland_protocols::wp::viewporter::client::wp_viewport; use wayland_protocols::xdg::decoration::zv1::client::zxdg_toplevel_decoration_v1; +use wayland_protocols::xdg::shell::client::xdg_popup; +use wayland_protocols::xdg::shell::client::xdg_positioner; use wayland_protocols::xdg::shell::client::xdg_surface; use wayland_protocols::xdg::shell::client::xdg_toplevel::{self}; use wayland_protocols::{ @@ -34,8 +36,8 @@ use gpui::{ PlatformAtlas, PlatformDisplay, PlatformInput, PlatformInputHandler, PlatformWindow, Point, PromptButton, PromptLevel, RequestFrameOptions, ResizeEdge, Scene, Size, Tiling, WindowAppearance, WindowBackgroundAppearance, WindowBounds, WindowControlArea, WindowControls, - WindowDecorations, WindowKind, WindowParams, layer_shell::LayerShellNotSupportedError, px, - size, + WindowDecorations, WindowKind, WindowParams, layer_shell::LayerShellNotSupportedError, + popup::PopupOptions, px, size, }; use gpui_wgpu::{CompositorGpuHint, WgpuRenderer, WgpuSurfaceConfig, wgpu}; @@ -93,7 +95,9 @@ pub struct WaylandWindowState { surface_state: WaylandSurfaceState, acknowledged_first_configure: bool, parent: Option, - children: FxHashSet, + /// Child surfaces mapped to whether they block this window's input (dialogs + /// block, popups don't). Children are closed before this window closes. + children: FxHashMap, pub surface: wl_surface::WlSurface, app_id: Option, appearance: WindowAppearance, @@ -129,6 +133,7 @@ pub struct WaylandWindowState { pub enum WaylandSurfaceState { Xdg(WaylandXdgSurfaceState), LayerShell(WaylandLayerSurfaceState), + Popup(WaylandPopupSurfaceState), } impl WaylandSurfaceState { @@ -137,6 +142,7 @@ impl WaylandSurfaceState { globals: &Globals, params: &WindowParams, parent: Option, + popup_grab: Option<(u32, wl_seat::WlSeat)>, target_output: Option, ) -> anyhow::Result { // For layer_shell windows, create a layer surface instead of an xdg surface @@ -186,6 +192,54 @@ impl WaylandSurfaceState { })); } + if let WindowKind::AnchoredPopup(options) = ¶ms.kind { + let Some(parent) = parent.as_ref() else { + return Err(anyhow::anyhow!("popup parent window not found")); + }; + + let positioner = build_popup_positioner( + globals, + options, + params.bounds.size, + parent.window_geometry(), + ); + + let xdg_surface = globals + .wm_base + .get_xdg_surface(&surface, &globals.qh, surface.id()); + + // A layer-shell parent takes a null xdg parent and is attached via the layer + // surface. Every other surface kind has an xdg_surface to parent to directly. + let xdg_popup = if let Some(parent_layer_surface) = parent.layer_surface() { + let xdg_popup = xdg_surface.get_popup(None, &positioner, &globals.qh, surface.id()); + parent_layer_surface.get_popup(&xdg_popup); + xdg_popup + } else { + xdg_surface.get_popup( + parent.xdg_surface().as_ref(), + &positioner, + &globals.qh, + surface.id(), + ) + }; + positioner.destroy(); + + if let Some((serial, seat)) = popup_grab { + xdg_popup.grab(&seat, serial); + } + + // Non-blocking: the parent keeps its input so it can dismiss the popup on + // clicks in its own window. + parent.add_child(surface.id(), false); + + return Ok(WaylandSurfaceState::Popup(WaylandPopupSurfaceState { + xdg_surface, + xdg_popup, + options: options.clone(), + next_reposition_token: Cell::new(0), + })); + } + // All other WindowKinds result in a regular xdg surface let xdg_surface = globals .wm_base @@ -206,7 +260,7 @@ impl WaylandSurfaceState { }); if let Some(parent) = parent.as_ref() { - parent.add_child(surface.id()); + parent.add_child(surface.id(), true); } dialog @@ -246,6 +300,65 @@ pub struct WaylandLayerSurfaceState { layer_surface: zwlr_layer_surface_v1::ZwlrLayerSurfaceV1, } +pub struct WaylandPopupSurfaceState { + xdg_surface: xdg_surface::XdgSurface, + xdg_popup: xdg_popup::XdgPopup, + // Kept so the popup can be re-anchored via `xdg_popup.reposition` when resized. + options: PopupOptions, + next_reposition_token: Cell, +} + +fn build_popup_positioner( + globals: &Globals, + options: &PopupOptions, + size: Size, + parent_geometry: Bounds, +) -> xdg_positioner::XdgPositioner { + let positioner = globals.wm_base.create_positioner(&globals.qh, ()); + // A zero or negative size is a protocol error. + positioner.set_size( + f32::from(size.width).max(1.0) as i32, + f32::from(size.height).max(1.0) as i32, + ); + + // The protocol wants the anchor rect relative to the parent's window geometry, while + // `options.anchor_rect` is in gpui window coordinates (surface-local). A rect extending + // outside the geometry or with a zero size is a protocol error, so translate, then clamp + // to at least one pixel inside the geometry, pulling the origin inward at the edges. + let anchor_rect = Bounds { + origin: options.anchor_rect.origin - parent_geometry.origin, + size: options.anchor_rect.size, + }; + let one = Point::new(px(1.0), px(1.0)); + let geometry_bottom_right: Point = parent_geometry.size.into(); + let top_left = anchor_rect + .origin + .min(&(geometry_bottom_right - one)) + .max(&Point::default()); + let bottom_right = anchor_rect + .bottom_right() + .min(&geometry_bottom_right) + .max(&(top_left + one)); + let anchor_rect = Bounds::from_corners(top_left, bottom_right); + positioner.set_anchor_rect( + f32::from(anchor_rect.origin.x) as i32, + f32::from(anchor_rect.origin.y) as i32, + f32::from(anchor_rect.size.width) as i32, + f32::from(anchor_rect.size.height) as i32, + ); + + positioner.set_anchor(super::popup::wayland_anchor(options.anchor)); + positioner.set_gravity(super::popup::wayland_gravity(options.gravity)); + positioner.set_constraint_adjustment(super::popup::wayland_constraint_adjustment( + options.constraint_adjustment, + )); + positioner.set_offset( + f32::from(options.offset.x) as i32, + f32::from(options.offset.y) as i32, + ); + positioner +} + impl WaylandSurfaceState { fn ack_configure(&self, serial: u32) { match self { @@ -255,6 +368,9 @@ impl WaylandSurfaceState { WaylandSurfaceState::LayerShell(WaylandLayerSurfaceState { layer_surface, .. }) => { layer_surface.ack_configure(serial); } + WaylandSurfaceState::Popup(WaylandPopupSurfaceState { xdg_surface, .. }) => { + xdg_surface.ack_configure(serial); + } } } @@ -274,6 +390,28 @@ impl WaylandSurfaceState { } } + fn xdg_surface(&self) -> Option<&xdg_surface::XdgSurface> { + match self { + WaylandSurfaceState::Xdg(WaylandXdgSurfaceState { xdg_surface, .. }) => { + Some(xdg_surface) + } + WaylandSurfaceState::Popup(WaylandPopupSurfaceState { xdg_surface, .. }) => { + Some(xdg_surface) + } + WaylandSurfaceState::LayerShell(_) => None, + } + } + + fn layer_surface(&self) -> Option<&zwlr_layer_surface_v1::ZwlrLayerSurfaceV1> { + if let WaylandSurfaceState::LayerShell(WaylandLayerSurfaceState { layer_surface, .. }) = + self + { + Some(layer_surface) + } else { + None + } + } + fn set_geometry(&self, x: i32, y: i32, width: i32, height: i32) { match self { WaylandSurfaceState::Xdg(WaylandXdgSurfaceState { xdg_surface, .. }) => { @@ -283,6 +421,34 @@ impl WaylandSurfaceState { // cannot set window position of a layer surface layer_surface.set_size(width as u32, height as u32); } + WaylandSurfaceState::Popup(WaylandPopupSurfaceState { xdg_surface, .. }) => { + xdg_surface.set_window_geometry(x, y, width, height); + } + } + } + + // Re-anchors a mapped popup at a new size via `xdg_popup.reposition`. Repositioning an + // unmapped popup (before the first configure) is a protocol error. + fn reposition_popup( + &self, + globals: &Globals, + size: Size, + parent_geometry: Bounds, + ) { + if let WaylandSurfaceState::Popup(WaylandPopupSurfaceState { + xdg_popup, + options, + next_reposition_token, + .. + }) = self + && xdg_popup.version() >= xdg_popup::REQ_REPOSITION_SINCE + { + let token = next_reposition_token.get(); + next_reposition_token.set(token.wrapping_add(1)); + + let positioner = build_popup_positioner(globals, options, size, parent_geometry); + xdg_popup.reposition(&positioner, token); + positioner.destroy(); } } @@ -307,6 +473,15 @@ impl WaylandSurfaceState { WaylandSurfaceState::LayerShell(WaylandLayerSurfaceState { layer_surface }) => { layer_surface.destroy(); } + WaylandSurfaceState::Popup(WaylandPopupSurfaceState { + xdg_surface, + xdg_popup, + .. + }) => { + // Role object before its xdg_surface, as with the toplevel above. + xdg_popup.destroy(); + xdg_surface.destroy(); + } } } } @@ -374,7 +549,7 @@ impl WaylandWindowState { surface_state, acknowledged_first_configure: false, parent, - children: FxHashSet::default(), + children: FxHashMap::default(), surface, app_id: options.app_id, blur: None, @@ -523,11 +698,18 @@ impl WaylandWindow { params: WindowParams, appearance: WindowAppearance, parent: Option, + popup_grab: Option<(u32, wl_seat::WlSeat)>, target_output: Option, ) -> anyhow::Result<(Self, ObjectId)> { let surface = globals.compositor.create_surface(&globals.qh, ()); - let surface_state = - WaylandSurfaceState::new(&surface, &globals, ¶ms, parent.clone(), target_output)?; + let surface_state = WaylandSurfaceState::new( + &surface, + &globals, + ¶ms, + parent.clone(), + popup_grab, + target_output, + )?; if let Some(fractional_scale_manager) = globals.fractional_scale_manager.as_ref() { fractional_scale_manager.get_fractional_scale(&surface, &globals.qh, surface.id()); @@ -575,18 +757,39 @@ impl WaylandWindowStatePtr { self.state.borrow().surface_state.toplevel().cloned() } + /// The `xdg_surface` backing this window, if it has one. Used to anchor child popups. + pub fn xdg_surface(&self) -> Option { + self.state.borrow().surface_state.xdg_surface().cloned() + } + + /// The layer-shell surface backing this window, if it is one. Used to anchor child popups. + pub fn layer_surface(&self) -> Option { + self.state.borrow().surface_state.layer_surface().cloned() + } + + /// This window's xdg window geometry in surface-local coordinates. Child popup anchor + /// rectangles are relative to it, while gpui coordinates are surface-local. + pub fn window_geometry(&self) -> Bounds { + let state = self.state.borrow(); + inset_by_tiling( + state.bounds.map_origin(|_| px(0.0)), + state.inset(), + state.tiling, + ) + } + pub fn ptr_eq(&self, other: &Self) -> bool { Rc::ptr_eq(&self.state, &other.state) } - pub fn add_child(&self, child: ObjectId) { + pub fn add_child(&self, child: ObjectId, blocking: bool) { let mut state = self.state.borrow_mut(); - state.children.insert(child); + state.children.insert(child, blocking); } pub fn is_blocked(&self) -> bool { let state = self.state.borrow(); - !state.children.is_empty() + state.children.values().any(|&blocking| blocking) } pub fn frame(&self) { @@ -889,6 +1092,35 @@ impl WaylandWindowStatePtr { } } + // Returns `true` if the popup should be closed. + pub fn handle_popup_event(&self, event: xdg_popup::Event) -> bool { + match event { + // Only the size is needed, the position is the compositor's. The following + // xdg_surface.configure applies the change. + xdg_popup::Event::Configure { width, height, .. } => { + let size = if width <= 0 || height <= 0 { + None + } else { + Some(size(px(width as f32), px(height as f32))) + }; + + self.state.borrow_mut().in_progress_configure = Some(InProgressConfigure { + size, + fullscreen: false, + maximized: false, + resizing: false, + tiling: Tiling::default(), + }); + + false + } + xdg_popup::Event::PopupDone => true, + // Precedes the reposition's Configure, which does the work. The token is not needed. + xdg_popup::Event::Repositioned { .. } => false, + _ => false, + } + } + #[allow(clippy::mutable_key_type)] pub fn handle_surface_event( &self, @@ -1025,8 +1257,7 @@ impl WaylandWindowStatePtr { pub fn close(&self) { let state = self.state.borrow(); let client = state.client.get_client(); - #[allow(clippy::mutable_key_type)] - let children = state.children.clone(); + let children = state.children.keys().cloned().collect::>(); drop(state); for child in children { @@ -1192,6 +1423,23 @@ impl PlatformWindow for WaylandWindow { let state = self.borrow(); let state_ptr = self.0.clone(); + // A popup's placement is the compositor's, so a resize re-runs the positioner and the + // configure reply drives the buffer resize. Before the first configure the popup is + // unmapped and cannot reposition, but the initial positioner already carries the size. + if matches!(state.surface_state, WaylandSurfaceState::Popup(_)) { + if state.acknowledged_first_configure { + let parent_geometry = state + .parent + .as_ref() + .map(|parent| parent.window_geometry()) + .unwrap_or_default(); + state + .surface_state + .reposition_popup(&state.globals, size, parent_geometry); + } + return; + } + // Keep window geometry consistent with configure handling. On Wayland, window geometry is // surface-local: resizing should not attempt to translate the window; the compositor // controls placement. We also account for client-side decoration insets and tiling. diff --git a/crates/gpui_linux/src/linux/x11/window.rs b/crates/gpui_linux/src/linux/x11/window.rs index d2edb72408b..a459ed03b5d 100644 --- a/crates/gpui_linux/src/linux/x11/window.rs +++ b/crates/gpui_linux/src/linux/x11/window.rs @@ -7,7 +7,7 @@ use gpui::{ Pixels, PlatformAtlas, PlatformDisplay, PlatformInput, PlatformInputHandler, PlatformWindow, Point, PromptButton, PromptLevel, RequestFrameOptions, ResizeEdge, ScaledPixels, Scene, Size, Tiling, WindowAppearance, WindowBackgroundAppearance, WindowBounds, WindowControlArea, - WindowDecorations, WindowKind, WindowParams, px, + WindowDecorations, WindowKind, WindowParams, popup::PopupNotSupportedError, px, }; use gpui_wgpu::{CompositorGpuHint, WgpuRenderer, WgpuSurfaceConfig}; @@ -428,6 +428,12 @@ impl X11WindowState { supports_xinput_gestures: bool, is_bgr: bool, ) -> anyhow::Result { + // Native popups are not implemented on X11 yet. Rejecting lets callers fall back to + // gpui's in-window popovers. + if let WindowKind::AnchoredPopup(_) = params.kind { + return Err(PopupNotSupportedError.into()); + } + let x_screen_index = params .display_id .map_or(x_main_screen_index, |did| u64::from(did) as usize); diff --git a/crates/gpui_macos/src/platform.rs b/crates/gpui_macos/src/platform.rs index 8266cade4d2..eed9e90bcd3 100644 --- a/crates/gpui_macos/src/platform.rs +++ b/crates/gpui_macos/src/platform.rs @@ -31,7 +31,8 @@ use gpui::{ Action, AnyWindowHandle, BackgroundExecutor, ClipboardItem, CursorStyle, ForegroundExecutor, KeyContext, Keymap, Menu, MenuItem, OsMenu, OwnedMenu, PathPromptOptions, Platform, PlatformDisplay, PlatformKeyboardLayout, PlatformKeyboardMapper, PlatformTextSystem, - PlatformWindow, Result, SystemMenuType, Task, ThermalState, WindowAppearance, WindowParams, + PlatformWindow, Result, SystemMenuType, Task, ThermalState, WindowAppearance, WindowKind, + WindowParams, popup::PopupNotSupportedError, }; use gpui_util::{ResultExt, new_std_command}; use itertools::Itertools; @@ -640,6 +641,12 @@ impl Platform for MacPlatform { handle: AnyWindowHandle, options: WindowParams, ) -> Result> { + // Native popups are not implemented on macOS yet. Rejecting lets callers fall back to + // gpui's in-window popovers. + if let WindowKind::AnchoredPopup(_) = options.kind { + return Err(PopupNotSupportedError.into()); + } + let (cursor_visible, foreground_executor, background_executor, renderer_context) = { let guard = self.0.lock(); ( diff --git a/crates/gpui_macos/src/window.rs b/crates/gpui_macos/src/window.rs index d3d87608253..6221b141610 100644 --- a/crates/gpui_macos/src/window.rs +++ b/crates/gpui_macos/src/window.rs @@ -795,7 +795,9 @@ impl MacWindow { WindowKind::Normal => { msg_send![WINDOW_CLASS, alloc] } - WindowKind::PopUp => { + // `AnchoredPopup` is rejected in `MacPlatform::open_window`, grouped here only + // for exhaustiveness. + WindowKind::PopUp | WindowKind::AnchoredPopup(_) => { style_mask |= NSWindowStyleMaskNonactivatingPanel; msg_send![PANEL_CLASS, alloc] } @@ -988,7 +990,9 @@ impl MacWindow { let _: () = msg_send![native_window, setTabbingIdentifier:nil]; } } - WindowKind::PopUp => { + // `AnchoredPopup` is rejected in `MacPlatform::open_window`, grouped here only + // for exhaustiveness. + WindowKind::PopUp | WindowKind::AnchoredPopup(_) => { // Use a tracking area to allow receiving MouseMoved events even when // the window or application aren't active, which is often the case // e.g. for notification windows. diff --git a/crates/gpui_web/src/platform.rs b/crates/gpui_web/src/platform.rs index fecc39f368d..c39723ea5b2 100644 --- a/crates/gpui_web/src/platform.rs +++ b/crates/gpui_web/src/platform.rs @@ -8,7 +8,7 @@ use gpui::{ Action, AnyWindowHandle, BackgroundExecutor, ClipboardItem, CursorStyle, DummyKeyboardMapper, ForegroundExecutor, Keymap, Menu, MenuItem, PathPromptOptions, Platform, PlatformDisplay, PlatformKeyboardLayout, PlatformKeyboardMapper, PlatformTextSystem, PlatformWindow, Task, - ThermalState, WindowAppearance, WindowParams, + ThermalState, WindowAppearance, WindowKind, WindowParams, popup::PopupNotSupportedError, }; use gpui_wgpu::WgpuContext; use std::{ @@ -166,6 +166,12 @@ impl Platform for WebPlatform { handle: AnyWindowHandle, params: WindowParams, ) -> anyhow::Result> { + // Native popups are not implemented on the web yet. Rejecting lets callers fall back to + // gpui's in-window popovers. + if let WindowKind::AnchoredPopup(_) = params.kind { + return Err(PopupNotSupportedError.into()); + } + let context_ref = self.wgpu_context.borrow(); let context = context_ref.as_ref().ok_or_else(|| { anyhow::anyhow!("WebGPU context not initialized. Was Platform::run() called?") diff --git a/crates/gpui_windows/src/window.rs b/crates/gpui_windows/src/window.rs index 547ee7a9dde..76075a66b1c 100644 --- a/crates/gpui_windows/src/window.rs +++ b/crates/gpui_windows/src/window.rs @@ -411,6 +411,12 @@ impl WindowsWindow { params: WindowParams, creation_info: WindowCreationInfo, ) -> Result { + // Native popups are not implemented on Windows yet. Rejecting lets callers fall back to + // gpui's in-window popovers. + if let WindowKind::AnchoredPopup(_) = params.kind { + return Err(popup::PopupNotSupportedError.into()); + } + let WindowCreationInfo { icon, executor, From 23c0080d1dd5e8e550ede91c0d573658e5aabe76 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 8 Jul 2026 15:27:01 -0400 Subject: [PATCH 12/61] gpui_windows: Check composition attribute lookup (#60122) Checks that SetWindowCompositionAttribute was found before transmuting and calling the function pointer, making the composition setup a no-op when the undocumented export is unavailable. Release Notes: - N/A --- crates/gpui_windows/src/window.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/gpui_windows/src/window.rs b/crates/gpui_windows/src/window.rs index 76075a66b1c..5a4129b493c 100644 --- a/crates/gpui_windows/src/window.rs +++ b/crates/gpui_windows/src/window.rs @@ -1545,8 +1545,12 @@ fn set_window_composition_attribute(hwnd: HWND, color: Option, state: u32 .log_err() { let func_name = PCSTR::from_raw(c"SetWindowCompositionAttribute".as_ptr() as *const u8); + let Some(raw_set_window_composition_attribute) = GetProcAddress(user32, func_name) + else { + return; + }; let set_window_composition_attribute: SetWindowCompositionAttributeType = - std::mem::transmute(GetProcAddress(user32, func_name)); + std::mem::transmute(raw_set_window_composition_attribute); let mut color = color.unwrap_or_default(); let is_acrylic = state == 4; if is_acrylic && color.3 == 0 { From 664b7ecb40b83fa728bc40ef8f0385e35ea01287 Mon Sep 17 00:00:00 2001 From: Philipp Schaffrath Date: Wed, 8 Jul 2026 22:27:47 +0200 Subject: [PATCH 13/61] gpui: Fix hover state not clearing when mouse leaves window (#60275) # Objective Fix two gaps in element hover tracking at window boundaries. Hover was only re-evaluated on `MouseMove`, so when the pointer left the window no event fired `on_hover(false)` and the element stayed hovered. Symmetrically on Wayland, no `Motion` follows `Enter` until the pointer moves again, so hover was not established at the entry pixel. Both cases are easy to miss since most hover-styled elements don't sit flush against the window edge, but they surfaced while implementing layer_shell popups with input_regions, which should close when stop hovering. ## Solution The hover compare-and-fire logic in `div` is refactored into a shared `update_hover` closure, and a second listener on `MouseExitEvent` clears hover when the pointer leaves the window. It clears unconditionally because `MouseExited` doesn't update the tracked mouse position, so a hit test during that dispatch would still report the element as hovered. On Wayland, a `MouseMove` is synthesized at the entry position on `wl_pointer.enter`, mirroring the `MouseExited` already dispatched on `Leave`. ## Testing Tested manually on Wayland/Linux: hover on a window-edge element clears when the pointer leaves the window, and hover is established immediately when the pointer enters a surface with an element under the entry pixel. Not tested on other platforms. The `div` change relies on each platform's existing `MouseExited` dispatch: macOS and X11 emit it, so they get the exit fix too. Windows never dispatches `MouseExited` (`WM_MOUSELEAVE` only flips the window-level hover flag), so the stuck-hover case might remain there, unchanged from before. Before: https://github.com/user-attachments/assets/6af83bb3-de9d-40e2-a64c-bdefc98fc96d After: https://github.com/user-attachments/assets/721a9b5c-108a-499f-867e-da111836a34a Here is an example application to test this: ```rs #![cfg_attr(target_family = "wasm", no_main)] use gpui::{ App, Bounds, Context, Window, WindowBounds, WindowOptions, div, prelude::*, px, rgb, size, }; use gpui_platform::application; struct HoverExit { hovered: bool, } impl Render for HoverExit { fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { // Fills the whole window so its edge is the window edge: moving the mouse // out of the window is what exercises the MouseExited path. div() .id("hover-exit") .size_full() .flex() .justify_center() .items_center() .text_xl() .text_color(rgb(0xffffff)) .bg(if self.hovered { rgb(0x585f58) } else { rgb(0x505050) }) .child(if self.hovered { "HOVERED" } else { "not hovered" }) .on_hover(cx.listener(|this, hovered, _, cx| { this.hovered = *hovered; cx.notify(); })) } } fn run_example() { application().run(|cx: &mut App| { let bounds = Bounds::centered(None, size(px(240.), px(160.0)), cx); cx.open_window( WindowOptions { window_bounds: Some(WindowBounds::Windowed(bounds)), app_id: Some("gpui-hover-exit".to_string()), ..Default::default() }, |_, cx| cx.new(|_| HoverExit { hovered: false }), ) .unwrap(); cx.activate(true); }); } #[cfg(not(target_family = "wasm"))] fn main() { run_example(); } #[cfg(target_family = "wasm")] #[wasm_bindgen::prelude::wasm_bindgen(start)] pub fn start() { gpui_platform::web_init(); run_example(); } ``` ## 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 adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [ ] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable --- Release Notes: - Fixed element hover state not clearing when the mouse leaves the window --- crates/gpui/src/elements/div.rs | 34 +++++++++++++------ crates/gpui_linux/src/linux/wayland/client.rs | 11 +++++- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/crates/gpui/src/elements/div.rs b/crates/gpui/src/elements/div.rs index ebbce8f84c9..04692c86a55 100644 --- a/crates/gpui/src/elements/div.rs +++ b/crates/gpui/src/elements/div.rs @@ -2860,7 +2860,6 @@ impl Interactivity { } if let Some(hover_listener) = self.hover_listener.take() { - let hitbox = hitbox.clone(); let was_hovered = element_state .hover_listener_state .get_or_insert_with(Default::default) @@ -2869,22 +2868,35 @@ impl Interactivity { .pending_mouse_down .get_or_insert_with(Default::default) .clone(); - - window.on_mouse_event(move |_: &MouseMoveEvent, phase, window, cx| { - if phase != DispatchPhase::Bubble { - return; - } - let is_hovered = has_mouse_down.borrow().is_none() - && !cx.has_active_drag() - && hitbox.is_hovered(window); + let hover_listener = Rc::new(hover_listener); + let update_hover = move |is_hovered: bool, window: &mut Window, cx: &mut App| { let mut was_hovered = was_hovered.borrow_mut(); - if is_hovered != *was_hovered { *was_hovered = is_hovered; drop(was_hovered); - hover_listener(&is_hovered, window, cx); } + }; + + window.on_mouse_event({ + let update_hover = update_hover.clone(); + let hitbox = hitbox.clone(); + move |_: &MouseMoveEvent, phase, window, cx| { + if phase == DispatchPhase::Bubble { + let is_hovered = has_mouse_down.borrow().is_none() + && !cx.has_active_drag() + && hitbox.is_hovered(window); + update_hover(is_hovered, window, cx); + } + } + }); + + // The pointer can leave the window without a final MouseMove, so also + // clear hover on MouseExited. + window.on_mouse_event(move |_: &MouseExitEvent, phase, window, cx| { + if phase == DispatchPhase::Bubble { + update_hover(false, window, cx); + } }); } diff --git a/crates/gpui_linux/src/linux/wayland/client.rs b/crates/gpui_linux/src/linux/wayland/client.rs index 9522ebb8f0d..a4cc9fde9f1 100644 --- a/crates/gpui_linux/src/linux/wayland/client.rs +++ b/crates/gpui_linux/src/linux/wayland/client.rs @@ -1884,8 +1884,9 @@ impl Dispatch for WaylandClientStatePtr { surface_y, .. } => { + let position = point(px(surface_x as f32), px(surface_y as f32)); state.serial_tracker.update(SerialKind::MouseEnter, serial); - state.mouse_location = Some(point(px(surface_x as f32), px(surface_y as f32))); + state.mouse_location = Some(position); state.button_pressed = None; if let Some(window) = get_window(&mut state, &surface.id()) { @@ -1908,8 +1909,16 @@ impl Dispatch for WaylandClientStatePtr { ); } } + let modifiers = state.modifiers; drop(state); window.set_hovered(true); + // No Motion follows Enter unless the pointer keeps moving, so synthesize + // a MouseMove to establish hover at the entry position. + window.handle_input(PlatformInput::MouseMove(MouseMoveEvent { + position, + pressed_button: None, + modifiers, + })); } } wl_pointer::Event::Leave { .. } => { From dd68454633ac9fea8b42115ae09568e7395831e2 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 8 Jul 2026 16:37:57 -0400 Subject: [PATCH 14/61] Update wayland-backend to fix Wayland file dialog crash (#60621) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Wayland, closing a window in the brief gap between requesting a portal file dialog and ashpd exporting the window's surface (used to parent the dialog) crashed Zed with `Unknown opcode 0 for object @0` ([ZED-9KB](https://zed-dev.sentry.io/issues/7568720776/)). When the export request fails because the surface is already dead, wayland-scanner's generated code silently returns an inert proxy, and ashpd's `Drop` impl later sends `destroy` on it — which wayland-backend 0.3.11 answers with a panic, because it looks up the request opcode before checking whether the object is null. wayland-backend 0.3.15 fixes this ([Smithay/wayland-rs#890](https://github.com/Smithay/wayland-rs/issues/890)) by returning an error instead, which the generated destructor discards, so the drop becomes a harmless no-op. This bumps the lockfile to 0.3.15 and raises the version floor in `gpui_linux` so the fix can't silently regress via a fresh lockfile. Closes FR-100 Release Notes: - Fixed a crash on Linux (Wayland) when a window was closed just as a file dialog was being opened. --- Cargo.lock | 12 ++++++------ crates/gpui_linux/Cargo.toml | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3f71a7000d7..f65225de1bd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5344,9 +5344,9 @@ dependencies = [ [[package]] name = "dlib" -version = "0.5.2" +version = "0.5.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "330c60081dcc4c72131f8eb70510f1ac07223e5d4163db481a04a0befcffa412" +checksum = "ab8ecd87370524b461f8557c119c405552c396ed91fc0a8eec68679eab26f94a" dependencies = [ "libloading", ] @@ -20843,9 +20843,9 @@ dependencies = [ [[package]] name = "wayland-backend" -version = "0.3.11" +version = "0.3.15" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "673a33c33048a5ade91a6b139580fa174e19fb0d23f396dca9fa15f2e1e49b35" +checksum = "2857dd20b54e916ec7253b3d6b4d5c4d7d4ca2c33c2e11c6c76a99bd8744755d" dependencies = [ "cc", "downcast-rs", @@ -20929,9 +20929,9 @@ dependencies = [ [[package]] name = "wayland-sys" -version = "0.31.7" +version = "0.31.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "34949b42822155826b41db8e5d0c1be3a2bd296c747577a43a3e6daefc296142" +checksum = "d8eab23fefc9e41f8e841df4a9c707e8a8c4ed26e944ef69297184de2785e3be" dependencies = [ "dlib", "log", diff --git a/crates/gpui_linux/Cargo.toml b/crates/gpui_linux/Cargo.toml index a1e0531f3d5..262da8d4dff 100644 --- a/crates/gpui_linux/Cargo.toml +++ b/crates/gpui_linux/Cargo.toml @@ -96,7 +96,7 @@ scap = { workspace = true, optional = true } # Wayland calloop-wayland-source = { version = "0.4.1", optional = true } -wayland-backend = { version = "0.3.3", features = [ +wayland-backend = { version = "0.3.15", features = [ "client_system", "dlopen", ], optional = true } From 10504e3ce18bb1d2444baf1047fa4ea9835ff1cc Mon Sep 17 00:00:00 2001 From: morgankrey Date: Wed, 8 Jul 2026 15:54:51 -0500 Subject: [PATCH 15/61] Improve docs AI readiness (#59577) Context This PR makes the Zed docs easier for AI tools, search crawlers, and users to consume without changing the visible docs content. The current production docs are primarily optimized for browser navigation. They do not expose first-class Markdown URLs, an `llms.txt` index, page-level copy affordances, or machine-readable freshness metadata that let users and agents grab clean, current page content. Changes - Generate Markdown copies for docs pages during the mdBook postprocess step, including `/docs/index.md` as an alias for Getting Started. - Generate `/docs/llms.txt` from the mdBook chapter list, grouped by `SUMMARY.md` sections and annotated with page frontmatter descriptions. - Generate `/docs/sitemap.xml` with `` values for every docs page. - Emit machine-readable freshness metadata in HTML via `last-modified` and `article:modified_time` meta tags. - Generate Cloudflare Pages `_redirects` for `.html`, extensionless, and `.md` redirect variants, with channel-aware docs destinations. - Add discovery hints for agents and crawlers: `rel="llms.txt"`, `rel="alternate" type="text/markdown"`, and a short generated `llms.txt` directive in copied Markdown pages. - Update the docs proxy so `Accept: text/markdown`, `/docs.md`, and direct `.md` requests can resolve to the generated Markdown artifacts. - Move primary docs content earlier in the HTML source while preserving the visible layout, so crawlers and agent scorers encounter the article before sidebar chrome. - Move the existing copy-as-Markdown control from the top navigation into the page-title row, using the generated Markdown alternate link as the source of truth. - Split AI-discovery artifact generation out of `docs_preprocessor/src/main.rs` into a focused module. Best Practices Adopted - Use `llms.txt` as a concise navigation index, not a dump of full page content. - Link to absolute, canonical Markdown URLs from `llms.txt`. - Preserve the docs hierarchy in `llms.txt` instead of emitting a flat sitemap-like list. - Include short per-link descriptions from existing metadata rather than inventing summaries. - Keep `llms.txt`, Markdown copies, sitemap data, redirects, and freshness metadata generated from the same mdBook source to avoid drift. - Advertise Markdown alternates with standard HTML metadata and same-origin URLs. - Support both explicit Markdown URLs and content negotiation for clients that prefer Markdown. - Keep browser copy behavior pointed at generated Markdown alternate links instead of duplicating route inference in JavaScript. - Keep the copy-as-Markdown affordance in the page title row without duplicating header chrome controls. Validation - `cargo check -p docs_preprocessor` - `cargo test -p docs_preprocessor` - `./script/clippy -p docs_preprocessor` - `mdbook build ./docs --dest-dir=../target/deploy/docs/` - `node --check docs/theme/plugins.js` - `pnpm dlx prettier@3.5.0 docs/theme/plugins.js --check` - `git diff --check` - Local artifact checks confirmed generated Markdown pages, `llms.txt`, `sitemap.xml` lastmod values, HTML freshness metadata, Markdown alternate links, redirect targets, and preprocessed action/keybinding tags resolve as expected. - Worker URL rewrite mock covered `/docs/`, `/docs.md`, `/docs/index.md`, extensionless docs routes, `.html` routes, `/docs/llms.txt`, and `/docs/sitemap.xml`. - High-effort adversarial subagent review found blockers around channel-aware redirects, shallow-checkout date fallback, file size, duplicated Markdown path inference, and process spawning. Those were addressed. Remaining Notes - Local `python -m http.server` does not emulate Cloudflare Pages pretty URLs, `_redirects`, or the docs-proxy Worker, so full local `afdocs` still cannot prove content negotiation end to end. - Existing production remains unchanged until this PR is deployed through the docs workflow. - Production baseline `npx afdocs check https://zed.dev/docs/ --fixes --verbose` still reports the original failures before this PR is deployed: 12 passed, 8 failed, 3 skipped. Release Notes: - Improved docs AI-readiness by adding machine-readable discovery, Markdown access, and freshness metadata. --------- Co-authored-by: Katie Geer Co-authored-by: Ben Kunkle --- .cloudflare/docs-proxy/src/worker.js | 41 ++ crates/docs_preprocessor/Cargo.toml | 2 +- crates/docs_preprocessor/src/ai_discovery.rs | 549 +++++++++++++++++++ crates/docs_preprocessor/src/main.rs | 122 ++--- crates/docs_preprocessor/src/tests.rs | 61 +++ docs/book.toml | 4 +- docs/theme/css/chrome.css | 1 + docs/theme/css/general.css | 1 + docs/theme/index.hbs | 182 +++--- docs/theme/plugins.css | 64 +++ docs/theme/plugins.js | 173 ++++-- 11 files changed, 982 insertions(+), 218 deletions(-) create mode 100644 crates/docs_preprocessor/src/ai_discovery.rs create mode 100644 crates/docs_preprocessor/src/tests.rs diff --git a/.cloudflare/docs-proxy/src/worker.js b/.cloudflare/docs-proxy/src/worker.js index 08b0265fafb..99fca0c7116 100644 --- a/.cloudflare/docs-proxy/src/worker.js +++ b/.cloudflare/docs-proxy/src/worker.js @@ -1,6 +1,11 @@ export default { async fetch(request, _env, _ctx) { const url = new URL(request.url); + const acceptHeader = request.headers.get("Accept") || ""; + const wantsMarkdown = acceptHeader + .split(",") + .map((mediaType) => mediaType.split(";")[0].trim().toLowerCase()) + .includes("text/markdown"); if (url.pathname === "/docs/nightly") { url.hostname = "docs-nightly.pages.dev"; @@ -18,6 +23,14 @@ export default { url.hostname = "docs-anw.pages.dev"; } + if (url.pathname === "/docs.md") { + url.pathname = "/docs/getting-started.md"; + } + + if (wantsMarkdown) { + url.pathname = markdownPathFor(url.pathname); + } + let res = await fetch(url, request); if (res.status === 404) { @@ -27,3 +40,31 @@ export default { return res; }, }; + +function markdownPathFor(pathname) { + if (pathname === "/docs" || pathname === "/docs/") { + return "/docs/getting-started.md"; + } + + if (pathname.endsWith("/index.md")) { + return pathname.replace(/\/index\.md$/, "/getting-started.md"); + } + + if (pathname.endsWith(".md")) { + return pathname; + } + + if (pathname.endsWith(".html")) { + return pathname.replace(/\.html$/, ".md"); + } + + if (pathname.split("/").pop().includes(".")) { + return pathname; + } + + if (pathname.endsWith("/")) { + return `${pathname}getting-started.md`; + } + + return `${pathname}.md`; +} diff --git a/crates/docs_preprocessor/Cargo.toml b/crates/docs_preprocessor/Cargo.toml index 87e5110ff52..a422d2811db 100644 --- a/crates/docs_preprocessor/Cargo.toml +++ b/crates/docs_preprocessor/Cargo.toml @@ -27,4 +27,4 @@ workspace = true [[bin]] name = "docs_preprocessor" -path = "src/main.rs" \ No newline at end of file +path = "src/main.rs" diff --git a/crates/docs_preprocessor/src/ai_discovery.rs b/crates/docs_preprocessor/src/ai_discovery.rs new file mode 100644 index 00000000000..4f3f9195827 --- /dev/null +++ b/crates/docs_preprocessor/src/ai_discovery.rs @@ -0,0 +1,549 @@ +use anyhow::{Context, Result}; +use mdbook::BookItem; +use mdbook::book::Book; +use regex::Regex; +use std::collections::HashMap; +use std::path::{Path, PathBuf}; +use std::sync::OnceLock; + +use crate::FRONT_MATTER_COMMENT; + +#[derive(Debug)] +pub(crate) struct DocsPage { + section: String, + title: String, + description: Option, + pub(crate) source_path: PathBuf, + content: String, +} + +pub(crate) fn write_ai_discovery_artifacts( + pages: &[DocsPage], + destination: &Path, + site_url: &str, +) -> Result<()> { + copy_markdown_sources(destination, site_url, pages)?; + write_llms_txt(destination, site_url, pages)?; + write_sitemap_xml(destination, site_url, pages)?; + Ok(()) +} + +pub(crate) fn docs_pages(book: &Book) -> Result> { + let mut pages = Vec::new(); + let mut section = "Docs".to_string(); + for item in book.iter() { + let BookItem::Chapter(chapter) = item else { + if let BookItem::PartTitle(part_title) = item { + section.clone_from(part_title); + } + continue; + }; + let Some(source_path) = chapter.source_path.as_ref() else { + continue; + }; + if source_path == Path::new("SUMMARY.md") { + continue; + } + pages.push(DocsPage { + section: section.clone(), + title: chapter.name.clone(), + description: docs_page_description(&chapter.content), + source_path: source_path.clone(), + content: chapter.content.clone(), + }); + } + Ok(pages) +} + +fn copy_markdown_sources(destination: &Path, site_url: &str, pages: &[DocsPage]) -> Result<()> { + for page in pages { + let destination = destination.join(&page.source_path); + if let Some(parent) = destination.parent() { + std::fs::create_dir_all(parent).with_context(|| { + format!("failed to create markdown destination {}", parent.display()) + })?; + } + let contents = rewrite_docs_links(&markdown_source_contents(&page.content), site_url); + std::fs::write( + &destination, + add_llms_markdown_directive(&contents, site_url), + ) + .with_context(|| { + format!( + "failed to write markdown page {} to {}", + page.source_path.display(), + destination.display() + ) + })?; + } + let getting_started = destination.join("getting-started.md"); + if getting_started.exists() { + std::fs::copy(&getting_started, destination.join("index.md")) + .context("failed to write index.md markdown alias")?; + } + Ok(()) +} + +fn markdown_source_contents(contents: &str) -> String { + front_matter_comment_regex() + .replace(contents, "") + .trim_start() + .to_string() +} + +fn docs_page_description(contents: &str) -> Option { + docs_page_metadata(contents).and_then(|metadata| { + metadata + .get("description") + .map(|description| { + description + .trim() + .trim_matches('"') + .split_whitespace() + .collect::>() + .join(" ") + }) + .filter(|description| !description.is_empty()) + }) +} + +fn docs_page_metadata(contents: &str) -> Option> { + let captures = front_matter_comment_regex().captures(contents)?; + serde_json::from_str(&captures[1]).ok() +} + +fn front_matter_comment_regex() -> &'static Regex { + static FRONT_MATTER_COMMENT_REGEX: OnceLock = OnceLock::new(); + FRONT_MATTER_COMMENT_REGEX + .get_or_init(|| Regex::new(&FRONT_MATTER_COMMENT.replace("{}", "([^\\n]*)")).unwrap()) +} + +fn write_llms_txt(destination: &Path, site_url: &str, pages: &[DocsPage]) -> Result<()> { + let mut contents = String::new(); + contents.push_str("# Zed Docs\n\n"); + contents.push_str( + "> Official Zed documentation index with links to Markdown versions of each docs page.\n\n", + ); + contents.push_str( + "Use these links for concise Markdown copies of Zed documentation pages. Each linked page mirrors the corresponding `/docs/*.html` page without site navigation or styling.\n\n", + ); + let mut current_section = None; + for page in pages { + if current_section != Some(page.section.as_str()) { + if current_section.is_some() { + contents.push('\n'); + } + contents.push_str("## "); + contents.push_str(&markdown_text(&page.section)); + contents.push_str("\n\n"); + current_section = Some(page.section.as_str()); + } + contents.push_str("- ["); + contents.push_str(&markdown_text(&page.title)); + contents.push_str("]("); + contents.push_str(&absolute_docs_url(site_url, &page.source_path)); + contents.push(')'); + if let Some(description) = &page.description { + contents.push_str(": "); + contents.push_str(&markdown_text(description)); + } + contents.push('\n'); + } + std::fs::write(destination.join("llms.txt"), contents).context("failed to write llms.txt")?; + Ok(()) +} + +fn markdown_text(text: &str) -> String { + text.replace('\\', "\\\\") + .replace('[', "\\[") + .replace(']', "\\]") +} + +fn write_sitemap_xml(destination: &Path, site_url: &str, pages: &[DocsPage]) -> Result<()> { + let mut contents = String::new(); + contents.push_str("\n"); + contents.push_str("\n"); + for page in pages { + contents.push_str(" "); + contents.push_str(&xml_escape(&absolute_docs_url( + site_url, + &page.source_path.with_extension("html"), + ))); + contents.push_str(""); + contents.push_str("\n"); + } + contents.push_str("\n"); + std::fs::write(destination.join("sitemap.xml"), contents) + .context("failed to write sitemap.xml")?; + Ok(()) +} + +pub(crate) fn write_pages_redirects( + destination: &Path, + redirects: &[(String, String)], + site_url: &str, +) -> Result<()> { + let Some(deploy_root) = destination.parent() else { + return Ok(()); + }; + let mut contents = String::new(); + for (source, destination) in redirects { + write_redirect_line( + &mut contents, + &docs_path("/docs/", source), + &redirect_destination(site_url, destination), + ); + if let Some(extensionless_source) = strip_html_suffix(source) { + write_redirect_line( + &mut contents, + &docs_path("/docs/", &extensionless_source), + &redirect_destination( + site_url, + &strip_html_suffix(destination).unwrap_or_else(|| destination.to_string()), + ), + ); + } + if let Some(markdown_source) = html_path_to_markdown(source) { + if let Some(markdown_destination) = html_path_to_markdown(destination) { + write_redirect_line( + &mut contents, + &docs_path("/docs/", &markdown_source), + &redirect_destination(site_url, &markdown_destination), + ); + } + } + } + std::fs::write(deploy_root.join("_redirects"), contents) + .context("failed to write Cloudflare Pages _redirects")?; + Ok(()) +} + +pub(crate) fn write_markdown_redirect_aliases( + destination: &Path, + redirects: &[(String, String)], + site_url: &str, +) -> Result<()> { + for (source, redirect_destination_path) in redirects { + let Some(source_markdown) = html_path_to_markdown(source) else { + continue; + }; + let Some(destination_markdown) = html_path_to_markdown(redirect_destination_path) else { + continue; + }; + let source_markdown = destination.join(source_markdown.trim_start_matches('/')); + let destination_markdown = + destination.join(destination_markdown.trim_start_matches("/docs/")); + if !destination_markdown.exists() { + continue; + } + if let Some(parent) = source_markdown.parent() { + std::fs::create_dir_all(parent).with_context(|| { + format!( + "failed to create markdown alias directory {}", + parent.display() + ) + })?; + } + let contents = format!( + "# Moved\n\n> For the complete documentation index and Markdown links, see [llms.txt]({}).\n\nThis page moved to [the current docs page]({}).\n", + docs_url(site_url, Path::new("llms.txt")), + html_path_to_markdown(redirect_destination_path) + .map(|path| redirect_destination(site_url, &path)) + .unwrap_or_else(|| redirect_destination(site_url, redirect_destination_path)) + ); + std::fs::write(&source_markdown, contents).with_context(|| { + format!( + "failed to write markdown redirect alias from {} to {}", + redirect_destination_path, + source_markdown.display() + ) + })?; + } + Ok(()) +} + +fn write_redirect_line(contents: &mut String, source: &str, destination: &str) { + contents.push_str(source); + contents.push(' '); + contents.push_str(destination); + contents.push_str(" 301\n"); +} + +fn docs_path(site_url: &str, path: &str) -> String { + docs_url(site_url, Path::new(path.trim_start_matches('/'))) +} + +fn redirect_destination(site_url: &str, destination: &str) -> String { + if let Some(path) = destination.strip_prefix("/docs/") { + docs_url(site_url, Path::new(path)) + } else if destination == "/docs" { + docs_url(site_url, Path::new("")) + } else { + destination.to_string() + } +} + +fn strip_html_suffix(path: &str) -> Option { + let (path, fragment) = split_fragment(path); + let path = path.strip_suffix(".html")?; + Some(format!("{path}{fragment}")) +} + +fn html_path_to_markdown(path: &str) -> Option { + let (path, fragment) = split_fragment(path); + if !path.starts_with("/docs/") && path != "/docs" && !path.ends_with(".html") { + return None; + } + let markdown_path = path.strip_suffix(".html").unwrap_or(path); + Some(format!("{markdown_path}.md{fragment}")) +} + +fn split_fragment(path: &str) -> (&str, &str) { + match path.find('#') { + Some(index) => (&path[..index], &path[index..]), + None => (path, ""), + } +} + +pub(crate) fn rewrite_docs_links(contents: &str, site_url: &str) -> String { + const STABLE_DOCS_PREFIX: &str = "https://zed.dev/docs/"; + let channel_docs_prefix = absolute_docs_url(site_url, Path::new("")); + if channel_docs_prefix == STABLE_DOCS_PREFIX { + return contents.to_string(); + } + + let mut output = String::with_capacity(contents.len()); + let mut remaining = contents; + while let Some(index) = remaining.find(STABLE_DOCS_PREFIX) { + output.push_str(&remaining[..index]); + let after_prefix = &remaining[index + STABLE_DOCS_PREFIX.len()..]; + if after_prefix.starts_with("preview/") || after_prefix.starts_with("nightly/") { + output.push_str(STABLE_DOCS_PREFIX); + } else { + output.push_str(&channel_docs_prefix); + } + remaining = after_prefix; + } + output.push_str(remaining); + output +} + +pub(crate) fn add_markdown_alternate_link( + contents: &str, + html_file: &Path, + root_dir: &Path, + site_url: &str, +) -> String { + let Ok(relative_path) = html_file.strip_prefix(root_dir) else { + return contents.to_string(); + }; + let markdown_path = relative_path.with_extension("md"); + if !root_dir.join(&markdown_path).exists() { + return contents.to_string(); + } + let markdown_url = docs_url(site_url, &markdown_path); + let link = format!( + " \n", + markdown_url + ); + contents.replacen("", &(link + " "), 1) +} + +fn add_llms_markdown_directive(contents: &str, site_url: &str) -> String { + let directive = format!( + "> For the complete documentation index and Markdown links, see [llms.txt]({}).\n\n", + docs_url(site_url, Path::new("llms.txt")), + ); + if let Some(rest) = contents.strip_prefix("---\n") { + if let Some(frontmatter_end) = rest.find("\n---\n") { + let split_at = "---\n".len() + frontmatter_end + "\n---\n".len(); + let mut output = String::with_capacity(contents.len() + directive.len()); + output.push_str(&contents[..split_at]); + output.push('\n'); + output.push_str(&directive); + output.push_str(&contents[split_at..]); + return output; + } + } + + let mut output = String::with_capacity(contents.len() + directive.len()); + output.push_str(&directive); + output.push_str(contents); + output +} + +fn docs_url(site_url: &str, path: &Path) -> String { + let mut url = site_url.to_string(); + if !url.ends_with('/') { + url.push('/'); + } + url.push_str(&path.to_string_lossy().replace('\\', "/")); + url +} + +fn absolute_docs_url(site_url: &str, path: &Path) -> String { + let url = docs_url(site_url, path); + if url.starts_with("http://") || url.starts_with("https://") { + url + } else { + format!("https://zed.dev{}", url) + } +} + +fn xml_escape(value: &str) -> String { + value + .replace('&', "&") + .replace('<', "<") + .replace('>', ">") + .replace('"', """) + .replace('\'', "'") +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_add_llms_markdown_directive_inserts_after_frontmatter() { + let contents = "---\ntitle: Example\n---\n# Example\n"; + let output = add_llms_markdown_directive(contents, "/docs/"); + + assert!(output.starts_with("---\ntitle: Example\n---\n\n")); + assert!(output.contains( + "> For the complete documentation index and Markdown links, see [llms.txt](/docs/llms.txt)." + )); + } + + #[test] + fn test_redirect_destination_uses_channel_site_url_for_docs_paths() { + assert_eq!( + redirect_destination("/docs/preview/", "/docs/ai/overview.html"), + "/docs/preview/ai/overview.html" + ); + assert_eq!( + redirect_destination("/docs/preview/", "/community-links"), + "/community-links" + ); + } + + #[test] + fn test_rewrite_docs_links_uses_channel_site_url() { + assert_eq!( + rewrite_docs_links( + "See [Code Actions](https://zed.dev/docs/configuring-languages#code-actions) and [Preview](https://zed.dev/docs/preview/ai/overview.html).", + "/docs/preview/" + ), + "See [Code Actions](https://zed.dev/docs/preview/configuring-languages#code-actions) and [Preview](https://zed.dev/docs/preview/ai/overview.html)." + ); + } + + #[test] + fn test_docs_path_uses_channel_site_url() { + assert_eq!( + docs_path("/docs/preview/", "/assistant.md"), + "/docs/preview/assistant.md" + ); + } + + #[test] + fn test_write_pages_redirects_keeps_sources_on_internal_pages_path() -> Result<()> { + let deploy_root = std::env::temp_dir().join(format!( + "docs_preprocessor_pages_redirects_test_{}_{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH)? + .as_nanos() + )); + let destination = deploy_root.join("docs"); + std::fs::create_dir_all(&destination)?; + let redirects = vec![ + ( + "/assistant.html".to_string(), + "/docs/ai/overview.html".to_string(), + ), + ( + "/community/feedback.html".to_string(), + "/community-links".to_string(), + ), + ]; + + write_pages_redirects(&destination, &redirects, "/docs/preview/")?; + + assert_eq!( + std::fs::read_to_string(deploy_root.join("_redirects"))?, + "/docs/assistant.html /docs/preview/ai/overview.html 301\n\ +/docs/assistant /docs/preview/ai/overview 301\n\ +/docs/assistant.md /docs/preview/ai/overview.md 301\n\ +/docs/community/feedback.html /community-links 301\n\ +/docs/community/feedback /community-links 301\n" + ); + std::fs::remove_dir_all(&deploy_root)?; + Ok(()) + } + + #[test] + fn test_write_ai_discovery_artifacts_generates_agent_facing_metadata() -> Result<()> { + let destination = std::env::temp_dir().join(format!( + "docs_preprocessor_ai_discovery_test_{}_{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH)? + .as_nanos() + )); + std::fs::create_dir_all(&destination)?; + + let pages = vec![ + DocsPage { + section: "Docs".to_string(), + title: "Getting Started".to_string(), + description: Some("Start using Zed.".to_string()), + source_path: PathBuf::from("getting-started.md"), + content: format!( + "{}\n# Getting Started\n", + FRONT_MATTER_COMMENT.replace("{}", r#"{"description":"Start using Zed."}"#) + ), + }, + DocsPage { + section: "AI".to_string(), + title: "MCP".to_string(), + description: Some("Connect model context servers.".to_string()), + source_path: PathBuf::from("ai/mcp.md"), + content: format!( + "{}\n# MCP\n", + FRONT_MATTER_COMMENT + .replace("{}", r#"{"description":"Connect model context servers."}"#) + ), + }, + ]; + + write_ai_discovery_artifacts(&pages, &destination, "/docs/")?; + + let llms_txt = std::fs::read_to_string(destination.join("llms.txt"))?; + assert!(llms_txt.contains("## Docs")); + assert!(llms_txt.contains( + "- [Getting Started](https://zed.dev/docs/getting-started.md): Start using Zed." + )); + assert!(llms_txt.contains("## AI")); + assert!( + llms_txt.contains( + "- [MCP](https://zed.dev/docs/ai/mcp.md): Connect model context servers." + ) + ); + + let sitemap_xml = std::fs::read_to_string(destination.join("sitemap.xml"))?; + assert!(sitemap_xml.contains("https://zed.dev/docs/getting-started.html")); + assert!(sitemap_xml.contains("https://zed.dev/docs/ai/mcp.html")); + + let mcp_markdown = std::fs::read_to_string(destination.join("ai/mcp.md"))?; + assert!(mcp_markdown.starts_with( + "> For the complete documentation index and Markdown links, see [llms.txt](/docs/llms.txt).\n\n# MCP" + )); + assert!(!mcp_markdown.contains("ZED_META")); + + let index_markdown = std::fs::read_to_string(destination.join("index.md"))?; + assert!(index_markdown.contains("# Getting Started")); + + std::fs::remove_dir_all(&destination)?; + Ok(()) + } +} diff --git a/crates/docs_preprocessor/src/main.rs b/crates/docs_preprocessor/src/main.rs index df3b556d6a4..4b0c2ca04bc 100644 --- a/crates/docs_preprocessor/src/main.rs +++ b/crates/docs_preprocessor/src/main.rs @@ -1,4 +1,10 @@ use anyhow::{Context, Result}; +mod ai_discovery; + +use ai_discovery::{ + add_markdown_alternate_link, docs_pages, rewrite_docs_links, write_ai_discovery_artifacts, + write_markdown_redirect_aliases, write_pages_redirects, +}; use mdbook::BookItem; use mdbook::book::{Book, Chapter}; use mdbook::preprocess::CmdPreprocessor; @@ -188,7 +194,7 @@ fn handle_preprocessing() -> Result<()> { template_big_table_of_actions(&mut book); template_and_validate_keybindings(&mut book, &mut errors); template_and_validate_actions(&mut book, &mut errors); - template_and_validate_json_snippets(&mut book, &mut errors); + template_and_validate_json_snippets(&mut book, &mut errors)?; if !errors.is_empty() { const ANSI_RED: &str = "\x1b[31m"; @@ -252,7 +258,7 @@ fn format_binding(binding: String) -> String { } fn template_and_validate_keybindings(book: &mut Book, errors: &mut HashSet) { - let regex = Regex::new(r"\{#kb(?::(\w+))?\s+(.*?)\}").unwrap(); + let regex = Regex::new(r"(?s)\{#kb(?::(\w+))?\s+(.*?)\}").unwrap(); for_each_chapter_mut(book, |chapter| { chapter.content = regex @@ -300,7 +306,7 @@ fn template_and_validate_keybindings(book: &mut Book, errors: &mut HashSet) { - let regex = Regex::new(r"\{#action (.*?)\}").unwrap(); + let regex = Regex::new(r"(?s)\{#action\s+(.*?)\}").unwrap(); for_each_chapter_mut(book, |chapter| { chapter.content = regex @@ -379,7 +385,10 @@ fn find_binding_with_overlay( .or_else(|| find_binding(os, action)) } -fn template_and_validate_json_snippets(book: &mut Book, errors: &mut HashSet) { +fn template_and_validate_json_snippets( + book: &mut Book, + errors: &mut HashSet, +) -> Result<()> { let params = SettingsJsonSchemaParams { language_names: &[], font_names: &[], @@ -393,7 +402,7 @@ fn template_and_validate_json_snippets(book: &mut Book, errors: &mut HashSet
 Result<()> {
         .as_table_mut()
         .expect("output is table");
     let zed_html = output.remove("zed-html").expect("zed-html output defined");
+    let redirects = zed_html
+        .get("redirect")
+        .and_then(|redirects| redirects.as_table())
+        .map(|redirects| {
+            redirects
+                .iter()
+                .filter_map(|(source, destination)| {
+                    destination
+                        .as_str()
+                        .map(|destination| (source.clone(), destination.to_string()))
+                })
+                .collect::>()
+        });
     let default_description = zed_html
         .get("default-description")
         .expect("Default description not found")
@@ -694,6 +718,17 @@ fn handle_postprocessing() -> Result<()> {
     let amplitude_key = std::env::var("DOCS_AMPLITUDE_API_KEY").unwrap_or_default();
     let consent_io_instance = std::env::var("DOCS_CONSENT_IO_INSTANCE").unwrap_or_default();
     let docs_channel = std::env::var("DOCS_CHANNEL").unwrap_or_else(|_| "stable".to_string());
+    let site_url = std::env::var("MDBOOK_BOOK__SITE_URL")
+        .ok()
+        .filter(|site_url| !site_url.trim().is_empty())
+        .unwrap_or_else(|| {
+            match docs_channel.as_str() {
+                "nightly" => "/docs/nightly/",
+                "preview" => "/docs/preview/",
+                _ => "/docs/",
+            }
+            .to_string()
+        });
     let noindex = if docs_channel == "nightly" || docs_channel == "preview" {
         ""
     } else {
@@ -733,8 +768,10 @@ fn handle_postprocessing() -> Result<()> {
     }
 
     zlog::info!(logger => "Processing {} `.html` files", files.len());
+    let pages = docs_pages(&ctx.book)?;
+    write_ai_discovery_artifacts(&pages, &root_dir, &site_url)?;
     let meta_regex = Regex::new(&FRONT_MATTER_COMMENT.replace("{}", "(.*)")).unwrap();
-    for file in files {
+    for file in &files {
         let contents = std::fs::read_to_string(&file)?;
         let mut meta_description = None;
         let mut meta_title = None;
@@ -770,14 +807,19 @@ fn handle_postprocessing() -> Result<()> {
         let contents = contents.replace("#amplitude_key#", &litude_key);
         let contents = contents.replace("#consent_io_instance#", &consent_io_instance);
         let contents = contents.replace("#noindex#", noindex);
+        let contents = rewrite_docs_links(&contents, &site_url);
+        let contents = add_markdown_alternate_link(&contents, file, &root_dir, &site_url);
         let contents = title_regex()
             .replace(&contents, |_: ®ex::Captures| {
                 format!("{}", meta_title)
             })
             .to_string();
-        // let contents = contents.replace("#title#", &meta_title);
         std::fs::write(file, contents)?;
     }
+    if let Some(redirects) = redirects {
+        write_markdown_redirect_aliases(&root_dir, &redirects, &site_url)?;
+        write_pages_redirects(&root_dir, &redirects, &site_url)?;
+    }
     return Ok(());
 
     fn pretty_path<'a>(
@@ -906,66 +948,4 @@ fn keymap_schema_for_actions(
 }
 
 #[cfg(test)]
-mod tests {
-    use super::*;
-    use serde_json::json;
-
-    #[test]
-    fn test_find_binding_prefers_exact_match_over_parameterized() {
-        let keymap: KeymapFile = serde_json::from_value(json!([
-            {
-                "bindings": {
-                    "ctrl-tab": "agents_sidebar::ToggleThreadSwitcher",
-                    "ctrl-shift-tab": ["agents_sidebar::ToggleThreadSwitcher", { "select_last": true }]
-                }
-            }
-        ]))
-        .unwrap();
-
-        let binding = find_binding_in_keymap(&keymap, "agents_sidebar::ToggleThreadSwitcher");
-        assert_eq!(binding.as_deref(), Some("ctrl-tab"));
-    }
-
-    #[test]
-    fn test_find_binding_falls_back_to_parameterized_match() {
-        let keymap: KeymapFile = serde_json::from_value(json!([
-            {
-                "bindings": {
-                    "ctrl-shift-tab": ["agents_sidebar::ToggleThreadSwitcher", { "select_last": true }]
-                }
-            }
-        ]))
-        .unwrap();
-
-        let binding = find_binding_in_keymap(&keymap, "agents_sidebar::ToggleThreadSwitcher");
-        assert_eq!(binding.as_deref(), Some("ctrl-shift-tab"));
-    }
-
-    #[test]
-    fn test_find_binding_prefers_exact_match_regardless_of_order() {
-        let keymap: KeymapFile = serde_json::from_value(json!([
-            {
-                "bindings": {
-                    "ctrl-shift-tab": ["agents_sidebar::ToggleThreadSwitcher", { "select_last": true }],
-                    "ctrl-tab": "agents_sidebar::ToggleThreadSwitcher"
-                }
-            }
-        ]))
-        .unwrap();
-
-        let binding = find_binding_in_keymap(&keymap, "agents_sidebar::ToggleThreadSwitcher");
-        assert_eq!(binding.as_deref(), Some("ctrl-tab"));
-    }
-
-    #[test]
-    fn test_find_binding_later_section_overrides_earlier() {
-        let keymap: KeymapFile = serde_json::from_value(json!([
-            { "bindings": { "ctrl-a": "some::Action" } },
-            { "bindings": { "ctrl-b": "some::Action" } }
-        ]))
-        .unwrap();
-
-        let binding = find_binding_in_keymap(&keymap, "some::Action");
-        assert_eq!(binding.as_deref(), Some("ctrl-b"));
-    }
-}
+mod tests;
diff --git a/crates/docs_preprocessor/src/tests.rs b/crates/docs_preprocessor/src/tests.rs
new file mode 100644
index 00000000000..43438207d8c
--- /dev/null
+++ b/crates/docs_preprocessor/src/tests.rs
@@ -0,0 +1,61 @@
+use super::*;
+use serde_json::json;
+
+#[test]
+fn test_find_binding_prefers_exact_match_over_parameterized() {
+    let keymap: KeymapFile = serde_json::from_value(json!([
+        {
+            "bindings": {
+                "ctrl-tab": "agents_sidebar::ToggleThreadSwitcher",
+                "ctrl-shift-tab": ["agents_sidebar::ToggleThreadSwitcher", { "select_last": true }]
+            }
+        }
+    ]))
+    .unwrap();
+
+    let binding = find_binding_in_keymap(&keymap, "agents_sidebar::ToggleThreadSwitcher");
+    assert_eq!(binding.as_deref(), Some("ctrl-tab"));
+}
+
+#[test]
+fn test_find_binding_falls_back_to_parameterized_match() {
+    let keymap: KeymapFile = serde_json::from_value(json!([
+        {
+            "bindings": {
+                "ctrl-shift-tab": ["agents_sidebar::ToggleThreadSwitcher", { "select_last": true }]
+            }
+        }
+    ]))
+    .unwrap();
+
+    let binding = find_binding_in_keymap(&keymap, "agents_sidebar::ToggleThreadSwitcher");
+    assert_eq!(binding.as_deref(), Some("ctrl-shift-tab"));
+}
+
+#[test]
+fn test_find_binding_prefers_exact_match_regardless_of_order() {
+    let keymap: KeymapFile = serde_json::from_value(json!([
+        {
+            "bindings": {
+                "ctrl-shift-tab": ["agents_sidebar::ToggleThreadSwitcher", { "select_last": true }],
+                "ctrl-tab": "agents_sidebar::ToggleThreadSwitcher"
+            }
+        }
+    ]))
+    .unwrap();
+
+    let binding = find_binding_in_keymap(&keymap, "agents_sidebar::ToggleThreadSwitcher");
+    assert_eq!(binding.as_deref(), Some("ctrl-tab"));
+}
+
+#[test]
+fn test_find_binding_later_section_overrides_earlier() {
+    let keymap: KeymapFile = serde_json::from_value(json!([
+        { "bindings": { "ctrl-a": "some::Action" } },
+        { "bindings": { "ctrl-b": "some::Action" } }
+    ]))
+    .unwrap();
+
+    let binding = find_binding_in_keymap(&keymap, "some::Action");
+    assert_eq!(binding.as_deref(), Some("ctrl-b"));
+}
diff --git a/docs/book.toml b/docs/book.toml
index 2eed5f7907a..e1a48d1f7f0 100644
--- a/docs/book.toml
+++ b/docs/book.toml
@@ -45,10 +45,10 @@ enable = false
 "/assistant.html" = "/docs/assistant/assistant.html"
 "/assistant/assistant-panel.html" = "/docs/ai/agent-panel.html"
 "/assistant/assistant.html" = "/docs/ai/overview.html"
-"/assistant/commands.html" = "/docs/ai/text-threads.html"
+"/assistant/commands.html" = "/docs/ai/agent-panel.html"
 "/assistant/configuration.html" = "/docs/ai/quick-start.html"
 "/assistant/context-servers.html" = "/docs/ai/mcp.html"
-"/assistant/contexts.html" = "/docs/ai/text-threads.html"
+"/assistant/contexts.html" = "/docs/ai/agent-panel.html"
 "/assistant/inline-assistant.html" = "/docs/ai/inline-assistant.html"
 "/assistant/model-context-protocol.html" = "/docs/ai/mcp.html"
 "/assistant/prompting.html" = "/docs/ai/skills.html"
diff --git a/docs/theme/css/chrome.css b/docs/theme/css/chrome.css
index 8f5b40cc19e..f0169fd7c87 100644
--- a/docs/theme/css/chrome.css
+++ b/docs/theme/css/chrome.css
@@ -434,6 +434,7 @@ ul#searchresults span.teaser em {
 
 .sidebar {
   position: relative;
+  order: 0;
   width: var(--sidebar-width);
   flex-shrink: 0;
   display: flex;
diff --git a/docs/theme/css/general.css b/docs/theme/css/general.css
index 9c8077bad52..d32edaa8d61 100644
--- a/docs/theme/css/general.css
+++ b/docs/theme/css/general.css
@@ -201,6 +201,7 @@ hr {
 
 .page {
   outline: 0;
+  order: 1;
   flex: 1;
   display: flex;
   flex-direction: column;
diff --git a/docs/theme/index.hbs b/docs/theme/index.hbs
index 2c7786817aa..d5e0b620051 100644
--- a/docs/theme/index.hbs
+++ b/docs/theme/index.hbs
@@ -27,6 +27,7 @@
             })();
         
         {{ title }}
+        
         {{#if is_print }}
         
         {{/if}}
@@ -86,6 +87,9 @@
             document.body.classList.remove('no-js');
             document.body.classList.add('js');
         
+        

+            Agent documentation index: llms.txt. Markdown versions are available for docs pages.
+        

 
         

             

@@ -155,6 +159,95 @@
             

             {{/if}}
 
+            

+                

+                    

+                      {{{ content }}}
+                        

+                            {{#previous}}
+                            
+                            {{/previous}}
+                            {{#next}}
+                            
+                            {{/next}}
+                        

+                        
+                    

+                    

+                        
+                    

+                    
+                    
+                

+            

+