From ea6853d35ca2cc712f8a2f550036f53c43f06fe2 Mon Sep 17 00:00:00 2001 From: Ben Kunkle Date: Thu, 16 Oct 2025 08:13:34 -0500 Subject: [PATCH] Fix code actions migration (#40303) Closes #40270 Release Notes: - Fixed an issue with the settings migration to flatten `code_actions` format steps where comments would cause enabled code actions to be omitted from the migrated settings. If you were effected, restoring the settings file backup and allowing the migration to re-run will result in a valid settings file - Fixed an issue where automated settings and keymap file updates would occasionally assume 4-space indentation --- crates/keymap_editor/src/keymap_editor.rs | 14 +- crates/migrator/src/migrations.rs | 2 +- .../src/migrations/m_2025_10_01/settings.rs | 173 ++-- .../src/migrations/m_2025_10_02/settings.rs | 15 +- .../src/migrations/m_2025_10_10/settings.rs | 15 +- crates/migrator/src/migrator.rs | 873 ++++++++++-------- crates/migrator/src/patterns.rs | 1 + crates/migrator/src/patterns/settings.rs | 21 + crates/settings/src/settings_json.rs | 163 +++- crates/settings/src/settings_store.rs | 47 +- 10 files changed, 786 insertions(+), 538 deletions(-) diff --git a/crates/keymap_editor/src/keymap_editor.rs b/crates/keymap_editor/src/keymap_editor.rs index 76c14ccfe48dbd85b6b02249ac4a26a73c129f4f..fce98ef596e2fb77cdf8091755de39ea55dce615 100644 --- a/crates/keymap_editor/src/keymap_editor.rs +++ b/crates/keymap_editor/src/keymap_editor.rs @@ -23,7 +23,9 @@ use gpui::{ use language::{Language, LanguageConfig, ToOffset as _}; use notifications::status_toast::{StatusToast, ToastIcon}; use project::{CompletionDisplayOptions, Project}; -use settings::{BaseKeymap, KeybindSource, KeymapFile, Settings as _, SettingsAssets}; +use settings::{ + BaseKeymap, KeybindSource, KeymapFile, Settings as _, SettingsAssets, infer_json_indent_size, +}; use ui::{ ActiveTheme as _, App, Banner, BorrowAppContext, ContextMenu, IconButtonShape, Indicator, Modal, ModalFooter, ModalHeader, ParentElement as _, PopoverMenu, Render, Section, @@ -1198,13 +1200,12 @@ impl KeymapEditor { else { return; }; - let tab_size = cx.global::().json_tab_size(); self.previous_edit = Some(PreviousEdit::ScrollBarOffset( self.table_interaction_state.read(cx).scroll_offset(), )); let keyboard_mapper = cx.keyboard_mapper().clone(); cx.spawn(async move |_, _| { - remove_keybinding(to_remove, &fs, tab_size, keyboard_mapper.as_ref()).await + remove_keybinding(to_remove, &fs, keyboard_mapper.as_ref()).await }) .detach_and_notify_err(window, cx); } @@ -2288,7 +2289,6 @@ impl KeybindingEditorModal { fn save(&mut self, cx: &mut Context) -> Result<(), InputError> { let existing_keybind = self.editing_keybind.clone(); let fs = self.fs.clone(); - let tab_size = cx.global::().json_tab_size(); let mut new_keystrokes = self.validate_keystrokes(cx).map_err(InputError::error)?; new_keystrokes @@ -2367,7 +2367,6 @@ impl KeybindingEditorModal { &action_mapping, new_action_args.as_deref(), &fs, - tab_size, keyboard_mapper.as_ref(), ) .await @@ -3019,13 +3018,14 @@ async fn save_keybinding_update( action_mapping: &ActionMapping, new_args: Option<&str>, fs: &Arc, - tab_size: usize, keyboard_mapper: &dyn PlatformKeyboardMapper, ) -> anyhow::Result<()> { let keymap_contents = settings::KeymapFile::load_keymap_file(fs) .await .context("Failed to load keymap file")?; + let tab_size = infer_json_indent_size(&keymap_contents); + let existing_keystrokes = existing.keystrokes().unwrap_or_default(); let existing_context = existing.context().and_then(KeybindContextString::local_str); let existing_args = existing @@ -3089,7 +3089,6 @@ async fn save_keybinding_update( async fn remove_keybinding( existing: ProcessedBinding, fs: &Arc, - tab_size: usize, keyboard_mapper: &dyn PlatformKeyboardMapper, ) -> anyhow::Result<()> { let Some(keystrokes) = existing.keystrokes() else { @@ -3098,6 +3097,7 @@ async fn remove_keybinding( let keymap_contents = settings::KeymapFile::load_keymap_file(fs) .await .context("Failed to load keymap file")?; + let tab_size = infer_json_indent_size(&keymap_contents); let operation = settings::KeybindUpdateOperation::Remove { target: settings::KeybindUpdateTarget { diff --git a/crates/migrator/src/migrations.rs b/crates/migrator/src/migrations.rs index 1b8ede68b1eb8686325c896723d1fdc762d02b73..0b35a238c60fb85a8426b0477d9b017d61513028 100644 --- a/crates/migrator/src/migrations.rs +++ b/crates/migrator/src/migrations.rs @@ -103,7 +103,7 @@ pub(crate) mod m_2025_07_08 { pub(crate) mod m_2025_10_01 { mod settings; - pub(crate) use settings::SETTINGS_PATTERNS; + pub(crate) use settings::flatten_code_actions_formatters; } pub(crate) mod m_2025_10_02 { diff --git a/crates/migrator/src/migrations/m_2025_10_01/settings.rs b/crates/migrator/src/migrations/m_2025_10_01/settings.rs index 4f1e7a642f2fff3702886f9f37929976b8ad4d76..84cf95049154b44048e92982fd00a11a3514bc16 100644 --- a/crates/migrator/src/migrations/m_2025_10_01/settings.rs +++ b/crates/migrator/src/migrations/m_2025_10_01/settings.rs @@ -1,109 +1,74 @@ -use std::ops::Range; -use tree_sitter::{Query, QueryMatch}; +use crate::patterns::migrate_language_setting; +use anyhow::Result; +use serde_json::Value; -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; +pub fn flatten_code_actions_formatters(value: &mut Value) -> Result<()> { + migrate_language_setting(value, |value, _path| { + let Some(obj) = value.as_object_mut() else { + return Ok(()); }; - 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); + for key in ["formatter", "format_on_save"] { + let Some(formatter) = obj.get_mut(key) else { + continue; + }; + let new_formatter = match formatter { + Value::Array(arr) => { + let mut new_arr = Vec::new(); + let mut found_code_actions = false; + for item in arr { + let Some(obj) = item.as_object() else { + new_arr.push(item.clone()); + continue; + }; + let code_actions_obj = obj + .get("code_actions") + .and_then(|code_actions| code_actions.as_object()); + let Some(code_actions) = code_actions_obj else { + new_arr.push(item.clone()); + continue; + }; + found_code_actions = true; + for (name, enabled) in code_actions { + if !enabled.as_bool().unwrap_or(true) { + continue; + } + new_arr.push(serde_json::json!({ + "code_action": name + })); + } + } + if !found_code_actions { + continue; + } + Value::Array(new_arr) + } + Value::Object(obj) => { + let mut new_arr = Vec::new(); + let code_actions_obj = obj + .get("code_actions") + .and_then(|code_actions| code_actions.as_object()); + let Some(code_actions) = code_actions_obj else { + continue; + }; + for (name, enabled) in code_actions { + if !enabled.as_bool().unwrap_or(true) { + continue; + } + new_arr.push(serde_json::json!({ + "code_action": name + })); + } + if new_arr.len() == 1 { + new_arr.pop().unwrap() + } else { + Value::Array(new_arr) + } + } + _ => continue, + }; - 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; + obj.insert(key.to_string(), new_formatter); } - } - Some((replace_range, code_actions_str)) + return Ok(()); + }) } diff --git a/crates/migrator/src/migrations/m_2025_10_02/settings.rs b/crates/migrator/src/migrations/m_2025_10_02/settings.rs index 2434ae4d0e100ce58e4cfdd2eee1039188c1d7bc..cb0d63ca8570952818e74e021f5dd2edc2523786 100644 --- a/crates/migrator/src/migrations/m_2025_10_02/settings.rs +++ b/crates/migrator/src/migrations/m_2025_10_02/settings.rs @@ -1,19 +1,10 @@ use anyhow::Result; use serde_json::Value; +use crate::patterns::migrate_language_setting; + pub fn remove_formatters_on_save(value: &mut Value) -> Result<()> { - remove_formatters_on_save_inner(value, &[])?; - let languages = value - .as_object_mut() - .and_then(|obj| obj.get_mut("languages")) - .and_then(|languages| languages.as_object_mut()); - if let Some(languages) = languages { - for (language_name, language) in languages.iter_mut() { - let path = vec!["languages", language_name]; - remove_formatters_on_save_inner(language, &path)?; - } - } - Ok(()) + migrate_language_setting(value, remove_formatters_on_save_inner) } fn remove_formatters_on_save_inner(value: &mut Value, path: &[&str]) -> Result<()> { diff --git a/crates/migrator/src/migrations/m_2025_10_10/settings.rs b/crates/migrator/src/migrations/m_2025_10_10/settings.rs index 1d07be71a139b60e4b362d26c68b25922f04a233..694f887b5962bb1f1e0f66eb4562edd10351868a 100644 --- a/crates/migrator/src/migrations/m_2025_10_10/settings.rs +++ b/crates/migrator/src/migrations/m_2025_10_10/settings.rs @@ -1,19 +1,10 @@ use anyhow::Result; use serde_json::Value; +use crate::patterns::migrate_language_setting; + pub fn remove_code_actions_on_format(value: &mut Value) -> Result<()> { - remove_code_actions_on_format_inner(value, &[])?; - let languages = value - .as_object_mut() - .and_then(|obj| obj.get_mut("languages")) - .and_then(|languages| languages.as_object_mut()); - if let Some(languages) = languages { - for (language_name, language) in languages.iter_mut() { - let path = vec!["languages", language_name]; - remove_code_actions_on_format_inner(language, &path)?; - } - } - Ok(()) + migrate_language_setting(value, remove_code_actions_on_format_inner) } fn remove_code_actions_on_format_inner(value: &mut Value, path: &[&str]) -> Result<()> { diff --git a/crates/migrator/src/migrator.rs b/crates/migrator/src/migrator.rs index aea11f98c460cc6c72120138ca1068be9ea60923..7b4182b00509effb0d05c64ea737b1d615c5b467 100644 --- a/crates/migrator/src/migrator.rs +++ b/crates/migrator/src/migrator.rs @@ -74,6 +74,7 @@ fn run_migrations(text: &str, migrations: &[MigrationType]) -> Result = None; + let json_indent_size = settings::infer_json_indent_size(¤t_text); for migration in migrations.iter() { let migrated_text = match migration { MigrationType::TreeSitter(patterns, query) => migrate(¤t_text, patterns, query)?, @@ -92,7 +93,7 @@ fn run_migrations(text: &str, migrations: &[MigrationType]) -> Result Result> { migrations::m_2025_07_08::SETTINGS_PATTERNS, &SETTINGS_QUERY_2025_07_08, ), - MigrationType::TreeSitter( - migrations::m_2025_10_01::SETTINGS_PATTERNS, - &SETTINGS_QUERY_2025_10_01, - ), + MigrationType::Json(migrations::m_2025_10_01::flatten_code_actions_formatters), MigrationType::Json(migrations::m_2025_10_02::remove_formatters_on_save), MigrationType::TreeSitter( migrations::m_2025_10_03::SETTINGS_PATTERNS, @@ -328,10 +326,6 @@ 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 -); define_query!( SETTINGS_QUERY_2025_10_03, migrations::m_2025_10_03::SETTINGS_PATTERNS @@ -351,10 +345,11 @@ mod tests { use super::*; use unindent::Unindent as _; + #[track_caller] 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_str_eq!(expected, migrated); } _ => { pretty_assertions::assert_eq!(migrated.as_deref(), expected); @@ -372,6 +367,7 @@ mod tests { assert_migrated_correctly(migrated, output); } + #[track_caller] fn assert_migrate_settings_with_migrations( migrations: &[MigrationType], input: &str, @@ -1343,24 +1339,28 @@ mod tests { fn test_flatten_code_action_formatters_basic_array() { assert_migrate_settings( &r#"{ - "formatter": [ - { - "code_actions": { - "included-1": true, - "included-2": true, - "excluded": false, - } - } - ] - }"# + "formatter": [ + { + "code_actions": { + "included-1": true, + "included-2": true, + "excluded": false, + } + } + ] + }"# .unindent(), Some( &r#"{ - "formatter": [ - { "code_action": "included-1" }, - { "code_action": "included-2" } - ] - }"# + "formatter": [ + { + "code_action": "included-1" + }, + { + "code_action": "included-2" + } + ] + }"# .unindent(), ), ); @@ -1370,21 +1370,25 @@ mod tests { fn test_flatten_code_action_formatters_basic_object() { assert_migrate_settings( &r#"{ - "formatter": { - "code_actions": { - "included-1": true, - "excluded": false, - "included-2": true - } - } - }"# + "formatter": { + "code_actions": { + "included-1": true, + "excluded": false, + "included-2": true + } + } + }"# .unindent(), Some( &r#"{ - "formatter": [ - { "code_action": "included-1" }, - { "code_action": "included-2" } - ] + "formatter": [ + { + "code_action": "included-1" + }, + { + "code_action": "included-2" + } + ] }"# .unindent(), ), @@ -1394,47 +1398,57 @@ mod tests { #[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, - } - }, - ] - }"#, + &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, + } + }, + ] + }"# + .unindent(), Some( - r#"{ - "formatter": [ - { "code_action": "included-1" }, - { "code_action": "included-2" }, - { - "language_server": "ruff" - }, - { "code_action": "included-3" }, - { "code_action": "included-4" }, - ] - }"#, + &r#"{ + "formatter": [ + { + "code_action": "included-1" + }, + { + "code_action": "included-2" + }, + { + "language_server": "ruff" + }, + { + "code_action": "included-3" + }, + { + "code_action": "included-4" + } + ] + }"# + .unindent(), ), ); } @@ -1443,55 +1457,63 @@ mod tests { 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, - } - }, - ] - } + "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" }, - ] - } - } - }"# + "languages": { + "Rust": { + "formatter": [ + { + "code_action": "included-1" + }, + { + "code_action": "included-2" + }, + { + "language_server": "ruff" + }, + { + "code_action": "included-3" + }, + { + "code_action": "included-4" + } + ] + } + } + }"# .unindent(), ), ); @@ -1502,100 +1524,120 @@ mod tests { { 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, - } - }, - ] - } + "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" }, - ] - } - } - }"# + "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(), ), ); @@ -1604,153 +1646,185 @@ mod tests { #[test] fn test_flatten_code_action_formatters_array_with_format_on_save_and_multiple_languages() { assert_migrate_settings_with_migrations( - &[MigrationType::TreeSitter( - migrations::m_2025_10_01::SETTINGS_PATTERNS, - &SETTINGS_QUERY_2025_10_01, + &[MigrationType::Json( + migrations::m_2025_10_01::flatten_code_actions_formatters, )], &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, - } - }, - ] - } + "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" }, - ] - } - } - }"# + &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(), ), ); @@ -1952,25 +2026,25 @@ mod tests { migrations::m_2025_10_10::remove_code_actions_on_format, )], &r#"{ - "code_actions_on_format": { - "a": true, - "b": false, - "c": true - } - }"# + "code_actions_on_format": { + "a": true, + "b": false, + "c": true + } + }"# .unindent(), Some( &r#"{ - "formatter": [ - { - "code_action": "a" - }, - { - "code_action": "c" - } - ] - } - "# + "formatter": [ + { + "code_action": "a" + }, + { + "code_action": "c" + } + ] + } + "# .unindent(), ), ); @@ -2163,12 +2237,12 @@ mod tests { ] }, "Python": { - "formatter": [ - { - "code_action": "source.organizeImports" - } - ] + "formatter": [ + { + "code_action": "source.organizeImports" } + ] + } } }"# .unindent(), @@ -2212,4 +2286,53 @@ mod tests { ), ); } + + #[test] + fn test_code_action_formatters_issue() { + assert_migrate_settings_with_migrations( + &[MigrationType::Json( + migrations::m_2025_10_01::flatten_code_actions_formatters, + )], + &r#" + { + "languages": { + "Python": { + "language_servers": ["ruff"], + "format_on_save": "on", + "formatter": [ + { + "code_actions": { + // Fix all auto-fixable lint violations + "source.fixAll.ruff": true, + // Organize imports + "source.organizeImports.ruff": true + } + } + ] + } + } + }"# + .unindent(), + Some( + &r#" + { + "languages": { + "Python": { + "language_servers": ["ruff"], + "format_on_save": "on", + "formatter": [ + { + "code_action": "source.fixAll.ruff" + }, + { + "code_action": "source.organizeImports.ruff" + } + ] + } + } + }"# + .unindent(), + ), + ); + } } diff --git a/crates/migrator/src/patterns.rs b/crates/migrator/src/patterns.rs index 3848baf23ba0d324995f18e3a53921948291153b..4132c93d9367a8dee200200e03dcc46ee073e67f 100644 --- a/crates/migrator/src/patterns.rs +++ b/crates/migrator/src/patterns.rs @@ -10,4 +10,5 @@ pub(crate) use settings::{ SETTINGS_ASSISTANT_PATTERN, SETTINGS_ASSISTANT_TOOLS_PATTERN, SETTINGS_DUPLICATED_AGENT_PATTERN, SETTINGS_EDIT_PREDICTIONS_ASSISTANT_PATTERN, SETTINGS_LANGUAGES_PATTERN, SETTINGS_NESTED_KEY_VALUE_PATTERN, SETTINGS_ROOT_KEY_VALUE_PATTERN, + migrate_language_setting, }; diff --git a/crates/migrator/src/patterns/settings.rs b/crates/migrator/src/patterns/settings.rs index 72fd02b153a5cf6e3158790f1c5d09a9f643ebf9..a068cce23b013a3435188c03ceebe866883c4e6d 100644 --- a/crates/migrator/src/patterns/settings.rs +++ b/crates/migrator/src/patterns/settings.rs @@ -108,3 +108,24 @@ pub const SETTINGS_DUPLICATED_AGENT_PATTERN: &str = r#"(document (#eq? @agent1 "agent") (#eq? @agent2 "agent") )"#; + +/// Migrate language settings, +/// calls `migrate_fn` with the top level object as well as all language settings under the "languages" key +/// Fails early if `migrate_fn` returns an error at any point +pub fn migrate_language_setting( + value: &mut serde_json::Value, + migrate_fn: fn(&mut serde_json::Value, path: &[&str]) -> anyhow::Result<()>, +) -> anyhow::Result<()> { + migrate_fn(value, &[])?; + let languages = value + .as_object_mut() + .and_then(|obj| obj.get_mut("languages")) + .and_then(|languages| languages.as_object_mut()); + if let Some(languages) = languages { + for (language_name, language) in languages.iter_mut() { + let path = vec!["languages", language_name]; + migrate_fn(language, &path)?; + } + } + Ok(()) +} diff --git a/crates/settings/src/settings_json.rs b/crates/settings/src/settings_json.rs index 555a48e9f0972d708eaf9aaaaaf467852ccf7dd6..5e83b11b339245c9e8beb6038cb8c5532b551ad8 100644 --- a/crates/settings/src/settings_json.rs +++ b/crates/settings/src/settings_json.rs @@ -262,8 +262,8 @@ pub fn replace_value_in_json_text>( } else { // We don't have the key, construct the nested objects let new_value = construct_json_value(&key_path[depth..], new_value); - let indent_prefix_len = 4 * depth; - let mut new_val = to_pretty_json(&new_value, 4, indent_prefix_len); + let indent_prefix_len = tab_size * depth; + let mut new_val = to_pretty_json(&new_value, tab_size, indent_prefix_len); if depth == 0 { new_val.push('\n'); } @@ -628,6 +628,100 @@ pub fn append_top_level_array_value_in_json_text( } } +/// Infers the indentation size used in JSON text by analyzing the tree structure. +/// Returns the detected indent size, or a default of 2 if no indentation is found. +pub fn infer_json_indent_size(text: &str) -> usize { + const MAX_INDENT_SIZE: usize = 64; + + let mut parser = tree_sitter::Parser::new(); + parser + .set_language(&tree_sitter_json::LANGUAGE.into()) + .unwrap(); + + let Some(syntax_tree) = parser.parse(text, None) else { + return 4; + }; + + let mut cursor = syntax_tree.walk(); + let mut indent_counts = [0u32; MAX_INDENT_SIZE]; + + // Traverse the tree to find indentation patterns + fn visit_node( + cursor: &mut tree_sitter::TreeCursor, + indent_counts: &mut [u32; MAX_INDENT_SIZE], + depth: usize, + ) { + if depth >= 3 { + return; + } + let node = cursor.node(); + let node_kind = node.kind(); + + // For objects and arrays, check the indentation of their first content child + if matches!(node_kind, "object" | "array") { + let container_column = node.start_position().column; + let container_row = node.start_position().row; + + if cursor.goto_first_child() { + // Skip the opening bracket + loop { + let child = cursor.node(); + let child_kind = child.kind(); + + // Look for the first actual content (pair for objects, value for arrays) + if (node_kind == "object" && child_kind == "pair") + || (node_kind == "array" + && !matches!(child_kind, "[" | "]" | "," | "comment")) + { + let child_column = child.start_position().column; + let child_row = child.start_position().row; + + // Only count if the child is on a different line + if child_row > container_row && child_column > container_column { + let indent = child_column - container_column; + if indent > 0 && indent < MAX_INDENT_SIZE { + indent_counts[indent] += 1; + } + } + break; + } + + if !cursor.goto_next_sibling() { + break; + } + } + cursor.goto_parent(); + } + } + + // Recurse to children + if cursor.goto_first_child() { + loop { + visit_node(cursor, indent_counts, depth + 1); + if !cursor.goto_next_sibling() { + break; + } + } + cursor.goto_parent(); + } + } + + visit_node(&mut cursor, &mut indent_counts, 0); + + // Find the indent size with the highest count + let mut max_count = 0; + let mut max_indent = 4; + + for (indent, &count) in indent_counts.iter().enumerate() { + if count > max_count { + max_count = count; + max_indent = indent; + } + } + + if max_count == 0 { 2 } else { max_indent } +} + pub fn to_pretty_json( value: &impl Serialize, indent_size: usize, @@ -2486,4 +2580,69 @@ mod tests { .unindent(), ) } + + #[test] + fn test_infer_json_indent_size() { + let json_2_spaces = r#"{ + "key1": "value1", + "nested": { + "key2": "value2", + "array": [ + 1, + 2, + 3 + ] + } +}"#; + assert_eq!(infer_json_indent_size(json_2_spaces), 2); + + let json_4_spaces = r#"{ + "key1": "value1", + "nested": { + "key2": "value2", + "array": [ + 1, + 2, + 3 + ] + } +}"#; + assert_eq!(infer_json_indent_size(json_4_spaces), 4); + + let json_8_spaces = r#"{ + "key1": "value1", + "nested": { + "key2": "value2" + } +}"#; + assert_eq!(infer_json_indent_size(json_8_spaces), 8); + + let json_single_line = r#"{"key": "value", "nested": {"inner": "data"}}"#; + assert_eq!(infer_json_indent_size(json_single_line), 2); + + let json_empty = r#"{}"#; + assert_eq!(infer_json_indent_size(json_empty), 2); + + let json_array = r#"[ + { + "id": 1, + "name": "first" + }, + { + "id": 2, + "name": "second" + } +]"#; + assert_eq!(infer_json_indent_size(json_array), 2); + + let json_mixed = r#"{ + "a": { + "b": { + "c": "value" + } + }, + "d": "value2" +}"#; + assert_eq!(infer_json_indent_size(json_mixed), 2); + } } diff --git a/crates/settings/src/settings_store.rs b/crates/settings/src/settings_store.rs index 6f22e27d980699e79741a4b3895c4eae456b8536..4731c38379f787b497354c3066f8f5eecb1332ca 100644 --- a/crates/settings/src/settings_store.rs +++ b/crates/settings/src/settings_store.rs @@ -33,6 +33,7 @@ pub type EditorconfigProperties = ec4rs::Properties; use crate::{ ActiveSettingsProfileName, FontFamilyName, IconThemeName, LanguageSettingsContent, LanguageToSettingsMap, SettingsJsonSchemaParams, ThemeName, VsCodeSettings, WorktreeId, + infer_json_indent_size, merge_from::MergeFrom, parse_json_with_comments, settings_content::{ @@ -637,7 +638,7 @@ impl SettingsStore { let mut key_path = Vec::new(); let mut edits = Vec::new(); - let tab_size = self.json_tab_size(); + let tab_size = infer_json_indent_size(&text); let mut text = text.to_string(); update_value_in_json_text( &mut text, @@ -650,10 +651,6 @@ impl SettingsStore { edits } - pub fn json_tab_size(&self) -> usize { - 2 - } - /// Sets the default settings via a JSON string. /// /// The string should contain a JSON object with a default value for every setting. @@ -1540,9 +1537,9 @@ mod tests { }) }, r#"{ - "tabs": { - "git_status": true - } + "tabs": { + "git_status": true + } } "# .unindent(), @@ -1557,9 +1554,9 @@ mod tests { .unindent(), |settings| settings.title_bar.get_or_insert_default().show_branch_name = Some(true), r#"{ - "title_bar": { - "show_branch_name": true - } + "title_bar": { + "show_branch_name": true + } } "# .unindent(), @@ -1584,7 +1581,7 @@ mod tests { .unindent(), r#" { "editor.tabSize": 37 } "#.to_owned(), r#"{ - "tab_size": 37 + "tab_size": 37 } "# .unindent(), @@ -1637,9 +1634,9 @@ mod tests { .unindent(), r#"{ "workbench.editor.decorations.colors": true }"#.to_owned(), r#"{ - "tabs": { - "git_status": true - } + "tabs": { + "git_status": true + } } "# .unindent(), @@ -1655,11 +1652,11 @@ mod tests { .unindent(), r#"{ "editor.fontFamily": "Cascadia Code, 'Consolas', Courier New" }"#.to_owned(), r#"{ - "buffer_font_fallbacks": [ - "Consolas", - "Courier New" - ], - "buffer_font_family": "Cascadia Code" + "buffer_font_fallbacks": [ + "Consolas", + "Courier New" + ], + "buffer_font_family": "Cascadia Code" } "# .unindent(), @@ -1695,16 +1692,16 @@ mod tests { .get_or_insert_default() .enabled = Some(true); }); - assert_eq!( + pretty_assertions::assert_str_eq!( actual, r#"{ - "git": { + "git": { "inline_blame": { - "enabled": true + "enabled": true } + } } - } - "# + "# .unindent() ); }