From b6b2b63370cb0141ef77306deca6c2dc887ead77 Mon Sep 17 00:00:00 2001 From: Pratik Karki Date: Thu, 16 Apr 2026 11:56:22 +0545 Subject: [PATCH] Gate `Format Selections` on whether the active formatter can actually format ranges (#53178) ## What Changed - compute range-format support from formatter configuration and language-server capabilities - show `Format Selections` only when at least one selected buffer has a range-capable formatter - keep Prettier-supported range formatting available without depending on LSP support - hide the action for formatter modes that cannot honor selections, such as external-command and code-action formatters - keep the action handler safe by rechecking support and returning early when range formatting is unavailable - restrict capability checks to the servers actually registered for the current buffer - add regression coverage for supported, unsupported, and mixed-server cases - document when `Format Selections` is available in both action docs and user docs 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 #25796 Release Notes: - Improved `Format Selections` so it is only shown when the active formatter supports range formatting. Testing I did: Here are the formatters I used: Screenshot_20260405_102047 Note: I've chosen three formatters: 1. Prettier, which supports range formatting 2. clangd (LSP) which also supports range formatting 3. gopls which do not support range formatting [Screencast_20260405_102431.webm](https://github.com/user-attachments/assets/69ce3f95-0b73-46a0-853d-3b5be6329dde) --------- Signed-off-by: Pratik Karki Co-authored-by: Kirill Bulatov --- crates/editor/src/actions.rs | 10 +- crates/editor/src/editor.rs | 22 +++ crates/editor/src/editor_tests.rs | 131 +++++++++++++++- crates/editor/src/element.rs | 16 +- crates/editor/src/mouse_context_menu.rs | 3 +- crates/project/src/lsp_store.rs | 140 +++++++++++++----- crates/project/src/project.rs | 6 + .../tests/integration/project_tests.rs | 81 +++++++++- docs/src/configuring-languages.md | 9 +- 9 files changed, 364 insertions(+), 54 deletions(-) diff --git a/crates/editor/src/actions.rs b/crates/editor/src/actions.rs index 077152bff0e5494d04c62a7874f7b2ffea28488c..4c0623006953c0aae9c718b2e894ba85b26074e0 100644 --- a/crates/editor/src/actions.rs +++ b/crates/editor/src/actions.rs @@ -552,11 +552,13 @@ actions!( Format, /// Formats only the selected text. /// + /// This action is only available when the active formatter can format ranges. /// When using a language server, this sends an LSP range formatting request for each - /// selection. When using Prettier, Prettier's own range formatting is used to format the - /// encompassing range of all selections, and resulting edits outside the selected ranges - /// are discarded. External command formatters do not support range formatting and are - /// skipped. + /// selection, and is hidden when the selected buffer's configured language server does + /// not advertise range-formatting support. When using Prettier, Prettier's own range + /// formatting is used to format the encompassing range of all selections, and resulting + /// edits outside the selected ranges are discarded. External command formatters do not + /// support range formatting and are skipped. FormatSelections, /// Goes to the declaration of the symbol at cursor. GoToDeclaration, diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index fbafb9a8bdae135230a2041def0de592cf0f52d0..f3dfef45783240042675667af4207797ae65b743 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -19608,6 +19608,28 @@ impl Editor { self.pending_rename.as_ref() } + fn can_format_selections(&self, cx: &App) -> bool { + if !self.mode.is_full() { + return false; + } + + let Some(project) = &self.project else { + return false; + }; + + let project = project.read(cx); + let multi_buffer = self.buffer.read(cx); + let snapshot = multi_buffer.snapshot(cx); + + self.selections + .disjoint_anchor_ranges() + .filter(|range| range.start != range.end) + .flat_map(|range| [range.start, range.end]) + .filter_map(|anchor| snapshot.anchor_to_buffer_anchor(anchor)) + .filter_map(|(_, buffer_snapshot)| multi_buffer.buffer(buffer_snapshot.remote_id())) + .any(|buffer| project.supports_range_formatting(&buffer, cx)) + } + fn format( &mut self, _: &Format, diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index b94e9f79b0ca2b155cb92b3fc43ba8795cee5244..38f4259b4d4c87f3243e49dec8f35991bd82f246 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -14199,8 +14199,9 @@ async fn test_autosave_with_dirty_buffers(cx: &mut TestAppContext) { ); } -async fn setup_range_format_test( +async fn setup_range_format_test_with_capabilities( cx: &mut TestAppContext, + capabilities: lsp::ServerCapabilities, ) -> ( Entity, Entity, @@ -14217,6 +14218,120 @@ async fn setup_range_format_test( 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, + ..FakeLspAdapter::default() + }, + ); + + let buffer = project + .update(cx, |project, cx| { + project.open_local_buffer(path!("/file.rs"), 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) +} + +async fn setup_range_format_test( + cx: &mut TestAppContext, +) -> ( + Entity, + Entity, + &mut gpui::VisualTestContext, + lsp::FakeLanguageServer, +) { + setup_range_format_test_with_capabilities( + cx, + lsp::ServerCapabilities { + document_range_formatting_provider: Some(lsp::OneOf::Left(true)), + ..lsp::ServerCapabilities::default() + }, + ) + .await +} + +fn refresh_editor_actions(cx: &mut VisualTestContext) { + cx.executor().run_until_parked(); + cx.update(|window, cx| { + let _ = window.draw(cx); + }); +} + +#[gpui::test] +async fn test_format_selections_action_available_when_range_formatting_is_supported( + cx: &mut TestAppContext, +) { + let (_, editor, cx, _) = setup_range_format_test(cx).await; + + editor.update_in(cx, |editor, window, cx| { + editor.set_text("one\ntwo\nthree\n", window, cx); + editor.change_selections(SelectionEffects::default(), window, cx, |s| { + s.select_ranges([Point::new(0, 0)..Point::new(1, 0)]); + }); + }); + + refresh_editor_actions(cx); + + assert!(cx.update(|window, cx| { window.is_action_available(&FormatSelections, cx) })); +} + +#[gpui::test] +async fn test_format_selections_action_hidden_without_range_formatting_support( + cx: &mut TestAppContext, +) { + let (_, editor, cx, _) = setup_range_format_test_with_capabilities( + cx, + lsp::ServerCapabilities { + document_formatting_provider: Some(lsp::OneOf::Left(true)), + document_range_formatting_provider: Some(lsp::OneOf::Left(false)), + ..lsp::ServerCapabilities::default() + }, + ) + .await; + + editor.update_in(cx, |editor, window, cx| { + editor.set_text("one\ntwo\nthree\n", window, cx); + editor.change_selections(SelectionEffects::default(), window, cx, |s| { + s.select_ranges([Point::new(0, 0)..Point::new(1, 0)]); + }); + }); + + refresh_editor_actions(cx); + + assert!(!cx.update(|window, cx| { window.is_action_available(&FormatSelections, cx) })); +} + +#[gpui::test] +async fn test_format_selections_action_hidden_without_range_capable_formatter( + cx: &mut TestAppContext, +) { + init_test(cx, |settings| { + settings.defaults.formatter = Some(FormatterList::Single(Formatter::External { + command: "awk".into(), + arguments: Some(vec!["{ print }".to_string()]), + })); + }); + + let fs = FakeFs::new(cx.executor()); + fs.insert_file(path!("/file.rs"), Default::default()).await; + + let project = Project::test(fs, [path!("/").as_ref()], cx).await; + let language_registry = project.read_with(cx, |project, _| project.languages().clone()); + language_registry.add(rust_lang()); + let _ = language_registry.register_fake_lsp( "Rust", FakeLspAdapter { capabilities: lsp::ServerCapabilities { @@ -14238,10 +14353,20 @@ async fn setup_range_format_test( 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(); + editor.update_in(cx, |editor, window, cx| { + editor.set_text("one\ntwo\nthree\n", window, cx); + editor.change_selections(SelectionEffects::default(), window, cx, |s| { + s.select_ranges([Point::new(0, 0)..Point::new(1, 0)]); + }); + }); - (project, editor, cx, fake_server) + refresh_editor_actions(cx); + + assert!(!cx.update(|window, cx| { window.is_action_available(&FormatSelections, cx) })); } #[gpui::test] diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index fa6b9d30b5b7123e8775ba1d8b65a79461e26ca1..e2b8a49f9eaa23340895779724660c3b46fdc0af 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -552,13 +552,15 @@ impl EditorElement { cx.propagate(); } }); - register_action(editor, window, |editor, action, window, cx| { - if let Some(task) = editor.format_selections(action, window, cx) { - editor.detach_and_notify_err(task, window, cx); - } else { - cx.propagate(); - } - }); + if editor.read(cx).can_format_selections(cx) { + register_action(editor, window, |editor, action, window, cx| { + if let Some(task) = editor.format_selections(action, window, cx) { + editor.detach_and_notify_err(task, window, cx); + } else { + cx.propagate(); + } + }); + } register_action(editor, window, |editor, action, window, cx| { if let Some(task) = editor.organize_imports(action, window, cx) { editor.detach_and_notify_err(task, window, cx); diff --git a/crates/editor/src/mouse_context_menu.rs b/crates/editor/src/mouse_context_menu.rs index 0028f52d3d91ca9e6ea660dec0628e7ca6b9e520..879384fb6e65fdd243f029e69f05182d35c3f6fc 100644 --- a/crates/editor/src/mouse_context_menu.rs +++ b/crates/editor/src/mouse_context_menu.rs @@ -219,6 +219,7 @@ pub fn deploy_context_menu( let evaluate_selection = window.is_action_available(&EvaluateSelectedText, cx); let run_to_cursor = window.is_action_available(&RunToCursor, cx); + let format_selections = window.is_action_available(&FormatSelections, cx); let disable_ai = DisableAiSettings::is_ai_disabled_for_buffer( editor.buffer.read(cx).as_singleton().as_ref(), cx, @@ -266,7 +267,7 @@ pub fn deploy_context_menu( .separator() .action("Rename Symbol", Box::new(Rename)) .action("Format Buffer", Box::new(Format)) - .when(has_selections, |cx| { + .when(format_selections, |cx| { cx.action("Format Selections", Box::new(FormatSelections)) }) .action( diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 47d6a7c0d171dd5db64f7c8c6c6be00fb332dc61..1488691afa9e91132d62bc378fbacdf6883cbf00 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -2340,9 +2340,8 @@ impl LocalLspStore { fn server_supports_formatting(server: &Arc) -> bool { let capabilities = server.capabilities(); let formatting = capabilities.document_formatting_provider.as_ref(); - let range_formatting = capabilities.document_range_formatting_provider.as_ref(); matches!(formatting, Some(p) if *p != OneOf::Left(false)) - || matches!(range_formatting, Some(p) if *p != OneOf::Left(false)) + || server_capabilities_support_range_formatting(&capabilities) } async fn format_via_lsp( @@ -5003,17 +5002,24 @@ impl LspStore { ) } - fn check_if_capable_for_proto_request( + fn relevant_server_ids_for_capability_check( &self, buffer: &Entity, - check: F, cx: &App, - ) -> bool - where - F: FnMut(&lsp::ServerCapabilities) -> bool, - { + ) -> Vec { + let buffer_id = buffer.read(cx).remote_id(); + if let Some(local) = self.as_local() { + return local + .buffers_opened_in_servers + .get(&buffer_id) + .into_iter() + .flatten() + .copied() + .collect(); + } + let Some(language) = buffer.read(cx).language().cloned() else { - return false; + return Vec::default(); }; let registered_language_servers = self .languages @@ -5030,10 +5036,81 @@ impl LspStore { // but only loaded on the server side) let is_relevant = registered_language_servers.contains(&server_status.name) || self.languages.is_lsp_adapter_available(&server_status.name); - is_relevant.then_some(server_id) + is_relevant.then_some(*server_id) }) - .filter_map(|server_id| self.lsp_server_capabilities.get(server_id)) - .any(check) + .collect() + } + + fn check_if_any_relevant_server_matches( + &self, + buffer: &Entity, + mut check: F, + cx: &App, + ) -> bool + where + F: FnMut(&LanguageServerStatus, &lsp::ServerCapabilities) -> bool, + { + self.relevant_server_ids_for_capability_check(buffer, cx) + .into_iter() + .filter_map(|server_id| { + Some(( + self.language_server_statuses.get(&server_id)?, + self.lsp_server_capabilities.get(&server_id)?, + )) + }) + .any(|(server_status, capabilities)| check(server_status, capabilities)) + } + + fn check_if_capable_for_proto_request( + &self, + buffer: &Entity, + mut check: F, + cx: &App, + ) -> bool + where + F: FnMut(&lsp::ServerCapabilities) -> bool, + { + self.check_if_any_relevant_server_matches(buffer, |_, capabilities| check(capabilities), cx) + } + + pub fn supports_range_formatting(&self, buffer: &Entity, cx: &App) -> bool { + let settings = LanguageSettings::for_buffer(buffer.read(cx), cx); + settings.formatter.as_ref().iter().any(|formatter| { + match formatter { + Formatter::None => false, + Formatter::Auto => { + settings.prettier.allowed + || self.check_if_capable_for_proto_request( + buffer, + server_capabilities_support_range_formatting, + cx, + ) + } + Formatter::Prettier => true, + Formatter::External { .. } => false, + Formatter::LanguageServer(settings::LanguageServerFormatterSpecifier::Current) => { + self.check_if_capable_for_proto_request( + buffer, + server_capabilities_support_range_formatting, + cx, + ) + } + Formatter::LanguageServer( + settings::LanguageServerFormatterSpecifier::Specific { name }, + ) => self.check_if_any_relevant_server_matches( + buffer, + |server_status, capabilities| { + server_status.name.0.as_ref() == name + && server_capabilities_support_range_formatting(capabilities) + }, + cx, + ), + // `FormatSelections` should only surface when a formatter can honor the + // selected ranges. Code actions can still run as part of formatting, but + // they operate on the whole buffer rather than the selected text. + Formatter::CodeAction(_) => false, + } + }) } fn all_capable_for_proto_request( @@ -5045,33 +5122,17 @@ impl LspStore { where F: FnMut(&lsp::LanguageServerName, &lsp::ServerCapabilities) -> bool, { - let Some(language) = buffer.read(cx).language().cloned() else { - return Vec::default(); - }; - let registered_language_servers = self - .languages - .lsp_adapters(&language.name()) + self.relevant_server_ids_for_capability_check(buffer, cx) .into_iter() - .map(|lsp_adapter| lsp_adapter.name()) - .collect::>(); - self.language_server_statuses - .iter() - .filter_map(|(server_id, server_status)| { - // Include servers that are either registered for this language OR - // available to be loaded (for SSH remote mode where adapters like - // ty/pylsp/pyright are registered via register_available_lsp_adapter - // but only loaded on the server side) - let is_relevant = registered_language_servers.contains(&server_status.name) - || self.languages.is_lsp_adapter_available(&server_status.name); - is_relevant.then_some((server_id, &server_status.name)) - }) - .filter_map(|(server_id, server_name)| { - self.lsp_server_capabilities - .get(server_id) - .map(|c| (server_id, server_name, c)) + .filter_map(|server_id| { + Some(( + server_id, + &self.language_server_statuses.get(&server_id)?.name, + self.lsp_server_capabilities.get(&server_id)?, + )) }) .filter(|(_, server_name, capabilities)| check(server_name, capabilities)) - .map(|(server_id, server_name, _)| (*server_id, server_name.clone())) + .map(|(server_id, server_name, _)| (server_id, server_name.clone())) .collect() } @@ -13287,6 +13348,13 @@ fn parse_register_capabilities( }) } +fn server_capabilities_support_range_formatting(capabilities: &lsp::ServerCapabilities) -> bool { + matches!( + capabilities.document_range_formatting_provider.as_ref(), + Some(provider) if *provider != OneOf::Left(false) + ) +} + fn subscribe_to_binary_statuses( languages: &Arc, cx: &mut Context<'_, LspStore>, diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 515a518e530e79eb7bdf2a3074e6bf12a5824027..83e069a0f80363234ce3e149f44a61aa786b0138 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -4122,6 +4122,12 @@ impl Project { }) } + pub fn supports_range_formatting(&self, buffer: &Entity, cx: &App) -> bool { + self.lsp_store + .read(cx) + .supports_range_formatting(buffer, cx) + } + pub fn definitions( &mut self, buffer: &Entity, diff --git a/crates/project/tests/integration/project_tests.rs b/crates/project/tests/integration/project_tests.rs index a26e95fdb9de20a35d92b85f5d7abb21047d2520..4d6899189e76c0083a4501570270e44b825329bd 100644 --- a/crates/project/tests/integration/project_tests.rs +++ b/crates/project/tests/integration/project_tests.rs @@ -45,7 +45,7 @@ use language::{ LanguageConfig, LanguageMatcher, LanguageName, LineEnding, ManifestName, ManifestProvider, ManifestQuery, OffsetRangeExt, Point, ToPoint, Toolchain, ToolchainList, ToolchainLister, ToolchainMetadata, - language_settings::{LanguageSettings, LanguageSettingsContent}, + language_settings::{Formatter, FormatterList, LanguageSettings, LanguageSettingsContent}, markdown_lang, rust_lang, tree_sitter_typescript, }; use lsp::{ @@ -4901,6 +4901,85 @@ async fn test_completions_with_carriage_returns(cx: &mut gpui::TestAppContext) { assert_eq!(completions[0].new_text, "fully\nQualified\nName"); } +#[gpui::test] +async fn test_supports_range_formatting_ignores_unrelated_language_servers( + cx: &mut gpui::TestAppContext, +) { + init_test(cx); + cx.update(|cx| { + SettingsStore::update_global(cx, |store, cx| { + store.update_user_settings(cx, |settings| { + settings.project.all_languages.defaults.formatter = Some(FormatterList::Single( + Formatter::LanguageServer(settings::LanguageServerFormatterSpecifier::Current), + )); + }); + }); + }); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + path!("/dir"), + json!({ + "a.ts": "", + "b.rs": "", + }), + ) + .await; + + let project = Project::test(fs, [path!("/dir").as_ref()], cx).await; + let language_registry = project.read_with(cx, |project, _| project.languages().clone()); + language_registry.add(typescript_lang()); + language_registry.add(rust_lang()); + + let mut typescript_language_servers = language_registry.register_fake_lsp( + "TypeScript", + FakeLspAdapter { + name: "typescript-fake-language-server", + capabilities: lsp::ServerCapabilities { + document_range_formatting_provider: Some(lsp::OneOf::Left(true)), + ..lsp::ServerCapabilities::default() + }, + ..FakeLspAdapter::default() + }, + ); + let mut rust_language_servers = language_registry.register_fake_lsp( + "Rust", + FakeLspAdapter { + name: "rust-fake-language-server", + capabilities: lsp::ServerCapabilities { + document_formatting_provider: Some(lsp::OneOf::Left(true)), + document_range_formatting_provider: Some(lsp::OneOf::Left(false)), + ..lsp::ServerCapabilities::default() + }, + ..FakeLspAdapter::default() + }, + ); + + let (typescript_buffer, _typescript_handle) = project + .update(cx, |project, cx| { + project.open_local_buffer_with_lsp(path!("/dir/a.ts"), cx) + }) + .await + .unwrap(); + let (rust_buffer, _rust_handle) = project + .update(cx, |project, cx| { + project.open_local_buffer_with_lsp(path!("/dir/b.rs"), cx) + }) + .await + .unwrap(); + + let _typescript_language_server = typescript_language_servers.next().await.unwrap(); + let _rust_language_server = rust_language_servers.next().await.unwrap(); + cx.executor().run_until_parked(); + + assert!(project.read_with(cx, |project, cx| { + project.supports_range_formatting(&typescript_buffer, cx) + })); + assert!(!project.read_with(cx, |project, cx| { + project.supports_range_formatting(&rust_buffer, cx) + })); +} + #[gpui::test(iterations = 10)] async fn test_apply_code_actions_with_commands(cx: &mut gpui::TestAppContext) { init_test(cx); diff --git a/docs/src/configuring-languages.md b/docs/src/configuring-languages.md index 46a10e80e3807c1dd57df2184e814b2abe8647d7..01c884622edd2e373bd3aabf2cd4b0c5339565ee 100644 --- a/docs/src/configuring-languages.md +++ b/docs/src/configuring-languages.md @@ -356,12 +356,17 @@ To run linter fixes automatically on save: Zed supports formatting only the selected text via `editor: format selections` ({#kb editor::FormatSelections}). How this works depends on the configured formatter: -- **Language server**: Sends an LSP range formatting request for each selection. This provides the most precise - selection-only formatting. +- The action is only shown when the active formatter can actually format ranges for at least one + selected buffer. +- **Language server**: Sends an LSP range formatting request for each selection. This provides the + most precise selection-only formatting, and is only available when the configured language server + advertises range-formatting support. - **Prettier**: Uses Prettier's built-in range formatting to format the encompassing range of all selections. Any resulting edits that fall outside the selected ranges are discarded, so only the selected code is modified. - **External commands**: External command formatters do not support range formatting and are skipped when formatting selections. +- **Code action formatters**: Code actions operate on the whole buffer, so they do not enable + `format selections` on their own. ### Integrating Formatting and Linting