bug#66191: erc-ibuffer.el: suspicious use of hash-table-size

bug#66191: erc-ibuffer.el: suspicious use of hash-table-size

[Mattias Engdegård]

In erc-ibuffer.el:

(define-ibuffer-column
 erc-members (:name "Users")
  (if (and (eq major-mode 'erc-mode)
	   (boundp 'erc-channel-users)
	   (hash-table-p erc-channel-users)
	   (> (hash-table-size erc-channel-users) 0))
     (number-to-string (hash-table-size erc-channel-users))
    ""))

Perhaps I'm mistaken but shouldn't hash-table-size be hash-table-count here?

bug#66191: erc-ibuffer.el: suspicious use of hash-table-size

[J.P.]

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

Mattias Engdegård <mattias.engdegard@gmail.com> writes:

> In erc-ibuffer.el:
>
> (define-ibuffer-column
>  erc-members (:name "Users")
>   (if (and (eq major-mode 'erc-mode)
> 	   (boundp 'erc-channel-users)
> 	   (hash-table-p erc-channel-users)
> 	   (> (hash-table-size erc-channel-users) 0))
>      (number-to-string (hash-table-size erc-channel-users))
>     ""))
>
> Perhaps I'm mistaken but shouldn't hash-table-size be hash-table-count here?

Nice find! I've attached a patch that should fix the issue. If no one
says anything, I will add it or something similar in a few days. Thanks.


[-- Attachment #2: 0001-5.6-Fix-wrong-User-column-count-in-erc-ibuffer.patch --]
[-- Type: text/x-patch, Size: 5109 bytes --]

From 1b3e16cc6085899d462f6fd954cbabfbc5aa4e7f Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Mon, 25 Sep 2023 06:06:13 -0700
Subject: [PATCH] [5.6] Fix wrong "User" column count in erc-ibuffer
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* etc/ERC-NEWS: Mention minor ibuffer UI changes.
* lisp/erc/erc-ibuffer.el (erc-ibuffer-target-server-name): Add option
to show server's buffer name instead of its dialed name, which is
unhelpful with multiple connections to the same proxy.
(ibuffer-make-column-erc-target): Optionally show buffer name instead
of `erc-session-server'.
(ibuffer-make-column-erc-members): Show tally of all server users for
non-target buffers, and show correct count for targets.  Thanks to
Mattias Engdegård for reporting this.
(erc-ibuffer-limit-map): Use "new" `define-ibuffer-filter'
API.  (Bug#66191)
---
 etc/ERC-NEWS            |  7 +++++++
 lisp/erc/erc-ibuffer.el | 30 +++++++++++++++++++++---------
 2 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/etc/ERC-NEWS b/etc/ERC-NEWS
index 05e933930e2..a5ed463cb5a 100644
--- a/etc/ERC-NEWS
+++ b/etc/ERC-NEWS
@@ -194,6 +194,13 @@ a weight of 'semi-bold'.  This was done to make buttons detectable and
 to spare users from resorting to tweaking these faces, or options like
 'erc-notice-highlight-type', just to achieve this effect.
 
+** A slightly more informative 'erc-ibuffer'.
+Users of ERC's Ibuffer integration might like to know that the "Users"
+column now reports total server users alongside membership tallies for
+target buffers.  And the new option 'erc-ibuffer-target-server-name'
+allows for showing a more human friendly name instead of the network
+process's peer address, which can be ambiguous.
+
 ** Improved interplay between buffer truncation and message logging.
 While most of these improvements are subtle, some affect everyday use.
 For example, users of the 'truncate' module may notice that truncation
diff --git a/lisp/erc/erc-ibuffer.el b/lisp/erc/erc-ibuffer.el
index 612814ac6da..cc16d0be258 100644
--- a/lisp/erc/erc-ibuffer.el
+++ b/lisp/erc/erc-ibuffer.el
@@ -27,6 +27,9 @@
 ;; needs work.  Usage:  Type / C-e C-h when in Ibuffer-mode to see new
 ;; limiting commands
 
+;; This library does not contain a module, but you can `require' it
+;; after loading `erc' to make use of its functionality.
+
 ;;; Code:
 
 (require 'ibuffer)
@@ -51,6 +54,13 @@ erc-ibuffer-dangerous-host-char
   "Char indicating a channel which had dangerous-host traffic lately (hidden)."
   :type 'character)
 
+(defcustom erc-ibuffer-target-server-name 'dialed
+  "What to show in the \"Target\" column for server buffers.
+With `dialed', show a TCP-like host:port pair, prefixed by the
+word \"Server \".  With `buffer', use the buffer's current name."
+  :package-version '(ERC . "5.6") ; FIXME sync on release
+  :type '(choice (const dialed) (const buffer)))
+
 (define-ibuffer-filter erc-server
   "Toggle current view to buffers which are related to ERC servers."
   (:description "erc servers"
@@ -101,8 +111,10 @@ erc-target
   (if (eq major-mode 'erc-mode)
       (cond ((and erc-server-process (processp erc-server-process)
 		  (eq (current-buffer) (process-buffer erc-server-process)))
-	     (concat "Server " erc-session-server ":"
-		     (erc-port-to-string erc-session-port)))
+             (if (eq erc-ibuffer-target-server-name 'dialed)
+                 (concat "Server " erc-session-server ":"
+                         (erc-port-to-string erc-session-port))
+               (buffer-name)))
 	    ((erc-channel-p (erc-default-target))
 	     (concat (erc-default-target)))
 	    ((erc-default-target)
@@ -118,11 +130,11 @@ erc-topic
 
 (define-ibuffer-column
  erc-members (:name "Users")
-  (if (and (eq major-mode 'erc-mode)
-	   (boundp 'erc-channel-users)
-	   (hash-table-p erc-channel-users)
-	   (> (hash-table-size erc-channel-users) 0))
-     (number-to-string (hash-table-size erc-channel-users))
+  (if-let ((table (or erc-channel-users erc-server-users))
+           ((hash-table-p table))
+           (count (hash-table-count table))
+           ((> count 0)))
+      (number-to-string count)
     ""))
 
 (define-ibuffer-column erc-away (:name "A")
@@ -145,6 +157,7 @@ erc-voice
       "+"
     " "))
 
+;; FIXME display user mode for server rows.
 (define-ibuffer-column erc-channel-modes (:name "Mode")
   (if (and (eq major-mode 'erc-mode)
 	   (or (> (length erc-channel-modes) 0)
@@ -177,8 +190,7 @@ erc-ibuffer-formats
 (defvar erc-ibuffer-limit-map nil
   "Prefix keymap to use for ERC related limiting.")
 (define-prefix-command 'erc-ibuffer-limit-map)
-;; FIXME: Where is `ibuffer-limit-by-erc-server' defined?
-(define-key 'erc-ibuffer-limit-map (kbd "s") 'ibuffer-limit-by-erc-server)
+(define-key 'erc-ibuffer-limit-map (kbd "s") #'ibuffer-filter-by-erc-server)
 (define-key ibuffer-mode-map (kbd "/ \C-e") 'erc-ibuffer-limit-map)
 
 (provide 'erc-ibuffer)
-- 
2.41.0

bug#66191: erc-ibuffer.el: suspicious use of hash-table-size

[J.P.]

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

> If no one says anything, I will add it or something similar in a few
> days.

I ended up only installing the bug fix portions, but I may open another
report for the feature parts eventually. Thanks and closing.