-
-
Notifications
You must be signed in to change notification settings - Fork 17.7k
apprise: add support for macOS desktop notifications #478344
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
Conversation
bedc1ff to
43ea6b7
Compare
|
| patches = lib.optionals stdenv.hostPlatform.isDarwin [ | ||
| (replaceVars ./add-terminal-notifier-path.patch { | ||
| terminalNotifierPath = "${lib.getExe' terminal-notifier "terminal-notifier"}"; | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Substituting the first path is probably a better idea. Applying patch only on darwin risks being ignored during updates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking a look!
Substituting the first path is probably a better idea.during updates.
Good point, I now just insert the nix path at the first position of the list.
Applying patch only on darwin risks being ignored
The patch is now applied for all platforms. I wasn't sure whether applying this unconditionally would pull in the dependency also under Linux (where terminal-notifier is not available).
43ea6b7 to
2ad4bff
Compare
| (replaceVars ./add-terminal-notifier-path.patch { | ||
| terminalNotifierPath = "${lib.getExe' terminal-notifier "terminal-notifier"}"; | ||
| }) | ||
| ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ]; | |
| postPatch = lib.optionalString stdenv.hostPlatform.isDarwin '' | |
| substituteInPlace apprise/plugins/macosx.py \ | |
| --replace-fail "/opt/homebrew/bin/terminal-notifier" "${lib.getExe' terminal-notifier "terminal-notifier"}" | |
| ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, sorry for the misunderstanding. I'm now using the substitute instead of the patch.
4d39176 to
a6d5eee
Compare
| ] | ||
| ++ lib.optionals stdenv.hostPlatform.isDarwin [ | ||
| terminal-notifier |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ] | |
| ++ lib.optionals stdenv.hostPlatform.isDarwin [ | |
| terminal-notifier |
It's not needed as you have set absolute path. And this "dependencies" corresponds to pyproject's, which is only related to python modules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, thanks for the tip! :)
Apprise supports desktop notifications on macOS (e.g., via `apprise -b "Hello World" "macosx://"`). This feature uses terminal-notifier under the hood. Unfortunately, the potential paths to terminal-notifier are hardcoded into apprise. Thus, nixpkgs's apprise does currently not support desktop notifications on macOS. This PR adds the terminal-notifier dependency under macOS and patches the hardcoded paths to use nixpkgs's terminal-notifier.
a6d5eee to
3e99122
Compare
|
Apprise supports desktop notifications on macOS (e.g., via
apprise -b "Hello World" "macosx://"). This feature uses terminal-notifier under the hood. Unfortunately, the potential paths to terminal-notifier are hardcoded into apprise. Thus, nixpkgs's apprise does currently not support desktop notifications on macOS.This PR adds the terminal-notifier dependency under macOS and patches the hardcoded paths to use nixpkgs's terminal-notifier.
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.