Stop prompting to migrate settings that contain no deprecations (#62818)
## Objective Zed can report that a settings file uses deprecated settings even when it contains no deprecations. A context server configured with only a `url` is enough to trigger the prompt, although the backup Zed offers to write is identical to the current file. Fixes #24967. ## Solution Two text-edit migrations cancel each other on `context_servers` entries: one adds a `source` key and a later one removes it. The round trip reindents the entry, and the migrator mistakes the leftover whitespace for a migration. Remove `m_2025_06_16` rather than comparing around that whitespace. It wrote `source` and `settings` into every entry, but `ContextServerSettingsContent` contains neither field today. `settings` is worse than inert: an empty entry such as `{ "settings": {} }` matches neither the `Stdio` nor `Http` variant of the untagged enum. Update `m_2025_06_27`, the only remaining consumer of the generated `"source": "custom"`, to detect legacy entries from the shape of `command` itself. This continues to flatten settings written before `source` existed. It also deliberately flattens an object `command` beside `"source": "extension"`, which the old migration skipped. The command shape is invalid regardless of the declared source, and a test pins the new behavior. `m_2025_11_25` remains unchanged because it still removes `source` from settings that genuinely carry it. This deletes one migration and edits another despite the warning at the top of `migrator.rs`. That warning protects later migrations that depend on earlier output, but this output has no remaining consumer. I left the policy unchanged rather than adding an exception in the same PR that relies on it, but I can document the exception if preferred. ## Testing - `cargo test -p migrator` passes, 86 tests. The suite loses the three tests that only exercised the deleted migration and gains `test_flatten_context_server_command_alongside_source`, covering a legacy entry that carries both `source` and an object `command`. - Restoring the `source` requirement on `m_2025_06_27` with the deletion in place turns `test_flatten_context_server_command` red. Its input is a pre-`source` entry, which is what makes the relaxation load-bearing rather than incidental. - To check by hand: add `"context_servers": { "grep": { "url": "https://mcp.grep.app" } }` to user settings and confirm no migration prompt appears. Add `"show_inline_completions": true` alongside it and confirm the prompt returns and leaves the context server block untouched. - I have not run those steps in a built editor. The tests call `migrate_settings`, which is all `should_migrate_settings` checks before raising the prompt, so that call is the whole untested gap. It’s all platform-independent text processing, so I did not test Linux or Windows. ## Self-Review Checklist: - [x] I’ve reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed’s UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable --- Release Notes: - Fixed the migration prompt appearing for settings that contain no deprecations. Context servers configured with only a `url` no longer trigger it. --------- Co-authored-by: MrSubidubi <finn@zed.dev>
This commit is contained in:
1 parent
2d6911db05
commit
7599be8e6b
7 files changed
+107
-354
No files matched your search
Generated
+1
@@ -10924,6 +10924,7 @@ dependencies = [
|
||||
"anyhow",
|
||||
"collections",
|
||||
"convert_case 0.11.0",
|
||||
"indoc",
|
||||
"log",
|
||||
"pretty_assertions",
|
||||
"serde_json",
|
||||
|
||||
Reference in new issue
Block a user