Merge code for desktop notification between MacOS and Linux #438
Conversation
|
Yes, I have been curious about why topgrade uses notify-send binary to send notifications on Linux:( |
It seems that all these platforms can use I just took a look at the
It would be great if we can support those BSDs and Windows through |
| #[cfg(target_os = "linux")] | ||
| { | ||
| if config.display_preamble() && terminal::supports_notify_send() && !config.skip_notify() { | ||
| print_warning("Due to a design issue with notify-send it could be that topgrade hangs when it's finished. | ||
| If this is the case on your system add the --skip-notify flag to the topgrade command or set skip_notify = true in the config file. | ||
| If you don't want this message to appear any longer set display_preamble = false in the config file. | ||
| For more information about this issue see https://askubuntu.com/questions/110969/notify-send-ignores-timeout and https://github.com/topgrade-rs/topgrade/issues/288."); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
We can get rid of that hang issue, so great
|
Great if notify-rust support all OSes, there is no need to use conditional compilation. |
|
And before merging the PR, would u like to squash your commits? So that the commits history looks clean and self-explanatory:) |
|
@SteveLauC no worries, I always squash and merge ;) |
Linux now use the crate notify-rust. BSD could use that crate too.
For Windows we could use winrt-notification.
Standards checklist:
cargo build)cargo fmt)cargo clippy)cargo test)If you developed a feature or a bug fix for someone else and you do not have the
means to test it, please tag this person here.