unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#46775: 27.1; ERC: Track: Modified channels doc and `erc-track-find-face' fixes
@ 2021-02-25 17:14 Olivier Certner
  2021-02-25 17:24 ` Olivier Certner
  2021-06-09 13:07 ` J.P.
  0 siblings, 2 replies; 5+ messages in thread
From: Olivier Certner @ 2021-02-25 17:14 UTC (permalink / raw)
  To: 46775

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







^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#46775: 27.1; ERC: Track: Modified channels doc and `erc-track-find-face' fixes
  2021-02-25 17:14 bug#46775: 27.1; ERC: Track: Modified channels doc and `erc-track-find-face' fixes Olivier Certner
@ 2021-02-25 17:24 ` Olivier Certner
  2021-02-26  8:51   ` Lars Ingebrigtsen
  2021-06-09 13:07 ` J.P.
  1 sibling, 1 reply; 5+ messages in thread
From: Olivier Certner @ 2021-02-25 17:24 UTC (permalink / raw)
  To: 46775

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








^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#46775: 27.1; ERC: Track: Modified channels doc and `erc-track-find-face' fixes
  2021-02-25 17:24 ` Olivier Certner
@ 2021-02-26  8:51   ` Lars Ingebrigtsen
  2021-02-26  9:03     ` Olivier Certner
  0 siblings, 1 reply; 5+ messages in thread
From: Lars Ingebrigtsen @ 2021-02-26  8:51 UTC (permalink / raw)
  To: Olivier Certner; +Cc: 46775

Olivier Certner <ocert.dev@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





^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#46775: 27.1; ERC: Track: Modified channels doc and `erc-track-find-face' fixes
  2021-02-26  8:51   ` Lars Ingebrigtsen
@ 2021-02-26  9:03     ` Olivier Certner
  0 siblings, 0 replies; 5+ messages in thread
From: Olivier Certner @ 2021-02-26  9:03 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 46775

