GNU bug report logs - #67220
30.0.50; ERC 5.6: Prefer parameter-driven MODE processing in ERC

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 67220 in the body.
You can then email your comments to 67220 AT debbugs.gnu.org in the normal way.

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

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

From: "J.P." <jp <at> neverwas.me>
To: bug-gnu-emacs <at> gnu.org
Subject: 30.0.50; ERC 5.6: Prefer parameter-driven MODE processing in ERC
Date: Wed, 15 Nov 2023 18:13:53 -0800

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

Tags: patch

In the early days of IRC, parsing a "MODE" command from the server was
comparatively straightforward. There were a few well known letters, some
taking a single argument, and a standard set of status prefixes. But
somewhere along the line, things got more complicated, and it seems ERC
never got the memo. While it may appear obvious that sticking to a
hard-coded, heuristics based approach doesn't really accommodate ERC's
core tenet of extensibility, the risk of shifting toward something more
parameter driven was probably never justifiable without a vocal demand.
Or an obvious bug.

From emacs -Q:

  1. Connect to Libera.Chat
  2. Create ##mychan
  3. /mode ##mychan +Qu

  debugger entered--Lisp error: (wrong-type-argument char-or-string-p nil)
    erc-downcase(nil)
    erc-update-current-channel-member(nil nil nil nil nil nil nil on ...)
    erc-update-channel-member("#libera" nil nil nil nil nil nil nil on)
    erc-update-modes("##mychan" "+Qu" "mynick" "user/foo" "Hi!")

The issue here is that ERC doesn't account for ISUPPORT parameters when
parsing MODE commands and dispatching handlers. Instead, it simply
assumes that +q (or +Q) means someone has just been promoted to a
channel owner.

I'll admit that although I've been aware of this basic issue for quite
some time, I've been hesitant to cross this bridge until 5.7+ because of
the potential pitfalls involved. In any case, with a concrete bug having
surfaced (courtesy of Corwin), the issue has been forced, and it's one
that can't really be papered over responsibly just to avoid holding up
the current release. My proposed means of addressing this is mainly
contained in the last of the attached patches. The approach comes down
to rewriting the most important bits and providing adapters to reroute
the rest accordingly. Comments welcome, as always. Thanks.


In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.38, cairo version 1.17.6) of 2023-11-15 built on localhost
Repository revision: ff1f82cbe3fa9aee354581f2798faaae7163ea44
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12014000
System Description: Fedora Linux 37 (Workstation Edition)

Configured using:
 'configure --enable-check-lisp-object-type --enable-checking=yes,glyphs
 'CFLAGS=-O0 -g3'
 PKG_CONFIG_PATH=:/usr/lib64/pkgconfig:/usr/share/pkgconfig'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  minibuffer-regexp-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec epa epg rfc6068 epg-config gnus-util
text-property-search time-date mm-decode mm-bodies mm-encode mail-parse
rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045
ietf-drums mm-util mail-prsvr mail-utils erc derived auth-source eieio
eieio-core password-cache json map format-spec erc-backend erc-networks
easy-mmode byte-opt bytecomp byte-compile erc-common inline erc-compat
cl-seq cl-macs gv pcase rx subr-x cl-loaddefs cl-lib erc-loaddefs rmc
iso-transl tooltip cconv eldoc paren electric uniquify ediff-hook
vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win
term/common-win x-dnd touch-screen tool-bar dnd fontset image regexp-opt
fringe tabulated-list replace newcomment text-mode lisp-mode prog-mode
register page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads dbusbind inotify lcms2 dynamic-setting system-font-setting
font-render-setting cairo gtk x-toolkit xinput2 x multi-tty move-toolbar
make-network-process emacs)

Memory information:
((conses 16 123590 9232) (symbols 48 10137 0) (strings 32 24791 2241)
 (string-bytes 1 837965) (vectors 16 14517)
 (vector-slots 8 204449 15354) (floats 8 24 31) (intervals 56 245 0)
 (buffers 984 10))

[0001-5.6-Use-caching-variant-of-erc-parse-prefix-internal.patch (text/x-patch, attachment)]

[0002-5.6-Fix-ISUPPORT-cache-misses-in-ERC-target-buffers.patch (text/x-patch, attachment)]

[0003-5.6-Rework-MODE-handling-in-ERC.patch (text/x-patch, attachment)]

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

