bug#35141: Add a default action to erc-desktop-notifications to switch to the originating buffer

[Alex Murray]

Hi Basil,

Thanks for the review - see attached for an updated version based on your feedback.

Cheers,

Alex

On Fri, 5 Apr 2019 at 12:43, Basil L. Contovounesios <contovob@tcd.ie> wrote:

Alex Murray <murray.alex@gmail.com> writes:

> Attached is a patch which converts erc-desktop-notifications.el to
> lexical-binding and adds a default action to the generated
> notifications so that when they are clicked we automatically switch to
> the originating buffer.

SGTM.

> From 52b63272eed8aaafcd931e833ad006dd425d6512 Mon Sep 17 00:00:00 2001
> From: Alex Murray <murray.alex@gmail.com>
> Date: Thu, 4 Apr 2019 16:49:48 +1030
> Subject: [PATCH] Switch to originating erc buffer when clicking desktop
>  notification
>
> * lisp/erc/erc-desktop-notifications.el: Switch to lexical-binding and
>   add a default action to the desktop notification which switches to
>   the buffer from which the notification originated. As such clicking
>   on the notification now automatically switches to that buffer.

Please mention the name of the function changed:

* lisp/erc/erc-desktop-notifications.el: Switch to lexical-binding.
(erc-notifications-notify): Add ...

Unless you have completed your copyright assignment for contributions to
Emacs, the commit message should also include:

Copyright-paperwork-exempt: yes

For more details, see the file CONTRIBUTE.

> diff --git a/ChangeLog.3 b/ChangeLog.3
> index 235c8bb180..3819d6b4fe 100644
> --- a/ChangeLog.3
> +++ b/ChangeLog.3
> @@ -1,3 +1,10 @@
> +2019-04-04  Alex Murray  <murray.alex@gmail.com>
> +
> +     * lisp/erc/erc-desktop-notifications.el: Use lexical-binding.
> +     (erc-notifications-notify): Add a default action to
> +     notifications which switches to the buffer from which the
> +     notification originated.
> +
>  2019-03-20  Eli Zaretskii  <eliz@gnu.org>
> 
>       Improve indexing of the user manual

ChangeLog files are no longer maintained by hand, but rather generated
from Git commit messages.

> diff --git a/lisp/erc/erc-desktop-notifications.el b/lisp/erc/erc-desktop-notifications.el
> index 56b93925ce..c1b812c119 100644
> --- a/lisp/erc/erc-desktop-notifications.el
> +++ b/lisp/erc/erc-desktop-notifications.el
> @@ -59,11 +59,17 @@
>  This will replace the last notification sent with this function."
>    (dbus-ignore-errors
>      (setq erc-notifications-last-notification
> -          (notifications-notify :bus erc-notifications-bus
> -                             :title (xml-escape-string nick)
> -                                :body (xml-escape-string msg)
> -                                :replaces-id erc-notifications-last-notification
> -                                :app-icon erc-notifications-icon))))
> +          (let ((channel (current-buffer)))
> +            (notifications-notify :bus erc-notifications-bus
> +                                  :title (format "%s in %s"
> +                                                 (xml-escape-string nick)
> +                                                 (buffer-name channel))

No need for buffer-name; you can pass buffers directly to "%s".

> +                                  :body (xml-escape-string msg)
> +                                  :replaces-id erc-notifications-last-notification
> +                                  :app-icon erc-notifications-icon
> +                                  :actions '("default" "Switch to buffer")
> +                                  :on-action #'(lambda (id action)
> +                                                 (switch-to-buffer channel)))))))

This should instead be something like the following:

  (lambda (&rest _)
    (pop-to-buffer channel))

In particular, lambdas don't need to be quoted with #', pop-to-buffer is
far better behaved than switch-to-buffer, and unused argument names
should either start with an underscore or be omitted.

Thanks,

--
Basil

0001-Switch-to-originating-erc-buffer-when-clicking-deskt.patch
Description: Text Data