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

Previous Next

Package: emacs;

Reported by: Tassilo Horn <thorn <at> fastmail.fm>

Date: Thu, 13 Feb 2025 11:29:01 UTC

Severity: wishlist

Tags: patch

To reply to this bug, email your comments to 76261 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#76261; Package emacs. (Thu, 13 Feb 2025 11:29:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Tassilo Horn <thorn <at> fastmail.fm>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Thu, 13 Feb 2025 11:29:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Tassilo Horn <thorn <at> fastmail.fm>
To: bug-gnu-emacs <at> gnu.org
Cc: Philip Kaludercic <philipk <at> posteo.net>
Subject: [PATCH] Add a mentions buffer feature to rcirc
Date: Thu, 13 Feb 2025 12:27:56 +0100
[Message part 1 (text/plain, inline)]
Tags: patch

Hi all,

rcirc-track-minor-mode is nice to navigate to mentions of your nick or
certain keywords (rcirc-keywords), however you can navigate there only
once.

The proposed feature (if enabled) collects such mentions in a separate
buffer *rcirc mentions* as clickable text, so that you can navigate to
the source of the mention as often as you like.

In its current state, the feature works fine but I guess the *rcirc
mentions* buffer should contain a bit more information, e.g., a copy of
the actual mentioning message.

I guess there's also a technical issue: right now, the linkage is to a
concrete buffer position (point value) which only works reliably if
rcirc-buffer-maximum-lines is nil and thus buffers aren't truncated.  I
guess I should switch to markers instead, right?

Anyway, I just wanted to get feedback if such a feature is acceptable?

Thanks,
Tassilo

In GNU Emacs 31.0.50 (build 7, x86_64-pc-linux-gnu, GTK+ Version
 3.24.48, cairo version 1.18.2) of 2025-02-13 built on thinkpad-t440p
Repository revision: 2e9413255fb8cbd0c8481ca6f863b37df3e9008d
Repository branch: master
System Description: Arch Linux

Configured using:
 'configure --without-native-compilation --with-modules --with-pgtk'

[0001-Add-a-mentions-buffer-feature-to-rcirc.patch (text/patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76261; Package emacs. (Fri, 14 Feb 2025 10:40:02 GMT) Full text and rfc822 format available.

Message #8 received at 76261 <at> debbugs.gnu.org (full text, mbox):

From: Tassilo Horn <tsdh <at> gnu.org>
To: 76261 <at> debbugs.gnu.org
Cc: Philip Kaludercic <philipk <at> posteo.net>
Subject: Re: bug#76261: [PATCH] Add a mentions buffer feature to rcirc
Date: Fri, 14 Feb 2025 11:39:16 +0100
[Message part 1 (text/plain, inline)]
Tassilo Horn <thorn <at> fastmail.fm> writes:

Hi again,

> The proposed feature (if enabled) collects such mentions in a separate
> buffer *rcirc mentions* as clickable text, so that you can navigate to
> the source of the mention as often as you like.

Attached is a revised version of the patch which uses markers instead of
point values now and formats the mentions nicely, see this screenshot:
[Screenshot-2025-02-14_113553.png (image/png, attachment)]
[Message part 3 (text/plain, inline)]
Bye,
Tassilo
[0001-Add-a-mentions-buffer-feature-to-rcirc.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76261; Package emacs. (Fri, 14 Feb 2025 14:05:01 GMT) Full text and rfc822 format available.

Message #11 received at 76261 <at> debbugs.gnu.org (full text, mbox):

From: Tassilo Horn <tsdh <at> gnu.org>
To: 76261 <at> debbugs.gnu.org
Cc: Philip Kaludercic <philipk <at> posteo.net>
Subject: Re: bug#76261: [PATCH] Add a mentions buffer feature to rcirc
Date: Fri, 14 Feb 2025 15:04:28 +0100
[Message part 1 (text/plain, inline)]
Tassilo Horn <tsdh <at> gnu.org> writes:

>> The proposed feature (if enabled) collects such mentions in a
>> separate buffer *rcirc mentions* as clickable text, so that you can
>> navigate to the source of the mention as often as you like.
>
> Attached is a revised version of the patch which uses markers instead
> of point values now and formats the mentions nicely, see this
> screenshot:

And here's a third version which adds a separate major mode for the
mentions buffer including movement commands to TAB (and C-TAB) to the
next (previous) mention link.

[0001-Add-a-mentions-buffer-feature-to-rcirc.patch (text/x-patch, attachment)]
[Message part 3 (text/plain, inline)]
Bye,
Tassilo

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76261; Package emacs. (Fri, 14 Feb 2025 20:25:02 GMT) Full text and rfc822 format available.

Message #14 received at 76261 <at> debbugs.gnu.org (full text, mbox):

From: Tassilo Horn <tsdh <at> gnu.org>
To: 76261 <at> debbugs.gnu.org
Cc: Philip Kaludercic <philipk <at> posteo.net>
Subject: Re: bug#76261: [PATCH] Add a mentions buffer feature to rcirc
Date: Fri, 14 Feb 2025 21:24:00 +0100
Tassilo Horn <tsdh <at> gnu.org> writes:

> And here's a third version which adds a separate major mode for the
> mentions buffer including movement commands to TAB (and C-TAB) to the
> next (previous) mention link.

Obviously, there was a bug in there.  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.

Bye,
Tassilo




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76261; Package emacs. (Fri, 14 Feb 2025 23:14:01 GMT) Full text and rfc822 format available.

Message #17 received at 76261 <at> debbugs.gnu.org (full text, mbox):

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

> Tassilo Horn <tsdh <at> gnu.org> writes:
>
>> And here's a third version which adds a separate major mode for the
>> mentions buffer including movement commands to TAB (and C-TAB) to the
>> next (previous) mention link.
>
> Obviously, there was a bug in there.  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?

> Bye,
> Tassilo




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76261; Package emacs. (Sat, 15 Feb 2025 06:52:02 GMT) Full text and rfc822 format available.

Message #20 received at 76261 <at> debbugs.gnu.org (full text, mbox):

From: Tassilo Horn <tsdh <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 76261 <at> debbugs.gnu.org
Subject: Re: bug#76261: [PATCH] Add a mentions buffer feature to rcirc
Date: Sat, 15 Feb 2025 07:51:11 +0100
Philip Kaludercic <philipk <at> posteo.net> 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. :-)

Thanks,
Tassilo




Severity set to 'wishlist' from 'normal' Request was from Stefan Kangas <stefankangas <at> gmail.com> to control <at> debbugs.gnu.org. (Wed, 19 Feb 2025 02:11:04 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76261; Package emacs. (Wed, 19 Feb 2025 12:14:01 GMT) Full text and rfc822 format available.

Message #25 received at 76261 <at> debbugs.gnu.org (full text, mbox):

From: Tassilo Horn <tsdh <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 76261 <at> debbugs.gnu.org
Subject: Re: bug#76261: [PATCH] Add a mentions buffer feature to rcirc
Date: Wed, 19 Feb 2025 13:12:55 +0100
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. :-)

Bye,
Tassilo




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76261; Package emacs. (Thu, 20 Feb 2025 20:49:02 GMT) Full text and rfc822 format available.

Message #28 received at 76261 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Tassilo Horn <tsdh <at> gnu.org>
Cc: 76261 <at> debbugs.gnu.org
Subject: Re: 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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76261; Package emacs. (Fri, 21 Feb 2025 09:11:01 GMT) Full text and rfc822 format available.

Message #31 received at 76261 <at> debbugs.gnu.org (full text, mbox):

From: Tassilo Horn <tsdh <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 76261 <at> debbugs.gnu.org
Subject: Re: bug#76261: [PATCH] Add a mentions buffer feature to rcirc
Date: Fri, 21 Feb 2025 10:09:41 +0100
Philip Kaludercic <philipk <at> posteo.net> writes:

Hi Philip,

>> 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 ^^)

:-)

