GNU bug report logs - #64301
30.0.50; ERC 5.6: Make speaker labels easier to work with

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 64301 in the body.
You can then email your comments to 64301 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: Make speaker labels easier to work with
Date: Mon, 26 Jun 2023 06:50:17 -0700

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

Tags: patch

Currently, modules that operate on inserted messages use heuristics,
like delimiting characters and face properties, to find the bounds of
the "speaker label" (my term for the stylized nick prepended to the
start of every displayed message). I think it's worth making these
boundaries easier and more reliable to detect. At a bare minimum, having
a specific text property or field spanning the nick portion should
provide enough information to identify other regions of interest
preceding the actual message (in most cases). The attached patch
attempts to do this.

However, it may be useful in the long run to provide an internal
interface for influencing how this happens. Use cases include hiding or
altering bookend styling (currently hard-coded to angle brackets) and
using alternate display names for nicks themselves. One approach for
implementing something like this was proposed in an iteration of the
change set from bug#60933 [1] but was ultimately removed prior to
installation.

The attached patch also fixes a somewhat related bug discovered by
incal. From emacs -Q:

  - Set `erc-format-nick-function' to `erc-format-@nick'
  - Customize `erc-nick-prefix-face' to make it easily noticeable
  - Connect to some network
  - Create a new dummy channel and say something
  - Notice that your speaker nick is decorated with the status prefix
    "@" but that it lacks the face changes you made earlier

The only thing giving me pause about this bug is that there's no
(surviving) mention of it in the usual places, and the offending code
has been around since at least 2006. So, it's possible we're still
missing something here.

Thanks.

[1] https://lists.gnu.org/archive/html/emacs-erc/2023-04/msg00018.html
    See `erc--format-speaker-functions' in the third patch
    0003-5.6-Use-getter-for-finding-users-in-erc-server-PRIVM.patch.


In GNU Emacs 30.0.50 (build 4, x86_64-pc-linux-gnu, GTK+ Version
 3.24.37, cairo version 1.17.6) of 2023-06-25 built on localhost
Repository revision: a6de0d22e4209e2c75dbf1e8c005dfc9d8c64cce
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
  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 derived 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 auth-source cl-seq
eieio eieio-core cl-macs password-cache json subr-x map format-spec
cl-loaddefs cl-lib erc-backend erc-networks byte-opt gv bytecomp
byte-compile erc-common erc-compat 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 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
move-toolbar gtk x-toolkit xinput2 x multi-tty make-network-process
emacs)

Memory information:
((conses 16 64551 8809) (symbols 48 8582 0) (strings 32 23258 1697)
 (string-bytes 1 673861) (vectors 16 15002)
 (vector-slots 8 207268 8157) (floats 8 24 37) (intervals 56 231 0)
 (buffers 976 10))

[0001-5.6-Don-t-clobber-erc-nick-prefix-face.patch (text/x-patch, attachment)]

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

From: "J.P." <jp <at> neverwas.me>
To: 64301 <at> debbugs.gnu.org
Cc: emacs-erc <at> gnu.org
Subject: Re: bug#64301: 30.0.50; ERC 5.6: Make speaker labels easier to work
 with
Date: Wed, 05 Jul 2023 07:03:16 -0700

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

v2. Add `erc-ctcp' text property to inserted CTCP ACTION messages.
Demote `erc-fill-spaced-commands' from a user option to an internal
variable. Dedent action messages with module `fill-wrap'. Combine faces
in `erc-display-message' when called with a list `type' arg. Have
`erc-put-text-property' conditionally combine prop values instead of
clobber. Apply `invisible' property to white space around stamps.


This iteration includes a number of changes, the most important being
one that looks severe but is largely mechanical. It concerns a bug
that's been around forever and, as such, is now part of the foundation.
Basically, the `type' parameter of the function `erc-display-message'
determines a special handler that applies styling to a message. When the
parameter is a list, the function calls each handler in turn, left to
right, which clobbers existing faces instead of combines them. The net
effect (in most cases) is identical to calling the function with the
last member alone. Despite this, it would seem ending up with multiple,
"layered" faces was the authors' original intent based on the ubiquity
of call sites featuring the list variant.

