Only prune stored profiles this manager loaded or saved - #139
Merged
Merged
Conversation
SaveAsync treated every stored profile missing from memory as deleted, so a manager that never called InitializeAsync deleted every profile on disk when it saved. The manager now records the ids of the profiles it loaded or saved, and SaveAsync deletes only those that are no longer in memory. Profiles it never saw are left alone, and deleting a loaded or previously saved profile still sticks as #106 requires. Fixes #124 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X65VbCUfvG15D4o8afrvpk
|
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 #124
What was wrong
Since the #106 fix,
KeybindingManager.SaveAsyncdeleted every stored profile that wasn't in memory. On a manager that never calledInitializeAsync, that was every profile on disk. In the three-manager scenario from the issue, saving from manager B permanently deleteddefaultandvim.Change
KeybindingManagernow records the ids of the stored profiles it has seen: the onesInitializeAsyncloaded and the onesSaveAsyncwrote.SaveAsyncdeletes only ids from that set that are no longer in memory, then updates the set.IProfileManageris unchanged. Tracking calls toDeleteProfiledirectly would have meant adding an interface member, because callers delete throughmanager.Profiles, which is an injectedIProfileManager. Tracking the ids the manager has seen gives the same result without that change.Lockguards the set.Lockcomes from Polyfill on net8, and the library builds for net8.0, net9.0 and net10.0.Tests
SaveWithoutInitializeTestscovers:With the fix reverted, the regression test fails: the reload shows only
other, notdefault,other,vim. Full suite: 105/105 pass.🤖 Generated with Claude Code
https://claude.ai/code/session_01X65VbCUfvG15D4o8afrvpk
Generated by Claude Code