emersion / mako

A lightweight Wayland notification daemon
https://wayland.emersion.fr/mako
MIT License
2.25k stars 139 forks source link

Support for running abitrary scripts on notification #38

Closed adrubesh closed 3 years ago

adrubesh commented 6 years ago

Taken from my previous issue on playing a notification chime. Perhaps the same rules for #3 could be used to run user-defined scripts so that they could be notification/urgency/etc specific?

emersion commented 6 years ago

Could be something like:

[urgency=critical]
exec=mpv ~/critical.ogg

Where exec would be executed with /bin/sh.

vilhalmer commented 6 years ago

There is an explicit "sound" capability in the spec which specifies hints that must be supported, perhaps we should support this as well and then offer some way to expand the content of hints in a format to pass as arguments to the exec'd tool?

emersion commented 6 years ago

If we add the "sound" capability, it means we must support the sound-file hint (and sound-name). So we could have something like:

# %S (or something else) expands to sound-file
exec-sound=mpv %S

[urgency=critical]
default-sound=~/critical.ogg
vilhalmer commented 6 years ago

I was picturing something more like:

[urgency=critical]
exec=mpv %H(sound-file)

to keep it from being special, but I can see it would be tricky to figure out what to do if the app passed sound-name vs sound-file.

emersion commented 6 years ago

Indeed that's a much better idea. And we could just scan formats for %H(sound-file) to decide whether we should expose the "sound" capability or not.

IMHO we should always expand sound-name to get a full filepath, not sure how to do that though.

vilhalmer commented 6 years ago

The sound naming spec doesn't say anything about resolving them to paths, on Arch at least they appear to live in /usr/share/sounds/freedesktop. I'm not sure if it's worth supporting sound-name or not, the notification spec only mentions sound-file and suppress-sound as required:

The server supports sounds on notifications. If returned, the server must support the "sound-file" and "suppress-sound" hints.

emersion commented 6 years ago

Yeah. We could just decide not to support it, check if any apps in the wild use it, and consider adding support if we find any.

vilhalmer commented 6 years ago

Ah, I found it in a related spec: http://0pointer.de/public/sound-theme-spec.html#directory_layout

Edit: this seems to be the reference, and only, implementation: http://0pointer.de/lennart/projects/libcanberra/

alecmev commented 5 years ago

I'd like to mention a usecase: timers / alarms. It would be nice to be able to play a looping sound, until the notification expires or gets dismissed.

This could be accomplished with exec, when it's implemented, given that mako kills the process when the notification is closed (maybe it could be exec-ephemeral instead of exec, if you'd like to fork by default), but that isn't perfect, since the sound to play needs to be defined on mako's side.

A hint could probably be added, allowing to pass a path of a script to execute, but that likely has negative security implications.

If you do go ahead with implementing the sound capability, then, please, consider adding a custom hint for looping, e.g. x-mako-sound-loop. The spec appears to be okay with that:

Third parties, when defining their own hints, should discuss the possibility of standardizing on the hint with other parties, preferably in a place such as the xdg mailing list at freedesktop.org. If it warrants a standard, it will be added to the table above. If no consensus is reached, the hint name should be in the form of "x-vendor-name."

I can see there being a need for a couple more hints, like sound-loop-limit and sound-loop-padding, but that's quality-of-life stuff. Looping, however, isn't possible at all right now, sans using some library instead of notify-send and subscribing to NotificationClosed.

The sound-specific discussion should probably be moved to the original issue a new issue though.

vilhalmer commented 5 years ago

Rather than forcing mako to keep track of processes, it might be better to add a way to exec a command on dismiss. Then you could use pkill or whatever other fancier method you wanted to stop the alarm. I don't think custom hints are a good solution, I'd rather have the exec'd program handle that.

alecmev commented 5 years ago

That works too :slightly_smiling_face: But specifics of exec wasn't the point, I was talking about the inability to configure the sound to play on the side of the notification sender, if you want looping. But, regardless, this is not a big deal, just something that I went looking for recently, and didn't find. I'm guessing if you're serious enough about looping, you can ditch shell scripting and subscribe to some events.

RyanDwyer commented 5 years ago

Another use case: In sway, being able to manually set a window as urgent. For example:

[app-name="geary"]
exec=swaymsg [app_id="geary"] urgent enable

In some cases the user might want to only run their command if the notification title or body contains certain phrases. To allow this you could do any of the following:

  1. Add title and body to the criteria fields and implement regex matching in criteria.
  2. Set environment variables containing all the notification fields before executing the command. The user could then set the command to a shell script and do further checks there.
  3. Allow the user to set placeholders, such as exec=somescript {app_name} {title} {body}. This is similar to the option 2, but is probably a worse option because you have to handle escaping.
emersion commented 5 years ago

We probably want (1) anyway to allow changing style options. I also agree (2) would be useful (and (3) is probably not a good idea).

vilhalmer commented 5 years ago

Option 1 is definitely on the todo list, I always intended for those to be available to match. Option 3 certainly does add some complexity, but we have previously discussed needing to do something similar for sound support so you can pass the name of the sound to your player of choice. We could probably use the environment variable approach there as well, though that would then require a one-line wrapper script to actually call the player which is a bit awkward.

Perhaps we expose things as environment variables, but then exec using /bin/sh to allow them to be expanded without any extra effort on our part?

emersion commented 5 years ago

Perhaps we expose things as environment variables, but then exec using /bin/sh to allow them to be expanded without any extra effort on our part?

Oh yeah for sure. It's pretty handy to be able to use redirections, pipes and other sh features in the config.

ghost commented 5 years ago

This is a really nice bike shed. Scripts is how people did things in the 80s. Nowadays we have msgpack, cbor, capnp, protobuf, etc, even things like json for those of us who like to throw our money in the garbage by constantly parsing text to try to make code from it. Mpv has a JSON socket. So if you're interested in better designs, take a look at that instead of bike shedding obsolete ideas. Heck, sway also has a json socket!

May the shell burn in hell forever!

emersion commented 5 years ago

Sorry, but this is not really constructive. Please keep this kind of comment for yourself next time. For the record, we already have IPC via D-Bus and we could add a signal for new notifications.

vilhalmer commented 5 years ago

I've been thinking about this again today and have come up with a slight complication: if we use this for sound support and also for arbitrary other things, it's going to be very difficult to override it in criteria in a useful way. It seems that in many cases you would want both the global (for sound, probably) and any criteria execs to be run. The way things work now, only the winning criteria's command will be run.

Do we want to figure out a new configuration mechanic to allow stacking these instead of overriding? Or do we just say that all decision making must be delegated to an external script? I don't really like the second option, because it makes the configuration file no longer the source of truth and forces a bunch of complexity onto the user when writing the script.

emersion commented 5 years ago

Maybe we can handle this in the function that merges configurations?

vilhalmer commented 5 years ago

Are you thinking just always implicitly stack them together? We definitely could, though it would behave differently than all the other options. I also wonder what the best way to allow actual overriding would be then, for example if you wanted to disable sounds for a particular app.

emersion commented 5 years ago

It's true that the = sign makes it awkward, because it really feels like it sets something.

I see the config file as being similar to sway's, where it's clear that two consecutive exec commands don't override each other (whereas other commands, like mouse settings, do override each other).

Grissess commented 4 years ago

You're not wrong on the awkwardness, but the systemd-exec unit files do the same thing; you can specify Exec*=... as many times as you want (within the section), and it will not only do all of them, but it will do them in order. Stacking instead of overriding might seem like a bit of a "gotcha", but it's not without precedent.

Glad to see some progress in the last 4 hours, by the way; this last issue has been the only one keeping me running dunst on sway--which doesn't work all too well, by the way, but I depend on that "beep" for getting my attention :)

emersion commented 3 years ago

Another use-case is described here: https://github.com/emersion/mako/issues/283#issue-646646085

tl;dr

on-button-left=exec makoctl menu -n $id wofi -d -p "Select Action..."
ian-s-mcb commented 3 years ago

Message from the future:

After installing the latest version of mako, the systemd service needs to be restarted (not just a makoctl reload), otherwise you'll get the following error when using the on-notify=exec ...: Call failed: Unable to parse configuration file

This command will do the trick: systemctl --user daemon-reload && systemctl --user restart mako