Authoritative intent aside, in the intervening decades, other code was
written that expects the current clobbering behavior. An example of this
exists in the default value of `erc-track-faces-priority-list', which
contains `erc-error-face' alone rather than paired with
`erc-notice-face'. There may indeed be others that will go undetected
should we make this "fix," which I'm still in favor of despite the
attendant risk. Though improved call-site readability is one upside, I'm
more interested in the UX possibilities such layering opens up. Subtle
benefits can already be seen after applying these changes, for example,
in text inserted for outgoing /me commands, which now sport a
combination of `erc-input-face' and `erc-action-face'.

In a bid to mitigate potential breakage, at least internally, I've gone
ahead and mass-replaced all instances of

  (erc-display-message parsed '(notice error) ...)

with

  (erc-display-message parsed 'error ...)

which amounts to 38 incisions in total. Once again, if we leave things
as is, users who've customized `erc-track-faces-priority-list' won't
ever see error-colored text in their mode line, which is a deal breaker.

The other major change involves making time stamps more sensitive to
existing invisible text by combining `stamp'-owned spec members with
existing properties and by extending those propertized areas to include
surrounding white space. Previously, the `stamp' module went to great
lengths to avoid applying stamps to invisible messages entirely.
However, this policy caused uneven output in logs with `left-handed'
stamps and in formerly hidden messages mentioning designated "fools".
It's true that what I'm proposing would mark a complete reversal, but
it's not as drastic as it sounds. To see the effect, do something like

  (setq erc-insert-timestamp-function #'erc-insert-timestamp-left
        erc-timestamp-only-if-changed-flag nil
        erc-text-matched-hook '(erc-log-matches erc-hide-fools)
        erc-fools '("somebot"))

After connecting, say something to trigger "somebot" while ensuring
`erc-toggle-timestamps' still works. Then do

  M-: (remove-from-invisibility-spec 'erc-match) RET

and notice that the revealed text has timestamps in all the right
places. And if you have the `log' module enabled, notice that stamps are
likewise present on all messages.

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

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

[0002-5.6-Simplify-erc-button-add-nickname-buttons.patch (text/x-patch, attachment)]

[0003-5.6-Add-text-props-for-CTCPs-and-speakers-in-ERC.patch (text/x-patch, attachment)]

[0004-5.6-Handle-composite-faces-better-in-erc-display-mes.patch (text/x-patch, attachment)]

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

From: "J.P." <jp <at> neverwas.me>
To: 64301 <at> debbugs.gnu.org
Cc: emacs-erc <at> gnu.org
Subject: Re: bug#64301: 30.0.50; ERC 5.6: Make speaker labels easier to work
 with
Date: Sat, 08 Jul 2023 07:19:26 -0700

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

v3. Fix problem calculating column width. Add command to toggle fool
invisibility. Add test for hidden date stamps.

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

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

[0002-5.6-Simplify-erc-button-add-nickname-buttons.patch (text/x-patch, attachment)]

[0003-5.6-Add-text-props-for-CTCPs-and-speakers-in-ERC.patch (text/x-patch, attachment)]

[0004-5.6-Handle-composite-faces-better-in-erc-display-mes.patch (text/x-patch, attachment)]

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

From: "J.P." <jp <at> neverwas.me>
To: 64301-done <at> debbugs.gnu.org
Cc: emacs-erc <at> gnu.org
Subject: Re: bug#64301: 30.0.50; ERC 5.6: Make speaker labels easier to work
 with
Date: Thu, 13 Jul 2023 19:20:08 -0700

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

> v2. [...] Combine faces in `erc-display-message' when called with a
> list `type' arg.
> [...]
>
> This iteration includes a number of changes, the most important being
> one that looks severe but is largely mechanical. It concerns a bug
> that's been around forever and, as such, is now part of the foundation.
> Basically, the `type' parameter of the function `erc-display-message'
> determines a special handler that applies styling to a message. When the
> parameter is a list, the function calls each handler in turn, left to
> right, which clobbers existing faces instead of combines them. The net
> effect (in most cases) is identical to calling the function with the
> last member alone. Despite this, it would seem ending up with multiple,
> "layered" faces was the authors' original intent based on the ubiquity
> of call sites featuring the list variant.
>
> Authoritative intent aside, in the intervening decades, other code was
> written that expects the current clobbering behavior. An example of this
> exists in the default value of `erc-track-faces-priority-list', which
> contains `erc-error-face' alone rather than paired with
> `erc-notice-face'. There may indeed be others that will go undetected
> should we make this "fix," which I'm still in favor of despite the
> attendant risk.

Actually, I decided against doing this after all [1]. There was simply no
telling what kind of damage might result from these changes rippling
through two decades of third-party code.

> Though improved call-site readability is one upside, I'm more
> interested in the UX possibilities such layering opens up. Subtle
> benefits can already be seen after applying these changes, for
> example, in text inserted for outgoing /me commands, which now sport a
> combination of `erc-input-face' and `erc-action-face'.
>
> In a bid to mitigate potential breakage, at least internally, I've gone
> ahead and mass-replaced all instances of

>   (erc-display-message parsed '(notice error) ...)

> with

>   (erc-display-message parsed 'error ...)

> which amounts to 38 incisions in total. Once again, if we leave things
> as is, users who've customized `erc-track-faces-priority-list' won't
> ever see error-colored text in their mode line, which is a deal breaker.

Instead of all that, I've implemented an uglier and less transparent
(but also non-disruptive) way for `erc-display-message' to perform such
layering. Basically, callers can request the effect by specifying a
slight variant of its list `type' parameter, namely, one headed by the
symbol t, e.g,

  (erc-display-message parsed '(t notice error) ...)

Without the t, this function acts as it always has. With it, the
function composes faces atop one another, left to right. Existing call
sites obviously won't benefit, but at least they'll be shielded from
harm. Hope that's an agreeable compromise.

Thanks and closing.

[1] https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=d45770e8

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

From: "J.P." <jp <at> neverwas.me>
To: 64301-done <at> debbugs.gnu.org
Cc: emacs-erc <at> gnu.org
Subject: Re: bug#64301: 30.0.50; ERC 5.6: Make speaker labels easier to work
 with
Date: Sat, 15 Jul 2023 07:05:51 -0700

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

Some functionality I introduced for wrangling text properties in
erc-match.el was shoddily done and not very usable from the perspective
of another module building on erc-match as a foundation. The attached
patch attempts to correct that. It also ditches "erc-" namespacing for
invisibility spec members because the potential for collision seems
pretty low given that we're only talking ERC buffers. (Hopefully, I'm
not missing any downsides here.)

[0001-5.6-Don-t-bother-namespacing-ERC-s-own-invisibility-.patch (text/x-patch, attachment)]

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

From: "J.P." <jp <at> neverwas.me>
To: 64301-done <at> debbugs.gnu.org
Cc: emacs-erc <at> gnu.org
Subject: Re: bug#64301: 30.0.50; ERC 5.6: Make speaker labels easier to work
 with
Date: Thu, 20 Jul 2023 06:29:13 -0700

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

v2 (invisibility API). Move primary message-hiding function to erc.el.
Invert meaning of invisible-bounds switch and make old non-nil behavior
new default.

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

[0001-5.6-Improve-ERC-s-internal-invisibility-API.patch (text/x-patch, attachment)]

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

From: "J.P." <jp <at> neverwas.me>
To: 64301 <at> debbugs.gnu.org
Cc: emacs-erc <at> gnu.org
Subject: Re: bug#64301: 30.0.50; ERC 5.6: Make speaker labels easier to work
 with
Date: Sun, 23 Jul 2023 07:00:32 -0700

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

> v2 (invisibility API). Move primary message-hiding function to erc.el.
> Invert meaning of invisible-bounds switch and make old non-nil behavior
> new default.

This was added as af547c4bbe8 "Improve ERC's internal invisibility API".

Previous Next

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