settings: Fix VS Code import appending duplicate file associations (#61355)
Closes #56536 ## What & why Running `zed: import vs code settings` more than once kept re-adding the same file extensions to `file_types`, so `files.associations` entries grew without bound: ```json "file_types": { "c": ["*.keymap", "*.keymap", "*.keymap"] } ``` The import merges the VS Code-derived settings into the user's existing settings, and `file_types` values were `ExtendingVec`s, whose `merge_from` appends every incoming value unconditionally. Per review feedback, instead of changing `ExtendingVec`'s merge semantics (it also backs ssh/wsl connections, `private_files`, etc.), this adds a new `ExtendingSet` type that actually behaves like a set, and switches `FileTypeMap`'s file associations to it: - Backed by an insertion-order-preserving `IndexSet`, so users' pattern order round-trips through the settings file unchanged (the import machinery diffs serialized old/new content to produce edits; a sorted set would reorder existing arrays). - Merging only accumulates new values, so re-importing an already-present association is a no-op. - Since it's a real set, deserialization also collapses duplicates that earlier imports already wrote into the settings file. - `ExtendingVec` and all its other users are untouched. ## Testing - Added a `test_vscode_import` case that re-imports an already-present `files.associations` entry and asserts the extension isn't duplicated. It fails before the change and passes after. - `cargo test -p settings -p settings_content -p language` pass. - `./script/clippy -p settings -p settings_content -p language -p recent_projects -p json_schema_store` is clean. Release Notes: - Fixed importing VS Code settings repeatedly adding duplicate `file_types` entries for `files.associations`. --------- Co-authored-by: Christopher Biscardi <chris@christopherbiscardi.com>
This commit is contained in:
1 parent
d749d04034
commit
28605024d7
5 files changed
+64
-10
No files matched your search
@@ -36,7 +36,7 @@ use crate::{
|
||||
LanguageToSettingsMap, LspSettings, LspSettingsMap, SemanticTokenRules, ThemeName,
|
||||
UserSettingsContentExt, VsCodeSettings, WorktreeId,
|
||||
settings_content::{
|
||||
ExtendingVec, ExtensionsSettingsContent, ProfileBase, ProjectSettingsContent,
|
||||
ExtendingSet, ExtensionsSettingsContent, ProfileBase, ProjectSettingsContent,
|
||||
RootUserSettings, SettingsContent, UserSettingsContent, merge_from::MergeFrom,
|
||||
},
|
||||
};
|
||||
@@ -1203,7 +1203,7 @@ impl SettingsStore {
|
||||
});
|
||||
|
||||
let file_type_patterns_ref =
|
||||
generator.subschema_for::<ExtendingVec<String>>().to_value();
|
||||
generator.subschema_for::<ExtendingSet<String>>().to_value();
|
||||
replace_subschema::<FileTypeMap>(generator, || {
|
||||
json_schema!({
|
||||
"type": "object",
|
||||
@@ -2616,6 +2616,32 @@ mod tests {
|
||||
.unindent(),
|
||||
cx,
|
||||
);
|
||||
|
||||
// re-importing a file association that is already present should be
|
||||
// idempotent rather than appending a duplicate extension (#56536)
|
||||
check_vscode_import(
|
||||
&mut store,
|
||||
r#"{
|
||||
"file_types": {
|
||||
"c": ["*.keymap"]
|
||||
}
|
||||
}
|
||||
"#
|
||||
.unindent(),
|
||||
r#"{ "files.associations": { "*.keymap": "c" } }"#.to_owned(),
|
||||
r#"{
|
||||
"base_keymap": "VSCode",
|
||||
"minimap": {
|
||||
"show": "always"
|
||||
},
|
||||
"file_types": {
|
||||
"c": ["*.keymap"]
|
||||
}
|
||||
}
|
||||
"#
|
||||
.unindent(),
|
||||
cx,
|
||||
);
|
||||
}
|
||||
|
||||
#[track_caller]
|
||||
|
||||
@@ -644,11 +644,15 @@ impl VsCodeSettings {
|
||||
|
||||
fn file_types(&self) -> Option<FileTypeMap> {
|
||||
// vscodes file association map is inverted from ours, so we flip the mapping before merging
|
||||
let mut associations: HashMap<Arc<str>, ExtendingVec<String>> = HashMap::default();
|
||||
let mut associations: HashMap<Arc<str>, ExtendingSet<String>> = HashMap::default();
|
||||
let map = self.read_value("files.associations")?.as_object()?;
|
||||
for (k, v) in map {
|
||||
let Some(v) = v.as_str() else { continue };
|
||||
associations.entry(v.into()).or_default().0.push(k.clone());
|
||||
associations
|
||||
.entry(v.into())
|
||||
.or_default()
|
||||
.0
|
||||
.insert(k.clone());
|
||||
}
|
||||
skip_default(FileTypeMap(associations))
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user