GNU bug report logs - #47711
27.1; Deferred highlighting support in `completion-all-completions', `vertico--all-completions`

Previous Next

Full log

View this message in rfc822 format

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: Daniel Mendler <mail <at> daniel-mendler.de>, Eli Zaretskii <eliz <at> gnu.org>, João Távora <joaotavora <at> gmail.com>, 47711 <at> debbugs.gnu.org
Subject: bug#47711: bug#48841: bug#47711: bug#48841: bug#47711: [PATCH VERSION 2] Add new `completion-filter-completions` API and deferred highlighting
Date: Fri, 27 Oct 2023 14:12:20 -0400

>>> We could, for example, have a period when we warn about returned
>>> non-matches. string-match-p is not free, but it's not very expensive either.
>> The problem is that I dislike `completion-regexp-list` :-)
> When we do use it, we can avoid copying all the strings to a new
> list. Skipping consing this way can really move the needle at the level of
> optimization we're discussing now.

Oh, don't get me wrong, I like the functionality it offers, I just
dislike the way it works.

>> More seriously, since it's a dynbound variable it can have unwanted
>> effects in nested calls to `all/try-completions`, so it's safer to
>> ignore that variable because its binding is not always "meant for us" :-(
> I guess it would be more precise if it was a function argument, e.g. the
> first argument to 'fancy-all-completions' or somesuch that all completion
> tables are supposed to use inside. OTOH, I suppose that might hinder those
> that use external programs.

In my "work in progress" (not touched since last December :-( ),
I replace `all-completions` with:

    (cl-defgeneric completion-table-fetch-matches ( table pattern
                                                    &optional pre session)
      "Return candidates matching PATTERN in the completion TABLE.
    For tables with subfields, PRE is the text found before PATTERN such that
       (let ((len (length PRE)))
         (equal (completion-table-boundaries TABLE PRE len) (cons len len)))
    
    Return a list of strings or a list of cons cells whose car is a string.
    SESSION if non-nil is a hash-table where we can stash arbitrary auxiliary
    info to avoid recomputing it between calls of the same \"session\".")

`pattern`s can take various shapes.  In my WiP code, I implement 4 kinds
of patterns: prefix, glob, regexp, and predicate.  Now, we don't want
completion tables to have to handle each and every one of those pattern
kinds (the set of which is extensible via CLOS methods), so there's
a middleman:

    (cl-defgeneric completion-pattern-convert (to pattern)
      "Convert PATTERN to be of type TO.
    Returns a pair (PRED . NEWPATTERN) where NEWPATTERN is of type TO
    and should match everything that PATTERN matches.  PRED is nil
    if NEWPATTERN matches exactly the same candidates as PATTERN
    and otherwise it is a function that takes a candidate and returns non-nil if the
    candidate also matches PATTERN.  PRED should not presume that the candidate
    has already been filtered by NEWPATTERN."

So the fallback definition of `completion-table-fetch-matches`, which
relies on the old API looks like:

    (defun completion-table--fetch-legacy (table pattern &optional pre _session)
      (pcase-let ((`(,pred . ,regexp)
                   (completion-pattern-convert 'regexp pattern))
                  (`(,ppred . ,prefix)
                   (completion-pattern-convert 'prefix pattern)))
        (let ((matches
               (let ((completion-regexp-list (if ppred (list regexp)))
                     (case-fold-search completion-ignore-case))
                 (all-completions (concat pre prefix) table))))
          (if (null pred)
              matches
            (seq-filter pred matches)))))

This is of course incorrect because `all-completions` could ignore
`completion-regexp-list`, in which case we should use `ppred` instead of
`pred` on the last 3 lines :-)

It has the disadvantage that every completion-table basically needs to
start by calling `completion-pattern-convert` so as to convert the
pattern to the format that it supports.  But I think it's still better
than the current API where completion tables "have to" obey the prefix
string, the `completion-regexp-list`, and the predicate (and where the
latter two are often nil so tables tends to ignore them, and since
tables ignore them callers don't use them, etc...).


        Stefan

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.