From: "J.P." <jp <at> neverwas.me>
To: 67220 <at> debbugs.gnu.org
Cc: emacs-erc <at> gnu.org
Subject: Re: bug#67220: 30.0.50; ERC 5.6: Prefer parameter-driven MODE
 processing in ERC
Date: Fri, 17 Nov 2023 10:30:22 -0800

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

v2. Account for nonstandard CHANMODES beyond type D. Make mode-letter
handling more extensible and modular. Provide convenience macro for
caching processed data originating from ISUPPORT values. Retain original
parsed channel-mode data.

[0000-v1-v2.diff (text/x-patch, attachment)]

[0001-5.6-Make-wrangling-ISUPPORT-data-more-convenient-in-.patch (text/x-patch, attachment)]

[0002-5.6-Use-caching-variant-of-erc-parse-prefix-internal.patch (text/x-patch, attachment)]

[0003-5.6-Rework-MODE-processing-in-ERC.patch (text/x-patch, attachment)]

Message #13 received at 67220-done <at> debbugs.gnu.org (full text, mbox):

From: "J.P." <jp <at> neverwas.me>
To: 67220-done <at> debbugs.gnu.org
Cc: emacs-erc <at> gnu.org
Subject: Re: bug#67220: 30.0.50; ERC 5.6: Prefer parameter-driven MODE
 processing in ERC
Date: Sat, 18 Nov 2023 14:14:36 -0800

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

> v2. Account for nonstandard CHANMODES beyond type D. Make mode-letter
> handling more extensible and modular. Provide convenience macro for
> caching processed data originating from ISUPPORT values. Retain original
> parsed channel-mode data.

This has been installed as

  cca7956c82d * Favor ISUPPORT params for MODE processing in ERC

Closing for now.

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

From: "J.P." <jp <at> neverwas.me>
To: 67220 <at> debbugs.gnu.org
Cc: emacs-erc <at> gnu.org
Subject: Re: bug#67220: 30.0.50; ERC 5.6: Prefer parameter-driven MODE
 processing in ERC
Date: Tue, 21 Nov 2023 06:30:34 -0800

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

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

> "J.P." <jp <at> neverwas.me> writes:
>
>> v2. Account for nonstandard CHANMODES beyond type D. Make mode-letter
>> handling more extensible and modular. Provide convenience macro for
>> caching processed data originating from ISUPPORT values. Retain original
>> parsed channel-mode data.
>
> This has been installed as
>
>   cca7956c82d * Favor ISUPPORT params for MODE processing in ERC
>
> Closing for now.

Unfortunately, this latest round of changes messed up a pretty basic but
important aspect of channel-mode parsing. As a result, the ERC on HEAD
confuses modes that take parameters with those that don't. Worst case is
thought to be that strange values may be assigned to the variables
`erc-channel-user-limit' and `erc-channel-key'. Fix attached.

[0001-5.6-Don-t-associate-type-D-channel-modes-with-args-i.patch (text/x-patch, attachment)]

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

From: "J.P." <jp <at> neverwas.me>
To: 67220 <at> debbugs.gnu.org
Cc: emacs-erc <at> gnu.org
Subject: Re: bug#67220: 30.0.50; ERC 5.6: Prefer parameter-driven MODE
 processing in ERC
Date: Fri, 24 Nov 2023 14:13:15 -0800

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

> Unfortunately, this latest round of changes messed up a pretty basic but
> important aspect of channel-mode parsing. As a result, the ERC on HEAD
> confuses modes that take parameters with those that don't. Worst case is
> thought to be that strange values may be assigned to the variables
> `erc-channel-user-limit' and `erc-channel-key'. Fix attached.

The remedial changes mentioned have been lumped in with this commit:

  5bc84a0c9e4 * Cache UI string for channel modes in ERC

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

From: "J.P." <jp <at> neverwas.me>
To: 67220 <at> debbugs.gnu.org
Cc: emacs-erc <at> gnu.org
Subject: Re: bug#67220: 30.0.50; ERC 5.6: Prefer parameter-driven MODE
 processing in ERC
Date: Thu, 18 Jan 2024 17:21:29 -0800

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

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

