GNU bug report logs - #46775
27.1; ERC: Track: Modified channels doc and `erc-track-find-face' fixes

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 46775 in the body.
You can then email your comments to 46775 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: Olivier Certner <ocert.dev <at> free.fr>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.1;
 ERC: Track: Modified channels doc and `erc-track-find-face' fixes
Date: Thu, 25 Feb 2021 18:14:25 +0100

Several small documentation changes and one minor fix.  List of (separate) 
changes:
1. Fix documentation of `erc-modified-channels-alist' about its structure.
2. Fix documentation of `erc-modified-channels-alist' about which channels it 
contains and when.  Add references to docstring of `erc-make-mode-line-buffer-
name'.
3. Rewrite `erc-track-find-face' to clarify what it does (& minor performance 
improvement; no functional changes).
4. `erc-track-modified-channels': Fix a perceived bug in the use of `erc-
track-find-face' (changes mode line faces selection in some cases).

URL and branch of the repository having the corresponding patches to be posted 
after the bug is open. Changes rebased on master.

-- 
Olivier Certner

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

From: Olivier Certner <ocert.dev <at> free.fr>
To: 46775 <at> debbugs.gnu.org
Subject: 27.1;
 ERC: Track: Modified channels doc and `erc-track-find-face' fixes
Date: Thu, 25 Feb 2021 18:24:28 +0100

Pull request showing the commits:
https://github.com/OlCe2/emacs/pull/1

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Olivier Certner <ocert.dev <at> free.fr>
Cc: 46775 <at> debbugs.gnu.org
Subject: Re: bug#46775: 27.1; ERC: Track: Modified channels doc and
 `erc-track-find-face' fixes
Date: Fri, 26 Feb 2021 09:51:25 +0100

Olivier Certner <ocert.dev <at> free.fr> writes:

> Pull request showing the commits:
> https://github.com/OlCe2/emacs/pull/1

Can you post the patch here in the bug tracker?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Olivier Certner <ocert.dev <at> free.fr>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 46775 <at> debbugs.gnu.org
Subject: Re: bug#46775: 27.1;
 ERC: Track: Modified channels doc and `erc-track-find-face' fixes
Date: Fri, 26 Feb 2021 10:03:23 +0100

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

> Can you post the patch here in the bug tracker?

Sure. Attached (there are 4 patches, hopefully making it easier to understand 
the changes).

-- 
Olivier Certner

[0001-ERC-Track-Fix-modified-channels-alist-s-documentatio.patch (text/x-patch, attachment)]

[0002-ERC-Track-Clarify-documentation-on-tracked-buffers-a.patch (text/x-patch, attachment)]

[0003-ERC-Track-Rewrite-and-rename-erc-track-find-face.patch (text/x-patch, attachment)]

[0004-ERC-Track-Fix-a-perceived-minor-bug-in-mode-line-fac.patch (text/x-patch, attachment)]

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

From: "J.P." <jp <at> neverwas.me>
To: Olivier Certner <ocert.dev <at> free.fr>
Cc: emacs-erc <at> gnu.org, 46775 <at> debbugs.gnu.org
Subject: Re: bug#46775: 27.1; ERC: Track: Modified channels doc and
 `erc-track-find-face' fixes
Date: Wed, 09 Jun 2021 06:07:17 -0700

Olivier Certner <ocert.dev <at> free.fr> writes:

> Several small documentation changes and one minor fix.  List of (separate) 
> changes:
> 1. Fix documentation of `erc-modified-channels-alist' about its structure.
> 2. Fix documentation of `erc-modified-channels-alist' about which channels it 
> contains and when.  Add references to docstring of `erc-make-mode-line-buffer-
> name'.

The bit about the structure makes perfect sense, and the revised
language is much clearer and easier to understand, at least to me.

> 3. Rewrite `erc-track-find-face' to clarify what it does (& minor performance 
> improvement; no functional changes).

The updates to the doc string here are likewise superb and make things
crystal clear.

Although sometimes, for drooling cretins like me, a unit test is worth a
thousand words: way easier to take in the whole cause-and-effect of it
all (push lever -> get pellet). Also, as a native EngRish speaker, I'm
only 51% sure you meant s/necessary means/necessarily means/ in that
last paragraph. In general though, I feel little optimizations like this
are more than welcome because this stuff runs more or less constantly.

> 4. `erc-track-modified-channels': Fix a perceived bug in the use of `erc-
> track-find-face' (changes mode line faces selection in some cases).

For this one, something like a detailed repro would be nice. But I
suppose that's rather involved/tricky without fancier tooling. So I'll
just take your word for it because (1) it sounds plausible and (2)
you're way more familiar with this module than I.

Regarding this module generally, it's definitely another one whose
layout confuses me. For example, I guess we can't add keymaps to minor
modes defined with `define-erc-module'? IOW, why the
`erc-track-minor-mode-map' instead of a `erc-track-mode-map'? I'm also
confused by the 001 RPL_WELCOME hook, but for now, I'll just assume it's
a vestige of a simpler time when `erc-user-is-active' looked different
and maybe did more. Or (more likely) there's something I'm not seeing
just yet. Anyway, I mention this stuff because you may be the best/only
qualified person to address such concerns at present. Would you be
willing to audit this file for redundancy/cruft/obsolescence at some
point before the next release? Thanks.

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

From: Olivier Certner <olce.emacs <at> certner.fr>
To: 46775 <at> debbugs.gnu.org
Subject: Updated patches
Date: Tue, 06 Jul 2021 16:23:14 +0200

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

Patches slightly edited (commit message, doc text) and rebased on top of a 
recent 'master'. Ready to apply. Easy to backport to 27 as well.

-- 
Olivier Certner

[0001-ERC-Track-Fix-modified-channels-alist-s-documentatio.patch (text/x-patch, attachment)]

[0002-ERC-Track-Clarify-documentation-on-tracked-buffers-a.patch (text/x-patch, attachment)]

[0003-ERC-Track-Rewrite-and-rename-erc-track-find-face.patch (text/x-patch, attachment)]

[0004-ERC-Track-Fix-a-perceived-minor-bug-in-mode-line-fac.patch (text/x-patch, attachment)]

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

From: Olivier Certner <ocert.dev <at> free.fr>
To: "J.P." <jp <at> neverwas.me>
Cc: emacs-erc <at> gnu.org, 46775 <at> debbugs.gnu.org
Subject: Re: bug#46775: 27.1;
 ERC: Track: Modified channels doc and `erc-track-find-face' fixes
