Re: [elpa] externals/notmuch-indicator f563869b6a 2/4: Tweak how the keymap is implemented

[Stefan Monnier <monnier@iro.umontreal.ca>]

> branch: externals/notmuch-indicator
> commit f563869b6a5e5e12fd791bc109232ed45064f7a7
> Author: Protesilaos Stavrou <info@protesilaos.com>
> Commit: Protesilaos Stavrou <info@protesilaos.com>
>
>     Tweak how the keymap is implemented
>     
>     This continues the work done by Henrik Kjerringvåg in commit f5c9b2e.
>     Henrik does not require to assign copyright to the Free Software
>     Foundation for that commit, as it is within the ~15 line threshold.
> ---
>  notmuch-indicator.el | 22 +++++++++++++---------
>  1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/notmuch-indicator.el b/notmuch-indicator.el
> index 89e93054ee..9b598910e2 100644
> --- a/notmuch-indicator.el
> +++ b/notmuch-indicator.el
> @@ -166,17 +166,21 @@ option `notmuch-indicator-refresh-count'."
>     (shell-command-to-string
>      (format "notmuch count %s" terms))))
>  
> +(defvar notmuch-indicator-mode-map
> +  (let ((map (make-sparse-keymap)))
> +    map)
> +  "Keymap for the Notmuch indicator.")
> +
>  (defun notmuch-indicator--format-label (label count face terms)
>    "Format LABEL, COUNT, FACE and TERMS of `notmuch-indicator-args'."
> -  (let ((map (make-sparse-keymap)))
> -    (define-key map [mode-line mouse-1]
> -                (lambda () (interactive) (notmuch-search terms)))
> -    (propertize
> -     (if (and face label)
> -         (format "%s%s " (propertize label 'face face) count)
> -       (format "%s%s " (or label "") count))
> -     'help-echo "mouse-1: Open notmuch search"
> -     'local-map map)))
> +  (define-key notmuch-indicator-mode-map [mode-line mouse-1]
> +              (lambda () (interactive) (notmuch-search terms)))
> +  (propertize
> +   (if (and face label)
> +       (format "%s%s " (propertize label 'face face) count)
> +     (format "%s%s " (or label "") count))
> +   'help-echo (format "mouse-1: Open notmuch search for `%s'" terms)
> +   'local-map notmuch-indicator-mode-map))

Hmm... really?  The new `define-key` will overwrite the binding added in
the previous call to `notmuch-indicator--format-label`, so if you have
several elements in `notmuch-indicator-args`, they'll all use the last
element's `terms` when you click `mouse-1` on them.

While I'm here:

> +   (if (and face label)
> +       (format "%s%s " (propertize label 'face face) count)
> +     (format "%s%s " (or label "") count))

can be rewritten to

    
        (format "%s%s "
                (if (and face label) (propertize label 'face face)
                  (or label "")
                count)

Oh, and I see elsewhere that you call `mapconcat` with only 2 args,
which is a new feature in Emacs-29 whereas the package claims to want to
support Emacs-27.


        Stefan