Fix FormatSelections to only format selected ranges, not entire document#51593
Fix FormatSelections to only format selected ranges, not entire document#51593SomeoneToIgnore merged 8 commits intozed-industries:mainfrom
Conversation
SomeoneToIgnore
left a comment
There was a problem hiding this comment.
Nice to see this working well.
One curious thing to test manually (as due to the way Prettier is mocked now we cannot test the real formatting, alas) is the multi-char treatment, e.g. what if there's a bunch of √ before and after and within the selection for Prettier, does Zed behave normally?
I'll test this manually. |
af8d15c to
a1214db
Compare
|
@SomeoneToIgnore I've added more tests with help from LLM. And have also added documentation as per the discussion. Please review again. |
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. Signed-off-by: Pratik Karki <pratik@prertik.com> diff --git a/crates/prettier/src/prettier.rs b/crates/prettier/src/prettier.rs index b0fd57f..c42d307 100644 --- a/crates/prettier/src/prettier.rs +++ b/crates/prettier/src/prettier.rs @@ -9,7 +9,7 @@ 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, @@ -348,6 +348,7 @@ impl Prettier { buffer: &Entity<Buffer>, buffer_path: Option<PathBuf>, ignore_dir: Option<PathBuf>, + range_utf16: Option<Range<usize>>, request_timeout: Duration, cx: &mut AsyncApp, ) -> anyhow::Result<Diff> { @@ -478,6 +479,8 @@ impl Prettier { plugins, prettier_options, ignore_path, + range_start: range_utf16.as_ref().map(|r| r.start), + range_end: range_utf16.as_ref().map(|r| r.end), }, }) }) @@ -651,6 +654,10 @@ struct FormatOptions { path: Option<PathBuf>, prettier_options: Option<HashMap<String, serde_json::Value>>, ignore_path: Option<PathBuf>, + #[serde(skip_serializing_if = "Option::is_none")] + range_start: Option<usize>, + #[serde(skip_serializing_if = "Option::is_none")] + range_end: Option<usize>, } #[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] diff --git a/crates/prettier/src/prettier_server.js b/crates/prettier/src/prettier_server.js index b3d8a66..917095f 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 b08aa8d..18cca7e 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -74,7 +74,8 @@ use language::{ 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, + ToOffset, ToOffsetUtf16, + ToPointUtf16, Toolchain, Transaction, Unclipped, language_settings::{ AllLanguageSettings, FormatOnSave, Formatter, LanguageSettings, all_language_settings, }, @@ -1722,30 +1723,59 @@ impl LocalLspStore { zlog::trace!(logger => "formatting"); let _timer = zlog::time!(logger => "Formatting buffer via prettier"); - 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) + let range_utf16 = buffer.ranges.as_ref().and_then(|ranges| { + if ranges.is_empty() { + return None; + } + Some(buffer.handle.read_with(cx, |buffer, _cx| { + let snapshot = buffer.snapshot(); + let mut min_start = usize::MAX; + let mut max_end = 0usize; + for range in ranges { + let start = range.start.to_offset_utf16(&snapshot).0; + let end = range.end.to_offset_utf16(&snapshot).0; + min_start = min_start.min(start); + max_end = max_end.max(end); + } + min_start..max_end + })) + }); + + 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, + range_utf16, + cx, + ) .await .transpose()?; - let Some(diff) = diff else { - zlog::trace!(logger => "No changes"); - return Ok(()); + let Some(diff) = diff else { + zlog::trace!(logger => "No changes"); + return Ok(()); }; - extend_formatting_transaction( - buffer, - formatting_transaction_id, - cx, - |buffer, cx| { - buffer.apply_diff(diff, cx); - }, - )?; - } - Formatter::External { command, arguments } => { - let logger = zlog::scoped!(logger => "command"); - zlog::trace!(logger => "formatting"); - let _timer = zlog::time!(logger => "Formatting buffer via external command"); + extend_formatting_transaction( + buffer, + formatting_transaction_id, + cx, + |buffer, cx| { + buffer.apply_diff(diff, cx); + }, + )?; + } + Formatter::External { command, arguments } => { + let logger = zlog::scoped!(logger => "command"); + + if buffer.ranges.is_some() { + zlog::trace!(logger => "External formatter does not support range formatting; skipping"); + continue; + } + + zlog::trace!(logger => "formatting"); + let _timer = zlog::time!(logger => "Formatting buffer via external command"); let diff = Self::format_via_external_command(buffer, &command, arguments.as_deref(), cx) diff --git a/crates/project/src/prettier_store.rs b/crates/project/src/prettier_store.rs index 95150fd..c29722f 100644 --- a/crates/project/src/prettier_store.rs +++ b/crates/project/src/prettier_store.rs @@ -736,6 +736,7 @@ pub fn prettier_plugins_for_language( pub(super) async fn format_with_prettier( prettier_store: &WeakEntity<PrettierStore>, buffer: &Entity<Buffer>, + range_utf16: Option<std::ops::Range<usize>>, cx: &mut AsyncApp, ) -> Option<Result<language::Diff>> { 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/prettier/src/prettier.rs b/crates/prettier/src/prettier.rs index b0fd57f..c42d307 100644 --- a/crates/prettier/src/prettier.rs +++ b/crates/prettier/src/prettier.rs @@ -9,7 +9,7 @@ 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, @@ -348,6 +348,7 @@ impl Prettier { buffer: &Entity<Buffer>, buffer_path: Option<PathBuf>, ignore_dir: Option<PathBuf>, + range_utf16: Option<Range<usize>>, request_timeout: Duration, cx: &mut AsyncApp, ) -> anyhow::Result<Diff> { @@ -478,6 +479,8 @@ impl Prettier { plugins, prettier_options, ignore_path, + range_start: range_utf16.as_ref().map(|r| r.start), + range_end: range_utf16.as_ref().map(|r| r.end), }, }) }) @@ -651,6 +654,10 @@ struct FormatOptions { path: Option<PathBuf>, prettier_options: Option<HashMap<String, serde_json::Value>>, ignore_path: Option<PathBuf>, + #[serde(skip_serializing_if = "Option::is_none")] + range_start: Option<usize>, + #[serde(skip_serializing_if = "Option::is_none")] + range_end: Option<usize>, } #[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] diff --git a/crates/prettier/src/prettier_server.js b/crates/prettier/src/prettier_server.js index b3d8a66..917095f 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 d36a456..8db2253 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -74,7 +74,8 @@ use language::{ 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, + TextBufferSnapshot, ToOffset, ToOffsetUtf16, + ToPointUtf16, Toolchain, Transaction, Unclipped, language_settings::{ AllLanguageSettings, FormatOnSave, Formatter, LanguageSettings, all_language_settings, }, @@ -1714,30 +1715,59 @@ impl LocalLspStore { zlog::trace!(logger => "formatting"); let _timer = zlog::time!(logger => "Formatting buffer via prettier"); - 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) + let range_utf16 = buffer.ranges.as_ref().and_then(|ranges| { + if ranges.is_empty() { + return None; + } + Some(buffer.handle.read_with(cx, |buffer, _cx| { + let snapshot = buffer.snapshot(); + let mut min_start = usize::MAX; + let mut max_end = 0usize; + for range in ranges { + let start = range.start.to_offset_utf16(&snapshot).0; + let end = range.end.to_offset_utf16(&snapshot).0; + min_start = min_start.min(start); + max_end = max_end.max(end); + } + min_start..max_end + })) + }); + + 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, + range_utf16, + cx, + ) .await .transpose()?; - let Some(diff) = diff else { - zlog::trace!(logger => "No changes"); - return Ok(()); + let Some(diff) = diff else { + zlog::trace!(logger => "No changes"); + return Ok(()); }; - extend_formatting_transaction( - buffer, - formatting_transaction_id, - cx, - |buffer, cx| { - buffer.apply_diff(diff, cx); - }, - )?; - } - Formatter::External { command, arguments } => { - let logger = zlog::scoped!(logger => "command"); - zlog::trace!(logger => "formatting"); - let _timer = zlog::time!(logger => "Formatting buffer via external command"); + extend_formatting_transaction( + buffer, + formatting_transaction_id, + cx, + |buffer, cx| { + buffer.apply_diff(diff, cx); + }, + )?; + } + Formatter::External { command, arguments } => { + let logger = zlog::scoped!(logger => "command"); + + if buffer.ranges.is_some() { + zlog::trace!(logger => "External formatter does not support range formatting; skipping"); + continue; + } + + zlog::trace!(logger => "formatting"); + let _timer = zlog::time!(logger => "Formatting buffer via external command"); let diff = Self::format_via_external_command(buffer, &command, arguments.as_deref(), cx) diff --git a/crates/project/src/prettier_store.rs b/crates/project/src/prettier_store.rs index 95150fd..c29722f 100644 --- a/crates/project/src/prettier_store.rs +++ b/crates/project/src/prettier_store.rs @@ -736,6 +736,7 @@ pub fn prettier_plugins_for_language( pub(super) async fn format_with_prettier( prettier_store: &WeakEntity<PrettierStore>, buffer: &Entity<Buffer>, + range_utf16: Option<std::ops::Range<usize>>, cx: &mut AsyncApp, ) -> Option<Result<language::Diff>> { 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));
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. Signed-off-by: Pratik Karki <pratik@prertik.com> diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 18cca7e..7e156a1 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -1723,23 +1723,30 @@ impl LocalLspStore { zlog::trace!(logger => "formatting"); let _timer = zlog::time!(logger => "Formatting buffer via prettier"); - let range_utf16 = buffer.ranges.as_ref().and_then(|ranges| { - if ranges.is_empty() { - return None; + 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 = usize::MAX; + let mut max_end_utf16 = 0usize; + let mut byte_ranges = Vec::with_capacity(ranges.len()); + for range in ranges { + let start_utf16 = range.start.to_offset_utf16(&snapshot).0; + let end_utf16 = range.end.to_offset_utf16(&snapshot).0; + min_start_utf16 = min_start_utf16.min(start_utf16); + max_end_utf16 = max_end_utf16.max(end_utf16); + + 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)) } - Some(buffer.handle.read_with(cx, |buffer, _cx| { - let snapshot = buffer.snapshot(); - let mut min_start = usize::MAX; - let mut max_end = 0usize; - for range in ranges { - let start = range.start.to_offset_utf16(&snapshot).0; - let end = range.end.to_offset_utf16(&snapshot).0; - min_start = min_start.min(start); - max_end = max_end.max(end); - } - min_start..max_end - })) - }); + _ => (None, None), + }; let prettier = lsp_store.read_with(cx, |lsp_store, _cx| { lsp_store.prettier_store().unwrap().downgrade() @@ -1752,11 +1759,24 @@ impl LocalLspStore { ) .await .transpose()?; - let Some(diff) = diff else { + 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"); + continue; + } + } + extend_formatting_transaction( buffer, formatting_transaction_id, diff --git a/crates/project/src/prettier_store.rs b/crates/project/src/prettier_store.rs index c29722f..e800700 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, @@ -736,7 +736,7 @@ pub fn prettier_plugins_for_language( pub(super) async fn format_with_prettier( prettier_store: &WeakEntity<PrettierStore>, buffer: &Entity<Buffer>, - range_utf16: Option<std::ops::Range<usize>>, + range_utf16: Option<Range<usize>>, cx: &mut AsyncApp, ) -> Option<Result<language::Diff>> { let prettier_instance = prettier_store diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 8db2253..81f72d7 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -1715,23 +1715,30 @@ impl LocalLspStore { zlog::trace!(logger => "formatting"); let _timer = zlog::time!(logger => "Formatting buffer via prettier"); - let range_utf16 = buffer.ranges.as_ref().and_then(|ranges| { - if ranges.is_empty() { - return None; + 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 = usize::MAX; + let mut max_end_utf16 = 0usize; + let mut byte_ranges = Vec::with_capacity(ranges.len()); + for range in ranges { + let start_utf16 = range.start.to_offset_utf16(&snapshot).0; + let end_utf16 = range.end.to_offset_utf16(&snapshot).0; + min_start_utf16 = min_start_utf16.min(start_utf16); + max_end_utf16 = max_end_utf16.max(end_utf16); + + 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)) } - Some(buffer.handle.read_with(cx, |buffer, _cx| { - let snapshot = buffer.snapshot(); - let mut min_start = usize::MAX; - let mut max_end = 0usize; - for range in ranges { - let start = range.start.to_offset_utf16(&snapshot).0; - let end = range.end.to_offset_utf16(&snapshot).0; - min_start = min_start.min(start); - max_end = max_end.max(end); - } - min_start..max_end - })) - }); + _ => (None, None), + }; let prettier = lsp_store.read_with(cx, |lsp_store, _cx| { lsp_store.prettier_store().unwrap().downgrade() @@ -1744,11 +1751,24 @@ impl LocalLspStore { ) .await .transpose()?; - let Some(diff) = diff else { + 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"); + continue; + } + } + extend_formatting_transaction( buffer, formatting_transaction_id, diff --git a/crates/project/src/prettier_store.rs b/crates/project/src/prettier_store.rs index c29722f..e800700 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, @@ -736,7 +736,7 @@ pub fn prettier_plugins_for_language( pub(super) async fn format_with_prettier( prettier_store: &WeakEntity<PrettierStore>, buffer: &Entity<Buffer>, - range_utf16: Option<std::ops::Range<usize>>, + range_utf16: Option<Range<usize>>, cx: &mut AsyncApp, ) -> Option<Result<language::Diff>> { let prettier_instance = prettier_store
Signed-off-by: Pratik Karki <pratik@prertik.com> diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 7e156a1..2d167ca 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -1790,7 +1790,7 @@ impl LocalLspStore { let logger = zlog::scoped!(logger => "command"); if buffer.ranges.is_some() { - zlog::trace!(logger => "External formatter does not support range formatting; skipping"); + zlog::debug!(logger => "External formatter does not support range formatting; skipping"); continue; } diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 81f72d7..fad30e2 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -1782,7 +1782,7 @@ impl LocalLspStore { let logger = zlog::scoped!(logger => "command"); if buffer.ranges.is_some() { - zlog::trace!(logger => "External formatter does not support range formatting; skipping"); + zlog::debug!(logger => "External formatter does not support range formatting; skipping"); continue; }
Signed-off-by: Pratik Karki <pratik@prertik.com> diff --git a/crates/prettier/src/prettier.rs b/crates/prettier/src/prettier.rs index c42d307..db58d2b 100644 --- a/crates/prettier/src/prettier.rs +++ b/crates/prettier/src/prettier.rs @@ -3,7 +3,7 @@ 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; @@ -348,7 +348,7 @@ impl Prettier { buffer: &Entity<Buffer>, buffer_path: Option<PathBuf>, ignore_dir: Option<PathBuf>, - range_utf16: Option<Range<usize>>, + range_utf16: Option<Range<OffsetUtf16>>, request_timeout: Duration, cx: &mut AsyncApp, ) -> anyhow::Result<Diff> { @@ -479,8 +479,8 @@ impl Prettier { plugins, prettier_options, ignore_path, - range_start: range_utf16.as_ref().map(|r| r.start), - range_end: range_utf16.as_ref().map(|r| r.end), + range_start: range_utf16.as_ref().map(|r| r.start.0), + range_end: range_utf16.as_ref().map(|r| r.end.0), }, }) }) diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 2d167ca..b6dc9ca 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -73,7 +73,7 @@ use language::{ Bias, BinaryStatus, Buffer, BufferRow, BufferSnapshot, CachedLspAdapter, Capability, CodeLabel, Diagnostic, DiagnosticEntry, DiagnosticSet, DiagnosticSourceKind, Diff, File as _, Language, LanguageName, LanguageRegistry, LocalFile, LspAdapter, LspAdapterDelegate, LspInstaller, - ManifestDelegate, ManifestName, ModelineSettings, Patch, PointUtf16, TextBufferSnapshot, + ManifestDelegate, ManifestName, ModelineSettings, OffsetUtf16, Patch, PointUtf16, TextBufferSnapshot, ToOffset, ToOffsetUtf16, ToPointUtf16, Toolchain, Transaction, Unclipped, language_settings::{ @@ -1728,14 +1728,14 @@ impl LocalLspStore { let (utf16_range, byte_ranges) = buffer.handle.read_with(cx, |buffer, _cx| { let snapshot = buffer.snapshot(); - let mut min_start_utf16 = usize::MAX; - let mut max_end_utf16 = 0usize; + 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).0; - let end_utf16 = range.end.to_offset_utf16(&snapshot).0; - min_start_utf16 = min_start_utf16.min(start_utf16); - max_end_utf16 = max_end_utf16.max(end_utf16); + 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); diff --git a/crates/project/src/prettier_store.rs b/crates/project/src/prettier_store.rs index e800700..b66f2d5 100644 --- a/crates/project/src/prettier_store.rs +++ b/crates/project/src/prettier_store.rs @@ -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,7 +736,7 @@ pub fn prettier_plugins_for_language( pub(super) async fn format_with_prettier( prettier_store: &WeakEntity<PrettierStore>, buffer: &Entity<Buffer>, - range_utf16: Option<Range<usize>>, + range_utf16: Option<Range<OffsetUtf16>>, cx: &mut AsyncApp, ) -> Option<Result<language::Diff>> { let prettier_instance = prettier_store diff --git a/crates/prettier/src/prettier.rs b/crates/prettier/src/prettier.rs index c42d307..db58d2b 100644 --- a/crates/prettier/src/prettier.rs +++ b/crates/prettier/src/prettier.rs @@ -3,7 +3,7 @@ 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; @@ -348,7 +348,7 @@ impl Prettier { buffer: &Entity<Buffer>, buffer_path: Option<PathBuf>, ignore_dir: Option<PathBuf>, - range_utf16: Option<Range<usize>>, + range_utf16: Option<Range<OffsetUtf16>>, request_timeout: Duration, cx: &mut AsyncApp, ) -> anyhow::Result<Diff> { @@ -479,8 +479,8 @@ impl Prettier { plugins, prettier_options, ignore_path, - range_start: range_utf16.as_ref().map(|r| r.start), - range_end: range_utf16.as_ref().map(|r| r.end), + range_start: range_utf16.as_ref().map(|r| r.start.0), + range_end: range_utf16.as_ref().map(|r| r.end.0), }, }) }) diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index fad30e2..1591652 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -73,7 +73,7 @@ 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, + LspInstaller, ManifestDelegate, ManifestName, ModelineSettings, OffsetUtf16, Patch, PointUtf16, TextBufferSnapshot, ToOffset, ToOffsetUtf16, ToPointUtf16, Toolchain, Transaction, Unclipped, language_settings::{ @@ -1720,14 +1720,14 @@ impl LocalLspStore { let (utf16_range, byte_ranges) = buffer.handle.read_with(cx, |buffer, _cx| { let snapshot = buffer.snapshot(); - let mut min_start_utf16 = usize::MAX; - let mut max_end_utf16 = 0usize; + 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).0; - let end_utf16 = range.end.to_offset_utf16(&snapshot).0; - min_start_utf16 = min_start_utf16.min(start_utf16); - max_end_utf16 = max_end_utf16.max(end_utf16); + 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); diff --git a/crates/project/src/prettier_store.rs b/crates/project/src/prettier_store.rs index e800700..b66f2d5 100644 --- a/crates/project/src/prettier_store.rs +++ b/crates/project/src/prettier_store.rs @@ -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,7 +736,7 @@ pub fn prettier_plugins_for_language( pub(super) async fn format_with_prettier( prettier_store: &WeakEntity<PrettierStore>, buffer: &Entity<Buffer>, - range_utf16: Option<Range<usize>>, + range_utf16: Option<Range<OffsetUtf16>>, cx: &mut AsyncApp, ) -> Option<Result<language::Diff>> { let prettier_instance = prettier_store
Signed-off-by: Pratik Karki <pratik@prertik.com> diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 1591652..c936295 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -74,8 +74,7 @@ use language::{ CodeLabelExt, Diagnostic, DiagnosticEntry, DiagnosticSet, DiagnosticSourceKind, Diff, File as _, Language, LanguageName, LanguageRegistry, LocalFile, LspAdapter, LspAdapterDelegate, LspInstaller, ManifestDelegate, ManifestName, ModelineSettings, OffsetUtf16, Patch, PointUtf16, - TextBufferSnapshot, ToOffset, ToOffsetUtf16, - ToPointUtf16, Toolchain, Transaction, Unclipped, + TextBufferSnapshot, ToOffset, ToOffsetUtf16, ToPointUtf16, Toolchain, Transaction, Unclipped, language_settings::{ AllLanguageSettings, FormatOnSave, Formatter, LanguageSettings, all_language_settings, }, @@ -1716,78 +1715,78 @@ impl LocalLspStore { let _timer = zlog::time!(logger => "Formatting buffer via prettier"); 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), - }; + 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, - range_utf16, - cx, - ) - .await - .transpose()?; - let Some(mut diff) = diff else { - zlog::trace!(logger => "No changes"); - return Ok(()); + 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, + 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"); - continue; - } + 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"); + continue; } - - extend_formatting_transaction( - buffer, - formatting_transaction_id, - cx, - |buffer, cx| { - buffer.apply_diff(diff, cx); - }, - )?; } - 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"); - continue; - } + extend_formatting_transaction( + buffer, + formatting_transaction_id, + cx, + |buffer, cx| { + buffer.apply_diff(diff, cx); + }, + )?; + } + 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"); + continue; + } - zlog::trace!(logger => "formatting"); - let _timer = zlog::time!(logger => "Formatting buffer via external command"); + zlog::trace!(logger => "formatting"); + let _timer = zlog::time!(logger => "Formatting buffer via external command"); let diff = Self::format_via_external_command(buffer, &command, arguments.as_deref(), cx)
Signed-off-by: Pratik Karki <pratik@prertik.com>
Signed-off-by: Pratik Karki <pratik@prertik.com> diff --git a/crates/editor/src/actions.rs b/crates/editor/src/actions.rs index a51ee78..f4b4c69 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 f285d13..5ec9aeb 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -20443,10 +20443,9 @@ async fn test_language_server_restart_due_to_settings_change(cx: &mut TestAppCon }, ); - let _window = cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); - let _buffer = project + let buffer = project .update(cx, |project, cx| { - project.open_local_buffer_with_lsp(path!("/a/main.rs"), cx) + project.open_local_buffer(path!("/file.ts"), cx) }) .await .unwrap(); @@ -21638,6 +21637,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/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index c936295..10fb447 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -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,6 +1716,10 @@ 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) = @@ -1764,7 +1770,7 @@ impl LocalLspStore { }); if diff.edits.is_empty() { zlog::trace!(logger => "No changes within selection"); - continue; + return Ok(()); } } @@ -1782,7 +1788,7 @@ impl LocalLspStore { if buffer.ranges.is_some() { zlog::debug!(logger => "External formatter does not support range formatting; skipping"); - continue; + return Ok(()); } zlog::trace!(logger => "formatting"); diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 0f9ad1a..96b82a1 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 485d843..46a10e8 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:
Signed-off-by: Pratik Karki <pratik@prertik.com>
b4533f8 to
fddd818
Compare
SomeoneToIgnore
left a comment
There was a problem hiding this comment.
Great, thank you so much for the fix, this should solve most of the format on select -related issues it seems.
Congratulations with your first contribution!
When
editor: format selectionsget invoked, the Prettier and external formatter branches informat_buffer_locallyignored the selection ranges entirely, causing the whole document to be formatted.rangeStart/rangeEndoptions in the format request.Part of #25796
Before you mark this PR as ready for review, make sure that you have:
Release Notes:
Current Behaviour:
original.behaviour.webm
New Behaviour:
new.behaviour.webm