Yeah, that seems about the right version.  (I've already pulled from
master again so the hashes are different but the commit message suggests
it's the latest version).

>> 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".

Alright.  I'll change that.

>> (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.

Fine.

>> (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

Haha, sqashing artifacts. :-)

>> ---
>>  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.

Alright.

>> +  :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?

I guess I just cargo-culted.

>> +(defun rcirc-update-mentions-buffer (type)
>
> This should be documented with a proper docstring!

And/or probably be a "private" function, right?

>> +(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?

Wouldn't that show the mentions buffer in the current/selected window?
The point here is to just select-window the window already showing the
mentians buffer if there's any.

>> +      (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?

My personal preference would be to have it in core, just because it's
simpler to implement, maintain, and document.  And it doesn't feel like
a big burden for the rest of rcirc.  I'd need one or two additional
hooks for implementing it as a package, I think.

While looking at it, I'm not sure it's right that currently the feature
obeys rcirc-ignore-buffer-activity-flag and rcirc-dim-nicks.  I mean, a
mention is a mention and given that the mentions buffer is "for later
reference" and doesn't disturb you in any way, it's probably better to
log those, too, in contrast to the activity mode-line indicator.

Bye,
Tassilo




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#76261; Package emacs. (Fri, 21 Feb 2025 14:18:02 GMT) Full text and rfc822 format available.

Message #34 received at 76261 <at> debbugs.gnu.org (full text, mbox):

From: Tassilo Horn <tsdh <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 76261 <at> debbugs.gnu.org
Subject: Re: bug#76261: [PATCH] Add a mentions buffer feature to rcirc
Date: Fri, 21 Feb 2025 15:17:11 +0100
Tassilo Horn <tsdh <at> gnu.org> writes:

Hi Philip,

I've addressed all of your comments and updated the branch now.

Well, except the question about display-buffer-same-window where I think
it would do the wrong thing.

>> 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?
>
> My personal preference would be to have it in core, just because it's
> simpler to implement, maintain, and document.  And it doesn't feel
> like a big burden for the rest of rcirc.  I'd need one or two
> additional hooks for implementing it as a package, I think.
>
> While looking at it, I'm not sure it's right that currently the
> feature obeys rcirc-ignore-buffer-activity-flag and rcirc-dim-nicks.
> I mean, a mention is a mention and given that the mentions buffer is
> "for later reference" and doesn't disturb you in any way, it's
> probably better to log those, too, in contrast to the activity
> mode-line indicator.

Yeah, that wasn't right.  It also was dependent on the default value of
rcirc-markup-text-functions.  It's no user option but it could still
happen that someone removes, e.g., rcirc-markup-keywords.

So now I test directly in rcirc-print if some kind(s) of mention was
done which makes it even simpler (just one instead of three entry
points).

Bye,
Tassilo




This bug report was last modified 118 days ago.

Previous Next


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