From b7f166ab409f24116716473849a124859a14dddd Mon Sep 17 00:00:00 2001 From: Pratik Karki Date: Tue, 31 Mar 2026 15:33:22 +0545 Subject: [PATCH] Fix FormatSelections to only format selected ranges, not entire document (#51593) When `editor: format selections` get invoked, the Prettier and external formatter branches in `format_buffer_locally` ignored the selection ranges entirely, causing the whole document to be formatted. - Thread selection ranges as UTF-16 offsets through to Prettier via `rangeStart/rangeEnd` options in the format request. - Skip external formatters when ranges are present, since they have no mechanism for range formatting. - Create diff edits and apply them for JSON-like languages. For single-expression languages like JSON, it wasn't respecting the range commands from Prettier. So, filter the diff edits returned by Prettier to retain only those overlapping with the user's selection byte ranges, ensuring changes outside the selection are never applied. Part of #25796 Before you mark this PR as ready for review, make sure that you have: - [x] Added a solid test coverage and/or screenshots from doing manual testing - [x] Done a self-review taking into account security and performance aspects - [x] Aligned any UI changes with the [UI checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) Release Notes: - Fixed FormatSelections to only format selected ranges, not the entire document where prettier is supported. Current Behaviour: [original behaviour.webm](https://github.com/user-attachments/assets/d5f0cb48-4c3f-44aa-89a9-975f31fce92d) New Behaviour: [new behaviour.webm](https://github.com/user-attachments/assets/41e04b90-f37f-43e1-b8ed-4622684454b1) --------- Signed-off-by: Pratik Karki --- crates/editor/src/actions.rs | 6 + crates/editor/src/editor_tests.rs | 159 +++++++++++++++++++++++++ crates/prettier/src/prettier.rs | 41 +++++-- crates/prettier/src/prettier_server.js | 9 ++ crates/project/src/lsp_store.rs | 67 ++++++++++- crates/project/src/prettier_store.rs | 14 ++- crates/project/src/project.rs | 2 + docs/src/configuring-languages.md | 12 ++ 8 files changed, 294 insertions(+), 16 deletions(-) diff --git a/crates/editor/src/actions.rs b/crates/editor/src/actions.rs index a51ee787f8b531aa650d13afee2cf9550c2a26fa..f4b4c69679ebd4ab0f9080cd7d110fd4e87259c4 100644 --- a/crates/editor/src/actions.rs +++ b/crates/editor/src/actions.rs @@ -545,6 +545,12 @@ actions!( /// Formats the entire document. Format, /// Formats only the selected text. + /// + /// 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. FormatSelections, /// Goes to the declaration of the symbol at cursor. GoToDeclaration, diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index dc84e826f6a74ae439137e1eb7592e3b2c6413a8..48c92f0f22762f95b1d6ef681951355a340d221e 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -22070,6 +22070,165 @@ async fn test_document_format_with_prettier_explicit_language(cx: &mut TestAppCo ); } +#[gpui::test] +async fn test_range_format_with_prettier(cx: &mut TestAppContext) { + init_test(cx, |settings| { + settings.defaults.formatter = Some(FormatterList::Single(Formatter::Prettier)) + }); + + let fs = FakeFs::new(cx.executor()); + fs.insert_file(path!("/file.ts"), Default::default()).await; + + let project = Project::test(fs, [path!("/file.ts").as_ref()], cx).await; + let language_registry = project.read_with(cx, |project, _| project.languages().clone()); + + language_registry.add(Arc::new(Language::new( + LanguageConfig { + name: "TypeScript".into(), + matcher: LanguageMatcher { + path_suffixes: vec!["ts".to_string()], + ..Default::default() + }, + ..Default::default() + }, + Some(tree_sitter_typescript::LANGUAGE_TYPESCRIPT.into()), + ))); + update_test_language_settings(cx, &|settings| { + settings.defaults.prettier.get_or_insert_default().allowed = Some(true); + }); + + let test_plugin = "test_plugin"; + let _ = language_registry.register_fake_lsp( + "TypeScript", + FakeLspAdapter { + prettier_plugins: vec![test_plugin], + ..Default::default() + }, + ); + + let prettier_range_format_suffix = project::TEST_PRETTIER_RANGE_FORMAT_SUFFIX; + let buffer = project + .update(cx, |project, cx| { + project.open_local_buffer(path!("/file.ts"), cx) + }) + .await + .unwrap(); + + let buffer_text = "one\ntwo\nthree\nfour\nfive\n"; + 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| { + editor.set_text(buffer_text, window, cx) + }); + + cx.executor().run_until_parked(); + + editor.update_in(cx, |editor, window, cx| { + editor.change_selections(SelectionEffects::default(), window, cx, |s| { + s.select_ranges([Point::new(1, 0)..Point::new(3, 0)]) + }); + }); + + let format = editor + .update_in(cx, |editor, window, cx| { + editor.format_selections(&FormatSelections, window, cx) + }) + .unwrap(); + format.await.unwrap(); + + assert_eq!( + editor.update(cx, |editor, cx| editor.text(cx)), + format!("one\ntwo{prettier_range_format_suffix}\nthree\nfour\nfive\n"), + "Range formatting (via test prettier) was not applied to the buffer text", + ); +} + +#[gpui::test] +async fn test_range_format_with_prettier_explicit_language(cx: &mut TestAppContext) { + init_test(cx, |settings| { + settings.defaults.formatter = Some(FormatterList::Single(Formatter::Prettier)) + }); + + let fs = FakeFs::new(cx.executor()); + fs.insert_file(path!("/file.settings"), Default::default()) + .await; + + let project = Project::test(fs, [path!("/file.settings").as_ref()], cx).await; + let language_registry = project.read_with(cx, |project, _| project.languages().clone()); + + let ts_lang = Arc::new(Language::new( + LanguageConfig { + name: "TypeScript".into(), + matcher: LanguageMatcher { + path_suffixes: vec!["ts".to_string()], + ..LanguageMatcher::default() + }, + prettier_parser_name: Some("typescript".to_string()), + ..LanguageConfig::default() + }, + Some(tree_sitter_typescript::LANGUAGE_TYPESCRIPT.into()), + )); + + language_registry.add(ts_lang.clone()); + + update_test_language_settings(cx, &|settings| { + settings.defaults.prettier.get_or_insert_default().allowed = Some(true); + }); + + let test_plugin = "test_plugin"; + let _ = language_registry.register_fake_lsp( + "TypeScript", + FakeLspAdapter { + prettier_plugins: vec![test_plugin], + ..Default::default() + }, + ); + + let prettier_range_format_suffix = project::TEST_PRETTIER_RANGE_FORMAT_SUFFIX; + let buffer = project + .update(cx, |project, cx| { + project.open_local_buffer(path!("/file.settings"), cx) + }) + .await + .unwrap(); + + project.update(cx, |project, cx| { + project.set_language_for_buffer(&buffer, ts_lang, cx) + }); + + let buffer_text = "one\ntwo\nthree\nfour\nfive\n"; + 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| { + editor.set_text(buffer_text, window, cx) + }); + + cx.executor().run_until_parked(); + + editor.update_in(cx, |editor, window, cx| { + editor.change_selections(SelectionEffects::default(), window, cx, |s| { + s.select_ranges([Point::new(1, 0)..Point::new(3, 0)]) + }); + }); + + let format = editor + .update_in(cx, |editor, window, cx| { + editor.format_selections(&FormatSelections, window, cx) + }) + .unwrap(); + format.await.unwrap(); + + assert_eq!( + editor.update(cx, |editor, cx| editor.text(cx)), + format!("one\ntwo{prettier_range_format_suffix}\ntypescript\nthree\nfour\nfive\n"), + "Range formatting (via test prettier) was not applied with explicit language", + ); +} + #[gpui::test] async fn test_addition_reverts(cx: &mut TestAppContext) { init_test(cx, |_| {}); diff --git a/crates/prettier/src/prettier.rs b/crates/prettier/src/prettier.rs index b0fd57f6980ca0f0f4d6d95ecd0e994ab80b2016..be4b35a6450eec645fde1343d4e9d27f0a695ef1 100644 --- a/crates/prettier/src/prettier.rs +++ b/crates/prettier/src/prettier.rs @@ -3,13 +3,13 @@ use collections::{HashMap, HashSet}; use fs::Fs; use gpui::{AsyncApp, Entity}; use language::language_settings::{LanguageSettings, PrettierSettings}; -use language::{Buffer, Diff, Language}; +use language::{Buffer, Diff, Language, OffsetUtf16}; use lsp::{LanguageServer, LanguageServerId}; use node_runtime::NodeRuntime; use paths::default_prettier_dir; use serde::{Deserialize, Serialize}; use std::{ - ops::ControlFlow, + ops::{ControlFlow, Range}, path::{Path, PathBuf}, sync::Arc, time::Duration, @@ -48,6 +48,8 @@ const TAILWIND_PRETTIER_PLUGIN_PACKAGE_NAME: &str = "prettier-plugin-tailwindcss #[cfg(any(test, feature = "test-support"))] pub const FORMAT_SUFFIX: &str = "\nformatted by test prettier"; +#[cfg(any(test, feature = "test-support"))] +pub const RANGE_FORMAT_SUFFIX: &str = "\nrange formatted by test prettier"; impl Prettier { pub const CONFIG_FILE_NAMES: &'static [&'static str] = &[ @@ -348,6 +350,7 @@ impl Prettier { buffer: &Entity, buffer_path: Option, ignore_dir: Option, + range_utf16: Option>, request_timeout: Duration, cx: &mut AsyncApp, ) -> anyhow::Result { @@ -478,6 +481,8 @@ impl Prettier { plugins, prettier_options, ignore_path, + range_start: range_utf16.as_ref().map(|r| r.start.0), + range_end: range_utf16.as_ref().map(|r| r.end.0), }, }) }) @@ -501,8 +506,6 @@ impl Prettier { { Some("rust") => anyhow::bail!("prettier does not support Rust"), Some(_other) => { - let mut formatted_text = buffer.text() + FORMAT_SUFFIX; - let buffer_language = buffer.language().map(|language| language.as_ref()); let language_settings = LanguageSettings::for_buffer(buffer, cx); @@ -513,9 +516,29 @@ impl Prettier { prettier_settings, )?; - if let Some(parser) = parser { - formatted_text = format!("{formatted_text}\n{parser}"); - } + let formatted_text = if let Some(range) = &range_utf16 { + let text = buffer.text(); + let start_byte = buffer.offset_utf16_to_offset(range.start); + let insert_at = text[start_byte..] + .find('\n') + .map(|pos| start_byte + pos) + .unwrap_or(text.len()); + let mut suffix = RANGE_FORMAT_SUFFIX.to_string(); + if let Some(parser) = &parser { + suffix = format!("{suffix}\n{parser}"); + } + let mut result = String::new(); + result.push_str(&text[..insert_at]); + result.push_str(&suffix); + result.push_str(&text[insert_at..]); + result + } else { + let mut text = buffer.text() + FORMAT_SUFFIX; + if let Some(parser) = &parser { + text = format!("{text}\n{parser}"); + } + text + }; Ok(buffer.diff(formatted_text, cx)) } @@ -651,6 +674,10 @@ struct FormatOptions { path: Option, prettier_options: Option>, ignore_path: Option, + #[serde(skip_serializing_if = "Option::is_none")] + range_start: Option, + #[serde(skip_serializing_if = "Option::is_none")] + range_end: Option, } #[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] diff --git a/crates/prettier/src/prettier_server.js b/crates/prettier/src/prettier_server.js index b3d8a660a40d6f629ba63847f5e00d91046b7cd7..917095fea6896f18459b9ba024dfc183a1e28314 100644 --- a/crates/prettier/src/prettier_server.js +++ b/crates/prettier/src/prettier_server.js @@ -199,12 +199,21 @@ async function handleMessage(message, prettier) { ? resolvedConfig.plugins : params.options.plugins; + const rangeOptions = {}; + if (params.options.rangeStart != null) { + rangeOptions.rangeStart = params.options.rangeStart; + } + if (params.options.rangeEnd != null) { + rangeOptions.rangeEnd = params.options.rangeEnd; + } + const options = { ...(params.options.prettierOptions || prettier.config), ...resolvedConfig, plugins, parser: params.options.parser, filepath: params.options.filepath, + ...rangeOptions }; process.stderr.write( `Resolved config: ${JSON.stringify(resolvedConfig)}, will format file '${ diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index d36a45692bec13e2ab4c9b21d1ee4da6879ab6fc..10fb447a6f6c7867212a4622d084deb4fcea91a2 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -73,8 +73,8 @@ use language::{ Bias, BinaryStatus, Buffer, BufferRow, BufferSnapshot, CachedLspAdapter, Capability, CodeLabel, CodeLabelExt, Diagnostic, DiagnosticEntry, DiagnosticSet, DiagnosticSourceKind, Diff, File as _, Language, LanguageName, LanguageRegistry, LocalFile, LspAdapter, LspAdapterDelegate, - LspInstaller, ManifestDelegate, ManifestName, ModelineSettings, Patch, PointUtf16, - TextBufferSnapshot, ToOffset, ToPointUtf16, Toolchain, Transaction, Unclipped, + LspInstaller, ManifestDelegate, ManifestName, ModelineSettings, OffsetUtf16, Patch, PointUtf16, + TextBufferSnapshot, ToOffset, ToOffsetUtf16, ToPointUtf16, Toolchain, Transaction, Unclipped, language_settings::{ AllLanguageSettings, FormatOnSave, Formatter, LanguageSettings, all_language_settings, }, @@ -149,6 +149,8 @@ pub use language::Location; pub use lsp_store::inlay_hints::{CacheInlayHints, InvalidationStrategy}; #[cfg(any(test, feature = "test-support"))] pub use prettier::FORMAT_SUFFIX as TEST_PRETTIER_FORMAT_SUFFIX; +#[cfg(any(test, feature = "test-support"))] +pub use prettier::RANGE_FORMAT_SUFFIX as TEST_PRETTIER_RANGE_FORMAT_SUFFIX; pub use semantic_tokens::{ BufferSemanticToken, BufferSemanticTokens, RefreshForServer, SemanticTokenStylizer, TokenType, }; @@ -1714,17 +1716,64 @@ impl LocalLspStore { zlog::trace!(logger => "formatting"); let _timer = zlog::time!(logger => "Formatting buffer via prettier"); + // When selection ranges are provided (via FormatSelections), we pass the + // encompassing UTF-16 range to Prettier so it can scope its formatting. + // After diffing, we filter the resulting edits to only keep those that + // overlap with the original byte-level selection ranges. + let (range_utf16, byte_ranges) = match buffer.ranges.as_ref() { + Some(ranges) if !ranges.is_empty() => { + let (utf16_range, byte_ranges) = + buffer.handle.read_with(cx, |buffer, _cx| { + let snapshot = buffer.snapshot(); + let mut min_start_utf16 = OffsetUtf16(usize::MAX); + let mut max_end_utf16 = OffsetUtf16(0); + let mut byte_ranges = Vec::with_capacity(ranges.len()); + for range in ranges { + let start_utf16 = range.start.to_offset_utf16(&snapshot); + let end_utf16 = range.end.to_offset_utf16(&snapshot); + min_start_utf16.0 = min_start_utf16.0.min(start_utf16.0); + max_end_utf16.0 = max_end_utf16.0.max(end_utf16.0); + + let start_byte = range.start.to_offset(&snapshot); + let end_byte = range.end.to_offset(&snapshot); + byte_ranges.push(start_byte..end_byte); + } + (min_start_utf16..max_end_utf16, byte_ranges) + }); + (Some(utf16_range), Some(byte_ranges)) + } + _ => (None, None), + }; + let prettier = lsp_store.read_with(cx, |lsp_store, _cx| { lsp_store.prettier_store().unwrap().downgrade() })?; - let diff = prettier_store::format_with_prettier(&prettier, &buffer.handle, cx) - .await - .transpose()?; - let Some(diff) = diff else { + let diff = prettier_store::format_with_prettier( + &prettier, + &buffer.handle, + range_utf16, + cx, + ) + .await + .transpose()?; + let Some(mut diff) = diff else { zlog::trace!(logger => "No changes"); return Ok(()); }; + if let Some(byte_ranges) = byte_ranges { + diff.edits.retain(|(edit_range, _)| { + byte_ranges.iter().any(|selection_range| { + edit_range.start < selection_range.end + && edit_range.end > selection_range.start + }) + }); + if diff.edits.is_empty() { + zlog::trace!(logger => "No changes within selection"); + return Ok(()); + } + } + extend_formatting_transaction( buffer, formatting_transaction_id, @@ -1736,6 +1785,12 @@ impl LocalLspStore { } Formatter::External { command, arguments } => { let logger = zlog::scoped!(logger => "command"); + + if buffer.ranges.is_some() { + zlog::debug!(logger => "External formatter does not support range formatting; skipping"); + return Ok(()); + } + zlog::trace!(logger => "formatting"); let _timer = zlog::time!(logger => "Formatting buffer via external command"); diff --git a/crates/project/src/prettier_store.rs b/crates/project/src/prettier_store.rs index 95150fda070e488cd9d6d43238c5aa99515aa271..b66f2d5e0c041e104cf109a48b6bad249b492b88 100644 --- a/crates/project/src/prettier_store.rs +++ b/crates/project/src/prettier_store.rs @@ -1,5 +1,5 @@ use std::{ - ops::ControlFlow, + ops::{ControlFlow, Range}, path::{Path, PathBuf}, sync::Arc, time::Duration, @@ -15,7 +15,7 @@ use futures::{ }; use gpui::{AppContext as _, AsyncApp, Context, Entity, EventEmitter, Task, WeakEntity}; use language::{ - Buffer, LanguageRegistry, LocalFile, + Buffer, LanguageRegistry, LocalFile, OffsetUtf16, language_settings::{Formatter, LanguageSettings}, }; use lsp::{LanguageServer, LanguageServerId, LanguageServerName}; @@ -736,6 +736,7 @@ pub fn prettier_plugins_for_language( pub(super) async fn format_with_prettier( prettier_store: &WeakEntity, buffer: &Entity, + range_utf16: Option>, cx: &mut AsyncApp, ) -> Option> { let prettier_instance = prettier_store @@ -772,7 +773,14 @@ pub(super) async fn format_with_prettier( }); let format_result = prettier - .format(buffer, buffer_path, ignore_dir, request_timeout, cx) + .format( + buffer, + buffer_path, + ignore_dir, + range_utf16, + request_timeout, + cx, + ) .await .with_context(|| format!("{} failed to format buffer", prettier_description)); diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 0f9ad1a4356d72bd15688d64a3909dd73dbaad35..96b82a16930543028b7588a843433c6a70bf34e6 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -150,6 +150,8 @@ pub use fs::*; pub use language::Location; #[cfg(any(test, feature = "test-support"))] pub use prettier::FORMAT_SUFFIX as TEST_PRETTIER_FORMAT_SUFFIX; +#[cfg(any(test, feature = "test-support"))] +pub use prettier::RANGE_FORMAT_SUFFIX as TEST_PRETTIER_RANGE_FORMAT_SUFFIX; pub use task_inventory::{ BasicContextProvider, ContextProviderWithTasks, DebugScenarioContext, Inventory, TaskContexts, TaskSourceKind, diff --git a/docs/src/configuring-languages.md b/docs/src/configuring-languages.md index 485d843fd480177376cf4e5e990fc495e2bb60a7..46a10e80e3807c1dd57df2184e814b2abe8647d7 100644 --- a/docs/src/configuring-languages.md +++ b/docs/src/configuring-languages.md @@ -351,6 +351,18 @@ To run linter fixes automatically on save: } ``` +### Formatting Selections + +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. +- **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. + ### Integrating Formatting and Linting Zed allows you to run both formatting and linting on save. Here's an example that uses Prettier for formatting and ESLint for linting JavaScript files: