From b3108e1ad23387327d6cc7ebfdb20307878dd5c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Tue, 23 Jul 2024 11:53:30 +0200 Subject: [PATCH] notify: separate active notifications from unfinished kitty notifications This fixes an issue where it wasn't possible to trigger multiple notifications with the same kitty notification ID. This is something that works in kitty, and there's no reason why it shouldn't work. The issue was that we track stdout, and the notification helper's PID in the notification struct. Thus, when a notification is being displayed, we can't re-use the same notification struct instance for another notification. This patch fixes this by adding a new notification list, 'active_notifications'. Whenever we detect that we need to track the helper (notification want's to either focus the window on activation, or send an event to the application), we add a copy of the notification to the 'active' list. The notification can then be removed from the 'kitty' list, allowing kitty notifications to re-use the same ID over and over again, even if old notifications are still being displayed. --- notify.c | 36 +++++++++++++++++++++++++----------- notify.h | 2 +- osc.c | 25 +++++++++++++------------ terminal.c | 21 ++++++++++++++++----- terminal.h | 3 ++- 5 files changed, 57 insertions(+), 30 deletions(-) diff --git a/notify.c b/notify.c index 67cf5bd1..af89838b 100644 --- a/notify.c +++ b/notify.c @@ -75,7 +75,7 @@ fdm_notify_stdout(struct fdm *fdm, int fd, int events, void *data) /* Find notification */ - tll_foreach(term->notifications, it) { + tll_foreach(term->active_notifications, it) { if (it->item.stdout_fd == fd) { notif = &it->item; break; @@ -124,7 +124,7 @@ notif_done(struct reaper *reaper, pid_t pid, int status, void *data) { struct terminal *term = data; - tll_foreach(term->notifications, it) { + tll_foreach(term->active_notifications, it) { struct notification *notif = &it->item; if (notif->pid != pid) continue; @@ -148,17 +148,17 @@ notif_done(struct reaper *reaper, pid_t pid, int status, void *data) } notify_free(term, notif); - tll_remove(term->notifications, it); + tll_remove(term->active_notifications, it); return; } } bool -notify_notify(const struct terminal *term, struct notification *notif) +notify_notify(struct terminal *term, struct notification *notif) { xassert(notif->xdg_token == NULL); xassert(notif->pid == 0); - xassert(notif->stdout_fd == 0); + xassert(notif->stdout_fd <= 0); xassert(notif->stdout_data == NULL); notif->pid = -1; @@ -187,6 +187,8 @@ notify_notify(const struct terminal *term, struct notification *notif) } } + bool track_notification = notif->focus || notif->report; + LOG_DBG("notify: title=\"%s\", body=\"%s\"", title, body); xassert(title != NULL); @@ -230,13 +232,25 @@ notify_notify(const struct terminal *term, struct notification *notif) LOG_DBG(" argv[%zu] = \"%s\"", i, argv[i]); int stdout_fds[2] = {-1, -1}; - if ((notif->focus || notif->report) && - pipe2(stdout_fds, O_CLOEXEC | O_NONBLOCK) < 0) - { - LOG_WARN("failed to create stdout pipe"); - /* Non-fatal */ + if (track_notification) { + if (pipe2(stdout_fds, O_CLOEXEC | O_NONBLOCK) < 0) { + LOG_WARN("failed to create stdout pipe"); + track_notification = false; + /* Non-fatal */ + } else { + tll_push_back(term->active_notifications, *notif); + notif->id = NULL; + notif->title = NULL; + notif->body = NULL; + notif->icon_id = NULL; + notif->icon_symbolic_name= NULL; + notif->icon_data = NULL; + notif->icon_data_sz = 0; + notif = &tll_back(term->active_notifications); + } } + if (stdout_fds[0] >= 0) { xassert(notif->xdg_token == NULL); fdm_add(term->fdm, stdout_fds[0], EPOLLIN, @@ -247,7 +261,7 @@ notify_notify(const struct terminal *term, struct notification *notif) int devnull = open("/dev/null", O_RDONLY); pid_t pid = spawn( term->reaper, NULL, argv, devnull, stdout_fds[1], -1, - ¬if_done, (void *)term, NULL); + track_notification ? ¬if_done : NULL, (void *)term, NULL); if (stdout_fds[1] >= 0) { /* Close write-end of stdout pipe */ diff --git a/notify.h b/notify.h index 55ace52d..90fbf9fc 100644 --- a/notify.h +++ b/notify.h @@ -60,7 +60,7 @@ struct notification_icon { char *tmp_file_on_disk; }; -bool notify_notify(const struct terminal *term, struct notification *notif); +bool notify_notify(struct terminal *term, struct notification *notif); void notify_free(struct terminal *term, struct notification *notif); void notify_icon_add(struct terminal *term, const char *id, diff --git a/osc.c b/osc.c index 5a001b7a..41a0dc68 100644 --- a/osc.c +++ b/osc.c @@ -727,7 +727,7 @@ kitty_notification(struct terminal *term, char *string) /* Search for an existing (d=0) notification to update */ struct notification *notif = NULL; - tll_foreach(term->notifications, it) { + tll_foreach(term->kitty_notifications, it) { if (strcmp(it->item.id, id) == 0) { /* Found existing notification */ notif = &it->item; @@ -736,16 +736,17 @@ kitty_notification(struct terminal *term, char *string) } if (notif == NULL) { - tll_push_front(term->notifications, ((struct notification){ + tll_push_front(term->kitty_notifications, ((struct notification){ .id = id, .when = when, .urgency = urgency, .focus = focus, .report = report, + .stdout_fd = -1, })); id = NULL; /* Prevent double free */ - notif = &tll_front(term->notifications); + notif = &tll_front(term->kitty_notifications); } if (notif->pid > 0) { @@ -833,15 +834,15 @@ kitty_notification(struct terminal *term, char *string) * The checks for title|body is to handle notifications that * only load icon data into the icon cache */ - if ((notif->title != NULL || notif->body != NULL) && - !notify_notify(term, notif)) - { - tll_foreach(term->notifications, it) { - if (&it->item == notif) { - notify_free(term, &it->item); - tll_remove(term->notifications, it); - break; - } + if (notif->title != NULL || notif->body != NULL) { + notify_notify(term, notif); + } + + tll_foreach(term->kitty_notifications, it) { + if (&it->item == notif) { + notify_free(term, &it->item); + tll_remove(term->kitty_notifications, it); + break; } } } diff --git a/terminal.c b/terminal.c index eadb9dbc..dc4f37b6 100644 --- a/terminal.c +++ b/terminal.c @@ -1313,7 +1313,8 @@ term_init(const struct config *conf, struct fdm *fdm, struct reaper *reaper, #if defined(FOOT_IME_ENABLED) && FOOT_IME_ENABLED .ime_enabled = true, #endif - .notifications = tll_init(), + .kitty_notifications = tll_init(), + .active_notifications = tll_init(), }; pixman_region32_init(&term->render.last_overlay_clip); @@ -1818,9 +1819,14 @@ term_destroy(struct terminal *term) tll_remove(term->ptmx_paste_buffers, it); } - tll_foreach(term->notifications, it) { + tll_foreach(term->kitty_notifications, it) { notify_free(term, &it->item); - tll_remove(term->notifications, it); + tll_remove(term->kitty_notifications, it); + } + + tll_foreach(term->active_notifications, it) { + notify_free(term, &it->item); + tll_remove(term->active_notifications, it); } for (size_t i = 0; i < ALEN(term->notification_icons); i++) @@ -2031,9 +2037,14 @@ term_reset(struct terminal *term, bool hard) tll_remove(term->alt.sixel_images, it); } - tll_foreach(term->notifications, it) { + tll_foreach(term->kitty_notifications, it) { notify_free(term, &it->item); - tll_remove(term->notifications, it); + tll_remove(term->kitty_notifications, it); + } + + tll_foreach(term->active_notifications, it) { + notify_free(term, &it->item); + tll_remove(term->active_notifications, it); } for (size_t i = 0; i < ALEN(term->notification_icons); i++) diff --git a/terminal.h b/terminal.h index 7b726c62..92d1e8f5 100644 --- a/terminal.h +++ b/terminal.h @@ -801,7 +801,8 @@ struct terminal { /* Notifications that either haven't been sent yet, or have been sent but not yet dismissed */ - tll(struct notification) notifications; + tll(struct notification) kitty_notifications; + tll(struct notification) active_notifications; struct notification_icon notification_icons[32]; char *foot_exe;