> Tags: patch
>
> In the early days of IRC, parsing a "MODE" command from the server was
> comparatively straightforward. There were a few well known letters, some
> taking a single argument, and a standard set of status prefixes. But
> somewhere along the line, things got more complicated, and it seems ERC
> never got the memo. While it may appear obvious that sticking to a
> hard-coded, heuristics based approach doesn't really accommodate ERC's
> core tenet of extensibility, the risk of shifting toward something more
> parameter driven was probably never justifiable without a vocal demand.

In the initial set of changes, I only partially implemented PREFIX-aware
channel-membership handling (here and in bug#67677, for the formatting
side). The main reason for this omission was that I mistakenly assumed
the lack of a valid use case for doing so. However, a latent clue in our
own test suite attesting to the contrary was staring me in the face the
whole time (until I unceremoniously erased it [1]). Since then, I've
come around on this and now think we might as well see it through the
somewhat arduous last mile. See attached.

Thanks.

[1] https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=4939f413
    ^ Grep for "Yqaohv".

[0001-5.6-Actually-derive-channel-membership-from-PREFIX-i.patch (text/x-patch, attachment)]

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

From: "J.P." <jp <at> neverwas.me>
To: 67220 <at> debbugs.gnu.org
Cc: emacs-erc <at> gnu.org
Subject: Re: bug#67220: 30.0.50; ERC 5.6: Prefer parameter-driven MODE
 processing in ERC
Date: Thu, 25 Jan 2024 13:45:33 -0800

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

> "J.P." <jp <at> neverwas.me> writes:
>
>> Tags: patch
>>
>> In the early days of IRC, parsing a "MODE" command from the server was
>> comparatively straightforward. There were a few well known letters, some
>> taking a single argument, and a standard set of status prefixes. But
>> somewhere along the line, things got more complicated, and it seems ERC
>> never got the memo. While it may appear obvious that sticking to a
>> hard-coded, heuristics based approach doesn't really accommodate ERC's
>> core tenet of extensibility, the risk of shifting toward something more
>> parameter driven was probably never justifiable without a vocal demand.
>
> In the initial set of changes, I only partially implemented PREFIX-aware
> channel-membership handling (here and in bug#67677, for the formatting
> side). The main reason for this omission was that I mistakenly assumed
> the lack of a valid use case for doing so. However, a latent clue in our
> own test suite attesting to the contrary was staring me in the face the
> whole time (until I unceremoniously erased it [1]). Since then, I've
> come around on this and now think we might as well see it through the
> somewhat arduous last mile. See attached.
>
> Thanks.
>
> [1] https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=4939f413
>     ^ Grep for "Yqaohv".

The mentioned changes have been installed as:

  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=aedc8b55

This bug is already closed.

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

From: "J.P." <jp <at> neverwas.me>
To: 67220 <at> debbugs.gnu.org
Cc: emacs-erc <at> gnu.org
Subject: Re: bug#67220: 30.0.50; ERC 5.6: Prefer parameter-driven MODE
 processing in ERC
Date: Tue, 13 Feb 2024 17:45:32 -0800

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

This commit

  e69bd59ec59784b2f646e93355d4d63f41426cfc
  
  Honor arbitrary CHANTYPES in ERC
  
  * lisp/erc/erc.el (erc-channel-p): Favor "CHANTYPES" ISUPPORT item
  before falling back to well known prefixes.
  * test/lisp/erc/erc-tests.el (erc-channel-p): Add test.  Arbitrarily
  bundled with bug#60954.

introduced "smarter" handling of CHANTYPES but overlooked a subtlety
regarding how ERC interprets empty vs. missing ISUPPORT values. As
implied in a comment for the function `erc--parse-isupport-value',

  ;; https://tools.ietf.org/html/draft-brocklesby-irc-isupport-03#section-2
  ;;
  ;; > The server SHOULD send "X", not "X="; this is the normalized form.
  ;;
  ;; Note: for now, assume the server will only send non-empty values,

ERC punts on this. Indeed, it's always treated "X=" as having a value
and thus deserving of ("X" . "") in `erc-server-parameters', whereas
it's always seen "X" as more of a flag/switch with no associated value,
hence ("X").

It turns out a not entirely frivolous use case for abiding by that RFC
draft and *not* distinguishing between the two forms has arisen.
Basically, a server may choose to support no channels whatsoever for a
subset of clients, limiting them to direct messages only. To accommodate
this, ERC will need to interpret both "CHANTYPES" and "CHANTYPES=" as
expressing such a policy instead of sticking with its current behavior
of only doing so for the "=" form and treating "CHANTYPES" as equivalent
to ${default/fallback} (and thus also to "-CHANTYPES", which is clearly
wrong).