[-- Attachment #1: Type: text/plain, Size: 164 bytes --]

> 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

[-- Attachment #2: 0001-ERC-Track-Fix-modified-channels-alist-s-documentatio.patch --]
[-- Type: text/x-patch, Size: 1862 bytes --]

From 868b042845211ced41cf751ac842e7d0b82da188 Mon Sep 17 00:00:00 2001
From: Olivier Certner <olce.emacs@certner.fr>
Date: Wed, 13 Jan 2021 17:39:54 +0100
Subject: [PATCH 1/4] ERC: Track: Fix modified channels alist's documentation
 about its structure

* lisp/erc/erc-track.el (erc-modified-channels-alist): Fix the
docstring, which reports a proper list structure whereas it is a
dotted list, where the last cdr is sometimes a proper list, making it
sometimes a proper list.
---
 lisp/erc/erc-track.el | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/lisp/erc/erc-track.el b/lisp/erc/erc-track.el
index 56f66563ad..e53f7fb22d 100644
--- a/lisp/erc/erc-track.el
+++ b/lisp/erc/erc-track.el
@@ -280,14 +280,17 @@ nil            - don't add to mode line."
 
 (defvar erc-modified-channels-alist nil
   "An ALIST used for tracking channel modification activity.
-Each element looks like (BUFFER COUNT FACE) where BUFFER is a buffer
-object of the channel the entry corresponds to, COUNT is a number
-indicating how often activity was noticed, and FACE is the face to use
-when displaying the buffer's name.  See `erc-track-faces-priority-list',
-and `erc-track-showcount'.
+Each element is a list with form (BUFFER COUNT . FACE) where
+BUFFER is a buffer object of the channel the entry corresponds
+to, COUNT is a number indicating how often activity was noticed,
+and FACE is a face (or a list of faces) to use when displaying the
+buffer's name.
 
 Entries in this list should only happen for buffers where activity occurred
-while the buffer was not visible.")
+while the buffer was not visible.
+
+See also `erc-track-faces-priority-list',
+`erc-track-faces-normal-list' and `erc-track-showcount'.")
 
 (defcustom erc-track-showcount nil
   "If non-nil, count of unseen messages will be shown for each channel."
-- 
2.30.0


[-- Attachment #3: 0002-ERC-Track-Clarify-documentation-on-tracked-buffers-a.patch --]
[-- Type: text/x-patch, Size: 3058 bytes --]

From c3a0486a19e442d84596ce6b595b111afcd9b9d1 Mon Sep 17 00:00:00 2001
From: Olivier Certner <olce.emacs@certner.fr>
Date: Thu, 14 Jan 2021 18:20:26 +0100
Subject: [PATCH 2/4] ERC: Track: Clarify documentation on tracked buffers and
 add references

* lisp/erc/erc-track.el (erc-modified-channels-alist): Clarify what
the list contains.  Add references to variables and functions involved
in displaying tracked buffers from this list.

* lisp/erc/erc-track.el (erc-make-mode-line-buffer-name): Describe
exactly the algorithm, and reference custom variables that influence
it.
---
 lisp/erc/erc-track.el | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/lisp/erc/erc-track.el b/lisp/erc/erc-track.el
index e53f7fb22d..8b9f88ee2a 100644
--- a/lisp/erc/erc-track.el
+++ b/lisp/erc/erc-track.el
@@ -283,14 +283,19 @@ nil            - don't add to mode line."
 Each element is a list with form (BUFFER COUNT . FACE) where
 BUFFER is a buffer object of the channel the entry corresponds
 to, COUNT is a number indicating how often activity was noticed,
-and FACE is a face (or a list of faces) to use when displaying the
-buffer's name.
+and FACE is a face (or a list of faces, combined as usual) to use
+when displaying the buffer's name in the mode line.
 
-Entries in this list should only happen for buffers where activity occurred
-while the buffer was not visible.
+Entries in this list are only added/updated for buffers that were
+not visible when activity occurred in them, and are removed for
+each buffer as soon as it becomes visible again (or if the server
+is disconnected, provided `erc-track-remove-disconnected-buffers'
+is true).
 
-See also `erc-track-faces-priority-list',
-`erc-track-faces-normal-list' and `erc-track-showcount'.")
+For how the face is chosen for a buffer, see
+`erc-track-find-face' and `erc-track-priority-faces-only'.  For
+how buffers are then displayed in the mode line, see
+`erc-modified-channels-display'.")
 
 (defcustom erc-track-showcount nil
   "If non-nil, count of unseen messages will be shown for each channel."
@@ -652,8 +657,14 @@ ARGS are ignored."
   "The face to use when mouse is over channel names in the mode line.")
 
 (defun erc-make-mode-line-buffer-name (string buffer &optional faces count)
-  "Return STRING as a button that switches to BUFFER when clicked.
-If FACES are provided, color STRING with them."
+  "Returns a button that switches to BUFFER when clicked.
+STRING is the string in the button.  It is possibly suffixed with
+the number of unread messages, according to variables
+`erc-track-showcount' and `erc-track-showcount-string'.
+
+If `erc-track-use-faces' is true and FACES are provided, format
+STRING with them. When the mouse hovers above the button, STRING
+is displayed according to `erc-track-mouse-face'."
   ;; We define a new sparse keymap every time, because 1. this data
   ;; structure is very small, the alternative would require us to
   ;; defvar a keymap, 2. the user is not interested in customizing it
-- 
2.30.0


[-- Attachment #4: 0003-ERC-Track-Rewrite-and-rename-erc-track-find-face.patch --]
[-- Type: text/x-patch, Size: 6250 bytes --]

From 548496c384ce115d92554604ab6ecc28e35ac190 Mon Sep 17 00:00:00 2001
From: Olivier Certner <olce.emacs@certner.fr>
Date: Thu, 14 Jan 2021 18:26:38 +0100
Subject: [PATCH 3/4] ERC: Track: Rewrite and rename `erc-track-find-face'

For clarification purposes.  No functional changes.

* lisp/erc/erc-track.el (erc-track-find-face): Rename it to
`erc-track-select-face'.  Rewrite it so that it is very clear what is
the current algorithm, changing the parameters it takes.  No
functional changes. Performance improvements.  Clarify the
documentation and remove the part on some faces being lists, which
clearly doesn't apply.

* lisp/erc/erc-track.el (erc-track-modified-channels): Replace calls
to `erc-track-find-face' by calls to `erc-track-select-face',
preserving the existing behavior.

* lisp/erc/erc-track.el (erc-modified-channels-alist): Change the
reference to `erc-track-select-face' in the documentation following
the rename.
---
 lisp/erc/erc-track.el | 81 ++++++++++++++++++++++++-------------------
 1 file changed, 46 insertions(+), 35 deletions(-)

diff --git a/lisp/erc/erc-track.el b/lisp/erc/erc-track.el
index 8b9f88ee2a..d596685fee 100644
--- a/lisp/erc/erc-track.el
+++ b/lisp/erc/erc-track.el
@@ -293,7 +293,7 @@ is disconnected, provided `erc-track-remove-disconnected-buffers'
 is true).
 
 For how the face is chosen for a buffer, see
-`erc-track-find-face' and `erc-track-priority-faces-only'.  For
+`erc-track-select-face' and `erc-track-priority-faces-only'.  For
 how buffers are then displayed in the mode line, see
 `erc-modified-channels-display'.")
 
@@ -760,38 +760,45 @@ Use `erc-make-mode-line-buffer-name' to create buttons."
   (when (called-interactively-p 'interactive)
     (erc-modified-channels-display)))
 
-(defun erc-track-find-face (faces)
-  "Return the face to use in the mode line from the faces in FACES.
-If `erc-track-faces-priority-list' is set, the one from FACES who
-is first in that list will be used.  If nothing matches or if
-`erc-track-faces-priority-list' is not set, the default mode-line
-faces will be used.
+(defun erc-track-select-face (cur-face new-faces)
+  "Return the face to use in the mode line.
 
-If `erc-track-faces-normal-list' is non-nil, use it to produce a
-blinking effect that indicates channel activity when the first
-element in FACES and the highest-ranking face among the rest of
-FACES are both members of `erc-track-faces-normal-list'.
+CUR-FACE is the face currently used in the mode line (for the
+current buffer).  NEW-FACES is the list of new faces that have
+just been seen (in the current buffer).
 
-If one of the faces is a list, then it will be ranked according
-to its highest-tanking face member.  A list of faces including
-that member will take priority over just the single member
-element."
+Initially, the selected face is the one with highest priority in
+`erc-track-faces-priority-list' (i.e., the one closest to the
+head of the list) among CUR-FACE and NEW-FACES.  If nothing
+matches (including if `erc-track-faces-priority-list' is not
+set), the default mode-line faces will be used (NIL is returned).
+
+If the selected face is still CUR-FACE (highest priority), and
+the highest priority face in NEW-FACES alone is different (which
+necessary means it has lower priority than CUR-FACE), and both
+are in `erc-track-faces-normal-list', then the latter is selected
+instead.  This has the effect of allowing the current mode line
+face, if a member of `erc-track-faces-normal-list', to be
+replaced by another with lower priority from the new faces, if
+that with highest priority in the new ones is also a member of
+`erc-track-faces-normal-list'."
   (let ((choice (catch 'face
-		  (dolist (candidate erc-track-faces-priority-list)
-		    (when (member candidate faces)
-		      (throw 'face candidate)))))
-	(no-first (and erc-track-faces-normal-list
-		       (catch 'face
-			 (dolist (candidate erc-track-faces-priority-list)
-			   (when (member candidate (cdr faces))
-			     (throw 'face candidate)))))))
-    (cond ((null choice)
-	   nil)
-	  ((and (member choice erc-track-faces-normal-list)
-		(member no-first erc-track-faces-normal-list))
-	   no-first)
-	  (t
-	   choice))))
+                  (dolist (candidate erc-track-faces-priority-list)
+                    (when (or (equal candidate cur-face)
+                              (member candidate new-faces))
+                      (throw 'face candidate))))))
+    (when choice
+      (if (and (equal choice cur-face)
+               (member choice erc-track-faces-normal-list))
+          (let ((only-in-new
+                 (catch 'face
+                   (dolist (candidate erc-track-faces-priority-list)
+                     (when (member candidate new-faces)
+                       (throw 'face candidate))))))
+            (if (member only-in-new erc-track-faces-normal-list)
+                only-in-new
+              choice))
+        choice))))
 
 (defun erc-track-modified-channels ()
   "Hook function for `erc-insert-post-hook' to check if the current
@@ -831,17 +838,21 @@ is in `erc-mode'."
 		;; Add buffer, faces and counts
 		(setq erc-modified-channels-alist
 		      (cons (cons (current-buffer)
-				  (cons 1 (erc-track-find-face faces)))
+				  (cons 1 (erc-track-select-mode-line-face
+                                           (car faces)
+                                           (cdr faces))))
 			    erc-modified-channels-alist))
 	      ;; Else modify the face for the buffer, if necessary.
 	      (when faces
 		(let* ((cell (assq (current-buffer)
 				   erc-modified-channels-alist))
 		       (old-face (cddr cell))
-		       (new-face (erc-track-find-face
-				  (if old-face
-				      (cons old-face faces)
-				    faces))))
+		       (new-face (if old-face
+                                     (erc-track-select-mode-line-face old-face
+                                                                      faces)
+                                   (erc-track-select-mode-line-face
+                                    (car faces)
+                                    (cdr faces)))))
 		  (setcdr cell (cons (1+ (cadr cell)) new-face)))))
 	    ;; And display it
 	    (erc-modified-channels-display)))
-- 
2.30.0


[-- Attachment #5: 0004-ERC-Track-Fix-a-perceived-minor-bug-in-mode-line-fac.patch --]
[-- Type: text/x-patch, Size: 2011 bytes --]

From e4de7a54136e72ed69c392c71fb57cf6e6a49d60 Mon Sep 17 00:00:00 2001
From: Olivier Certner <olce.emacs@certner.fr>
Date: Thu, 14 Jan 2021 18:36:08 +0100
Subject: [PATCH 4/4] ERC: Track: Fix a perceived minor bug in mode line face
 selection

* lisp/erc/erc-track.el (erc-track-modified-channels): Fix what is a
probable bug when a new insert event happens for a buffer that was not
tracked or for which no mode line face was selected: In this case,
stop treating the latest buffer's face (first in list) as the previous
one, which could be overriden with an older one (in FACES' rest), as
if it had happened after.
---
 lisp/erc/erc-track.el | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/lisp/erc/erc-track.el b/lisp/erc/erc-track.el
index d596685fee..f0fa1fbed2 100644
--- a/lisp/erc/erc-track.el
+++ b/lisp/erc/erc-track.el
@@ -838,21 +838,14 @@ is in `erc-mode'."
 		;; Add buffer, faces and counts
 		(setq erc-modified-channels-alist
 		      (cons (cons (current-buffer)
-				  (cons 1 (erc-track-select-mode-line-face
-                                           (car faces)
-                                           (cdr faces))))
+				  (cons 1 (erc-track-select-face nil faces)))
 			    erc-modified-channels-alist))
 	      ;; Else modify the face for the buffer, if necessary.
 	      (when faces
 		(let* ((cell (assq (current-buffer)
 				   erc-modified-channels-alist))
 		       (old-face (cddr cell))
-		       (new-face (if old-face
-                                     (erc-track-select-mode-line-face old-face
-                                                                      faces)
-                                   (erc-track-select-mode-line-face
-                                    (car faces)
-                                    (cdr faces)))))
+		       (new-face (erc-track-select-face old-face faces)))
 		  (setcdr cell (cons (1+ (cadr cell)) new-face)))))
 	    ;; And display it
 	    (erc-modified-channels-display)))
-- 
2.30.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#46775: 27.1; ERC: Track: Modified channels doc and `erc-track-find-face' fixes
  2021-02-25 17:14 bug#46775: 27.1; ERC: Track: Modified channels doc and `erc-track-find-face' fixes Olivier Certner
  2021-02-25 17:24 ` Olivier Certner
@ 2021-06-09 13:07 ` J.P.
  1 sibling, 0 replies; 5+ messages in thread
From: J.P. @ 2021-06-09 13:07 UTC (permalink / raw)
  To: Olivier Certner; +Cc: emacs-erc, 46775

Olivier Certner <ocert.dev@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.





^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-06-09 13:07 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-25 17:14 bug#46775: 27.1; ERC: Track: Modified channels doc and `erc-track-find-face' fixes Olivier Certner
2021-02-25 17:24 ` Olivier Certner
2021-02-26  8:51   ` Lars Ingebrigtsen
2021-02-26  9:03     ` Olivier Certner
2021-06-09 13:07 ` J.P.

unofficial mirror of bug-gnu-emacs@gnu.org 

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://yhetil.org/emacs-bugs/0 emacs-bugs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 emacs-bugs emacs-bugs/ https://yhetil.org/emacs-bugs \
		bug-gnu-emacs@gnu.org
	public-inbox-index emacs-bugs

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.yhetil.org/yhetil.emacs.bugs
	nntp://news.gmane.io/gmane.emacs.bugs


code repositories for project(s) associated with this inbox:

	https://git.savannah.gnu.org/cgit/emacs.git

AGPL code for this site: git clone http://ou63pmih66umazou.onion/public-inbox.git