Repository navigation
Use the registered IKeybindingService in KeybindingManager [patch] - #210
Merged
Merged
Conversation
KeybindingManager always created its own KeybindingService, so a custom or decorated IKeybindingService registered with the container never took effect and the container held two keybinding services. The manager now takes the service through its primary constructor, which dependency injection selects, and KeybindingManagerFactory.CreateManager() passes the registered one. The three-argument constructor still builds a KeybindingService for standalone use. Fixes #208 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW49jMQ3NXp3nePHu2u
|
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 #208
What changed
KeybindingManagernow has a primary constructor(ICommandRegistry, IProfileManager, IKeybindingRepository, IKeybindingService). Because it has the most resolvable parameters, Microsoft.Extensions.DependencyInjection selects it, somanager.Keybindingsis theIKeybindingServiceregistered in the container. That covers both the default registration and a custom or decorated one. The analyzer (IDE0290) asked for a primary constructor, which is why the class was converted.KeybindingServiceover the given registry and profile manager. Thestring dataDirectoryconstructor is unchanged.KeybindingManagerFactory.CreateManager()passes the registeredIKeybindingServicewhen there is one.Before this change, the container held two different keybinding services, even with the default
AddKeybindingregistration: the registered one and the one the manager created.Tests
KeybindingServiceInjectionTestsis new. It checks thatmanager.Keybindingsis the same instance as the registered service in four cases: a custom registration, the default singleton registration, the scoped registration, andfactory.CreateManager(). It also checks that the three-argument constructor still builds its ownKeybindingService.Without the fix, the four same-instance tests fail. With the fix, the full suite passes (334/334 on this branch) and
Keybinding.slnbuilds with no warnings.Generated by Claude Code