I think it's worth correcting this in ERC 5.6. Proposed changes
attached. (The first patch is unrelated.)

Thanks.

[0001-5.6-Ignore-the-TGT-LIST-parameter-in-erc-open.patch (text/x-patch, attachment)]

[0002-5.6-Normalize-ISUPPORT-params-with-empty-values-in-E.patch (text/x-patch, attachment)]

[0003-5.6-Use-modern-fallback-for-channel-name-detection-i.patch (text/x-patch, attachment)]

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

From: "J.P." <jp <at> neverwas.me>
To: 67220 <at> debbugs.gnu.org
Cc: emacs-erc <at> gnu.org
Subject: Re: bug#67220: 30.0.50; ERC 5.6: Prefer parameter-driven MODE
 processing in ERC
Date: Tue, 20 Feb 2024 17:14:39 -0800

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

> It turns out a not entirely frivolous use case for abiding by that RFC
> draft and *not* distinguishing between the two forms has arisen.
> Basically, a server may choose to support no channels whatsoever for a
> subset of clients, limiting them to direct messages only. To accommodate
> this, ERC will need to interpret both "CHANTYPES" and "CHANTYPES=" as
> expressing such a policy instead of sticking with its current behavior
> of only doing so for the "=" form and treating "CHANTYPES" as equivalent
> to ${default/fallback} (and thus also to "-CHANTYPES", which is clearly
> wrong).
>
> I think it's worth correcting this in ERC 5.6. Proposed changes
> attached. (The first patch is unrelated.)

These changes now live on master as

  3d87e343276 * Use modern fallback for channel name detection in ERC
  25d15391f26 * Normalize ISUPPORT params with empty values in ERC

If anyone experiences new difficulties related to detecting channel
names, these are likely to blame.

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

From: "J.P." <jp <at> neverwas.me>
To: 67220 <at> debbugs.gnu.org
Cc: emacs-erc <at> gnu.org
Subject: Re: bug#67220: 30.0.50; ERC 5.6: Prefer parameter-driven MODE
 processing in ERC
Date: Sat, 13 Apr 2024 15:17:00 -0700

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

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

>> I think it's worth correcting this in ERC 5.6. Proposed changes
>> attached. (The first patch is unrelated.)
>
> These changes now live on master as
>
>   3d87e343276 * Use modern fallback for channel name detection in ERC
>   25d15391f26 * Normalize ISUPPORT params with empty values in ERC
>
> If anyone experiences new difficulties related to detecting channel
> names, these are likely to blame.

A regression involving `erc-query-buffer-p' has surfaced that basically
makes the function unsuable in many situations. The root cause is some
combination of stupdiity and laziness on my part, as usual. The attached
patch should fix the issue. Thanks to Libera user mekeor for reporting
this.

[0001-Fix-regression-involving-erc-query-buffer-p.patch (text/x-patch, attachment)]

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

From: "J.P." <jp <at> neverwas.me>
To: 67220 <at> debbugs.gnu.org
Cc: emacs-erc <at> gnu.org
Subject: Re: bug#67220: 30.0.50; ERC 5.6: Prefer parameter-driven MODE
 processing in ERC
Date: Tue, 23 Apr 2024 15:35:42 -0700

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

> "J.P." <jp <at> neverwas.me> writes:
>
>>> I think it's worth correcting this in ERC 5.6. Proposed changes
>>> attached. (The first patch is unrelated.)
>>
>> These changes now live on master as
>>
>>   3d87e343276 * Use modern fallback for channel name detection in ERC
>>   25d15391f26 * Normalize ISUPPORT params with empty values in ERC
>>
>> If anyone experiences new difficulties related to detecting channel
>> names, these are likely to blame.
>
> A regression involving `erc-query-buffer-p' has surfaced that basically
> makes the function unsuable in many situations. The root cause is some
> combination of stupdiity and laziness on my part, as usual. The attached
> patch should fix the issue. Thanks to Libera user mekeor for reporting
> this.

This has been installed as

  473189ab690 Fix regression involving erc-query-buffer-p

This bug is already closed.

Previous Next

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