What's wrong
ProfileManager.DeleteProfile removes the profile from memory only.
KeybindingManager.SaveAsync then upserts each in-memory profile, one at a time:
IReadOnlyCollection<Profile> profiles = Profiles.GetAllProfiles();
await AsyncBatchHelper.ForEachAsync(profiles, Repository.SaveProfileAsync)
JsonKeybindingRepository.SaveProfileAsync loads every profile already on disk, removes only the one with the same id, appends the new one, and rewrites the file. A profile that is on disk but no longer in memory is carried over unchanged.
IKeybindingRepository.DeleteProfileAsync exists and is implemented, but nothing in the library calls it.
Reproduction (verified against current main)
- Create profiles
default and vim, then call SaveAsync().
- Call
Profiles.DeleteProfile("vim"), then SaveAsync(). The in-memory profile list is now default.
- Create a fresh
KeybindingManager on the same directory and call InitializeAsync(). The profiles are vim, default.
Why it matters
Users cannot delete a profile for good: it reappears on every launch, and any bindings it holds come back with it. Renaming a profile to a new id also leaves the old id on disk.
Suggested fix
Have SaveAsync persist the profile set as a whole, for example with a new IKeybindingRepository.SaveAllProfilesAsync(IEnumerable<Profile>) that writes profiles.json once. This also removes the current O(n²) pattern of reading and rewriting the whole file once per profile.
Alternatively, delete the stored ids that are no longer in memory, or call Repository.DeleteProfileAsync from the delete path.
Acceptance criteria: a test covers delete, save, and reload, and asserts the deleted profile is gone.
What's wrong
ProfileManager.DeleteProfileremoves the profile from memory only.KeybindingManager.SaveAsyncthen upserts each in-memory profile, one at a time:JsonKeybindingRepository.SaveProfileAsyncloads every profile already on disk, removes only the one with the same id, appends the new one, and rewrites the file. A profile that is on disk but no longer in memory is carried over unchanged.IKeybindingRepository.DeleteProfileAsyncexists and is implemented, but nothing in the library calls it.Reproduction (verified against current
main)defaultandvim, then callSaveAsync().Profiles.DeleteProfile("vim"), thenSaveAsync(). The in-memory profile list is nowdefault.KeybindingManageron the same directory and callInitializeAsync(). The profiles arevim, default.Why it matters
Users cannot delete a profile for good: it reappears on every launch, and any bindings it holds come back with it. Renaming a profile to a new id also leaves the old id on disk.
Suggested fix
Have
SaveAsyncpersist the profile set as a whole, for example with a newIKeybindingRepository.SaveAllProfilesAsync(IEnumerable<Profile>)that writesprofiles.jsononce. This also removes the current O(n²) pattern of reading and rewriting the whole file once per profile.Alternatively, delete the stored ids that are no longer in memory, or call
Repository.DeleteProfileAsyncfrom the delete path.Acceptance criteria: a test covers delete, save, and reload, and asserts the deleted profile is gone.