Skip to content

Return snapshots from GetAllChords and BoundCommands - #130

Merged
matt-edmondson merged 1 commit into
mainfrom
fix/get-all-chords-snapshot-127
Sep 27, 2026
Merged

matt-edmondson merged 1 commit into
mainfrom
fix/get-all-chords-snapshot-127

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #127

What changed

Profile.GetAllChords() returned Chords.AsReadOnly(), a wrapper over the live dictionary, and Profile.BoundCommands returned Chords.Keys. So a value handed to the caller changed after a later BindChord/UnbindChord, and binding while iterating GetAllChords() threw InvalidOperationException: Collection was modified.

Both now return copies (new Dictionary<,>(Chords).AsReadOnly() and [.. Chords.Keys]). That matches the other collection getters (GetAllProfiles, GetAllCommands), which already return snapshots. KeybindingService.GetAllChords() and GetAllChords(profileId) pass the profile's result through, so they get the fix too.

Tests

New GetAllChordsSnapshotTests:

  • a dictionary returned by GetAllChords() / GetAllChords("p") doesn't change after a later bind and unbind
  • binding while enumerating GetAllChords() doesn't throw
  • BoundCommands doesn't change after a later bind

All three fail on main and pass with the fix. The full suite passes: 90/90.

🤖 Generated with Claude Code

https://claude.ai/code/session_01G89GFC9HBcemu37GarNas1


Generated by Claude Code

Profile.GetAllChords() wrapped the live chord dictionary and BoundCommands
exposed its key collection, so a returned value changed after later binds
and binding while iterating threw "Collection was modified". Both now copy,
matching the other collection getters in the API.

Fixes #127

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01G89GFC9HBcemu37GarNas1
@sonarqubecloud

Copy link
Copy Markdown

@matt-edmondson
matt-edmondson merged commit 0d91a38 into main Sep 27, 2026
12 checks passed
@matt-edmondson
matt-edmondson deleted the fix/get-all-chords-snapshot-127 branch September 27, 2026 04:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GetAllChords() returns a live view, so it changes under the caller and throws "Collection was modified" when binding while iterating

1 participant