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

Program hangs when notification is visible #111

Closed
shadoWalker89 opened this issue Sep 10, 2024 · 3 comments
Closed

Program hangs when notification is visible #111

shadoWalker89 opened this issue Sep 10, 2024 · 3 comments

Comments

@shadoWalker89
Copy link
Contributor

Hi,

I noticed that when the notification is visible the program hangs.
I also found this issue #88
For my use case, i'm building a cli for internal use within our company, the developer will start a task and go do something else, and if something happens i want to notify him.

For our use case i don't really need the assertion if the notification was displayed or not, i just need to display a notification and let the program continue it's job, the interruption caused by the notification is not good.

I have two propositions

1- Allow to send a notification asynchronously by adding a new method called sendAsync(), this will not be a breaking change because this is a new method. Any one using this will know that he will not be getting an assertion if the notification was displayed or not.
I really hope you consider this option, if so i'm willing to send a PR

2- Extract a new method in the CliBasedNotifier called dispatch (the name could be something else)
The only job of this method is to run/start the process, this way you will allow the developer to override this method on his custom notifier that will extend the CliBasedNotifier and override this method to his liking, in my case i will be using the start() method

class CliBasedNotifier
{

    public function send(Notification $notification): bool
    {
        $this->dispatch($proccess);
    }

    protected function dispatch(Process $process)
    {
        $process->run();
    }
}

In both cases i'm willing to make a PR but i really like the first proposition, because it's built in into the package, and does really need any overrides.

@pyrech
Copy link
Member

pyrech commented Sep 11, 2024

hi @shadoWalker89

Thanks for your suggestions.

Could you run the script example/index.php and share the output to confirm which driver is used on your system.

Unless I'm wrong, this blocking behaviour was only seen on one of the driver used on Windows, aka SnoreToast. This is due to how this software works as it probably allows to receive a user response direcly from an input inside the notification. But this feature is not supported on JoliNotif.

While I agree this behaviour is quite annoying, I don't want to change the whole public interface of this library. as it works well for all other platform/driver. Instead, here are the solutions I can think of right now, in order of preference:

  • see if the software provides an option to return early
  • change how this specific driver works and make a $process->run() instead of a start() (like you suggested) without impacting too much code
  • try to find (or build) another tiny notification software to replace SnoreToast

@shadoWalker89
Copy link
Contributor Author

This is the output

Notification successfully sent with Joli\JoliNotif\Notifier\SnoreToastNotifier

I already checked the source code before opening this issue and i saw for windows there is 2 notifiers, notifu is for windows 7, i tried notifu and it's the same behavior it hangs

see if the software provides an option to return early
I checked snore toast options and i don't see any option that allow to return early

I went ahead and made a PR according to my second suggestion
https://github.com/jolicode/JoliNotif/pull/112/files

@pyrech
Copy link
Member

pyrech commented Sep 30, 2024

Fixed in #112 and released in v2.7.3. Thanks @shadoWalker89 💛

@pyrech pyrech closed this as completed Sep 30, 2024
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

2 participants