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

Add application and expire time to notify-send #61

Merged
merged 3 commits into from Mar 26, 2020

Conversation

@genofire
Copy link
Contributor

genofire commented May 12, 2019

It is better then notify-send as application name.

@marzocchi marzocchi closed this Feb 17, 2020
@genofire

This comment has been minimized.

Copy link
Contributor Author

genofire commented Feb 18, 2020

Why was this PR closed (without any comment) ?

I really like to see a Application in the notification, for better filtering.
(also a configurable expire-time -t)

@marzocchi

This comment has been minimized.

Copy link
Owner

marzocchi commented Feb 18, 2020

hello! I closed because I lost of track of it and it fell out of date with master, feel free to rebase and also add support for -t, thanks!

@marzocchi marzocchi reopened this Feb 18, 2020
@marzocchi marzocchi closed this Feb 24, 2020
@genofire

This comment has been minimized.

Copy link
Contributor Author

genofire commented Feb 24, 2020

hey, let me some time to manage it - you need 6 month to take a look into this PR ....

@marzocchi marzocchi reopened this Feb 25, 2020
Signed-off-by: genofire <geno+dev@fireorbit.de>
@genofire genofire force-pushed the genofire:patch-1 branch from c8f73fe to 63384ae Mar 22, 2020
@genofire

This comment has been minimized.

Copy link
Contributor Author

genofire commented Mar 22, 2020

Thanks for waiting - what do you think about this version?

Copy link
Owner

marzocchi left a comment

thanks! it's fine, just left a couple of suggestions for improvements

xdotool/functions Outdated Show resolved Hide resolved
xdotool/functions Outdated Show resolved Hide resolved
@genofire

This comment has been minimized.

Copy link
Contributor Author

genofire commented Mar 22, 2020

I improve app-name by using the first word of notification message.


title=$(notification-title "$type" time_elapsed "$time_elapsed")

if [[ -n "$icon" ]]; then
icon_option="-i $icon"
fi

notify-send ${=icon_option} "$title" "$message"
if [[ -n "$app_name" ]]; then

This comment has been minimized.

Copy link
@marzocchi

marzocchi Mar 24, 2020

Owner

I think we can go all-in and just make it so there's always an -a option for notify-send, either the first word of the command or a pre-set one.

Suggested change
if [[ -n "$app_name" ]]; then
if [[ -z "$app_name" ]]; then
app_name_option="-a ${message%% *}"
message="${message#* }"
else
app_name_option="-a $app_name"
fi
@@ -124,7 +133,7 @@ function store-window-id() {
# the terminal used to type the command, as this function is run as a prexec
# hook.
if [[ -z "$WINDOWID" || "$always_check" == 'yes' ]]; then
zstyle ':notify:*' window-id "$(xdotool getwindowfocus)"
# zstyle ':notify:*' window-id "$(xdotool getwindowfocus)"

This comment has been minimized.

Copy link
@marzocchi

marzocchi Mar 25, 2020

Owner

i think we need this, no?

This comment has been minimized.

Copy link
@genofire

genofire Mar 26, 2020

Author Contributor

Oh sorry - i only need this in wayland enviroments (like gnome, sway, ...).
That should not be here in this PR.


title=$(notification-title "$type" time_elapsed "$time_elapsed")

if [[ -n "$icon" ]]; then
icon_option="-i $icon"
fi

notify-send ${=icon_option} "$title" "$message"
if [[ "$app_name" -eq "AUTO" ]]; then

This comment has been minimized.

Copy link
@marzocchi

marzocchi Mar 25, 2020

Owner

i don't think we need a special value to have an app_name: if you set the default value as an empty string in notify.plugin.zsh (as for example success-sound), then here you can check if app_name is an empty string, and act accordingly.

@genofire genofire force-pushed the genofire:patch-1 branch 3 times, most recently from 859a3c4 to 8c621c9 Mar 26, 2020
@genofire genofire changed the title Add application to notify-send Add application and expire time to notify-send Mar 26, 2020
@genofire genofire force-pushed the genofire:patch-1 branch from 8c621c9 to 4734250 Mar 26, 2020
@marzocchi marzocchi merged commit 8a4abe7 into marzocchi:master Mar 26, 2020
@marzocchi

This comment has been minimized.

Copy link
Owner

marzocchi commented Mar 26, 2020

thanks!

@genofire genofire deleted the genofire:patch-1 branch Mar 26, 2020
@genofire

This comment has been minimized.

Copy link
Contributor Author

genofire commented Mar 26, 2020

you are welcome - i do not know how to solve wayland issue yet ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.