diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index df0c833ab0..7fbb781f0f 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -76,7 +76,7 @@ use std::{ sync::Arc, time::{Duration, Instant}, }; -use text::{Anchor, BufferId, LineEnding, OffsetRangeExt}; +use text::{Anchor, BufferId, LineEnding, OffsetRangeExt, TransactionId}; use util::{ debug_panic, defer, maybe, merge_json_value_into, paths::SanitizedPath, post_inc, ResultExt, TryFutureExt as _, @@ -1098,7 +1098,6 @@ impl LocalLspStore { async fn format_locally( lsp_store: WeakEntity, mut buffers: Vec, - target: &LspFormatTarget, push_to_history: bool, trigger: FormatTrigger, mut cx: AsyncApp, @@ -1131,25 +1130,16 @@ impl LocalLspStore { let mut project_transaction = ProjectTransaction::default(); for buffer in &buffers { - let (primary_adapter_and_server, adapters_and_servers) = - lsp_store.update(&mut cx, |lsp_store, cx| { - let buffer = buffer.handle.read(cx); + let adapters_and_servers = lsp_store.update(&mut cx, |lsp_store, cx| { + let buffer = buffer.handle.read(cx); - let adapters_and_servers = lsp_store - .as_local() - .unwrap() - .language_servers_for_buffer(buffer, cx) - .map(|(adapter, lsp)| (adapter.clone(), lsp.clone())) - .collect::>(); - - let primary_adapter = lsp_store - .as_local() - .unwrap() - .primary_language_server_for_buffer(buffer, cx) - .map(|(adapter, lsp)| (adapter.clone(), lsp.clone())); - - (primary_adapter, adapters_and_servers) - })?; + lsp_store + .as_local() + .unwrap() + .language_servers_for_buffer(buffer, cx) + .map(|(adapter, lsp)| (adapter.clone(), lsp.clone())) + .collect::>() + })?; let settings = buffer.handle.update(&mut cx, |buffer, cx| { language_settings(buffer.language().map(|l| l.name()), buffer.file(), cx) @@ -1182,6 +1172,8 @@ impl LocalLspStore { buffer.end_transaction(cx) })?; + let initial_transaction_id = whitespace_transaction_id; + // Apply the `code_actions_on_format` before we run the formatter. let code_actions = deserialize_code_actions(&settings.code_actions_on_format); #[allow(clippy::nonminimal_bool)] @@ -1200,278 +1192,99 @@ impl LocalLspStore { .await?; } - // Apply language-specific formatting using either the primary language server - // or external command. - // 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 primary_server_and_path = primary_language_server - .as_ref() - .zip(buffer.abs_path.as_ref()); - let prettier_settings = buffer.handle.read_with(&cx, |buffer, cx| { language_settings(buffer.language().map(|l| l.name()), buffer.file(), cx) .prettier .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 formatters = match (trigger, &settings.format_on_save) { + (FormatTrigger::Save, FormatOnSave::Off) => &[], + (FormatTrigger::Save, FormatOnSave::List(formatters)) => formatters.as_ref(), + (FormatTrigger::Manual, _) | (FormatTrigger::Save, FormatOnSave::On) => { + match &settings.formatter { + SelectedFormatter::Auto => { + if prettier_settings.allowed { + std::slice::from_ref(&Formatter::Prettier) + } else { + std::slice::from_ref(&Formatter::LanguageServer { name: None }) + } + } + SelectedFormatter::List(formatter_list) => formatter_list.as_ref(), + } } }; - - let mut format_operations: Vec = vec![]; - { - match trigger { - FormatTrigger::Save => { - match &settings.format_on_save { - FormatOnSave::Off => { - // nothing - } - FormatOnSave::On => { - match &settings.formatter { - SelectedFormatter::Auto => { - // do the auto-format: prefer prettier, fallback to primary language server - let diff = { - if prettier_settings.allowed { - Self::perform_format( - &Formatter::Prettier, - buffer, - ranges, - primary_server_and_path, - lsp_store.clone(), - &settings, - &adapters_and_servers, - push_to_history, - &mut project_transaction, - &mut cx, - ) - .await - } else { - Self::perform_format( - &Formatter::LanguageServer { name: None }, - buffer, - ranges, - primary_server_and_path, - lsp_store.clone(), - &settings, - &adapters_and_servers, - push_to_history, - &mut project_transaction, - &mut cx, - ) - .await - } - }?; - - if let Some(op) = diff { - format_operations.push(op); - } - } - SelectedFormatter::List(formatters) => { - for formatter in formatters.as_ref() { - let diff = Self::perform_format( - formatter, - buffer, - ranges, - primary_server_and_path, - lsp_store.clone(), - &settings, - &adapters_and_servers, - push_to_history, - &mut project_transaction, - &mut cx, - ) - .await?; - if let Some(op) = diff { - format_operations.push(op); - } - - // format with formatter - } - } - } - } - FormatOnSave::List(formatters) => { - for formatter in formatters.as_ref() { - let diff = Self::perform_format( - formatter, - buffer, - ranges, - primary_server_and_path, - lsp_store.clone(), - &settings, - &adapters_and_servers, - push_to_history, - &mut project_transaction, - &mut cx, - ) - .await?; - if let Some(op) = diff { - format_operations.push(op); - } - } - } - } - } - FormatTrigger::Manual => { - match &settings.formatter { - SelectedFormatter::Auto => { - // do the auto-format: prefer prettier, fallback to primary language server - let diff = { - if prettier_settings.allowed { - Self::perform_format( - &Formatter::Prettier, - buffer, - ranges, - primary_server_and_path, - lsp_store.clone(), - &settings, - &adapters_and_servers, - push_to_history, - &mut project_transaction, - &mut cx, - ) - .await - } else { - let formatter = Formatter::LanguageServer { - name: primary_language_server - .as_ref() - .map(|server| server.name().to_string()), - }; - Self::perform_format( - &formatter, - buffer, - ranges, - primary_server_and_path, - lsp_store.clone(), - &settings, - &adapters_and_servers, - push_to_history, - &mut project_transaction, - &mut cx, - ) - .await - } - }?; - - if let Some(op) = diff { - format_operations.push(op) - } - } - SelectedFormatter::List(formatters) => { - for formatter in formatters.as_ref() { - // format with formatter - let diff = Self::perform_format( - formatter, - buffer, - ranges, - primary_server_and_path, - lsp_store.clone(), - &settings, - &adapters_and_servers, - push_to_history, - &mut project_transaction, - &mut cx, - ) - .await?; - if let Some(op) = diff { - format_operations.push(op); - } - } - } - } - } - } - } - - buffer.handle.update(&mut cx, |b, cx| { - // If the buffer had its whitespace formatted and was edited while the language-specific - // formatting was being computed, avoid applying the language-specific formatting, because - // it can't be grouped with the whitespace formatting in the undo history. - if let Some(transaction_id) = whitespace_transaction_id { - if b.peek_undo_stack() - .map_or(true, |e| e.transaction_id() != transaction_id) - { - format_operations.clear(); - } - } - - // Apply any language-specific formatting, and group the two formatting operations - // in the buffer's undo history. - for operation in format_operations { - match operation { - FormatOperation::Lsp(edits) => { - b.edit(edits, None, cx); - } - FormatOperation::External(diff) => { - b.apply_diff(diff, cx); - } - FormatOperation::Prettier(diff) => { - b.apply_diff(diff, cx); - } - } - - if let Some(transaction_id) = whitespace_transaction_id { - b.group_until_transaction(transaction_id); - } else if let Some(transaction) = project_transaction.0.get(&buffer.handle) { - b.group_until_transaction(transaction.id) - } - } - - if let Some(transaction) = b.finalize_last_transaction().cloned() { - if !push_to_history { - b.forget_transaction(transaction.id); - } - project_transaction - .0 - .insert(buffer.handle.clone(), transaction); - } - })?; + Self::execute_formatters( + lsp_store.clone(), + formatters, + buffer, + &settings, + &adapters_and_servers, + push_to_history, + initial_transaction_id, + &mut project_transaction, + &mut cx, + ) + .await?; } Ok(project_transaction) } #[allow(clippy::too_many_arguments)] - async fn perform_format( - formatter: &Formatter, - buffer: &FormattableBuffer, - ranges: Option<&Vec>>, - primary_server_and_path: Option<(&Arc, &PathBuf)>, + async fn execute_formatters( lsp_store: WeakEntity, + formatters: &[Formatter], + buffer: &FormattableBuffer, settings: &LanguageSettings, adapters_and_servers: &[(Arc, Arc)], push_to_history: bool, - transaction: &mut ProjectTransaction, + mut initial_transaction_id: Option, + project_transaction: &mut ProjectTransaction, cx: &mut AsyncApp, - ) -> Result, anyhow::Error> { - let result = match formatter { - Formatter::LanguageServer { name } => { - if let Some((language_server, buffer_abs_path)) = primary_server_and_path { + ) -> anyhow::Result<()> { + let mut prev_transaction_id = initial_transaction_id; + + for formatter in formatters { + let operation = match formatter { + Formatter::LanguageServer { name } => { + let Some(language_server) = lsp_store.update(cx, |lsp_store, cx| { + lsp_store + .as_local() + .unwrap() + .primary_language_server_for_buffer(buffer.handle.read(cx), cx) + .map(|(_, lsp)| lsp.clone()) + })? + else { + continue; + }; + let Some(buffer_abs_path) = buffer.abs_path.as_ref() else { + continue; + }; + let language_server = if let Some(name) = name { adapters_and_servers .iter() .find_map(|(adapter, server)| { - adapter.name.0.as_ref().eq(name.as_str()).then_some(server) + adapter + .name + .0 + .as_ref() + .eq(name.as_str()) + .then_some(server.clone()) }) .unwrap_or(language_server) } else { language_server }; - let result = if let Some(ranges) = ranges { + let result = if let Some(ranges) = &buffer.ranges { Self::format_ranges_via_lsp( &lsp_store, - &buffer, + &buffer.handle, ranges, buffer_abs_path, - language_server, + &language_server, settings, cx, ) @@ -1482,7 +1295,7 @@ impl LocalLspStore { &lsp_store, &buffer.handle, buffer_abs_path, - language_server, + &language_server, settings, cx, ) @@ -1491,51 +1304,114 @@ impl LocalLspStore { }; Some(FormatOperation::Lsp(result)) - } else { + } + Formatter::Prettier => { + let prettier = lsp_store.update(cx, |lsp_store, _cx| { + lsp_store.prettier_store().unwrap().downgrade() + })?; + prettier_store::format_with_prettier(&prettier, &buffer.handle, cx) + .await + .transpose()? + } + Formatter::External { command, arguments } => { + Self::format_via_external_command(buffer, command, arguments.as_deref(), cx) + .await + .context(format!( + "failed to format via external command {:?}", + command + ))? + .map(FormatOperation::External) + } + Formatter::CodeActions(code_actions) => { + let code_actions = deserialize_code_actions(code_actions); + if !code_actions.is_empty() { + Self::execute_code_actions_on_servers( + &lsp_store, + adapters_and_servers, + code_actions, + &buffer.handle, + push_to_history, + project_transaction, + cx, + ) + .await?; + let buf_transaction_id = + project_transaction.0.get(&buffer.handle).map(|t| t.id); + // NOTE: same logic as in buffer.handle.update below + if initial_transaction_id.is_none() { + initial_transaction_id = buf_transaction_id; + } + if buf_transaction_id.is_some() { + prev_transaction_id = buf_transaction_id; + } + } None } - } - Formatter::Prettier => { - let prettier = lsp_store.update(cx, |lsp_store, _cx| { - lsp_store.prettier_store().unwrap().downgrade() - })?; - prettier_store::format_with_prettier(&prettier, &buffer.handle, cx) - .await - .transpose()? - } - Formatter::External { command, arguments } => { - Self::format_via_external_command(buffer, command, arguments.as_deref(), cx) - .await - .context(format!( - "failed to format via external command {:?}", - command - ))? - .map(FormatOperation::External) - } - Formatter::CodeActions(code_actions) => { - let code_actions = deserialize_code_actions(code_actions); - if !code_actions.is_empty() { - Self::execute_code_actions_on_servers( - &lsp_store, - adapters_and_servers, - code_actions, - &buffer.handle, - push_to_history, - transaction, - cx, - ) - .await?; + }; + let Some(operation) = operation else { + continue; + }; + + let should_continue_formatting = buffer.handle.update(cx, |b, cx| { + // If a previous format succeeded and the buffer was edited while the language-specific + // formatting information for this format was being computed, avoid applying the + // language-specific formatting, because it can't be grouped with the previous formatting + // in the undo history. + let should_continue_formatting = match (prev_transaction_id, b.peek_undo_stack()) { + (Some(prev_transaction_id), Some(last_history_entry)) => { + let last_history_transaction_id = last_history_entry.transaction_id(); + let is_same_as_prev = last_history_transaction_id == prev_transaction_id; + is_same_as_prev + } + (Some(_), None) => false, + (_, _) => true, + }; + + if should_continue_formatting { + // Apply any language-specific formatting, and group the two formatting operations + // in the buffer's undo history. + let this_transaction_id = match operation { + FormatOperation::Lsp(edits) => b.edit(edits, None, cx), + FormatOperation::External(diff) => b.apply_diff(diff, cx), + FormatOperation::Prettier(diff) => b.apply_diff(diff, cx), + }; + if initial_transaction_id.is_none() { + initial_transaction_id = this_transaction_id; + } + if this_transaction_id.is_some() { + prev_transaction_id = this_transaction_id; + } } - None + + if let Some(transaction_id) = initial_transaction_id { + b.group_until_transaction(transaction_id); + } else if let Some(transaction) = project_transaction.0.get(&buffer.handle) { + b.group_until_transaction(transaction.id) + } + return should_continue_formatting; + })?; + if !should_continue_formatting { + break; } - }; - anyhow::Ok(result) + } + + buffer.handle.update(cx, |b, _cx| { + if let Some(transaction) = b.finalize_last_transaction().cloned() { + if !push_to_history { + b.forget_transaction(transaction.id); + project_transaction + .0 + .insert(buffer.handle.clone(), transaction); + } + } + })?; + return Ok(()); } pub async fn format_ranges_via_lsp( this: &WeakEntity, - buffer: &FormattableBuffer, - ranges: &Vec>, + buffer_handle: &Entity, + ranges: &[Range], abs_path: &Path, language_server: &Arc, settings: &LanguageSettings, @@ -1563,7 +1439,7 @@ impl LocalLspStore { // // TODO: Instead of using current snapshot, should use the latest snapshot sent to // LSP. - let snapshot = buffer.handle.read(cx).snapshot(); + let snapshot = buffer_handle.read(cx).snapshot(); for range in ranges { lsp_ranges.push(range_to_lsp(range.to_point_utf16(&snapshot))?); } @@ -1590,7 +1466,7 @@ impl LocalLspStore { if let Some(lsp_edits) = lsp_edits { this.update(cx, |this, cx| { this.as_local_mut().unwrap().edits_from_lsp( - &buffer.handle, + &buffer_handle, lsp_edits, language_server.server_id(), None, @@ -2779,10 +2655,10 @@ impl LocalLspStore { #[derive(Debug)] pub struct FormattableBuffer { - id: BufferId, handle: Entity, abs_path: Option, env: Option>, + ranges: Option>>, } pub struct RemoteLspStore { @@ -7041,18 +6917,27 @@ impl LspStore { })? .await; + let ranges = match &target { + LspFormatTarget::Buffers => None, + LspFormatTarget::Ranges(ranges) => { + let Some(ranges) = ranges.get(&id) else { + return Err(anyhow!("No format ranges provided for buffer")); + }; + Some(ranges.clone()) + } + }; + formattable_buffers.push(FormattableBuffer { - id, handle, abs_path, env, + ranges, }); } let result = LocalLspStore::format_locally( lsp_store.clone(), formattable_buffers, - &target, push_to_history, trigger, cx.clone(),