Repository navigation
Refuse a save that can't be loaded back, and load a hand-added $schema [patch] - #382
Open
matt-edmondson wants to merge 1 commit into
Open
matt-edmondson wants to merge 1 commit into
matt-edmondson wants to merge 1 commit into
Conversation
…a [patch] ReferenceHandler.Preserve writes a dictionary key starting with '$' but throws on reading it, so the next load archived the file as corrupt and reset every setting. Save() now reads the document back first and throws JsonException without touching the file. A top-level "$schema" added by hand for editor validation is dropped before deserializing. Fixes #376 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW44rALLJaHHtXa8D2t
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #376
What was wrong
AppData.JsonSerializerOptionsusesReferenceHandler.Preserve. Under that option System.Text.Json writes a dictionary key such as"$HOME", but throws when it reads it back.LoadOrCreatethen archived the file as.corruptand reset every setting, not only the dictionary. A top-level"$schema"added by hand for editor validation failed the same way.Change
This is the "at minimum" step from the issue and its triage comment:
Save()deserializes the document it is about to write, using the same exceptions the load path recovers from. If that fails, it throwsJsonExceptionnaming the type and the likely cause, before anything is written. The file on disk keeps its last good contents, so the data loss now shows up as an error at save time.LoadOrCreateremoves a top-level"$schema"property before deserializing. It only parses when the text contains"$schema". A document that can't be parsed goes to the deserializer unchanged and is archived as before. The next save does not write the$schemaback.The move away from
Preserveitself, which could also close #353, is left for a separate change. It needs a fallback reader for existing$id/$valuesfiles.Cost: every save now runs one extra deserialize of the settings document. Settings files are small.
Tests
New
DollarPropertyTests:SaveRefusesADollarKeyAndKeepsTheLastGoodFile($HOME,$schema,$id): the save throws, the last good save still loads, and nothing is archived.HandAddedSchemaPropertyLoads: this is the issue's{"$schema":...,"Theme":"Dark"}case.SchemaPropertyBeforePreservedReferenceMetadataLoads: a$schemaplaced before the file's own$id.OrdinaryDictionaryKeysStillRoundTrip(HOME,a$b) andNestedSchemaKeyIsStillUnreadable.With
AppData.csreverted, 5 of the 7 new cases fail. The two guards pass either way. Full suite on Linux / .NET 10: 152/152 passed. A Release build of the library succeeds for all six target frameworks.Generated by Claude Code