Date: Tue, 06 Jul 2021 18:34:48 +0200

> The updates to the doc string here are likewise superb and make things
> crystal clear.

Thanks!
 
> only 51% sure you meant s/necessary means/necessarily means/ in that

I fixed the typo in the updated patch.

> Regarding this module generally, it's definitely another one whose
> layout confuses me. For example, I guess we can't add keymaps to minor
> modes defined with `define-erc-module'? IOW, why the
> `erc-track-minor-mode-map' instead of a `erc-track-mode-map'?

The trick is that `erc-track-minor-mode' may be enabled or disabled 
independently of the whole module. In particular, it may not be activated at 
all if the (global) keybindings it wants to install, which don't abide by the 
rules for minor modes maps, are already bound at the time of module's 
activation.

> I'm also confused by the 001 RPL_WELCOME hook, but for now, I'll just assume
> it's a vestige of a simpler time when `erc-user-is-active' looked different
> and maybe did more. Or (more likely) there's something I'm not seeing
> just yet.

`erc-user-is-active' seems in fact to be used for more than one purpose. In 
particular, it also triggers an update of channels in the mode line. What's 
exactly the problem with hooking on 001?

> Anyway, I mention this stuff because you may be the best/only
> qualified person to address such concerns at present. Would you be
> willing to audit this file for redundancy/cruft/obsolescence at some
> point before the next release? Thanks.

Depending on available time and precise concerns that you may have, yes.

There is a lot of room for improvement in this module. In particular in 
determining how some user is "active" (none of the current possibilities 
really suits my needs).

-- 
Olivier Certner

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

From: "J.P." <jp <at> neverwas.me>
To: Olivier Certner <ocert.dev <at> free.fr>
Cc: emacs-erc <at> gnu.org, 46775 <at> debbugs.gnu.org
Subject: Re: bug#46775: 27.1; ERC: Track: Modified channels doc and
 `erc-track-find-face' fixes
Date: Wed, 07 Jul 2021 05:30:41 -0700

Olivier Certner <ocert.dev <at> free.fr> writes:

> The trick is that `erc-track-minor-mode' may be enabled or disabled 
> independently of the whole module. In particular, it may not be activated at 
> all if the (global) keybindings it wants to install, which don't abide by the 
> rules for minor modes maps, are already bound at the time of module's 
> activation.

Right, thanks for patiently explaining an RTFM-worthy question. So like
the doc string says: "for the sole purpose of providing the C-c C-SPC
and C-c C-@ keybindings" (basically for toggling during sessions). I
guess I was dazed into a stupor by encountering two global minor modes
for the same module with only the word "-minor" to distinguish them.

Anyway, it's obviously decades too late to reconsider modules doubling
as minor modes. But perhaps it's still worth reexamining
`erc-update-modules' at some point. As a quick aside:

Currently, we rely on a module's options for conveying per-connection
specifics, when supported. Some options, like `erc-enable-logging', are
buffer local but require extra attention to exploit on a per-connection
basis. And while the macro `erc-define-module' does have a `local-p'
param, I don't think it's actually being used anywhere. This is probably
in part due to `erc-update-modules' only ever enabling (and never
disabling) modules and also running just a hair too early, i.e., right
before a new ERC buffer is made current.

Perhaps a discussion dedicated to this topic may be beneficial at some
point.

> `erc-user-is-active' seems in fact to be used for more than one purpose. In 
> particular, it also triggers an update of channels in the mode line. What's 
> exactly the problem with hooking on 001?

The 001 hook only runs in server buffers. And when it does,
`erc-server-connected' is always nil.

> There is a lot of room for improvement in this module. In particular in 
> determining how some user is "active" (none of the current possibilities 
> really suits my needs).

Then hopefully we can (eventually) make it smarter. I get the feeling
this module, when fully realized, might provide an experience
that's actually superior to that offered by dedicated clients.

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

From: Amin Bandali <bandali <at> gnu.org>
To: Olivier Certner <olce.emacs <at> certner.fr>
Cc: 46775-done <at> debbugs.gnu.org
Subject: Re: bug#46775: 27.1; ERC: Track: Modified channels doc and
 `erc-track-find-face' fixes
Date: Sun, 12 Sep 2021 01:25:19 -0400

Hi Olivier,

Many thanks for your patches, and my apologies it's taken so long to get
them merged.

Olivier Certner writes:

> Patches slightly edited (commit message, doc text) and rebased on top of a 
> recent 'master'. Ready to apply. Easy to backport to 27 as well.

I applied (1) and (2) to 'emacs-27' and then merged that into 'master',
then applied (3) and (4) on 'master'; each with a few small tweaks.

Thanks again,
amin

Previous Next

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