bug#72826: 30.0.90; icomplete-in-buffer becomes unusably slow in large Eshell

[Sean Whitton <spwhitton@spwhitton.name>]

Hello,

On Fri 04 Oct 2024 at 05:33pm +03, Eli Zaretskii wrote:

> Thanks.  But the difference between the two cases is larger than the
> difference between the first and the second line of the prompt.  How
> do you explain that?  Also, why doesn't the first image show more
> candidates, so as to use all of the second screen line in the
> mini-window? is that expected?

It's expected, indeed.  The explanation for all the cases you ask about
here is that the parts of the algorithm subsequent to the computation of
prospects-max do not try especially hard to fill up prospects-max.

My patch to address this bug only attempts to correct the computation of
prospects-max, not to make any change to how that value is subsequently
used.  If you think the latter should be changed, I think that should be
a separate bug report.  (Speaking as a longterm Icomplete user, I don't
especially think it needs to change.)

I've prepared two diffs that display the problem and how my patch
fixes it.  Apply the first diff and then try the (completing-read ...)
examples from my previous message.  In the case of the multiline prompt,
you can see that the XXXXXXX do not extend as far as they should.

Then apply the second diff, and try the (completing-read ...) again.
Even though the algorithm doesn't aggressively use up prospects-max, the
XXXXXX extend to the correct position, demonstrating the correctness of
my reworked computation of prospects-max.

--8<---------------cut here---------------start------------->8---
diff --git a/lisp/icomplete.el b/lisp/icomplete.el
index 2ea5e36fa88..e2607812d5e 100644
--- a/lisp/icomplete.el
+++ b/lisp/icomplete.el
@@ -1074,11 +1079,19 @@ icomplete-completions
                     (put-text-property 0 (length first)
                                        'face 'icomplete-first-match first)
                     (push first prospects)))
-                (concat determ
-                        "{"
-                        (mapconcat #'identity prospects icomplete-separator)
-                        (concat (and limit (concat icomplete-separator ellipsis))
-                                "}")))
+                (let ((res
+                       (concat determ
+                               "{"
+                               (mapconcat #'identity prospects icomplete-separator)
+                               (concat (and limit (concat icomplete-separator ellipsis))
+                                       "}"))))
+                  ;; Show the space out of prospects-max we didn't use.
+                  (if (window-minibuffer-p)
+                      (concat res (make-string (- prospects-max
+                                                  (length res)
+                                                  (string-width (buffer-string)))
+                                               ?X))
+                    res)))
             ;; Restore the base-size info, since completion-all-sorted-completions
             ;; is cached.
             (if last (setcdr last base-size))))))))
--8<---------------cut here---------------end--------------->8---

--8<---------------cut here---------------start------------->8---
diff --git a/lisp/icomplete.el b/lisp/icomplete.el
index 2ea5e36fa88..c37788d85e3 100644
--- a/lisp/icomplete.el
+++ b/lisp/icomplete.el
@@ -1002,12 +1002,18 @@ icomplete-completions
                                   ((< compare (+ 2 (string-width ellipsis))) most)
                                   (t (concat ellipsis (substring most compare))))
                                  close-bracket)))
+               (temp (string-width
+                      (buffer-substring
+                       (save-excursion
+                         (goto-char (icomplete--field-beg))
+                         (pos-bol))
+                       (icomplete--field-end))))
                ;;"-prospects" - more than one candidate
                (prospects-len (+ (string-width
                                   (or determ (concat open-bracket close-bracket)))
                                  (string-width icomplete-separator)
                                  (+ 2 (string-width ellipsis)) ;; take {…} into account
-                                 (string-width (buffer-string))))
+                                 temp))
                (prospects-max
                 ;; Max total length to use, including the minibuffer content.
                 (* (+ icomplete-prospects-height
@@ -1074,11 +1080,19 @@ icomplete-completions
                     (put-text-property 0 (length first)
                                        'face 'icomplete-first-match first)
                     (push first prospects)))
-                (concat determ
-                        "{"
-                        (mapconcat #'identity prospects icomplete-separator)
-                        (concat (and limit (concat icomplete-separator ellipsis))
-                                "}")))
+                (let ((res
+                       (concat determ
+                               "{"
+                               (mapconcat #'identity prospects icomplete-separator)
+                               (concat (and limit (concat icomplete-separator ellipsis))
+                                       "}"))))
+                  ;; Show the space out of prospects-max we didn't use.
+                  (if (window-minibuffer-p)
+                      (concat res (make-string (- prospects-max
+                                                  (length res)
+                                                  temp)
+                                               ?X))
+                    res)))
             ;; Restore the base-size info, since completion-all-sorted-completions
             ;; is cached.
             (if last (setcdr last base-size))))))))
--8<---------------cut here---------------end--------------->8---

> But if your analysis is correct, does the value returned by
> string-pixel-width divided by default-font-width produce the correct
> result?

Do you mean to propose (/ (string-pixel-width (buffer-string))
                          (default-font-width))
as an alternative to my fix?

I've just tried that, and while it appears to be slightly faster than
the existing code, Icomplete in a large buffer remains unusably slow.

-- 
Sean Whitton