search

franciscolourenco / done

A fish-shell package to automatically receive notifications when long processes finish.
MIT License
766 stars 70 forks source link

Support transient notifications #102

Closed yajo closed 2 years ago

yajo commented 3 years ago

With this patch, if the user does set -U __done_notification_transient 1, these will disappear automatically on Linux systems.

See https://gist.github.com/carsondarling/6217755 for details.

franciscolourenco commented 3 years ago

@Yajo I'm thinking transient notifications should be the default, no?

ammgws commented 3 years ago

https://gitlab.freedesktop.org/xdg/xdg-specs/-/blob/master/notification/notification-spec.xml#L780

When set the server will treat the notification as transient and by-pass the server's persistence capability, if it should exist.

Does this mean this would override a user's timeout/etc settings in their notification daemon config?

Hints are a way to provide extra data to a notification server that the server may be able to make use of. Neither clients nor notification servers are required to support any hints. Both sides should assume that hints are not passed, and should ignore any hints they do not understand. The following table lists the standard hints as defined by this specification. Future hints may be proposed and added to this list over time. Once again, implementations are not required to support these.

What notification daemons currently implement this? A quick test with and grep of mako seemed to suggest it wasn't implemented there. Dunst?

Not sure about how other notification daemons are configured, but couldn't a user just set all notifications from done to have the specific timeout they wish in their notification daemon config? IIRC it should be possible with both mako and dunst.

yajo commented 3 years ago

transient notifications should be the default, no?

I think so, I was just trying to do the least aggressive PR possible.

Does this mean this would override a user's timeout/etc settings in their notification daemon config?

I think that means that the notification disappears. At least, that's what happens for me.

Over time, opening the notifications panel and seeing it full of 1 notification for each command that lasted more than 5 seconds is annoying. These bury the important notifications, or force me to click on them to avoid that accumulation, while I just want to know the command finished, with nothing more.

What notification daemons currently implement this?

On GNOME 3.38.4 this works. And some older GNOME versions too.

implementations are not required to support these

So we could make this not optional, and just pass the hint always, so if the notification manager understands it, the notification will disappear; and if it doesn't... well, it'll be the same as until today. Does that sound reasonable?

nzig commented 3 years ago

+1 for making this the default.

franciscolourenco commented 3 years ago

@Yajo are you positive that the hint will be ingored by implementations which don't support it? Thanks!

yajo commented 3 years ago

According to the spec:

Neither clients nor notification servers are required to support any hints. Both sides should assume that hints are not passed, and should ignore any hints they do not understand.

To my understanding, it should break nothing. But I have to admit I didn't test this on any other implementations.

yajo commented 3 years ago

So, anything else left to merge this?

franciscolourenco commented 3 years ago

@Yajo we would need a manual test at at least in another implementation

yajo commented 2 years ago

There you have:

KDE, running notify-send --hint=int:transient:1 --urgency=normal --icon=utilities-terminal --app-name=fish title message

notify

yajo commented 2 years ago

So, could we merge?

yajo commented 2 years ago

Hello, is there anything left here to merge?

franciscolourenco commented 2 years ago

Hi @Yajo sorry for the delay and thank you for pinging me on this. Let's merge! 🙏

khaveesh commented 2 years ago

mako supports the --expire-time flag of notify-send. So, my suggestion would be to send both this and the current transient hint to cover all possible implementations.

franciscolourenco commented 2 years ago

@khaveesh any chance providing options which are not supported might result in an error?

khaveesh commented 2 years ago

From notify-send manpage:

       -t, --expire-time=TIME
           The duration, in milliseconds, for the notification to appear on screen.

           (Ubuntu's Notify OSD and GNOME Shell both ignore this parameter.)

So, it shouldn't result in an error