diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index 6ef0d964def1f5..bacdcbd3b7405d 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -27,10 +27,10 @@ use language::{ }; use lsp::LanguageServerId; use parking_lot::Mutex; -use project::lsp_store::FormatTarget; use project::{ - lsp_store::FormatTrigger, search::SearchQuery, search::SearchResult, DiagnosticSummary, - HoverBlockKind, Project, ProjectPath, + lsp_store::{FormatTrigger, LspFormatTarget}, + search::{SearchQuery, SearchResult}, + DiagnosticSummary, HoverBlockKind, Project, ProjectPath, }; use rand::prelude::*; use serde_json::json; @@ -4400,9 +4400,9 @@ async fn test_formatting_buffer( .update(cx_b, |project, cx| { project.format( HashSet::from_iter([buffer_b.clone()]), + LspFormatTarget::Buffers, true, FormatTrigger::Save, - FormatTarget::Buffer, cx, ) }) @@ -4436,9 +4436,9 @@ async fn test_formatting_buffer( .update(cx_b, |project, cx| { project.format( HashSet::from_iter([buffer_b.clone()]), + LspFormatTarget::Buffers, true, FormatTrigger::Save, - FormatTarget::Buffer, cx, ) }) @@ -4546,9 +4546,9 @@ async fn test_prettier_formatting_buffer( .update(cx_b, |project, cx| { project.format( HashSet::from_iter([buffer_b.clone()]), + LspFormatTarget::Buffers, true, FormatTrigger::Save, - FormatTarget::Buffer, cx, ) }) @@ -4566,9 +4566,9 @@ async fn test_prettier_formatting_buffer( .update(cx_a, |project, cx| { project.format( HashSet::from_iter([buffer_a.clone()]), + LspFormatTarget::Buffers, true, FormatTrigger::Manual, - FormatTarget::Buffer, cx, ) }) diff --git a/crates/collab/src/tests/remote_editing_collaboration_tests.rs b/crates/collab/src/tests/remote_editing_collaboration_tests.rs index 0698bc4007a859..30fa54935eb10f 100644 --- a/crates/collab/src/tests/remote_editing_collaboration_tests.rs +++ b/crates/collab/src/tests/remote_editing_collaboration_tests.rs @@ -16,7 +16,7 @@ use language::{ }; use node_runtime::NodeRuntime; use project::{ - lsp_store::{FormatTarget, FormatTrigger}, + lsp_store::{FormatTrigger, LspFormatTarget}, ProjectPath, }; use remote::SshRemoteClient; @@ -472,9 +472,9 @@ async fn test_ssh_collaboration_formatting_with_prettier( .update(cx_b, |project, cx| { project.format( HashSet::from_iter([buffer_b.clone()]), + LspFormatTarget::Buffers, true, FormatTrigger::Save, - FormatTarget::Buffer, cx, ) }) @@ -509,9 +509,9 @@ async fn test_ssh_collaboration_formatting_with_prettier( .update(cx_a, |project, cx| { project.format( HashSet::from_iter([buffer_a.clone()]), + LspFormatTarget::Buffers, true, FormatTrigger::Manual, - FormatTarget::Buffer, cx, ) }) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index a9988c9eaa10d6..4a9aa79e306ae1 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -129,7 +129,7 @@ use multi_buffer::{ }; use project::{ buffer_store::BufferChangeSet, - lsp_store::{FormatTarget, FormatTrigger, OpenLspBufferHandle}, + lsp_store::{FormatTrigger, LspFormatTarget, OpenLspBufferHandle}, project_settings::{GitGutterSetting, ProjectSettings}, CodeAction, Completion, CompletionIntent, DocumentHighlight, InlayHint, Location, LocationLink, LspStore, Project, ProjectItem, ProjectTransaction, TaskSourceKind, @@ -986,6 +986,11 @@ impl InlayHintRefreshReason { } } +pub enum FormatTarget { + Buffers, + Ranges(Vec>), +} + pub(crate) struct FocusedBlock { id: BlockId, focus_handle: WeakFocusHandle, @@ -10237,7 +10242,7 @@ impl Editor { None => return None, }; - Some(self.perform_format(project, FormatTrigger::Manual, FormatTarget::Buffer, cx)) + Some(self.perform_format(project, FormatTrigger::Manual, FormatTarget::Buffers, cx)) } fn format_selections( @@ -10250,17 +10255,18 @@ impl Editor { None => return None, }; - let selections = self + let ranges = self .selections .all_adjusted(cx) .into_iter() + .map(|selection| selection.range()) .filter(|s| !s.is_empty()) .collect_vec(); Some(self.perform_format( project, FormatTrigger::Manual, - FormatTarget::Ranges(selections), + FormatTarget::Ranges(ranges), cx, )) } @@ -10272,15 +10278,41 @@ impl Editor { target: FormatTarget, cx: &mut ViewContext, ) -> Task> { - let buffer = self.buffer().clone(); - let mut buffers = buffer.read(cx).all_buffers(); - if trigger == FormatTrigger::Save { - buffers.retain(|buffer| buffer.read(cx).is_dirty()); - } + let buffer = self.buffer.clone(); + let (buffers, target) = match target { + FormatTarget::Buffers => { + let mut buffers = buffer.read(cx).all_buffers(); + if trigger == FormatTrigger::Save { + buffers.retain(|buffer| buffer.read(cx).is_dirty()); + } + (buffers, LspFormatTarget::Buffers) + } + FormatTarget::Ranges(selection_ranges) => { + let multi_buffer = buffer.read(cx); + let snapshot = multi_buffer.read(cx); + let mut buffers = HashSet::default(); + let mut buffer_id_to_ranges: BTreeMap>> = + BTreeMap::new(); + for selection_range in selection_ranges { + for (excerpt, buffer_range) in snapshot.range_to_buffer_ranges(selection_range) + { + let buffer_id = excerpt.buffer_id(); + let start = excerpt.buffer().anchor_before(buffer_range.start); + let end = excerpt.buffer().anchor_after(buffer_range.end); + buffers.insert(multi_buffer.buffer(buffer_id).unwrap()); + buffer_id_to_ranges + .entry(buffer_id) + .and_modify(|buffer_ranges| buffer_ranges.push(start..end)) + .or_insert_with(|| vec![start..end]); + } + } + (buffers, LspFormatTarget::Ranges(buffer_id_to_ranges)) + } + }; let mut timeout = cx.background_executor().timer(FORMAT_TIMEOUT).fuse(); let format = project.update(cx, |project, cx| { - project.format(buffers, true, trigger, target, cx) + project.format(buffers, target, true, trigger, cx) }); cx.spawn(|_, mut cx| async move { diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 0ba540f2b1fdb9..446bce8d6e29c8 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -7380,7 +7380,7 @@ async fn test_document_format_manual_trigger(cx: &mut gpui::TestAppContext) { editor.perform_format( project.clone(), FormatTrigger::Manual, - FormatTarget::Buffer, + FormatTarget::Buffers, cx, ) }) @@ -7418,7 +7418,7 @@ async fn test_document_format_manual_trigger(cx: &mut gpui::TestAppContext) { }); let format = editor .update(cx, |editor, cx| { - editor.perform_format(project, FormatTrigger::Manual, FormatTarget::Buffer, cx) + editor.perform_format(project, FormatTrigger::Manual, FormatTarget::Buffers, cx) }) .unwrap(); cx.executor().advance_clock(super::FORMAT_TIMEOUT); @@ -11293,7 +11293,7 @@ async fn test_document_format_with_prettier(cx: &mut gpui::TestAppContext) { editor.perform_format( project.clone(), FormatTrigger::Manual, - FormatTarget::Buffer, + FormatTarget::Buffers, cx, ) }) @@ -11312,7 +11312,7 @@ async fn test_document_format_with_prettier(cx: &mut gpui::TestAppContext) { editor.perform_format( project.clone(), FormatTrigger::Manual, - FormatTarget::Buffer, + FormatTarget::Buffers, cx, ) }); diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index 620fcd5ec40db2..25c7079a0991eb 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -2,8 +2,8 @@ use crate::{ editor_settings::SeedQuerySetting, persistence::{SerializedEditor, DB}, scroll::ScrollAnchor, - Anchor, Autoscroll, Editor, EditorEvent, EditorSettings, ExcerptId, ExcerptRange, MultiBuffer, - MultiBufferSnapshot, NavigationData, SearchWithinRange, ToPoint as _, + Anchor, Autoscroll, Editor, EditorEvent, EditorSettings, ExcerptId, ExcerptRange, FormatTarget, + MultiBuffer, MultiBufferSnapshot, NavigationData, SearchWithinRange, ToPoint as _, }; use anyhow::{anyhow, Context as _, Result}; use collections::HashSet; @@ -29,7 +29,6 @@ use rpc::proto::{self, update_view, PeerId}; use settings::Settings; use workspace::item::{Dedup, ItemSettings, SerializableItem, TabContentParams}; -use project::lsp_store::FormatTarget; use std::{ any::TypeId, borrow::Cow, @@ -756,7 +755,7 @@ impl Item for Editor { editor.perform_format( project.clone(), FormatTrigger::Save, - FormatTarget::Buffer, + FormatTarget::Buffers, cx, ) })? diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 90467ec33d98a0..a04388d9101fe3 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -36,11 +36,11 @@ use language::{ }, markdown, point_to_lsp, prepare_completion_documentation, proto::{deserialize_anchor, deserialize_version, serialize_anchor, serialize_version}, - range_from_lsp, Bias, Buffer, BufferSnapshot, CachedLspAdapter, CodeLabel, Diagnostic, - DiagnosticEntry, DiagnosticSet, Diff, Documentation, File as _, Language, LanguageName, - LanguageRegistry, LanguageServerBinaryStatus, LanguageToolchainStore, LocalFile, LspAdapter, - LspAdapterDelegate, Patch, PointUtf16, TextBufferSnapshot, ToOffset, ToPointUtf16, Transaction, - Unclipped, + range_from_lsp, range_to_lsp, Bias, Buffer, BufferSnapshot, CachedLspAdapter, CodeLabel, + Diagnostic, DiagnosticEntry, DiagnosticSet, Diff, Documentation, File as _, Language, + LanguageName, LanguageRegistry, LanguageServerBinaryStatus, LanguageToolchainStore, LocalFile, + LspAdapter, LspAdapterDelegate, Patch, PointUtf16, TextBufferSnapshot, ToOffset, ToPointUtf16, + Transaction, Unclipped, }; use lsp::{ notification::DidRenameFiles, CodeActionKind, CompletionContext, DiagnosticSeverity, @@ -77,7 +77,7 @@ use std::{ sync::Arc, time::{Duration, Instant}, }; -use text::{Anchor, BufferId, LineEnding, Point, Selection}; +use text::{Anchor, BufferId, LineEnding, OffsetRangeExt}; use util::{ debug_panic, defer, maybe, merge_json_value_into, post_inc, ResultExt, TryFutureExt as _, }; @@ -100,18 +100,9 @@ pub enum FormatTrigger { Manual, } -pub enum FormatTarget { - Buffer, - Ranges(Vec>), -} - -impl FormatTarget { - pub fn as_selections(&self) -> Option<&[Selection]> { - match self { - FormatTarget::Buffer => None, - FormatTarget::Ranges(selections) => Some(selections.as_slice()), - } - } +pub enum LspFormatTarget { + Buffers, + Ranges(BTreeMap>>), } // proto::RegisterBufferWithLanguageServer {} @@ -1075,9 +1066,9 @@ impl LocalLspStore { async fn format_locally( lsp_store: WeakModel, mut buffers: Vec, + target: &LspFormatTarget, push_to_history: bool, trigger: FormatTrigger, - target: FormatTarget, mut cx: AsyncAppContext, ) -> anyhow::Result { // Do not allow multiple concurrent formatting requests for the @@ -1182,7 +1173,7 @@ impl LocalLspStore { // Except for code actions, which are applied with all connected language servers. let primary_language_server = primary_adapter_and_server.map(|(_adapter, server)| server.clone()); - let server_and_buffer = primary_language_server + let primary_server_and_path = primary_language_server .as_ref() .zip(buffer.abs_path.as_ref()); @@ -1192,6 +1183,16 @@ impl LocalLspStore { .clone() })?; + let ranges = match target { + LspFormatTarget::Buffers => None, + LspFormatTarget::Ranges(ranges) => { + let Some(ranges) = ranges.get(&buffer.id) else { + return Err(anyhow!("No format ranges provided for buffer")); + }; + Some(ranges) + } + }; + let mut format_operations: Vec = vec![]; { match trigger { @@ -1208,10 +1209,10 @@ impl LocalLspStore { if prettier_settings.allowed { Self::perform_format( &Formatter::Prettier, - &target, - server_and_buffer, - lsp_store.clone(), buffer, + ranges, + primary_server_and_path, + lsp_store.clone(), &settings, &adapters_and_servers, push_to_history, @@ -1222,10 +1223,10 @@ impl LocalLspStore { } else { Self::perform_format( &Formatter::LanguageServer { name: None }, - &target, - server_and_buffer, - lsp_store.clone(), buffer, + ranges, + primary_server_and_path, + lsp_store.clone(), &settings, &adapters_and_servers, push_to_history, @@ -1244,10 +1245,10 @@ impl LocalLspStore { for formatter in formatters.as_ref() { let diff = Self::perform_format( formatter, - &target, - server_and_buffer, - lsp_store.clone(), buffer, + ranges, + primary_server_and_path, + lsp_store.clone(), &settings, &adapters_and_servers, push_to_history, @@ -1268,10 +1269,10 @@ impl LocalLspStore { for formatter in formatters.as_ref() { let diff = Self::perform_format( formatter, - &target, - server_and_buffer, - lsp_store.clone(), buffer, + ranges, + primary_server_and_path, + lsp_store.clone(), &settings, &adapters_and_servers, push_to_history, @@ -1294,10 +1295,10 @@ impl LocalLspStore { if prettier_settings.allowed { Self::perform_format( &Formatter::Prettier, - &target, - server_and_buffer, - lsp_store.clone(), buffer, + ranges, + primary_server_and_path, + lsp_store.clone(), &settings, &adapters_and_servers, push_to_history, @@ -1313,10 +1314,10 @@ impl LocalLspStore { }; Self::perform_format( &formatter, - &target, - server_and_buffer, - lsp_store.clone(), buffer, + ranges, + primary_server_and_path, + lsp_store.clone(), &settings, &adapters_and_servers, push_to_history, @@ -1336,10 +1337,10 @@ impl LocalLspStore { // format with formatter let diff = Self::perform_format( formatter, - &target, - server_and_buffer, - lsp_store.clone(), buffer, + ranges, + primary_server_and_path, + lsp_store.clone(), &settings, &adapters_and_servers, push_to_history, @@ -1408,10 +1409,10 @@ impl LocalLspStore { #[allow(clippy::too_many_arguments)] async fn perform_format( formatter: &Formatter, - format_target: &FormatTarget, - primary_server_and_buffer: Option<(&Arc, &PathBuf)>, - lsp_store: WeakModel, buffer: &FormattableBuffer, + ranges: Option<&Vec>>, + primary_server_and_path: Option<(&Arc, &PathBuf)>, + lsp_store: WeakModel, settings: &LanguageSettings, adapters_and_servers: &[(Arc, Arc)], push_to_history: bool, @@ -1420,7 +1421,7 @@ impl LocalLspStore { ) -> Result, anyhow::Error> { let result = match formatter { Formatter::LanguageServer { name } => { - if let Some((language_server, buffer_abs_path)) = primary_server_and_buffer { + if let Some((language_server, buffer_abs_path)) = primary_server_and_path { let language_server = if let Some(name) = name { adapters_and_servers .iter() @@ -1432,33 +1433,32 @@ impl LocalLspStore { language_server }; - match format_target { - FormatTarget::Buffer => Some(FormatOperation::Lsp( - Self::format_via_lsp( - &lsp_store, - &buffer.handle, - buffer_abs_path, - language_server, - settings, - cx, - ) - .await - .context("failed to format via language server")?, - )), - FormatTarget::Ranges(selections) => Some(FormatOperation::Lsp( - Self::format_range_via_lsp( - &lsp_store, - &buffer.handle, - selections.as_slice(), - buffer_abs_path, - language_server, - settings, - cx, - ) - .await - .context("failed to format ranges via language server")?, - )), - } + let result = if let Some(ranges) = ranges { + Self::format_ranges_via_lsp( + &lsp_store, + &buffer, + ranges, + buffer_abs_path, + language_server, + settings, + cx, + ) + .await + .context("failed to format ranges via language server")? + } else { + Self::format_via_lsp( + &lsp_store, + &buffer.handle, + buffer_abs_path, + language_server, + settings, + cx, + ) + .await + .context("failed to format via language server")? + }; + + Some(FormatOperation::Lsp(result)) } else { None } @@ -1500,10 +1500,10 @@ impl LocalLspStore { anyhow::Ok(result) } - pub async fn format_range_via_lsp( + pub async fn format_ranges_via_lsp( this: &WeakModel, - buffer: &Model, - selections: &[Selection], + buffer: &FormattableBuffer, + ranges: &Vec>, abs_path: &Path, language_server: &Arc, settings: &LanguageSettings, @@ -1523,14 +1523,23 @@ impl LocalLspStore { let text_document = lsp::TextDocumentIdentifier::new(uri); let lsp_edits = { - let ranges = selections.into_iter().map(|s| { - let start = lsp::Position::new(s.start.row, s.start.column); - let end = lsp::Position::new(s.end.row, s.end.column); - lsp::Range::new(start, end) - }); + let mut lsp_ranges = Vec::new(); + this.update(cx, |_this, cx| { + // TODO(#22930): In the case of formatting multibuffer selections, this buffer may + // not have been sent to the language server. This seems like a fairly systemic + // issue, though, the resolution probably is not specific to formatting. + // + // TODO: Instead of using current snapshot, should use the latest snapshot sent to + // LSP. + let snapshot = buffer.handle.read(cx).snapshot(); + for range in ranges { + lsp_ranges.push(range_to_lsp(range.to_point_utf16(&snapshot))?); + } + anyhow::Ok(()) + })??; let mut edits = None; - for range in ranges { + for range in lsp_ranges { if let Some(mut edit) = language_server .request::(lsp::DocumentRangeFormattingParams { text_document: text_document.clone(), @@ -1549,7 +1558,7 @@ impl LocalLspStore { if let Some(lsp_edits) = lsp_edits { this.update(cx, |this, cx| { this.as_local_mut().unwrap().edits_from_lsp( - buffer, + &buffer.handle, lsp_edits, language_server.server_id(), None, @@ -2734,6 +2743,7 @@ impl LocalLspStore { #[derive(Debug)] pub struct FormattableBuffer { + id: BufferId, handle: Model, abs_path: Option, env: Option>, @@ -6906,27 +6916,27 @@ impl LspStore { pub fn format( &mut self, buffers: HashSet>, + target: LspFormatTarget, push_to_history: bool, trigger: FormatTrigger, - target: FormatTarget, cx: &mut ModelContext, ) -> Task> { if let Some(_) = self.as_local() { - let buffers_with_paths = buffers + let buffers = buffers .into_iter() .map(|buffer_handle| { let buffer = buffer_handle.read(cx); let buffer_abs_path = File::from_dyn(buffer.file()) .and_then(|file| file.as_local().map(|f| f.abs_path(cx))); - (buffer_handle, buffer_abs_path) + (buffer_handle, buffer_abs_path, buffer.remote_id()) }) .collect::>(); cx.spawn(move |lsp_store, mut cx| async move { - let mut formattable_buffers = Vec::with_capacity(buffers_with_paths.len()); + let mut formattable_buffers = Vec::with_capacity(buffers.len()); - for (handle, abs_path) in buffers_with_paths { + for (handle, abs_path, id) in buffers { let env = lsp_store .update(&mut cx, |lsp_store, cx| { lsp_store.environment_for_buffer(&handle, cx) @@ -6934,6 +6944,7 @@ impl LspStore { .await; formattable_buffers.push(FormattableBuffer { + id, handle, abs_path, env, @@ -6943,9 +6954,9 @@ impl LspStore { let result = LocalLspStore::format_locally( lsp_store.clone(), formattable_buffers, + &target, push_to_history, trigger, - target, cx.clone(), ) .await; @@ -6956,6 +6967,14 @@ impl LspStore { result }) } else if let Some((client, project_id)) = self.upstream_client() { + // Don't support formatting ranges via remote + match target { + LspFormatTarget::Buffers => {} + LspFormatTarget::Ranges(_) => { + return Task::ready(Ok(ProjectTransaction::default())); + } + } + let buffer_store = self.buffer_store(); cx.spawn(move |lsp_store, mut cx| async move { let result = client @@ -7005,7 +7024,7 @@ impl LspStore { buffers.insert(this.buffer_store.read(cx).get_existing(buffer_id)?); } let trigger = FormatTrigger::from_proto(envelope.payload.trigger); - anyhow::Ok(this.format(buffers, false, trigger, FormatTarget::Buffer, cx)) + anyhow::Ok(this.format(buffers, LspFormatTarget::Buffers, false, trigger, cx)) })??; let project_transaction = format.await?; diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index d7ffd6421ebd80..ed36ea4330fea8 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -59,6 +59,7 @@ use lsp::{ LanguageServerId, LanguageServerName, MessageActionItem, }; use lsp_command::*; +use lsp_store::LspFormatTarget; use node_runtime::NodeRuntime; use parking_lot::Mutex; pub use prettier_store::PrettierStore; @@ -2632,13 +2633,13 @@ impl Project { pub fn format( &mut self, buffers: HashSet>, + target: LspFormatTarget, push_to_history: bool, trigger: lsp_store::FormatTrigger, - target: lsp_store::FormatTarget, cx: &mut ModelContext, ) -> Task> { self.lsp_store.update(cx, |lsp_store, cx| { - lsp_store.format(buffers, push_to_history, trigger, target, cx) + lsp_store.format(buffers, target, push_to_history, trigger, cx) }) }