Skip to content
New issue

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

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

Already on GitHub? # to your account

Use Windows API to spawn processes instead of powershell #1269

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

Dethada
Copy link
Contributor

@Dethada Dethada commented Feb 9, 2025

  • Switched to use win api for both process creation and termination for all cases except spawning of whkd, as it will spawn a visible console window if we use win api for its process creation. I suspect this can be solved on whkd side but I'll leave it as is for now
  • Addresses [BUG]: komorebic stop --ahk doesn't stop autohotkey because of trailing space in command line #1174 now that we don't have powershell adding trailing space to the commandline
  • Startup time has a very noticeable improvement, on my machine it dropped from about 2s to < 1s
  • Modified error printing to print to stderr

whkd still remains to be spawned with powershell as switch to spawn directly will cause a console
window to pop up
The previous commits handles the trailing space issue, this commit concludes the fix by handling the
the termination of AHK processes with winapi. Note the added check for komorebic.ahk without the
quote, as this is new behavior that can happen when the provided path has no spaces, introduced
after swapping over to winapi

fix LGUG2Z#1174
@Dethada
Copy link
Contributor Author

Dethada commented Feb 9, 2025

@LGUG2Z please test if masir launches and closes without issue, I don't have it installed so I didn't test that

@Dethada
Copy link
Contributor Author

Dethada commented Feb 10, 2025

If this PR is merged into whkd, we can swap whkd spawning to winapi as well, I have tested that it works however one problem is that with that change the startup seems to be too fast. I think komorebi-bar is crashing on startup because komorebi is not ready yet. I have to implement a solution for that.

The other problem I noticed is suppose I have config setup for my komorebi-bar then each time I run komorebic start --bar then n more instances of komorebi-bar will be started, where n is the number of bars setup in my config.

Let's hold off on merging this until I have these problems fixed.

@Dethada
Copy link
Contributor Author

Dethada commented Feb 10, 2025

I have addressed the start up crash problem in the bar. The bar will now attempt to connect to komorebi at a 1s interval instead of crashing upon failure to connect.

Regarding this I will handle it in a separate PR after this PR is merged

The other problem I noticed is suppose I have config setup for my komorebi-bar then each time I run komorebic start --bar then n more instances of komorebi-bar will be started, where n is the number of bars setup in my config.

This PR is ready for review now, but depends on whkd pr, else a console window will appear when whkd is started with komorebic

@LGUG2Z
Copy link
Owner

LGUG2Z commented Feb 15, 2025

I'm going to park this PR until the 0.1.35 dev cycle

@LGUG2Z LGUG2Z force-pushed the master branch 4 times, most recently from efeefb7 to 974e5a2 Compare February 23, 2025 20:16
@LGUG2Z LGUG2Z force-pushed the master branch 8 times, most recently from 6926478 to 75a0506 Compare March 6, 2025 23:34
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants