Fix #2919: keep a settings file that cannot be parsed - #4109
Merged
Merged
Conversation
ILSpy.xml is the only copy of everything a user puts in it - assembly lists above all, which people build up over years and, as the report shows, edit by hand. A file that fails to parse was replaced by defaults on the next save, which happens for something as incidental as a window position, so the data was gone before the user had a chance to notice anything was wrong. The file is now moved aside first, under a name that says what it is, and an earlier copy is never replaced: two bad starts in a row must not cost the file that still has the data. A typo in hand-written XML is usually one edit away from readable, so what matters is that it still exists. Telling the user is still not solved - that needs somewhere central to report it from, which the settings do not have yet - but the file is recoverable, and its name says why it is there. Assisted-by: Claude:claude-opus-5:Claude Code
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
1 participant
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 #2919.
ILSpy.xmlis the only copy of everything a user puts in it - assembly lists above all, whichpeople build up over years and, as the report shows, edit by hand. A file that fails to parse was
replaced by defaults on the next save, and a save happens for something as incidental as a window
position, so the data was gone before the user had a chance to notice anything was wrong.
The change
ILSpySettings.Updateis the one place that writes the file. When the file cannot be parsed it ismoved to
ILSpy.xml.broken-<timestamp>before the new one is written, and an earlier copy is neverreplaced - two bad starts in a row must not cost the file that still holds the data. A typo in
hand-written XML is usually one edit away from readable, so what matters is that it still exists.
Failing to move the file does not stop settings from being saved: keeping it is the point, and a
copy that cannot be made is not a reason to lose the session's settings too.
What this does not do
It does not tell the user. The thread's reading was that this is unfixable short of central
settings loading and saving:
That holds for notifying - there is nowhere central to report from, and
ICSharpCode.ILSpyXhasno UI to report to. It does not hold for the data loss itself, which is one
doc.Save(config)inone method. So this fixes the destructive half now and leaves the notification to the refactor,
with the file recoverable in the meantime and its name saying why it is there.
Verification
ILSpy.Tests/Settings/MalformedSettingsFileTests, four tests written failing first: what could notbe parsed is kept, the save still goes through, an earlier backup is not overwritten, and a
well-formed file is left completely alone.
ILSpy.Tests1263 tests, no failures.Prepared by an AI agent (Claude, claude-opus-5, via Claude Code) and reviewed by @siegfriedpammer.