From 6fbbdb351205a97b09f3e704f0c92f05a1b89c3a Mon Sep 17 00:00:00 2001 From: Ben Kunkle Date: Thu, 2 Oct 2025 09:48:15 -0500 Subject: [PATCH] settings: Flatten code actions formatters object (#39375) Closes #ISSUE Release Notes: - settings: Changed code action format in `formatter` and `format_on_save` settings. **Previous format:** ``` { "code_actions": { "source.organizeImports": true, "source.fixAll": true } } ``` **New format:** ``` [ {"code_action": "source.organizeImports"}, {"code_action": "source.fixAll"} ] ``` After #39246, code actions run sequentially in order. The structure now reflects this and aligns with other formatter options (e.g., language servers). Both the `formatter` and `format_on_save` settings will be auto-migrated. --- Cargo.lock | 1 + crates/editor/src/editor_tests.rs | 10 +- crates/migrator/Cargo.toml | 1 + crates/migrator/src/migrations.rs | 6 + .../src/migrations/m_2025_10_01/settings.rs | 109 +++ crates/migrator/src/migrator.rs | 435 +++++++++++- crates/project/src/lsp_store.rs | 626 +++++++++--------- .../settings/src/settings_content/language.rs | 4 +- 8 files changed, 861 insertions(+), 331 deletions(-) create mode 100644 crates/migrator/src/migrations/m_2025_10_01/settings.rs diff --git a/Cargo.lock b/Cargo.lock index bf7594a9093bddc9191ccd51ba3bc50ff0d0e084..631b1cfb0b60241f065b10816f36e684495e8a83 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9691,6 +9691,7 @@ dependencies = [ "streaming-iterator", "tree-sitter", "tree-sitter-json", + "unindent", "workspace-hack", ] diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index bd3cb0ef8e06bbef9709f4cd4225633f1c70b75e..1143540108b9fabda45af6e9b5c76543cb1a9c4e 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -11874,14 +11874,8 @@ async fn test_multiple_formatters(cx: &mut TestAppContext) { settings.defaults.remove_trailing_whitespace_on_save = Some(true); settings.defaults.formatter = Some(SelectedFormatter::List(FormatterList::Vec(vec![ Formatter::LanguageServer { name: None }, - Formatter::CodeActions( - [ - ("code-action-1".into(), true), - ("code-action-2".into(), true), - ] - .into_iter() - .collect(), - ), + Formatter::CodeAction("code-action-1".into()), + Formatter::CodeAction("code-action-2".into()), ]))) }); diff --git a/crates/migrator/Cargo.toml b/crates/migrator/Cargo.toml index aeb50e582e2cec34821ba9094514cd32c21204d8..b23a60e9e9f5699c0c6c95a732b9b8f2e0a9e130 100644 --- a/crates/migrator/Cargo.toml +++ b/crates/migrator/Cargo.toml @@ -24,3 +24,4 @@ workspace-hack.workspace = true [dev-dependencies] pretty_assertions.workspace = true +unindent.workspace = true diff --git a/crates/migrator/src/migrations.rs b/crates/migrator/src/migrations.rs index 9db597e9643866ba2fcbad00ea77d4de0c244fd1..a4db2df46437c8e78d3e610f78c5374898b842a2 100644 --- a/crates/migrator/src/migrations.rs +++ b/crates/migrator/src/migrations.rs @@ -99,3 +99,9 @@ pub(crate) mod m_2025_07_08 { pub(crate) use settings::SETTINGS_PATTERNS; } + +pub(crate) mod m_2025_10_01 { + mod settings; + + pub(crate) use settings::SETTINGS_PATTERNS; +} diff --git a/crates/migrator/src/migrations/m_2025_10_01/settings.rs b/crates/migrator/src/migrations/m_2025_10_01/settings.rs new file mode 100644 index 0000000000000000000000000000000000000000..4f1e7a642f2fff3702886f9f37929976b8ad4d76 --- /dev/null +++ b/crates/migrator/src/migrations/m_2025_10_01/settings.rs @@ -0,0 +1,109 @@ +use std::ops::Range; +use tree_sitter::{Query, QueryMatch}; + +use crate::MigrationPatterns; + +pub const SETTINGS_PATTERNS: MigrationPatterns = + &[(FORMATTER_PATTERN, migrate_code_action_formatters)]; + +const FORMATTER_PATTERN: &str = r#" + (object + (pair + key: (string (string_content) @formatter) (#any-of? @formatter "formatter" "format_on_save") + value: [ + (array + (object + (pair + key: (string (string_content) @code-actions-key) (#eq? @code-actions-key "code_actions") + value: (object + ((pair) @code-action ","?)* + ) + ) + ) @code-actions-obj + ) @formatter-array + (object + (pair + key: (string (string_content) @code-actions-key) (#eq? @code-actions-key "code_actions") + value: (object + ((pair) @code-action ","?)* + ) + ) + ) @code-actions-obj + ] + ) + ) +"#; + +pub fn migrate_code_action_formatters( + contents: &str, + mat: &QueryMatch, + query: &Query, +) -> Option<(Range, String)> { + let code_actions_obj_ix = query.capture_index_for_name("code-actions-obj")?; + let code_actions_obj_node = mat.nodes_for_capture_index(code_actions_obj_ix).next()?; + + let mut code_actions = vec![]; + + let code_actions_ix = query.capture_index_for_name("code-action")?; + for code_action_node in mat.nodes_for_capture_index(code_actions_ix) { + let Some(enabled) = code_action_node + .child_by_field_name("value") + .map(|n| n.kind() != "false") + else { + continue; + }; + if !enabled { + continue; + } + let Some(name) = code_action_node + .child_by_field_name("key") + .and_then(|n| n.child(1)) + .map(|n| &contents[n.byte_range()]) + else { + continue; + }; + code_actions.push(name); + } + + let indent = query + .capture_index_for_name("formatter") + .and_then(|ix| mat.nodes_for_capture_index(ix).next()) + .map(|node| node.start_position().column + 1) + .unwrap_or(2); + + let mut code_actions_str = code_actions + .into_iter() + .map(|code_action| format!(r#"{{ "code_action": "{}" }}"#, code_action)) + .collect::>() + .join(&format!(",\n{}", " ".repeat(indent))); + let is_array = query + .capture_index_for_name("formatter-array") + .map(|ix| mat.nodes_for_capture_index(ix).count() > 0) + .unwrap_or(false); + if !is_array { + code_actions_str.insert_str(0, &" ".repeat(indent)); + code_actions_str.insert_str(0, "[\n"); + code_actions_str.push('\n'); + code_actions_str.push_str(&" ".repeat(indent.saturating_sub(2))); + code_actions_str.push_str("]"); + } + let mut replace_range = code_actions_obj_node.byte_range(); + if is_array && code_actions_str.is_empty() { + let mut cursor = code_actions_obj_node.parent().unwrap().walk(); + cursor.goto_first_child(); + while cursor.node().id() != code_actions_obj_node.id() && cursor.goto_next_sibling() {} + while cursor.goto_next_sibling() + && (cursor.node().is_extra() + || cursor.node().is_missing() + || cursor.node().kind() == "comment") + {} + if cursor.node().kind() == "," { + // found comma, delete up to next node + while cursor.goto_next_sibling() + && (cursor.node().is_extra() || cursor.node().is_missing()) + {} + replace_range.end = cursor.node().range().start_byte; + } + } + Some((replace_range, code_actions_str)) +} diff --git a/crates/migrator/src/migrator.rs b/crates/migrator/src/migrator.rs index 2180a049d03daf5fcd2a60e1f1f7ddd0013c7d1f..788af803f89b2bea6951b1ed8f5932ac7c9b4177 100644 --- a/crates/migrator/src/migrator.rs +++ b/crates/migrator/src/migrator.rs @@ -164,6 +164,10 @@ pub fn migrate_settings(text: &str) -> Result> { migrations::m_2025_07_08::SETTINGS_PATTERNS, &SETTINGS_QUERY_2025_07_08, ), + ( + migrations::m_2025_10_01::SETTINGS_PATTERNS, + &SETTINGS_QUERY_2025_10_01, + ), ]; run_migrations(text, migrations) } @@ -278,6 +282,10 @@ define_query!( SETTINGS_QUERY_2025_07_08, migrations::m_2025_07_08::SETTINGS_PATTERNS ); +define_query!( + SETTINGS_QUERY_2025_10_01, + migrations::m_2025_10_01::SETTINGS_PATTERNS +); // custom query static EDIT_PREDICTION_SETTINGS_MIGRATION_QUERY: LazyLock = LazyLock::new(|| { @@ -291,6 +299,18 @@ static EDIT_PREDICTION_SETTINGS_MIGRATION_QUERY: LazyLock = LazyLock::new #[cfg(test)] mod tests { use super::*; + use unindent::Unindent as _; + + fn assert_migrated_correctly(migrated: Option, expected: Option<&str>) { + match (&migrated, &expected) { + (Some(migrated), Some(expected)) => { + pretty_assertions::assert_str_eq!(migrated, expected); + } + _ => { + pretty_assertions::assert_eq!(migrated.as_deref(), expected); + } + } + } fn assert_migrate_keymap(input: &str, output: Option<&str>) { let migrated = migrate_keymap(input).unwrap(); @@ -299,7 +319,7 @@ mod tests { fn assert_migrate_settings(input: &str, output: Option<&str>) { let migrated = migrate_settings(input).unwrap(); - pretty_assertions::assert_eq!(migrated.as_deref(), output); + assert_migrated_correctly(migrated, output); } fn assert_migrate_settings_with_migrations( @@ -1263,4 +1283,417 @@ mod tests { ), ); } + + #[test] + fn test_flatten_code_action_formatters_basic_array() { + assert_migrate_settings( + &r#"{ + "formatter": [ + { + "code_actions": { + "included-1": true, + "included-2": true, + "excluded": false, + } + } + ] + }"# + .unindent(), + Some( + &r#"{ + "formatter": [ + { "code_action": "included-1" }, + { "code_action": "included-2" } + ] + }"# + .unindent(), + ), + ); + } + + #[test] + fn test_flatten_code_action_formatters_basic_object() { + assert_migrate_settings( + &r#"{ + "formatter": { + "code_actions": { + "included-1": true, + "excluded": false, + "included-2": true + } + } + }"# + .unindent(), + Some( + &r#"{ + "formatter": [ + { "code_action": "included-1" }, + { "code_action": "included-2" } + ] + }"# + .unindent(), + ), + ); + } + + #[test] + fn test_flatten_code_action_formatters_array_with_multiple_action_blocks() { + assert_migrate_settings( + r#"{ + "formatter": [ + { + "code_actions": { + "included-1": true, + "included-2": true, + "excluded": false, + } + }, + { + "language_server": "ruff" + }, + { + "code_actions": { + "excluded": false, + "excluded-2": false, + } + } + // some comment + , + { + "code_actions": { + "excluded": false, + "included-3": true, + "included-4": true, + } + }, + ] + }"#, + Some( + r#"{ + "formatter": [ + { "code_action": "included-1" }, + { "code_action": "included-2" }, + { + "language_server": "ruff" + }, + { "code_action": "included-3" }, + { "code_action": "included-4" }, + ] + }"#, + ), + ); + } + + #[test] + fn test_flatten_code_action_formatters_array_with_multiple_action_blocks_in_languages() { + assert_migrate_settings( + &r#"{ + "languages": { + "Rust": { + "formatter": [ + { + "code_actions": { + "included-1": true, + "included-2": true, + "excluded": false, + } + }, + { + "language_server": "ruff" + }, + { + "code_actions": { + "excluded": false, + "excluded-2": false, + } + } + // some comment + , + { + "code_actions": { + "excluded": false, + "included-3": true, + "included-4": true, + } + }, + ] + } + } + }"# + .unindent(), + Some( + &r#"{ + "languages": { + "Rust": { + "formatter": [ + { "code_action": "included-1" }, + { "code_action": "included-2" }, + { + "language_server": "ruff" + }, + { "code_action": "included-3" }, + { "code_action": "included-4" }, + ] + } + } + }"# + .unindent(), + ), + ); + } + + #[test] + fn test_flatten_code_action_formatters_array_with_multiple_action_blocks_in_defaults_and_multiple_languages() + { + assert_migrate_settings( + &r#"{ + "formatter": { + "code_actions": { + "default-1": true, + "default-2": true, + "default-3": true, + "default-4": true, + } + } + "languages": { + "Rust": { + "formatter": [ + { + "code_actions": { + "included-1": true, + "included-2": true, + "excluded": false, + } + }, + { + "language_server": "ruff" + }, + { + "code_actions": { + "excluded": false, + "excluded-2": false, + } + } + // some comment + , + { + "code_actions": { + "excluded": false, + "included-3": true, + "included-4": true, + } + }, + ] + }, + "Python": { + "formatter": [ + { + "language_server": "ruff" + }, + { + "code_actions": { + "excluded": false, + "excluded-2": false, + } + } + // some comment + , + { + "code_actions": { + "excluded": false, + "included-3": true, + "included-4": true, + } + }, + ] + } + } + }"# + .unindent(), + Some( + &r#"{ + "formatter": [ + { "code_action": "default-1" }, + { "code_action": "default-2" }, + { "code_action": "default-3" }, + { "code_action": "default-4" } + ] + "languages": { + "Rust": { + "formatter": [ + { "code_action": "included-1" }, + { "code_action": "included-2" }, + { + "language_server": "ruff" + }, + { "code_action": "included-3" }, + { "code_action": "included-4" }, + ] + }, + "Python": { + "formatter": [ + { + "language_server": "ruff" + }, + { "code_action": "included-3" }, + { "code_action": "included-4" }, + ] + } + } + }"# + .unindent(), + ), + ); + } + + #[test] + fn test_flatten_code_action_formatters_array_with_format_on_save_and_multiple_languages() { + assert_migrate_settings( + &r#"{ + "formatter": { + "code_actions": { + "default-1": true, + "default-2": true, + "default-3": true, + "default-4": true, + } + }, + "format_on_save": [ + { + "code_actions": { + "included-1": true, + "included-2": true, + "excluded": false, + } + }, + { + "language_server": "ruff" + }, + { + "code_actions": { + "excluded": false, + "excluded-2": false, + } + } + // some comment + , + { + "code_actions": { + "excluded": false, + "included-3": true, + "included-4": true, + } + }, + ], + "languages": { + "Rust": { + "format_on_save": "prettier", + "formatter": [ + { + "code_actions": { + "included-1": true, + "included-2": true, + "excluded": false, + } + }, + { + "language_server": "ruff" + }, + { + "code_actions": { + "excluded": false, + "excluded-2": false, + } + } + // some comment + , + { + "code_actions": { + "excluded": false, + "included-3": true, + "included-4": true, + } + }, + ] + }, + "Python": { + "format_on_save": { + "code_actions": { + "on-save-1": true, + "on-save-2": true, + } + }, + "formatter": [ + { + "language_server": "ruff" + }, + { + "code_actions": { + "excluded": false, + "excluded-2": false, + } + } + // some comment + , + { + "code_actions": { + "excluded": false, + "included-3": true, + "included-4": true, + } + }, + ] + } + } + }"# + .unindent(), + Some( + &r#"{ + "formatter": [ + { "code_action": "default-1" }, + { "code_action": "default-2" }, + { "code_action": "default-3" }, + { "code_action": "default-4" } + ], + "format_on_save": [ + { "code_action": "included-1" }, + { "code_action": "included-2" }, + { + "language_server": "ruff" + }, + { "code_action": "included-3" }, + { "code_action": "included-4" }, + ], + "languages": { + "Rust": { + "format_on_save": "prettier", + "formatter": [ + { "code_action": "included-1" }, + { "code_action": "included-2" }, + { + "language_server": "ruff" + }, + { "code_action": "included-3" }, + { "code_action": "included-4" }, + ] + }, + "Python": { + "format_on_save": [ + { "code_action": "on-save-1" }, + { "code_action": "on-save-2" } + ], + "formatter": [ + { + "language_server": "ruff" + }, + { "code_action": "included-3" }, + { "code_action": "included-4" }, + ] + } + } + }"# + .unindent(), + ), + ); + } } diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 9d328f04763d36904490e91c158709aef5beae2b..d5d6f808abecadf127c06fb2e1c0e75ca9c8d2f7 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -1340,7 +1340,7 @@ impl LocalLspStore { // Formatter for `code_actions_on_format` that runs before // the rest of the formatters - let mut code_actions_on_format_formatter = None; + let mut code_actions_on_format_formatters = None; let should_run_code_actions_on_format = !matches!( (trigger, &settings.format_on_save), (FormatTrigger::Save, &FormatOnSave::Off) @@ -1352,9 +1352,15 @@ impl LocalLspStore { .any(|enabled| *enabled); if have_code_actions_to_run_on_format { zlog::trace!(logger => "going to run code actions on format"); - code_actions_on_format_formatter = Some(Formatter::CodeActions( - settings.code_actions_on_format.clone(), - )); + code_actions_on_format_formatters = Some( + settings + .code_actions_on_format + .iter() + .filter_map(|(action, enabled)| enabled.then_some(action)) + .cloned() + .map(Formatter::CodeAction) + .collect::>(), + ); } } @@ -1377,7 +1383,10 @@ impl LocalLspStore { } }; - let formatters = code_actions_on_format_formatter.iter().chain(formatters); + let formatters = code_actions_on_format_formatters + .iter() + .flatten() + .chain(formatters); for formatter in formatters { match formatter { @@ -1512,7 +1521,7 @@ impl LocalLspStore { }, )?; } - Formatter::CodeActions(code_actions) => { + 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"); @@ -1521,371 +1530,348 @@ impl LocalLspStore { zlog::warn!(logger => "Cannot format buffer that is not backed by a file on disk using code actions. Skipping"); continue; }; - if code_actions.iter().filter(|(_, enabled)| **enabled).count() == 0 { - zlog::trace!(logger => "No code action kinds enabled, skipping"); - continue; - } - // Note: this loop exists despite `Self::get_server_code_actions_from_action_kinds` taking a `Vec` - // because code actions can resolve edits when `Self::get_server_code_actions_from_action_kinds` is called, which - // are not updated to reflect edits from previous actions or actions from other servers when `resolve_code_actions` is called later - // which can result in conflicting edits and mangled buffer state - for (code_action_name, enabled) in code_actions { - if !enabled { + + 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!( + 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)); } - 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(); + if actions_and_servers.is_empty() { + zlog::warn!(logger => "No code actions were resolved, continuing"); + continue; + } - 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, + '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 - .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!( + }; + + zlog::trace!(logger => "Executing {}", describe_code_action(&action)); + + if let Err(err) = Self::try_resolve_code_action(server, &mut action).await { + zlog::error!( + logger => + "Failed to resolve {}. Error: {}", + 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!( 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).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; } + 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); + } - 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() - ); - }, - )?; + if edits.is_empty() { + zlog::warn!(logger => "No edits resolved from LSP"); + continue; } - if let Some(command) = action.lsp_action.command() { + 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()); + }, + )?; + } + + if let Some(command) = action.lsp_action.command() { + 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, + ); + + // bail early if command is invalid + 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 => - "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, + "Cannot execute a command {} not listed in the language server capabilities of server {}", + command.command, + server.name(), ); + continue; + } - // bail early if command is invalid - 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; - } + // noop so we just ensure buffer hasn't been edited since resolving code actions + extend_formatting_transaction( + buffer, + formatting_transaction_id, + cx, + |_, _| {}, + )?; + 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 execute_command_result = server + .request::( + lsp::ExecuteCommandParams { + command: command.command.clone(), + arguments: command.arguments.clone().unwrap_or_default(), + ..Default::default() + }, + ) + .await + .into_response(); - // noop so we just ensure buffer hasn't been edited since resolving code actions - extend_formatting_transaction( - buffer, - formatting_transaction_id, - cx, - |_, _| {}, - )?; - zlog::info!(logger => "Executing command {}", &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 mut project_transaction_command = lsp_store.update(cx, |this, _| { this.as_local_mut() .unwrap() .last_workspace_edits_by_language_server - .remove(&server.server_id()); + .remove(&server.server_id()) + .unwrap_or_default() })?; - let execute_command_result = server - .request::( - lsp::ExecuteCommandParams { - command: command.command.clone(), - arguments: command - .arguments - .clone() - .unwrap_or_default(), - ..Default::default() - }, - ) - .await - .into_response(); - - 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 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 => - "Successfully captured {} edits that resulted from command {}", - transaction.edit_ids.len(), - &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, ); - 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() { - let mut extra_buffers = String::new(); - for buffer in project_transaction_command.0.keys() { - buffer - .read_with(cx, |b, cx| { - if let Some(path) = b.project_path(cx) { - if !extra_buffers.is_empty() { - extra_buffers.push_str(", "); - } - extra_buffers.push_str(path.path.as_unix_str()); + if !project_transaction_command.0.is_empty() { + let mut extra_buffers = String::new(); + for buffer in project_transaction_command.0.keys() { + buffer + .read_with(cx, |b, cx| { + if let Some(path) = b.project_path(cx) { + if !extra_buffers.is_empty() { + extra_buffers.push_str(", "); } - }) - .ok(); - } - 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 + extra_buffers.push_str(path.path.as_unix_str()); + } + }) + .ok(); } + 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 } } } diff --git a/crates/settings/src/settings_content/language.rs b/crates/settings/src/settings_content/language.rs index d60fbe04eb9cdec9ddc36c7fd827bcfea499293a..6a3e87f205f95db633eeb2f99d2fff1462251e06 100644 --- a/crates/settings/src/settings_content/language.rs +++ b/crates/settings/src/settings_content/language.rs @@ -802,8 +802,8 @@ pub enum Formatter { /// The arguments to pass to the program. arguments: Option>, }, - /// Files should be formatted using code actions executed by language servers. - CodeActions(HashMap), + /// Files should be formatted using a code action executed by language servers. + CodeAction(String), } /// The settings for indent guides.