macos-notifications icon indicating copy to clipboard operation
macos-notifications copied to clipboard

Skip multiprocessing if no callback actions requested; Add sound param; Fix signal handling

Open michelcrypt4d4mus opened this issue 1 year ago • 2 comments

Changes:

  1. If there are no callbacks requested then there is no need for multiprocessing / listening / etc.
  2. Fix the signal handling - the argument to sys.exit() is just the return code for the shell; it does not re-raise the SIGINT to call sys.exit(signal.SIGINT). Now it properly re-raises the trapped signal.
  3. Add sound parameter as per the other PR https://github.com/Jorricks/macos-notifications/pull/25

In order to implement (1) I had to move the MacOSNotification class outside of the create_notification() method so it would not be redeclared every time create_notification() is called - see my comment.

Re: (2) I'm still not really convinced that trapping SIGINT in a package is the best idea and am kind of concerned about other unforeseen side effects but at least re-raising it seems to work as the user might expect.

Feel free to cherry pick and/or request changes.

michelcrypt4d4mus avatar May 18 '24 18:05 michelcrypt4d4mus

It turns out that because the MacOSNotification class isn't getting redefined with every call to create_notification() any more forking the initial NotificationProcess was unnecessary - you only need multithreading to listen for responses, not trigger the initial notification - so i removed NotificationProcess altogether on this PR.

michelcrypt4d4mus avatar May 19 '24 02:05 michelcrypt4d4mus

First of all, sorry for being so slow to respond. Second, I went over the MR and it looks very nice and ready to merge. Please rebase it/fix the conflicts and we can get it in.

Jorricks avatar Jun 16 '24 10:06 Jorricks