Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unsubscribed keybindings cause crash when used #444

Open
ghost opened this issue May 15, 2023 · 3 comments
Open

Unsubscribed keybindings cause crash when used #444

ghost opened this issue May 15, 2023 · 3 comments

Comments

@ghost
Copy link

ghost commented May 15, 2023

In my config, I use the UnsubsribeAll() method from the IKeybindManager class to get rid of any existing keybindings and apply my own. This works until I press a default keybinding. This causes Workspacer to crash. The log entry relating to the crash is below.

2023-05-15 12:26:13.6575|FATAL|workspacer.Program|exception occurred, quitting workspacer: System.Collections.Generic.KeyNotFoundException: The given key 'workspacer.Monitor' was not present in the dictionary.
   at System.Collections.Generic.Dictionary`2.get_Item(TKey key)
   at workspacer.WorkspaceContainer.GetWorkspaceForMonitor(IMonitor monitor) in D:\a\workspacer\workspacer\src\workspacer.Shared\Workspace\WorkspaceContainer.cs:line 172
   at workspacer.WorkspaceManager.SwitchToWorkspace(Int32 index) in D:\a\workspacer\workspacer\src\workspacer\Workspace\WorkspaceManager.cs:line 54
   at workspacer.KeybindManager.<SubscribeDefaults>b__28_25() in D:\a\workspacer\workspacer\src\workspacer\Keybinds\KeybindManager.cs:line 305
   at workspacer.KeybindManager.DoKeyboardEvent(Keys key, KeyModifiers modifiersPressed) in D:\a\workspacer\workspacer\src\workspacer\Keybinds\KeybindManager.cs:line 206
   at workspacer.KeybindManager.KbdHook(Int32 nCode, UIntPtr wParam, IntPtr lParam) in D:\a\workspacer\workspacer\src\workspacer\Keybinds\KeybindManager.cs:line 131|System.Collections.Generic.KeyNotFoundException: The given key 'workspacer.Monitor' was not present in the dictionary.
   at System.Collections.Generic.Dictionary`2.get_Item(TKey key)
   at workspacer.WorkspaceContainer.GetWorkspaceForMonitor(IMonitor monitor) in D:\a\workspacer\workspacer\src\workspacer.Shared\Workspace\WorkspaceContainer.cs:line 172
   at workspacer.WorkspaceManager.SwitchToWorkspace(Int32 index) in D:\a\workspacer\workspacer\src\workspacer\Workspace\WorkspaceManager.cs:line 54
   at workspacer.KeybindManager.<SubscribeDefaults>b__28_25() in D:\a\workspacer\workspacer\src\workspacer\Keybinds\KeybindManager.cs:line 305
   at workspacer.KeybindManager.DoKeyboardEvent(Keys key, KeyModifiers modifiersPressed) in D:\a\workspacer\workspacer\src\workspacer\Keybinds\KeybindManager.cs:line 206
   at workspacer.KeybindManager.KbdHook(Int32 nCode, UIntPtr wParam, IntPtr lParam) in D:\a\workspacer\workspacer\src\workspacer\Keybinds\KeybindManager.cs:line 131

Just simply invoking UnsubscribeAll() in the basic default config and then pressing one of the default keybindings causes a crash. I stripped down the config to just create 5 simple workspaces and not really do anything else. Still, the call to UnsubscribeAll() followed by trying to use an unbound keybind causes the application to crash.

The expected behavior that I'm anticipating is that keybindings that have been "unsubscribed" would be ignored by Workspacer. Somehow, this is not the case.

@ghost
Copy link
Author

ghost commented May 15, 2023

