Re: bug#75014: [PATCH] 30.0.92; ERC 5.6.0.30.1: Add interactive function to clear ERC channels' modified status

Hi Alex,

Alex Bochannek <[email protected]> writes:

> This small function is intended as an easy way to mark all channels as
> "read." This is useful when returning to an ERC session with multiple
> modified channels that are not displayed in a window. Their status is
> indicated in the mode line and in the ERC status sidebar, both of which
> this function clears.

Thanks for the patch. It seems quite useful.

Administrative note: this change should probably go in ERC 5.7 instead
of the upcoming 5.6.1 because, AFAIK, new features in point releases
should only address unfinished (and ideally pressing) business from the
current release or at least have some tenuous connection to a feature
introduced there. That said, this command is pretty basic, so maybe we
can squeeze it in without anyone complaining (although we should
probably still mention it in etc/ERC-NEWS).

>
> If this is a common enough task, it makes sense to bind this function to
> a key sequence, e.g.:
>
>   (keymap-global-set "C-c e c" 'erc-modified-channels-clear)

FTR, I'd rather ERC avoid modifying the global keymap (not to say that's
what you were suggesting here). However, if people really need a
predefined binding, I suppose we could maybe add something like this to
`erc-track-minor-mode-map'. As for the keys themselves, I've been told
there's a tacit convention these days to avoid "C-c {alnum}" because
such keys are quasi reserved for a user's own config and that we should
instead favor "C-c {modifier-sequence}-{alnum}" or a binding outside the
"C-c" prefix space. (Not sure how widely followed this practice is,
though.) A safer, perhaps more cowardly approach might be to just
recommend a binding in the command's doc string.

> Note: I am not familiar with the ERC code base, so if calling the
> function something else makes more sense (I originally called it
> `erc-reset-modified-channels') I have no objection to it.

As it happens, ERC hails from a time before prefixing a library's
symbols with its provided feature and/or group prefix was widely
adopted. For better or worse, we tend to abide by this convention
nowadays in new code, even though it may hamper discoverability at
times. IOW, we should probably ensure the name begins with "erc-track-".

Beyond that, I think it makes sense for related symbols within a library
to share a common prefix. But since doing so often affects readability
by contravening natural spoken syntax, e.g., "modified-channels-clear"
vs. "clear-modified-channels," I tend to reserve such rigid alignment
for "internal" symbols (those with two hyphens). I mention this because
abandoning the association with `erc-modified-channels-alist' means
there's no need to retain "channels" in the name, which may be more
faithful to reality, since the command affects mode-line indicators for
query and server buffers as well. As for "reset" vs. "clear," I have no
real preference, but "clear" maybe sounds more definitive because
"reset" might make someone wonder "reset to what"? (Or not.)

> From 65cbc68d2fcdff8654df53d8d9a0f4c7aeb12529 Mon Sep 17 00:00:00 2001
> From: Alex Bochannek <[email protected]>
> Date: Sat, 21 Dec 2024 11:39:08 -0800
> Subject: [PATCH] Add interactive function to clear ERC channels' modified status
>  status
>
> ---
>  lisp/erc/erc-track.el | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/lisp/erc/erc-track.el b/lisp/erc/erc-track.el
> index 97fb7e726bd..8d6f804a42b 100644
> --- a/lisp/erc/erc-track.el
> +++ b/lisp/erc/erc-track.el
> @@ -900,6 +900,13 @@ erc-modified-channels-remove-buffer
>    (when (called-interactively-p 'interactive)
>      (erc-modified-channels-display)))
>  
> +(defun erc-modified-channels-clear ()
> +  "Remove all buffers from `erc-modified-channels-alist'."

Should we also mention that this clears the mode line segment, or maybe
that's already obvious enough?

> +  (interactive)
> +  (setq erc-modified-channels-alist nil)
> +  (when (called-interactively-p 'interactive)
> +    (erc-modified-channels-display)))
> +

Perhaps we should just remove the conditional `when' construct and run
`erc-modified-channels-display' unguarded, the assumption being that
users doing this in Lisp code can set `erc-modified-channels-alist' to
nil as needed. Otherwise, if keeping the `when', should we heed the
scary admonition in the doc string for `called-interactively-p' and
accommodate keyboard macros as well, or is there a reason to exclude
them?

>  (defun erc-track-find-face (faces)
>    "Return the face to use in the mode line."
>    (declare (obsolete erc-track-select-mode-line-face "28.1"))
> -- 
> 2.39.5 (Apple Git-154)

Thanks,
J.P.