GNU bug report logs - #76261
[PATCH] Add a mentions buffer feature to rcirc

Previous Next

Full log

View this message in rfc822 format

From: Philip Kaludercic <philipk <at> posteo.net>
To: Tassilo Horn <tsdh <at> gnu.org>
Cc: 76261 <at> debbugs.gnu.org
Subject: bug#76261: [PATCH] Add a mentions buffer feature to rcirc
Date: Thu, 20 Feb 2025 20:48:16 +0000

Tassilo Horn <tsdh <at> gnu.org> writes:

> Tassilo Horn <tsdh <at> gnu.org> writes:
>
>>>> I'll stop sending patches and instead pushed my
>>>> feature/rcirc-mentions branch to https://git.sr.ht/~tsdh/emacs where
>>>> I can keep fixing issues when I find one.  I also added another
>>>> commit adding the texinfo docs, i.e., a new section in the rcirc
>>>> manual.
>>>>
>>>> I'll keep it rebased onto master.
>>>
>>> Can you ping me when I should check the state of the branch?
>>
>> Whenever you want.  I think I'm done. :-)
>
> I just threw away the rcirc-mentions-enabled defcustom for the feature
> and now enable it using minor-modes rcirc-log-mentions-mode or
> rcirc-global-log-mentions-mode which makes more sense.
>
> So now I'm really, really done and would welcome a review. :-)

So this is the patch in question, right:

> From e9961ff1ffdc4bb39c1dff7df58f5c7cef5cf448 Mon Sep 17 00:00:00 2001
> From: Tassilo Horn <tsdh <at> gnu.org>
> Date: Thu, 13 Feb 2025 10:39:31 +0100
> Subject: [PATCH] Add a mentions buffer feature to rcirc

> If the feature is enabled by activating rcirc-log-mentions-mode in a
> buffer or rcirc-global-log-mentions-mode, a buffer

(Not crucial, this just seems like a roundabout way to say what you are
trying to say... Not that I am any better at it ^^)

> rcirc-mentions-buffer-name is created and used for logging mentions of
> your nick or a keyword in rcirc-keywords.  The log lines link to the
> original position in the actual channel buffer where the mention
> occured.

> * lisp/net/rcirc.el (rcirc-mentions-buffer-name): New defcustom.

I think it is more conventional to refer to this as "user option".

> (rcirc-log-mentions-mode, rcirc-global-log-mentions-mode): New
> minor-modes.
> (rcirc-log-mentions-mode-map): New keymap for rcirc-log-mentions-mode.
> (rcirc-switch-to-mentions-buffer): New command bound in
> rcirc-log-mentions-mode-map.

You should add 'these-kinds-of' quotations for elisp symbols.

> (rcirc-mode): Enable rcirc-log-mentions-mode in the buffer if
> rcirc-global-log-mentions-mode is activated.
> (rcirc-mentions-buffer-mode): New major mode for the mentions buffer.
> (rcirc-mentions-next, rcirc-mentions-prev): New movement commands in the
> mentions buffer.
> (rcirc--get-last-message-bounds): New helper function.
> (rcirc-record-activity): Call rcirc-update-mentions-buffer if
> rcirc-log-mentions-mode is activated.
> (rcirc-update-mentions-buffer): New function.

> foo

?

> fix
> ---
>  lisp/net/rcirc.el | 140 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 140 insertions(+)

> diff --git a/lisp/net/rcirc.el b/lisp/net/rcirc.el
> index 3f81672182b..61e867377fe 100644
> --- a/lisp/net/rcirc.el
> +++ b/lisp/net/rcirc.el
> @@ -1482,6 +1482,43 @@ does not confuse the pairing."
>        (narrow-to-region rcirc-prompt-start-marker (point-max))
>        (funcall fallback char))))
 
