Skip to content

Added support for the Mac and Windows operating systems!!!#4

Open
philocalyst wants to merge 3 commits into
7mind:persistent-daemonfrom
philocalyst:but-mac-and-win
Open

Added support for the Mac and Windows operating systems!!!#4
philocalyst wants to merge 3 commits into
7mind:persistent-daemonfrom
philocalyst:but-mac-and-win

Conversation

@philocalyst

@philocalyst philocalyst commented Apr 11, 2026

Copy link
Copy Markdown

So happy to get this done! Matches the original code from kanata-vk-agent for mac, and window_tools for the windows backend. Includes tests and qa. Very much slop, but functioning slop!

@neko-kai neko-kai left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@philocalyst Ugh, given -6000 lines in main.rs, I take it the entire linux support was removed? 😨 The diff would have to be reduced substantially for review. Also, I haven't read through the later parts of the PR - so I'll just ask - have you managed to get integration tests running for either windows/mac? As in, using the real OS WM & message bus? For Linux that was a setup with a headless X11 & wayland and a private transient dbus

Comment thread qa/macos-keybinds.md Outdated
@@ -0,0 +1,47 @@
## macOS Backend (NSWorkspace polling)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@philocalyst All the backends should be async push-based - no polling. Daemon wakes up on events from the windowing system.

Comment thread qa/windows-keybinds.md Outdated
@@ -0,0 +1,51 @@
## Windows Backend (Win32 GetForegroundWindow polling)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

@philocalyst

philocalyst commented Apr 12, 2026 via email

Copy link
Copy Markdown
Author

Comment thread qa/macos-keybinds.md Outdated
- [ ] `class` field matches macOS bundle IDs (e.g., `com.apple.Safari`)
- [ ] Case-insensitive matching works (e.g., `com.apple.safari` matches Safari)
- [ ] Partial regex matches work (e.g., `com.apple` matches all Apple apps)
- [ ] Non-matching apps leave the current layer unchanged

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@philocalyst Not correct: non-matching apps should switch back to default layer, as with linux & windows below. Ideally that should have been caught by unit tests - perhaps the matching logic was duplicated instead of being shared between modules - and tested?

Comment thread src/daemon/macos.rs Outdated
@@ -0,0 +1,159 @@
//! macOS backend for kanata-switcher.
//!
//! Uses `NSWorkspaceDidActivateApplicationNotification` to receive push-based

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍️

Comment thread src/daemon/windows.rs Outdated
@@ -0,0 +1,257 @@
//! Windows backend for kanata-switcher.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@philocalyst Both windows and macos implementations are quite small. I wonder if anything could be done to reduce the diff from its current +8000 -6000, not very reviewable, state.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Frankly this is a problem of the codebase's single-file attitude, if we had moved to a trait system with file scoping the diff would literally be sub 500

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@philocalyst Fair enough! One way to deal with that would be if you could create a separate PR that just performs the refactoring to traits without changing logic - I could review that with difft and the IDE despite the high line count. Then after that is merged this PR would have a workable sub-500 diff.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You know what! I will do that!! You win lol

You'll have to test though, unless you could setup PR CI?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@philocalyst CI should work now as long as the branch includes latest commit 72ff404

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@philocalyst Hey, I've attempted to perform that refactor in #5 – I wonder whether the structure in that PR is close to what you had in mind

@neko-kai neko-kai May 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@philocalyst The main refactor to traits + test split has been merged into persistent-daemon branch. (The tests have been split by desktop backend, but not by platform - but windows/mac only have one focus backend each, so perhaps they could be regarded as backends first for that purpose)

@philocalyst

Copy link
Copy Markdown
Author

@neko-kai Now good I think!?!!

@neko-kai

Copy link
Copy Markdown
Member

@philocalyst Much better, but... there are no new integration tests. Are there are any in kanata-vk-agent or window_tools? I suspect some amount of automated testing is possible for macOS and Windows too

@philocalyst

Copy link
Copy Markdown
Author

Added the integration tests! @neko-kai

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.

2 participants