@@ -14648,6 +14648,107 @@ async fn test_organize_imports_manual_trigger(cx: &mut TestAppContext) {
);
}
+#[gpui::test]
+async fn test_formatter_failure_does_not_abort_subsequent_formatters(cx: &mut TestAppContext) {
+ init_test(cx, |settings| {
+ settings.defaults.formatter = Some(FormatterList::Vec(vec![
+ Formatter::LanguageServer(settings::LanguageServerFormatterSpecifier::Current),
+ Formatter::CodeAction("organize-imports".into()),
+ ]))
+ });
+
+ let fs = FakeFs::new(cx.executor());
+ fs.insert_file(path!("/file.rs"), "fn main() {}\n".into())
+ .await;
+
+ let project = Project::test(fs, [path!("/").as_ref()], cx).await;
+ let language_registry = project.read_with(cx, |project, _| project.languages().clone());
+ language_registry.add(rust_lang());
+
+ let mut fake_servers = language_registry.register_fake_lsp(
+ "Rust",
+ FakeLspAdapter {
+ capabilities: lsp::ServerCapabilities {
+ document_formatting_provider: Some(lsp::OneOf::Left(true)),
+ code_action_provider: Some(lsp::CodeActionProviderCapability::Simple(true)),
+ ..Default::default()
+ },
+ ..Default::default()
+ },
+ );
+
+ let buffer = project
+ .update(cx, |project, cx| {
+ project.open_local_buffer(path!("/file.rs"), cx)
+ })
+ .await
+ .unwrap();
+
+ let buffer = cx.new(|cx| MultiBuffer::singleton(buffer, cx));
+ let (editor, cx) = cx.add_window_view(|window, cx| {
+ build_editor_with_project(project.clone(), buffer, window, cx)
+ });
+
+ let fake_server = fake_servers.next().await.unwrap();
+
+ // Formatter #1 (LanguageServer) returns an error to simulate failure
+ fake_server.set_request_handler::<lsp::request::Formatting, _, _>(
+ move |_params, _| async move { Err(anyhow::anyhow!("Simulated formatter failure")) },
+ );
+
+ // Formatter #2 (CodeAction) returns a successful edit
+ fake_server.set_request_handler::<lsp::request::CodeActionRequest, _, _>(
+ move |_params, _| async move {
+ let uri = lsp::Uri::from_file_path(path!("/file.rs")).unwrap();
+ Ok(Some(vec![lsp::CodeActionOrCommand::CodeAction(
+ lsp::CodeAction {
+ kind: Some("organize-imports".into()),
+ edit: Some(lsp::WorkspaceEdit::new(
+ [(
+ uri,
+ vec![lsp::TextEdit::new(
+ lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(0, 0)),
+ "use std::io;\n".to_string(),
+ )],
+ )]
+ .into_iter()
+ .collect(),
+ )),
+ ..Default::default()
+ },
+ )]))
+ },
+ );
+
+ fake_server.set_request_handler::<lsp::request::CodeActionResolveRequest, _, _>({
+ move |params, _| async move { Ok(params) }
+ });
+
+ editor
+ .update_in(cx, |editor, window, cx| {
+ editor.perform_format(
+ project.clone(),
+ FormatTrigger::Manual,
+ FormatTarget::Buffers(editor.buffer().read(cx).all_buffers()),
+ window,
+ cx,
+ )
+ })
+ .unwrap()
+ .await;
+
+ // Formatter #1 (LanguageServer) failed, but formatter #2 (CodeAction) should have applied
+ editor.update(cx, |editor, cx| {
+ assert_eq!(editor.text(cx), "use std::io;\nfn main() {}\n");
+ });
+
+ // The entire format operation should undo as one transaction
+ editor.update_in(cx, |editor, window, cx| {
+ editor.undo(&Default::default(), window, cx);
+ assert_eq!(editor.text(cx), "fn main() {}\n");
+ });
+}
+
#[gpui::test]
async fn test_concurrent_format_requests(cx: &mut TestAppContext) {
init_test(cx, |_| {});
@@ -1611,28 +1611,6 @@ impl LocalLspStore {
})
})?;
- /// Apply edits to the buffer that will become part of the formatting transaction.
- /// Fails if the buffer has been edited since the start of that transaction.
- fn extend_formatting_transaction(
- buffer: &FormattableBuffer,
- formatting_transaction_id: text::TransactionId,
- cx: &mut AsyncApp,
- operation: impl FnOnce(&mut Buffer, &mut Context<Buffer>),
- ) -> anyhow::Result<()> {
- buffer.handle.update(cx, |buffer, cx| {
- let last_transaction_id = buffer.peek_undo_stack().map(|t| t.transaction_id());
- if last_transaction_id != Some(formatting_transaction_id) {
- anyhow::bail!("Buffer edited while formatting. Aborting")
- }
- buffer.start_transaction();
- operation(buffer, cx);
- if let Some(transaction_id) = buffer.end_transaction(cx) {
- buffer.merge_transactions(transaction_id, formatting_transaction_id);
- }
- Ok(())
- })
- }
-
// handle whitespace formatting
if settings.remove_trailing_whitespace_on_save {
zlog::trace!(logger => "removing trailing whitespace");
@@ -1702,508 +1680,532 @@ impl LocalLspStore {
} else {
formatter
};
- match formatter {
- Formatter::None => {
- zlog::trace!(logger => "skipping formatter 'none'");
- continue;
- }
- Formatter::Auto => unreachable!("Auto resolved above"),
- Formatter::Prettier => {
- let logger = zlog::scoped!(logger => "prettier");
- zlog::trace!(logger => "formatting");
- let _timer = zlog::time!(logger => "Formatting buffer via prettier");
+ if let Err(err) = Self::apply_formatter(
+ formatter,
+ &lsp_store,
+ buffer,
+ formatting_transaction_id,
+ &adapters_and_servers,
+ &settings,
+ request_timeout,
+ logger,
+ cx,
+ )
+ .await
+ {
+ zlog::error!(logger => "Formatter failed, skipping: {err:#}");
+ }
+ }
- 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)
+ Ok(())
+ }
+
+ async fn apply_formatter(
+ formatter: &Formatter,
+ lsp_store: &WeakEntity<LspStore>,
+ buffer: &FormattableBuffer,
+ formatting_transaction_id: clock::Lamport,
+ adapters_and_servers: &[(Arc<CachedLspAdapter>, Arc<LanguageServer>)],
+ settings: &LanguageSettings,
+ request_timeout: Duration,
+ logger: zlog::Logger,
+ cx: &mut AsyncApp,
+ ) -> anyhow::Result<()> {
+ match formatter {
+ Formatter::None => {
+ zlog::trace!(logger => "skipping formatter 'none'");
+ return Ok(());
+ }
+ Formatter::Auto => {
+ debug_panic!("Auto resolved above");
+ return Ok(());
+ }
+ Formatter::Prettier => {
+ let logger = zlog::scoped!(logger => "prettier");
+ 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)
+ .await
+ .transpose()?;
+ 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");
+
+ let diff =
+ Self::format_via_external_command(buffer, &command, arguments.as_deref(), cx)
.await
- .transpose()?;
- let Some(diff) = diff else {
- zlog::trace!(logger => "No changes");
- continue;
- };
+ .with_context(|| {
+ format!("Failed to format buffer via external command: {}", command)
+ })?;
+ 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::LanguageServer(specifier) => {
+ let logger = zlog::scoped!(logger => "language-server");
+ zlog::trace!(logger => "formatting");
+ let _timer = zlog::time!(logger => "Formatting buffer using language server");
- let diff = Self::format_via_external_command(
- buffer,
- &command,
- arguments.as_deref(),
+ let Some(buffer_path_abs) = buffer.abs_path.as_ref() else {
+ zlog::warn!(logger => "Cannot format buffer that is not backed by a file on disk using language servers. Skipping");
+ return Ok(());
+ };
+
+ let language_server = match specifier {
+ settings::LanguageServerFormatterSpecifier::Specific { name } => {
+ adapters_and_servers.iter().find_map(|(adapter, server)| {
+ if adapter.name.0.as_ref() == name {
+ Some(server.clone())
+ } else {
+ None
+ }
+ })
+ }
+ settings::LanguageServerFormatterSpecifier::Current => adapters_and_servers
+ .iter()
+ .find(|(_, server)| Self::server_supports_formatting(server))
+ .map(|(_, server)| server.clone()),
+ };
+
+ let Some(language_server) = language_server else {
+ log::debug!(
+ "No language server found to format buffer '{:?}'. Skipping",
+ buffer_path_abs.as_path().to_string_lossy()
+ );
+ return Ok(());
+ };
+
+ zlog::trace!(
+ logger =>
+ "Formatting buffer '{:?}' using language server '{:?}'",
+ buffer_path_abs.as_path().to_string_lossy(),
+ language_server.name()
+ );
+
+ let edits = if let Some(ranges) = buffer.ranges.as_ref() {
+ zlog::trace!(logger => "formatting ranges");
+ Self::format_ranges_via_lsp(
+ &lsp_store,
+ &buffer.handle,
+ ranges,
+ buffer_path_abs,
+ &language_server,
+ &settings,
cx,
)
.await
- .with_context(|| {
- format!("Failed to format buffer via external command: {}", command)
- })?;
- let Some(diff) = diff else {
- zlog::trace!(logger => "No changes");
- continue;
- };
-
- extend_formatting_transaction(
- buffer,
- formatting_transaction_id,
+ .context("Failed to format ranges via language server")?
+ } else {
+ zlog::trace!(logger => "formatting full");
+ Self::format_via_lsp(
+ &lsp_store,
+ &buffer.handle,
+ buffer_path_abs,
+ &language_server,
+ &settings,
cx,
- |buffer, cx| {
- buffer.apply_diff(diff, cx);
- },
- )?;
+ )
+ .await
+ .context("failed to format via language server")?
+ };
+
+ if edits.is_empty() {
+ zlog::trace!(logger => "No changes");
+ return Ok(());
}
- Formatter::LanguageServer(specifier) => {
- let logger = zlog::scoped!(logger => "language-server");
- zlog::trace!(logger => "formatting");
- let _timer = zlog::time!(logger => "Formatting buffer using language server");
+ extend_formatting_transaction(
+ buffer,
+ formatting_transaction_id,
+ cx,
+ |buffer, cx| {
+ buffer.edit(edits, None, cx);
+ },
+ )?;
+ }
+ Formatter::CodeAction(code_action_name) => {
+ let logger = zlog::scoped!(logger => "code-actions");
+ zlog::trace!(logger => "formatting");
+ let _timer = zlog::time!(logger => "Formatting buffer using code actions");
- let Some(buffer_path_abs) = buffer.abs_path.as_ref() else {
- zlog::warn!(logger => "Cannot format buffer that is not backed by a file on disk using language servers. Skipping");
- continue;
- };
+ let Some(buffer_path_abs) = buffer.abs_path.as_ref() else {
+ zlog::warn!(logger => "Cannot format buffer that is not backed by a file on disk using code actions. Skipping");
+ return Ok(());
+ };
- let language_server = match specifier {
- settings::LanguageServerFormatterSpecifier::Specific { name } => {
- adapters_and_servers.iter().find_map(|(adapter, server)| {
- if adapter.name.0.as_ref() == name {
- Some(server.clone())
- } else {
- None
- }
- })
- }
- settings::LanguageServerFormatterSpecifier::Current => adapters_and_servers
- .iter()
- .find(|(_, server)| Self::server_supports_formatting(server))
- .map(|(_, server)| server.clone()),
- };
+ let code_action_kind: CodeActionKind = code_action_name.clone().into();
+ zlog::trace!(logger => "Attempting to resolve code actions {:?}", &code_action_kind);
+
+ let mut actions_and_servers = Vec::new();
- let Some(language_server) = language_server else {
- log::debug!(
- "No language server found to format buffer '{:?}'. Skipping",
- buffer_path_abs.as_path().to_string_lossy()
+ for (index, (_, language_server)) in adapters_and_servers.iter().enumerate() {
+ let actions_result = Self::get_server_code_actions_from_action_kinds(
+ &lsp_store,
+ language_server.server_id(),
+ vec![code_action_kind.clone()],
+ &buffer.handle,
+ cx,
+ )
+ .await
+ .with_context(|| {
+ format!(
+ "Failed to resolve code action {:?} with language server {}",
+ code_action_kind,
+ language_server.name()
+ )
+ });
+ let Ok(actions) = actions_result else {
+ // note: it may be better to set result to the error and break formatters here
+ // but for now we try to execute the actions that we can resolve and skip the rest
+ zlog::error!(
+ logger =>
+ "Failed to resolve code action {:?} with language server {}",
+ code_action_kind,
+ language_server.name()
);
continue;
};
+ for action in actions {
+ actions_and_servers.push((action, index));
+ }
+ }
- zlog::trace!(
- logger =>
- "Formatting buffer '{:?}' using language server '{:?}'",
- buffer_path_abs.as_path().to_string_lossy(),
- language_server.name()
- );
+ if actions_and_servers.is_empty() {
+ zlog::warn!(logger => "No code actions were resolved, continuing");
+ return Ok(());
+ }
- let edits = if let Some(ranges) = buffer.ranges.as_ref() {
- zlog::trace!(logger => "formatting ranges");
- Self::format_ranges_via_lsp(
- &lsp_store,
- &buffer.handle,
- ranges,
- buffer_path_abs,
- &language_server,
- &settings,
- cx,
- )
- .await
- .context("Failed to format ranges via language server")?
- } else {
- zlog::trace!(logger => "formatting full");
- Self::format_via_lsp(
- &lsp_store,
- &buffer.handle,
- buffer_path_abs,
- &language_server,
- &settings,
- cx,
+ 'actions: for (mut action, server_index) in actions_and_servers {
+ let server = &adapters_and_servers[server_index].1;
+
+ let describe_code_action = |action: &CodeAction| {
+ format!(
+ "code action '{}' with title \"{}\" on server {}",
+ action
+ .lsp_action
+ .action_kind()
+ .unwrap_or("unknown".into())
+ .as_str(),
+ action.lsp_action.title(),
+ server.name(),
)
- .await
- .context("failed to format via language server")?
};
- if edits.is_empty() {
- zlog::trace!(logger => "No changes");
- continue;
- }
- extend_formatting_transaction(
- buffer,
- formatting_transaction_id,
- cx,
- |buffer, cx| {
- buffer.edit(edits, None, cx);
- },
- )?;
- }
- Formatter::CodeAction(code_action_name) => {
- let logger = zlog::scoped!(logger => "code-actions");
- zlog::trace!(logger => "formatting");
- let _timer = zlog::time!(logger => "Formatting buffer using code actions");
+ zlog::trace!(logger => "Executing {}", describe_code_action(&action));
- let Some(buffer_path_abs) = buffer.abs_path.as_ref() else {
- zlog::warn!(logger => "Cannot format buffer that is not backed by a file on disk using code actions. Skipping");
+ if let Err(err) =
+ Self::try_resolve_code_action(server, &mut action, request_timeout).await
+ {
+ zlog::error!(
+ logger =>
+ "Failed to resolve {}. Error: {}",
+ describe_code_action(&action),
+ err
+ );
continue;
- };
-
- let code_action_kind: CodeActionKind = code_action_name.clone().into();
- zlog::trace!(logger => "Attempting to resolve code actions {:?}", &code_action_kind);
-
- let mut actions_and_servers = Vec::new();
+ }
- for (index, (_, language_server)) in adapters_and_servers.iter().enumerate() {
- let actions_result = Self::get_server_code_actions_from_action_kinds(
- &lsp_store,
- language_server.server_id(),
- vec![code_action_kind.clone()],
- &buffer.handle,
- cx,
- )
- .await
- .with_context(|| {
- format!(
- "Failed to resolve code action {:?} with language server {}",
- code_action_kind,
- language_server.name()
- )
- });
- let Ok(actions) = actions_result else {
- // note: it may be better to set result to the error and break formatters here
- // but for now we try to execute the actions that we can resolve and skip the rest
- zlog::error!(
+ if let Some(edit) = action.lsp_action.edit().cloned() {
+ // NOTE: code below duplicated from `Self::deserialize_workspace_edit`
+ // but filters out and logs warnings for code actions that require unreasonably
+ // difficult handling on our part, such as:
+ // - applying edits that call commands
+ // which can result in arbitrary workspace edits being sent from the server that
+ // have no way of being tied back to the command that initiated them (i.e. we
+ // can't know which edits are part of the format request, or if the server is done sending
+ // actions in response to the command)
+ // - actions that create/delete/modify/rename files other than the one we are formatting
+ // as we then would need to handle such changes correctly in the local history as well
+ // as the remote history through the ProjectTransaction
+ // - actions with snippet edits, as these simply don't make sense in the context of a format request
+ // Supporting these actions is not impossible, but not supported as of yet.
+ if edit.changes.is_none() && edit.document_changes.is_none() {
+ zlog::trace!(
logger =>
- "Failed to resolve code action {:?} with language server {}",
- code_action_kind,
- language_server.name()
+ "No changes for code action. Skipping {}",
+ describe_code_action(&action),
);
continue;
- };
- for action in actions {
- actions_and_servers.push((action, index));
}
- }
-
- if actions_and_servers.is_empty() {
- zlog::warn!(logger => "No code actions were resolved, continuing");
- continue;
- }
- 'actions: for (mut action, server_index) in actions_and_servers {
- let server = &adapters_and_servers[server_index].1;
-
- let describe_code_action = |action: &CodeAction| {
- format!(
- "code action '{}' with title \"{}\" on server {}",
- action
- .lsp_action
- .action_kind()
- .unwrap_or("unknown".into())
- .as_str(),
- action.lsp_action.title(),
- server.name(),
- )
- };
+ let mut operations = Vec::new();
+ if let Some(document_changes) = edit.document_changes {
+ match document_changes {
+ lsp::DocumentChanges::Edits(edits) => operations.extend(
+ edits.into_iter().map(lsp::DocumentChangeOperation::Edit),
+ ),
+ lsp::DocumentChanges::Operations(ops) => operations = ops,
+ }
+ } else if let Some(changes) = edit.changes {
+ operations.extend(changes.into_iter().map(|(uri, edits)| {
+ lsp::DocumentChangeOperation::Edit(lsp::TextDocumentEdit {
+ text_document: lsp::OptionalVersionedTextDocumentIdentifier {
+ uri,
+ version: None,
+ },
+ edits: edits.into_iter().map(Edit::Plain).collect(),
+ })
+ }));
+ }
- zlog::trace!(logger => "Executing {}", describe_code_action(&action));
+ let mut edits = Vec::with_capacity(operations.len());
- if let Err(err) =
- Self::try_resolve_code_action(server, &mut action, request_timeout)
- .await
- {
- zlog::error!(
+ if operations.is_empty() {
+ zlog::trace!(
logger =>
- "Failed to resolve {}. Error: {}",
+ "No changes for code action. Skipping {}",
describe_code_action(&action),
- err
);
continue;
}
-
- if let Some(edit) = action.lsp_action.edit().cloned() {
- // NOTE: code below duplicated from `Self::deserialize_workspace_edit`
- // but filters out and logs warnings for code actions that require unreasonably
- // difficult handling on our part, such as:
- // - applying edits that call commands
- // which can result in arbitrary workspace edits being sent from the server that
- // have no way of being tied back to the command that initiated them (i.e. we
- // can't know which edits are part of the format request, or if the server is done sending
- // actions in response to the command)
- // - actions that create/delete/modify/rename files other than the one we are formatting
- // as we then would need to handle such changes correctly in the local history as well
- // as the remote history through the ProjectTransaction
- // - actions with snippet edits, as these simply don't make sense in the context of a format request
- // Supporting these actions is not impossible, but not supported as of yet.
- if edit.changes.is_none() && edit.document_changes.is_none() {
- zlog::trace!(
+ for operation in operations {
+ let op = match operation {
+ lsp::DocumentChangeOperation::Edit(op) => op,
+ lsp::DocumentChangeOperation::Op(_) => {
+ zlog::warn!(
+ logger =>
+ "Code actions which create, delete, or rename files are not supported on format. Skipping {}",
+ describe_code_action(&action),
+ );
+ continue 'actions;
+ }
+ };
+ let Ok(file_path) = op.text_document.uri.to_file_path() else {
+ zlog::warn!(
logger =>
- "No changes for code action. Skipping {}",
+ "Failed to convert URI '{:?}' to file path. Skipping {}",
+ &op.text_document.uri,
describe_code_action(&action),
);
- continue;
- }
-
- let mut operations = Vec::new();
- if let Some(document_changes) = edit.document_changes {
- match document_changes {
- lsp::DocumentChanges::Edits(edits) => operations.extend(
- edits.into_iter().map(lsp::DocumentChangeOperation::Edit),
- ),
- lsp::DocumentChanges::Operations(ops) => operations = ops,
- }
- } else if let Some(changes) = edit.changes {
- operations.extend(changes.into_iter().map(|(uri, edits)| {
- lsp::DocumentChangeOperation::Edit(lsp::TextDocumentEdit {
- text_document:
- lsp::OptionalVersionedTextDocumentIdentifier {
- uri,
- version: None,
- },
- edits: edits.into_iter().map(Edit::Plain).collect(),
- })
- }));
- }
-
- let mut edits = Vec::with_capacity(operations.len());
-
- if operations.is_empty() {
- zlog::trace!(
+ continue 'actions;
+ };
+ if &file_path != buffer_path_abs {
+ zlog::warn!(
logger =>
- "No changes for code action. Skipping {}",
+ "File path '{:?}' does not match buffer path '{:?}'. Skipping {}",
+ file_path,
+ buffer_path_abs,
describe_code_action(&action),
);
- continue;
+ continue 'actions;
}
- for operation in operations {
- let op = match operation {
- lsp::DocumentChangeOperation::Edit(op) => op,
- lsp::DocumentChangeOperation::Op(_) => {
+
+ let mut lsp_edits = Vec::new();
+ for edit in op.edits {
+ match edit {
+ Edit::Plain(edit) => {
+ if !lsp_edits.contains(&edit) {
+ lsp_edits.push(edit);
+ }
+ }
+ Edit::Annotated(edit) => {
+ if !lsp_edits.contains(&edit.text_edit) {
+ lsp_edits.push(edit.text_edit);
+ }
+ }
+ Edit::Snippet(_) => {
zlog::warn!(
logger =>
- "Code actions which create, delete, or rename files are not supported on format. Skipping {}",
+ "Code actions which produce snippet edits are not supported during formatting. Skipping {}",
describe_code_action(&action),
);
continue 'actions;
}
- };
- let Ok(file_path) = op.text_document.uri.to_file_path() else {
- zlog::warn!(
- logger =>
- "Failed to convert URI '{:?}' to file path. Skipping {}",
- &op.text_document.uri,
- describe_code_action(&action),
- );
- continue 'actions;
- };
- if &file_path != buffer_path_abs {
- zlog::warn!(
- logger =>
- "File path '{:?}' does not match buffer path '{:?}'. Skipping {}",
- file_path,
- buffer_path_abs,
- describe_code_action(&action),
- );
- continue 'actions;
- }
-
- let mut lsp_edits = Vec::new();
- for edit in op.edits {
- match edit {
- Edit::Plain(edit) => {
- if !lsp_edits.contains(&edit) {
- lsp_edits.push(edit);
- }
- }
- Edit::Annotated(edit) => {
- if !lsp_edits.contains(&edit.text_edit) {
- lsp_edits.push(edit.text_edit);
- }
- }
- Edit::Snippet(_) => {
- zlog::warn!(
- logger =>
- "Code actions which produce snippet edits are not supported during formatting. Skipping {}",
- describe_code_action(&action),
- );
- continue 'actions;
- }
- }
}
- let edits_result = lsp_store
- .update(cx, |lsp_store, cx| {
- lsp_store.as_local_mut().unwrap().edits_from_lsp(
- &buffer.handle,
- lsp_edits,
- server.server_id(),
- op.text_document.version,
- cx,
- )
- })?
- .await;
- let Ok(resolved_edits) = edits_result else {
- zlog::warn!(
- logger =>
- "Failed to resolve edits from LSP for buffer {:?} while handling {}",
- buffer_path_abs.as_path(),
- describe_code_action(&action),
- );
- continue 'actions;
- };
- edits.extend(resolved_edits);
- }
-
- if edits.is_empty() {
- zlog::warn!(logger => "No edits resolved from LSP");
- continue;
}
-
- extend_formatting_transaction(
- buffer,
- formatting_transaction_id,
- cx,
- |buffer, cx| {
- zlog::info!(
- "Applying edits {edits:?}. Content: {:?}",
- buffer.text()
- );
- buffer.edit(edits, None, cx);
- zlog::info!("Applied edits. New Content: {:?}", buffer.text());
- },
- )?;
+ let edits_result = lsp_store
+ .update(cx, |lsp_store, cx| {
+ lsp_store.as_local_mut().unwrap().edits_from_lsp(
+ &buffer.handle,
+ lsp_edits,
+ server.server_id(),
+ op.text_document.version,
+ cx,
+ )
+ })?
+ .await;
+ let Ok(resolved_edits) = edits_result else {
+ zlog::warn!(
+ logger =>
+ "Failed to resolve edits from LSP for buffer {:?} while handling {}",
+ buffer_path_abs.as_path(),
+ describe_code_action(&action),
+ );
+ continue 'actions;
+ };
+ edits.extend(resolved_edits);
}
- // bail early if command is invalid
- let Some(command) = action.lsp_action.command() else {
- continue;
- };
-
- zlog::warn!(
- logger =>
- "Executing code action command '{}'. This may cause formatting to abort unnecessarily as well as splitting formatting into two entries in the undo history",
- &command.command,
- );
-
- let server_capabilities = server.capabilities();
- let available_commands = server_capabilities
- .execute_command_provider
- .as_ref()
- .map(|options| options.commands.as_slice())
- .unwrap_or_default();
- if !available_commands.contains(&command.command) {
- zlog::warn!(
- logger =>
- "Cannot execute a command {} not listed in the language server capabilities of server {}",
- command.command,
- server.name(),
- );
+ if edits.is_empty() {
+ zlog::warn!(logger => "No edits resolved from LSP");
continue;
}
- // noop so we just ensure buffer hasn't been edited since resolving code actions
extend_formatting_transaction(
buffer,
formatting_transaction_id,
cx,
- |_, _| {},
+ |buffer, cx| {
+ zlog::info!(
+ "Applying edits {edits:?}. Content: {:?}",
+ buffer.text()
+ );
+ buffer.edit(edits, None, cx);
+ zlog::info!("Applied edits. New Content: {:?}", buffer.text());
+ },
)?;
- zlog::info!(logger => "Executing command {}", &command.command);
+ }
- lsp_store.update(cx, |this, _| {
- this.as_local_mut()
- .unwrap()
- .last_workspace_edits_by_language_server
- .remove(&server.server_id());
- })?;
+ let Some(command) = action.lsp_action.command() else {
+ continue;
+ };
- let execute_command_result = server
- .request::<lsp::request::ExecuteCommand>(
- lsp::ExecuteCommandParams {
- command: command.command.clone(),
- arguments: command.arguments.clone().unwrap_or_default(),
- ..Default::default()
- },
- request_timeout,
- )
- .await
- .into_response();
+ zlog::warn!(
+ logger =>
+ "Executing code action command '{}'. This may cause formatting to abort unnecessarily as well as splitting formatting into two entries in the undo history",
+ &command.command,
+ );
- if execute_command_result.is_err() {
- zlog::error!(
- logger =>
- "Failed to execute command '{}' as part of {}",
- &command.command,
- describe_code_action(&action),
- );
- continue 'actions;
- }
+ let server_capabilities = server.capabilities();
+ let available_commands = server_capabilities
+ .execute_command_provider
+ .as_ref()
+ .map(|options| options.commands.as_slice())
+ .unwrap_or_default();
+ if !available_commands.contains(&command.command) {
+ zlog::warn!(
+ logger =>
+ "Cannot execute a command {} not listed in the language server capabilities of server {}",
+ command.command,
+ server.name(),
+ );
+ continue;
+ }
- let mut project_transaction_command = lsp_store.update(cx, |this, _| {
- this.as_local_mut()
- .unwrap()
- .last_workspace_edits_by_language_server
- .remove(&server.server_id())
- .unwrap_or_default()
- })?;
+ extend_formatting_transaction(
+ buffer,
+ formatting_transaction_id,
+ cx,
+ |_, _| {},
+ )?;
+ zlog::info!(logger => "Executing command {}", &command.command);
- if let Some(transaction) =
- project_transaction_command.0.remove(&buffer.handle)
- {
- zlog::trace!(
- logger =>
- "Successfully captured {} edits that resulted from command {}",
- transaction.edit_ids.len(),
- &command.command,
- );
- let transaction_id_project_transaction = transaction.id;
- buffer.handle.update(cx, |buffer, _| {
- // it may have been removed from history if push_to_history was
- // false in deserialize_workspace_edit. If so push it so we
- // can merge it with the format transaction
- // and pop the combined transaction off the history stack
- // later if push_to_history is false
- if buffer.get_transaction(transaction.id).is_none() {
- buffer.push_transaction(transaction, Instant::now());
- }
- buffer.merge_transactions(
- transaction_id_project_transaction,
- formatting_transaction_id,
- );
- });
- }
+ lsp_store.update(cx, |this, _| {
+ this.as_local_mut()
+ .unwrap()
+ .last_workspace_edits_by_language_server
+ .remove(&server.server_id());
+ })?;
- if project_transaction_command.0.is_empty() {
- continue;
- }
+ let execute_command_result = server
+ .request::<lsp::request::ExecuteCommand>(
+ lsp::ExecuteCommandParams {
+ command: command.command.clone(),
+ arguments: command.arguments.clone().unwrap_or_default(),
+ ..Default::default()
+ },
+ request_timeout,
+ )
+ .await
+ .into_response();
- let mut extra_buffers = String::new();
- for buffer in project_transaction_command.0.keys() {
- buffer.read_with(cx, |b, cx| {
- let Some(path) = b.project_path(cx) else {
- return;
- };
+ if execute_command_result.is_err() {
+ zlog::error!(
+ logger =>
+ "Failed to execute command '{}' as part of {}",
+ &command.command,
+ describe_code_action(&action),
+ );
+ continue 'actions;
+ }
- if !extra_buffers.is_empty() {
- extra_buffers.push_str(", ");
- }
- extra_buffers.push_str(path.path.as_unix_str());
- });
- }
- zlog::warn!(
+ let mut project_transaction_command = lsp_store.update(cx, |this, _| {
+ this.as_local_mut()
+ .unwrap()
+ .last_workspace_edits_by_language_server
+ .remove(&server.server_id())
+ .unwrap_or_default()
+ })?;
+
+ if let Some(transaction) = project_transaction_command.0.remove(&buffer.handle)
+ {
+ zlog::trace!(
logger =>
- "Unexpected edits to buffers other than the buffer actively being formatted due to command {}. Impacted buffers: [{}].",
+ "Successfully captured {} edits that resulted from command {}",
+ transaction.edit_ids.len(),
&command.command,
- extra_buffers,
);
- // NOTE: if this case is hit, the proper thing to do is to for each buffer, merge the extra transaction
- // into the existing transaction in project_transaction if there is one, and if there isn't one in project_transaction,
- // add it so it's included, and merge it into the format transaction when its created later
+ let transaction_id_project_transaction = transaction.id;
+ buffer.handle.update(cx, |buffer, _| {
+ // it may have been removed from history if push_to_history was
+ // false in deserialize_workspace_edit. If so push it so we
+ // can merge it with the format transaction
+ // and pop the combined transaction off the history stack
+ // later if push_to_history is false
+ if buffer.get_transaction(transaction.id).is_none() {
+ buffer.push_transaction(transaction, Instant::now());
+ }
+ buffer.merge_transactions(
+ transaction_id_project_transaction,
+ formatting_transaction_id,
+ );
+ });
}
+
+ if project_transaction_command.0.is_empty() {
+ continue;
+ }
+
+ let mut extra_buffers = String::new();
+ for buffer in project_transaction_command.0.keys() {
+ buffer.read_with(cx, |b, cx| {
+ let Some(path) = b.project_path(cx) else {
+ return;
+ };
+
+ if !extra_buffers.is_empty() {
+ extra_buffers.push_str(", ");
+ }
+ extra_buffers.push_str(path.path.as_unix_str());
+ });
+ }
+ zlog::warn!(
+ logger =>
+ "Unexpected edits to buffers other than the buffer actively being formatted due to command {}. Impacted buffers: [{}].",
+ &command.command,
+ extra_buffers,
+ );
+ // NOTE: if this case is hit, the proper thing to do is to for each buffer, merge the extra transaction
+ // into the existing transaction in project_transaction if there is one, and if there isn't one in project_transaction,
+ // add it so it's included, and merge it into the format transaction when its created later
}
}
}
@@ -14542,3 +14544,25 @@ pub fn ensure_uniform_list_compatible_label(label: &mut CodeLabel) {
label.text = new_text;
}
+
+/// Apply edits to the buffer that will become part of the formatting transaction.
+/// Fails if the buffer has been edited since the start of that transaction.
+fn extend_formatting_transaction(
+ buffer: &FormattableBuffer,
+ formatting_transaction_id: text::TransactionId,
+ cx: &mut AsyncApp,
+ operation: impl FnOnce(&mut Buffer, &mut Context<Buffer>),
+) -> anyhow::Result<()> {
+ buffer.handle.update(cx, |buffer, cx| {
+ let last_transaction_id = buffer.peek_undo_stack().map(|t| t.transaction_id());
+ if last_transaction_id != Some(formatting_transaction_id) {
+ anyhow::bail!("Buffer edited while formatting. Aborting")
+ }
+ buffer.start_transaction();
+ operation(buffer, cx);
+ if let Some(transaction_id) = buffer.end_transaction(cx) {
+ buffer.merge_transactions(transaction_id, formatting_transaction_id);
+ }
+ Ok(())
+ })
+}