-
-
Notifications
You must be signed in to change notification settings - Fork 93
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
Add AbstractCliBasedDriver::launchProcess() #112
Conversation
This protected method is responsible for running the process responsible for sending the notification. It will allow any developer that wants to override how the process in launched
Hello @shadoWalker89. Thanks for your PR. I would make the windows drivers already override your new method, so out of the box, there are no longer blocking. Could you make the change, fix CS and add a note in the changelog (to specify windows drivers are no longer blocking) ? On a side note, when I talked about not changing the public interface, I was referring to the main NotifierInterface which is almost the only part of the "public api" of the library. I didn't want to add a new method in it. But almost every other classes and interfaces are internal and can be updated if needed 🙂 |
Hi, Thank you for your reply
As launching the process using the
Yup, i understood this, that is why in this PR i used the protected method to avoid breaking other people code by adding a new method to the interface |
That's the whole point. As this code is considered internal, I expect it to work by default without users having to modify it. To clarify: Windows notifier are blocking? Well, we don't need/want it, so let's bypass the exit code check, as the program running JoliNotif should not be paused for a single notification display 🙂 |
I only updated the notifu and SnoreToast and excluded the WslNotifySendDriver because i found that WslNotifySendDriver is not blocking even when using the run() method |
Thank you for your work @shadoWalker89 |
This PR is related to #111
This PR is made to allow displaying a notification in a non blocking way.
The program will display the notification and continue executing without waiting for the notification to close.
This PR adds a new protected method
launchProcess()
with a purpose of running / starting the process responsible for sending the notification.Since this is a protected method, it will not change the public interface and will just be used as a hook for developers that want to alter the behavior of how the process is launched by overriding it and use
$process->start()
instead of$process->run()