> +(defcustom rcirc-mentions-buffer-name "*rcirc mentions*"
> +  "The name of the mentions buffer.
> +Also see `rcirc-log-mentions-mode' and `rcirc-global-log-mentions-mode'."

I don't think you need to mention both here, but I would go into some
more detail than just "see also" here.

> +  :type 'string
> +  :version "31.1")
> +
> +(defvar-keymap rcirc-log-mentions-mode-map
> +  :doc "The keymap of `rcirc-log-mentions-mode'."
> +  "C-c C-." #'rcirc-switch-to-mentions-buffer)
> +
> +(define-minor-mode rcirc-log-mentions-mode
> +  "Minor mode to log mentions in the current channel in a separate buffer.
> +The name of this buffer is defined by `rcirc-mentions-buffer-name'.  In
> +this buffer, mentions of your nick or a keyword in `rcirc-keywords' will
> +be logged linking to the original message in the respective channel
> +buffer.
> +
> +Also see `rcirc-global-log-mentions-mode' to enable/disable
> +`rcirc-log-mentions-mode' in all rcirc buffers."
> +  :lighter " Mentions"
> +  :interactive (rcirc-mode)
> +  :keymap rcirc-log-mentions-mode-map)

The last :keymap should be superfluous, right?

> +
> +;;;###autoload
> +(define-minor-mode rcirc-global-log-mentions-mode
> +  "Global minor mode for `rcirc-log-mentions-mode'.
> +Enabling the mode activates `rcirc-log-mentions-mode' in all current and
> +future `rcirc-mode' buffers.  Disabling the mode deactivates it in all
> +current and future `rcirc-mode' buffers."
> +  :global t
> +  ;; Activate or deactivate it in all current `rcirc-mode' buffers.
> +  (let ((state (if rcirc-global-log-mentions-mode 1 -1)))
> +    (dolist (buf (buffer-list))
> +      (with-current-buffer buf
> +        (when (derived-mode-p #'rcirc-mode)
> +          (rcirc-log-mentions-mode state))))))
> +
>  (defun rcirc-mode (process target)
>    "Initialize an IRC buffer for writing with TARGET.
>  PROCESS is the process object used for communication.
> @@ -1556,6 +1593,11 @@ PROCESS is the process object used for communication.
>    (setq-local electric-pair-inhibit-predicate
>                #'rcirc--electric-pair-inhibit)
> 
> +  ;; Enable the local `rcirc-log-mentions-mode' when the global
> +  ;; `rcirc-global-log-mentions-mode' in activated.
> +  (when rcirc-global-log-mentions-mode
> +    (rcirc-log-mentions-mode 1))
> +
>    (run-mode-hooks 'rcirc-mode-hook))

>  (defun rcirc-update-prompt (&optional all)
> @@ -2466,6 +2508,100 @@ When there are no buffers with activity, bury all rcirc buffers."
>                   ""))))
>    (rcirc-update-activity-string))
 
> +(defun rcirc-mentions-next ()
> +  (interactive)
> +  (when-let* ((b (next-button (point))))
> +    (goto-char (overlay-start b))))
> +
> +(defun rcirc-mentions-prev ()
> +  (interactive)
> +  (when-let* ((b (previous-button (point))))
> +    (goto-char (overlay-start b))))
> +
> +(define-derived-mode rcirc-mentions-buffer-mode fundamental-mode
> +  "RcircMentions"
> +  "Major mode in the rcirc mentions buffer."
> +  (setq buffer-read-only t)
> +  ;; We want to see the original fontification of the channel buffer.
> +  (font-lock-mode 1))
> +
> +(keymap-set rcirc-mentions-buffer-mode-map
> +            "<tab>" #'rcirc-mentions-next)
> +(keymap-set rcirc-mentions-buffer-mode-map
> +            "C-<tab>" #'rcirc-mentions-prev)
> +
> +(defun rcirc--get-last-message-bounds ()
> +  ;; Assumes being called while the current buffer is the buffer with
> +  ;; the activity.
> +  (save-excursion
> +    (goto-char rcirc-prompt-start-marker)
> +    (goto-char (1- (point)))
> +    (let ((end (point)))
> +      (when-let* ((beg (previous-single-property-change
> +                        (point) 'rcirc-text)))
> +        (goto-char beg)
> +        (cons (line-beginning-position) end)))))
> +
> +(defun rcirc-update-mentions-buffer (type)

This should be documented with a proper docstring!

> +  ;; Assumes being called while the current buffer is the buffer with
> +  ;; the activity.
> +  (when-let* ((bounds (rcirc--get-last-message-bounds)))
> +    (let* ((msg (buffer-substring (car bounds) (cdr bounds)))
> +           (activity-marker (let ((m (make-marker)))
> +                              (set-marker m (car bounds))))
> +           (action
> +            (lambda (_button)
> +              (let ((buf (marker-buffer activity-marker)))
> +                (if (buffer-live-p buf)
> +                    (if-let* ((pos (marker-position activity-marker)))
> +                        (progn
> +                          (switch-to-buffer-other-window buf)
> +                          (goto-char activity-marker))
> +                      (message "The buffer %s has been truncated." buf))
> +                  (message "The originating buffer has disappeared."))))))
> +      (with-current-buffer (get-buffer-create
> +                            rcirc-mentions-buffer-name)
> +        (rcirc-mentions-buffer-mode)
> +        (let ((inhibit-read-only t)
> +              ;; Stay at the current position in the buffer when we are
> +              ;; not on the very last mention.
> +              (orig-pos (when (save-excursion
> +                                (text-property-search-forward
> +                                 'face t
> +                                 (lambda (_v pv)
> +                                   (and (listp pv)
> +                                        (eq 'separator-line
> +                                            (plist-get pv :inherit))
> +                                        (plist-get pv :extend)))))
> +                          (point))))
> +          (goto-char (point-max))
> +          (unless (bobp)
> +            (insert (make-separator-line)))
> +          (insert (rcirc-facify
> +                   (format-time-string
> +                    (string-trim-right rcirc-time-format)
> +                    (current-time))
> +                   'rcirc-timestamp)
> +                  ": ")
> +          (let ((link-beg (point)))
> +            (insert-button
> +             (format "%s mentioned in %s."
> +                     (if (eq type 'nick) "Nick" "Keyword")
> +                     (buffer-name (marker-buffer activity-marker)))
> +             'action action
> +             'follow-link t)
> +            (insert "\n" msg "\n")
> +            (goto-char (or orig-pos link-beg))))))))
> +
> +(defun rcirc-switch-to-mentions-buffer ()
> +  "Switch to the mentions buffer if it exists.
> +See `rcirc-log-mentions-mode' and `rcirc-mentions-buffer-name'."
> +  (interactive)
> +  (when-let* ((buf (get-buffer rcirc-mentions-buffer-name)))
> +    (if-let* ((win (get-buffer-window buf)))
> +        (select-window win)

Could you use `display-buffer-same-window' here?

> +      (switch-to-buffer buf))))
> +
>  (defvar rcirc-activity-functions nil
>    "Hook to be run when there is channel activity.
 
> @@ -2478,6 +2614,10 @@ activity.  Only run if the buffer is not visible and
>    (with-current-buffer buffer
>      (let ((old-activity rcirc-activity)
>            (old-types rcirc-activity-types))
> +      (when (and rcirc-log-mentions-mode
> +                 (memq type '(nick keyword))
> +                 (not (eq buffer rcirc-server-buffer)))
> +        (rcirc-update-mentions-buffer type))
>        (when (and (not (get-buffer-window (current-buffer) t))
>                   (not (and rcirc-track-ignore-server-buffer-flag
>                             (eq rcirc-server-buffer (current-buffer)))))
> -- 
> 2.45.3

Most of the code looks just fine, my main question is if we really want
to add this directly to rcirc or if it would be better to have it as a
extension on ELPA?

>
> Bye,
> Tassilo

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.