镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

KeybindingManager ignores a registered IKeybindingService: it always news up its own KeybindingService, so a custom or decorated service never takes effect #208

Closed

Description

@matt-edmondson

Plan reference

ARCHITECTURE_ANALYSIS.md L20-26 and L62-66 (status: draft):

  • Service Interfaces: All core services are interface-based, allowing custom implementations
  • Dependency Injection: All dependencies are injected via constructor

What exists today

  • IKeybindingService is registered in DI (Keybinding/Extensions/ServiceCollectionExtensions.cs:45, 92).
  • KeybindingManager exposes IKeybindingService Keybindings (Keybinding/KeybindingManager.cs:73).

What's missing

The manager's constructor hard-codes the implementation:

// Keybinding/KeybindingManager.cs:57
Keybindings = new KeybindingService(Commands, Profiles);

Neither constructor accepts an IKeybindingService. So an app that registers a decorator never gets it from the manager, for example a decorator that logs or audits rebinds, or one that enforces app-specific conflict rules. The DI container then holds two different keybinding services.

Reproduction (HEAD b035bdb)

var s = new ServiceCollection();
s.AddSingleton<IKeybindingService>(sp => new LoggingService(new KeybindingService(
    sp.GetRequiredService<ICommandRegistry>(), sp.GetRequiredService<IProfileManager>())));
s.AddKeybinding(tmpDir);
using var sp = s.BuildServiceProvider();
var registered = sp.GetRequiredService<IKeybindingService>();
var m = sp.GetRequiredService<KeybindingManager>();
Console.WriteLine($"registered: {registered.GetType().Name}; manager.Keybindings: {m.Keybindings.GetType().Name}; same instance: {ReferenceEquals(registered, m.Keybindings)}");
// registered: LoggingService; manager.Keybindings: KeybindingService; same instance: False

LoggingService is a pass-through decorator.

Acceptance criteria

  • Add a constructor that takes IKeybindingService alongside the existing three dependencies, and make it the one DI selects.
  • Keep the 3-argument constructor for standalone use. It goes on creating a KeybindingService.
  • KeybindingManagerFactory.CreateManager() passes the registered IKeybindingService.
  • Add a test asserting that manager.Keybindings is the instance registered in DI.

Dependencies

None. Related, but separate: #145 covers IKeybindingConfiguration not being read.

Activity

  1. matt-edmondson commented on Oct 31, 2026

    @matt-edmondson
    ContributorAuthor

    Triage


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

enhancementNew feature or request

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions