bug#73568: 30.0.91; which-key error in speedbar (wrong-type-argument wholenump -13)

[Justin Burkett <justin@burkett.cc>]

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

Thanks. Can you try the attached patch for the first problem?

I'm not sure what the best approach is with speedbar. I'm inclined to
disable which-key for that case, but maybe showing which-key in the
original frame is better?

On Mon, Sep 30, 2024 at 5:53 PM Stefan Kangas <stefankangas@gmail.com> wrote:
>
> Morgan Willcock <morgan@ice9.digital> writes:
>
> > An error is signalled when which-key is activated, point is within the
> > speedbar frame, and a portion of a key-sequence has been typed
> > (e.g. C-x).
> >
> > To generate an example backtrace:
> >
> >   emacs -Q \
> >       --eval "(which-key-mode)" \
> >       --eval "(speedbar-get-focus)" \
> >       --eval "(toggle-debug-on-error)" \
> >       --eval "(setq unread-command-events (listify-key-sequence \"\C-x\"))"
>
> Here's the error I get:
>
> Debugger entered--Lisp error: (wrong-type-argument wholenump -14)
>   make-string(-14 32)
>
> The problem occurs is in `which-key--pad-column': the speedbar frame is
> only 14 characters wide, and so can't fit longer strings than that.
>
> I actually see two bugs here:
>
> 1. Create a regular Emacs frame of width 14, enable `which-key-mode' and
>    press some key, e.g. C-h.  You now get a similar backtrace.  Probably
>    `which-key' should simply be smarter in this case, for example by
>    truncating instead of padding.
>
> 2. With a dframe, as in speedbar, I guess `which-key' should try to
>    display it's help window in the original frame?
>
> I'm copying in Justin Burkett.

[-- Attachment #2: 0001-Fix-which-key-layout-error-when-available-width-is-s.patch --]
[-- Type: application/octet-stream, Size: 4332 bytes --]

From a8015d0c3f5cc5c1d0b1b45f4b7c096dbfc92664 Mon Sep 17 00:00:00 2001
From: Justin Burkett <justin@burkett.cc>
Date: Tue, 1 Oct 2024 21:52:07 -0400
Subject: [PATCH] Fix which-key layout error when available width is small

* lisp/which-key.el (which-key--col-widths): Add more efficient function
for calculating column widths
(which-key--pad-column): Handle case where available width for which-key
buffer is smaller than the width of a single column.
---
 lisp/which-key.el | 71 ++++++++++++++++++++++++++++++++---------------
 1 file changed, 48 insertions(+), 23 deletions(-)

diff --git a/lisp/which-key.el b/lisp/which-key.el
index 232145f7fb5..6fd61462abe 100644
--- a/lisp/which-key.el
+++ b/lisp/which-key.el
@@ -39,6 +39,7 @@
 (require 'cl-lib)
 (require 'button)
 (require 'regexp-opt)
+(require 'seq)
 
 ;; For compiler
 (defvar evil-operator-shortcut-map)
@@ -2012,33 +2013,57 @@ which-key--join-columns
          (rows (apply #'cl-mapcar #'list padded)))
     (mapconcat (lambda (row) (mapconcat #'identity row " ")) rows "\n")))
 
-(defsubst which-key--max-len (keys index &optional initial-value)
-  "Find the max length of the INDEX element in each of KEYS."
-  (cl-reduce
-   (lambda (x y) (max x (which-key--string-width (nth index y))))
-   keys :initial-value (if initial-value initial-value 0)))
+(defun which-key--col-widths (keys)
+  "Find the max length of each column in KEYS.
+KEYS is assumed to be a list of lists of strings, where the sublists all
+have the same number of strings."
+  (let* ((n (length (car keys))))
+    (seq-reduce
+     (lambda (x y)
+       (seq-map-indexed
+        (lambda (e i)
+          (max (nth i x) (which-key--string-width e)))
+        y))
+     keys
+     (make-list n 0))))
 
 (defun which-key--pad-column (col-keys avl-width)
-  "Pad cells of COL-KEYS to AVL-WIDTH.
+  "Pad COL-KEYS cells to same width respecting AVL-WIDTH.
 Take a column of (key separator description) COL-KEYS,
 calculate the max width in the column and pad all cells out to
-that width."
-  (let* ((col-key-width  (+ which-key-add-column-padding
-                            (which-key--max-len col-keys 0)))
-         (col-sep-width  (which-key--max-len col-keys 1))
-         (avl-width      (- avl-width col-key-width col-sep-width))
-         (col-desc-width (min avl-width
-                              (which-key--max-len
-                               col-keys 2
-                               which-key-min-column-description-width)))
-         (col-width      (+ col-key-width col-sep-width col-desc-width))
-         (col-format     (concat "%" (int-to-string col-key-width) "s%s%s")))
-    (cons col-width
-          (mapcar (pcase-lambda (`(,key ,sep ,desc ,_doc))
-                    (concat
-                     (format col-format key sep desc)
-                     (make-string (- col-desc-width (string-width desc)) ?\s)))
-                  col-keys))))
+that width.
+
+If AVL-WIDTH is too small to contain the column, truncate the last
+element of COL-KEYS instead."
+  (let* ((col-widths (which-key--col-widths col-keys))
+         (key-col-width (+ which-key-add-column-padding
+                           (nth 0 col-widths)))
+         (sep-col-width (nth 1 col-widths))
+         (rem-avl-width (- avl-width key-col-width sep-col-width))
+         (desc-col-width (nth 2 col-widths))
+         (total-width (+ key-col-width sep-col-width desc-col-width)))
+    (cond
+     ((> which-key-min-column-description-width desc-col-width rem-avl-width)
+      ;; we can't fit the column
+      (cons 0 (make-list (length col-keys) "")))
+     ((> desc-col-width rem-avl-width)
+      ;; truncate the last column
+      (cons avl-width
+            (mapcar
+             (pcase-lambda (`(,key ,sep ,desc ,_doc))
+               (string-limit
+                (format (format "%%%ds%%s%%s" key-col-width) key sep desc)
+                avl-width))
+             col-keys)))
+     (t
+      ;; pad the last column
+      (cons total-width
+            (mapcar
+             (pcase-lambda (`(,key ,sep ,desc ,_doc))
+               (string-pad
+                (format (format "%%%ds%%s%%s" key-col-width) key sep desc)
+                total-width))
+             col-keys))))))
 
 (defun which-key--partition-list (n list)
   "Partition LIST into N-sized sublists."
-- 
2.46.2