Skip to content
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

Always send a transient notification with notify-send #118

Merged
merged 1 commit into from
Oct 3, 2021
Merged

Conversation

frazar
Copy link
Contributor

@frazar frazar commented Sep 30, 2021

Fixes #117

conf.d/done.fish Outdated Show resolved Hide resolved
conf.d/done.fish Outdated Show resolved Hide resolved
@franciscolourenco
Copy link
Owner

franciscolourenco commented Oct 1, 2021

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
Copy link
Contributor

yajo commented Oct 1, 2021

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
Copy link
Owner

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

@frazar frazar changed the title Check if __done_notification_transient is defined before using it Always send a transient notification Oct 1, 2021
@frazar frazar changed the title Always send a transient notification Always send a transient notification with notify-send Oct 1, 2021
@frazar
Copy link
Contributor Author

frazar commented Oct 1, 2021

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

Sure! Should be ok now

@ammgws
Copy link
Contributor

ammgws commented Oct 1, 2021

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

@franciscolourenco
Copy link
Owner

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
Copy link
Contributor

ammgws commented Oct 1, 2021

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

@franciscolourenco franciscolourenco merged commit c47d267 into franciscolourenco:master Oct 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Variable $__done_notification_transient is used but is not defined
4 participants