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

feat(refactor): refactor to allow generic usage of komokana #65

Merged
merged 4 commits into from
Jan 1, 2024

Conversation

psych3r
Copy link

@psych3r psych3r commented Dec 17, 2023

Resolves #64
Here's a summary of each module and what it does.

configuration:

  • Didn't change
  • Facilitates parsing the config file

kanata:

  • Responsible for managing kanata's tcpstream object
  • Responsible for connecting to kanata and starting kanata's listening loop

events:

  • Responsible for handling Show & FocusChange events

main:

  • Declares Provider trait
  • Parses configuration file
  • Creates shared (static) Kanata struct, connects to kanata & starts kanata loop
  • Creates struct Komokana that implements Provider & starts the Provider loop

windows:

  • Komorebi provider
  • Connects to komorebi and starts komorebi's listener loop

osx:

  • Placeholder for macOS provider

linux:

  • Placeholder for Linux provider

@psych3r
Copy link
Author

psych3r commented Dec 17, 2023

I thought it's better to introduce the macOS provider in a separate PR.
Also, I did minimal testing as I don't use komorebi.
My testing is focused on connecting to kanata (and reconnecting if needed).
It would be awesome if you tested the change with your configuration just to ensure that everything is working fine.

@LGUG2Z
Copy link
Owner

LGUG2Z commented Dec 17, 2023

Really appreciate your initiative and work on this! I'm travelling without a computer until the end of next week, once I'm back home I'll build and run this PR for a few days 🤞

@psych3r
Copy link
Author

psych3r commented Dec 21, 2023

Currently the event module relies on the Windows specific function GetKeyState due to virtual_key_overrides and virtual_key_ignores options.
I also think that the EVENT::Show variant is komorebi specific, and probably the title_overrides configuration option too.

I'm not sure whats the best way to go about this, but here are some options:

  1. remove the event module and make event handling part of the provider.
  2. have a feature for virtual_key that requires providers to implement a getKeyState function. And another feature for title_overrides.
    Use cfg feature conditionals where needed in the event and possibly the configuration module.

I am not a huge fan of the both option as I really like the idea of having a separate event module, and I don't think the features solution is particularly elegant.
What do you think?

Also, I think the exe option would benefit from supporting a matching strategy just like title.
Do you mind adding that?

@psych3r
Copy link
Author

psych3r commented Dec 21, 2023

I put virtual_key_overrides and virtual_key_ignores behind a feature virtual_keys.
It requires providers to implement a get_key_state function.
As for title_overrides, I just made the title sent to handle_event an Option<&str> for flexibility.
The virtual_keys feature is a Windows only feature for now.
Provider implementations for other platforms get to choose whether to support the feature or not.

@LGUG2Z LGUG2Z merged commit 0729975 into LGUG2Z:master Jan 1, 2024
1 check passed
@LGUG2Z
Copy link
Owner

LGUG2Z commented Mar 1, 2024

Hey @psych3r I'm going to have to back this PR out of master, at least temporarily; I have created a backup of the trunk with the changes integrated on https://github.com/LGUG2Z/komokana/tree/feature/refactor

Komokana is currently broken with komorebi due to named pipe and message size issues, so I need to switch over to a new UDS-based subscription implementation, however when doing this what the refactor currently on master this ends up deadlocking and I'm not able to understand where the issue is right now. I"m going to revert this out to get everything working again on master and then we can revisit this refactor afterwards.

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.

[feat] cross platform, generic komokana
2 participants