search

franciscolourenco / done

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

Always send a transient notification with notify-send #118

Closed frazar closed 2 years ago

frazar commented 2 years ago

Fixes #117

franciscolourenco commented 2 years ago

I'm sorry I merged the previous PR prematurely. __done_notification_transient is not even in the README and it is enabled by default. Do we even need this option?

yajo commented 2 years ago

IMHO not, but we just wanted to add a backwards compatibility layer in case this broke somebody's workflow. But you can remove it, yes.

franciscolourenco commented 2 years ago

@frazar would you like to remove this check completely in your PR? Thanks!

frazar commented 2 years ago

@frazar would you like to remove this check completely in your PR? Thanks!

Sure! Should be ok now

ammgws commented 2 years ago

Since the default behavior is changing, shouldn't it be mentioned in the docs?

franciscolourenco commented 2 years ago

Since the default behavior is changing, shouldn't it be mentioned in the docs?

I will add it at least to the release notes.

I wonder if it makes sense to put it in the readme, since transient notifications are the way it was always supposed to work. One concern is wether this option might prevent some users from configuring some notification from remaining in the screen. I would imagine some systems allow users to configure this or not at all?

ammgws commented 2 years ago

I might be misremembering. I just checked and have mine set to never expire in my mako config.

I think in general the config files for each notification daemon would take precedence so it should be OK