GNU bug report logs - #60936
30.0.50; ERC >5.5: Add erc-fill style based on visual-line-mode

Previous Next

Full log

View this message in rfc822 format

From: "J.P." <jp <at> neverwas.me>
To: 60936 <at> debbugs.gnu.org
Cc: emacs-erc <at> gnu.org
Subject: bug#60936: 30.0.50; ERC >5.5: Add erc-fill style based on visual-line-mode
Date: Mon, 03 Jul 2023 06:14:31 -0700

[Message part 1 (text/plain, inline)]

"J.P." <jp <at> neverwas.me> writes:

> A commit associated with this bug
>
>   d880a08f "Cement ordering of essential hook members in ERC"
>
> introduced a regression that basically nullifies the `match' module when
> a certain `erc-stamp' option is customized to a certain value. To
> reproduce from Emacs -Q:
>
>   - Set `erc-insert-timestamp-function' to `erc-insert-timestamp-left'
>
>   - Connect to any server
>
>   - Find the first mention of your nickname in the text of some early
>     numeric (often something like "Welcome to FooNet <nick>")
>
>   - Notice that it appears in plain `erc-notice-face' rather than
>     `erc-current-nick-face' (a "match" face)
>
> The attached patch should fix the issue. Thanks to Libera.Chat user jrm
> for reporting this bug.

Actually, the veracity of that claim is unclear and most likely bogus.
What is clear is that this approach is unsustainable because related
bugs are bound to crop up in the near future (if they haven't already).

Basically, in trying to code defensively around possibly encountering
unexpected text before inserted messages (such as leading stamps, white
space, decorations, etc.), my attempted solution traded superficial
robustness for a new dimension of complexity that's almost certainly
unsustainable. (This outcome was more or less predicted in the
justification for d880a08f "Cement ...", which this fix rather callously
contravened the spirit of.)

Anyway, to address all this, I think we should:

  1. Revert the previous attempted fix, which now exists on HEAD as

     commit 99d74dcd45938e2686d93eb5649800e14a88cd84
     Author: F. Jason Park <jp <at> neverwas.me>
     Date:   Tue Jun 27 20:47:26 2023 -0700
     
         Account for leading timestamps in erc-match
         
      lisp/erc/erc-match.el                |  41 ++++++++----
      test/lisp/erc/erc-scenarios-match.el | 120 +++++++++++++++++++++++++
      2 files changed, 149 insertions(+), 12 deletions(-)

  2. Undo the change of ordering for `erc-add-timestamp' and
     `erc-match-message' in `erc-insert-modify-hook' (from d880a08f
     "Cement ...").

  3. Take an entirely different tack bent on including (rather than
     omitting) time stamps from invisible messages. If not yet obvious,
     the impetus for the poor decision (of mine) to switch the order of
     those hook members was to improve the toggling of invisible
     elements created by the `match' module (and potentially others),
     and also to make logs less ragged when they feature invisible
     messages.

I'll go ahead and install the first of the attached patches (reverting
the misguided fix) and continue to iterate on the second, which proposes
the more comprehensive solution described in 3. Thanks.

> While we're at it, I'm thinking the option `erc-fill-spaced-commands',
> which has been on HEAD for a few months now, should be demoted to a
> plain variable, maybe even an internal one, because there aren't any
> obvious use cases for non-default values. Unless someone has a good
> argument to the contrary, I will do this in an accompanying patch to be
> installed along with this one. Thanks.

I've decided to instead lump this in with bug#64301 (speaker labels).

[0001-Revert-Account-for-leading-timestamps-in-erc-match.patch (text/x-patch, attachment)]

[0002-5.6-Respect-existing-invisibility-props-in-erc-stamp.patch (text/x-patch, attachment)]

Previous Next

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