As a really terrible workaround, I can overwrite all of the default keybindings with a junk action and this prevents the crash. However, these keys now become unusable for other purposes.

            manager.Subscribe(Alt, Keys.Escape,
                () => {}, "toggle enable/disable");

            manager.Subscribe(Alt | KeyModifiers.LShift, Keys.C,
                () => {}, "close focused window");

            manager.Subscribe(Alt | KeyModifiers.LShift, Keys.Space,
                () => {}, "previous layout");

            manager.Subscribe(Alt, Keys.N,
                () => {}, "reset layout");

            manager.Subscribe(Alt, Keys.J,
                () => {}, "focus next window");

            manager.Subscribe(Alt, Keys.K,
                () => {}, "focus previous window");

            manager.Subscribe(Alt, Keys.M,
                () => {}, "focus primary window");

            manager.Subscribe(Alt, Keys.Enter,
                () => {}, "swap focus and primary window");

            manager.Subscribe(Alt | KeyModifiers.LShift, Keys.J,
                () => {}, "swap focus and next window");

            manager.Subscribe(Alt | KeyModifiers.LShift, Keys.K,
                () => {}, "swap focus and previous window");

            manager.Subscribe(Alt, Keys.Oemcomma,
                () => {}, "increment # primary windows");

            manager.Subscribe(Alt, Keys.OemPeriod,
                () => {}, "decrement # primary windows");

            manager.Subscribe(Alt, Keys.T,
                () => {}, "toggle tiling for focused window");

            manager.Subscribe(Alt, Keys.Q, () => {}, "restart workspacer");

            manager.Subscribe(Alt, Keys.D8,
                () => {}, "switch to workspace 8");

            manager.Subscribe(Alt, Keys.D9,
                () => {}, "switch to workpsace 9");

            manager.Subscribe(Alt, Keys.Left,
                () => {}, "switch to previous workspace");

            manager.Subscribe(Alt, Keys.Right,
                () => {}, "switch to next workspace");

            manager.Subscribe(Alt | KeyModifiers.Control, Keys.Left,
                () => {}, "move window to previous workspace and switch to it");

            manager.Subscribe(Alt | KeyModifiers.Control, Keys.Right,
                () => {}, "move window to next workspace and switch to it");

            manager.Subscribe(Alt, Keys.Oemtilde,
                () => {}, "switch to last focused workspace");

            manager.Subscribe(Alt, Keys.W,
                () => {}, "focus monitor 1");

            manager.Subscribe(Alt, Keys.R,
                () => {}, "focus monitor 3");

            manager.Subscribe(Alt | KeyModifiers.LShift, Keys.W,
                () => {}, "move focused window to monitor 1");

            manager.Subscribe(Alt | KeyModifiers.LShift, Keys.E,
                () => {}, "move focused window to monitor 2");

            manager.Subscribe(Alt | KeyModifiers.LShift, Keys.D8,
                () => {}, "switch focused window to workspace 8");

            manager.Subscribe(Alt | KeyModifiers.LShift, Keys.D9,
                () => {}, "switch focused window to workspace 9");

            manager.Subscribe(Alt | KeyModifiers.LShift, Keys.Oem2,
                () => {}, "toggle keybind window");

What this tells me is that the IKeybindManager is cleared with UnsubscribeAll() but potentially not the real KeybindManager which has the real defaults. I wonder if the actual KeybindManager's keybindings are being executed and the error is due to a bad context.

@ghost
Copy link
Author

ghost commented May 15, 2023

I am not a native C# programmer, but I've been able to hack my way through setting up my config. If I can get my VSCode set up for C#, I may be able to debug this. I'll try it, when I have time.

If there are any devs who see this issue, I'd appreciate any input you can provide as to the intention of the IKeyBindManager versus the KeyBindManager. Why would keybinds fall through to the defaults when the user explicitly tries to unsubscribe from them?

@josteink
Copy link
Member

josteink commented Jun 1, 2023

I'd appreciate any input you can provide as to the intention of the IKeyBindManager versus the KeyBindManager

This is just a common construct to separate contract from implementation. It helps decouple projects and it makes code more testable in general. In your case, you should be able to focus on KeyBindManager only, since that is the important bit here (the implementation).

As for the actual issue, I honestly have no idea what's going on here. Definitely sounds like a bug.

The good news is that workspacer is fairly easy to just build and run locally, so you may be able to just load the source-code in Visual Studio, run it, and then reproduce in debug mode and just take it from there,

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

No branches or pull requests

1 participant