emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] (icomplete-vertical-mode): Add support for affixations and,


From: João Távora
Subject: Re: [PATCH] (icomplete-vertical-mode): Add support for affixations and, annotations
Date: Tue, 01 Jun 2021 15:30:17 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux)

Daniel Mendler <mail@daniel-mendler.de> writes:

> On 6/1/21 2:00 PM, João Távora wrote:
>>> * I observed an issue with the last candidate with `icomplete-scroll=t`
>>> - The candidate is one line off the minibuffer screen.
> Yes, I am using the newest master version. But the issue seems to be
> related somehow to my configuration, since I don't see it with emacs -Q.
> I have to investigate which setting does this.

Let me know about your findings.

>>> * Do you plan to support the `group-function`?
>> 
>> Maybe, but nothing in Emacs uses it so I can't test.  You can add it
>> too, of course. 
>
> There is `read-char-by-name` one can use for testing. Juri added the
> `group-function` there.

I didn't know how to use it, so I just M-: (read-char-by-name "thingy").
It worked decently, I guess, but I couldn't find any evidence of
"grouping".  What is the group function supposed to do here?  Does
Vertico use it?  What does it do with it?

>> I don't understand one thing about that protocol: why
>> the extra TRANSFORM argument flag?  Why not just have that function
>> return a tuple with the group title and transformed name?
>
> The design of the `group-function` has been extensively discussed. The
> flag is used instead of a tuple for performance reasons. The computation
> of the transformed candidate string may require string allocations,
> which are expensive if performed for every candidate.

That's right, I see your point.

They could return a lambda, which is cheap to allocate, to defer the
calculation.  Or just nil or #'identity if it's supposed to stay
unchanged.

   diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
   index ec21b7b93b..5c00e7bb81 100644
   --- a/lisp/minibuffer.el
   +++ b/lisp/minibuffer.el
   @@ -1450,11 +1450,14 @@ minibuffer--group-by
      "Group ELEMS by GROUP-FUN and sort groups by SORT-FUN."
      (let ((groups))
        (dolist (cand elems)
   -      (let* ((key (funcall group-fun cand nil))
   +      (let* ((key-and-transform (funcall group-fun cand))
   +             (key (car key-and-transform))
                 (group (assoc key groups)))
   -        (if group
   -            (setcdr group (cons cand (cdr group)))
   -          (push (list key cand) groups))))
   +        (cond (group
   +               (put-text-property 0 1 'completion--transform (cdr 
key-and-transform)
   +                                  cand)
   +               (setcdr group (cons cand (cdr group))))
   +              (push (list key cand) groups))))
        (setq groups (nreverse groups)
              groups (mapc (lambda (x)
                             (setcdr x (nreverse (cdr x))))

The protocol is slightly simpler since it returns the same type every
time.  More importantly, this avoids threading group-fn down a bunch of
functions in minibuffer.el, where it seems to be adding an awful lot of
complexity.

João



reply via email to

[Prev in Thread] Current Thread [Next in Thread]