[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#47711: bug#48841: bug#47711: bug#48841: bug#47711: [PATCH VERSION 2]
From: |
João Távora |
Subject: |
bug#47711: bug#48841: bug#47711: bug#48841: bug#47711: [PATCH VERSION 2] Add new `completion-filter-completions` API and deferred highlighting |
Date: |
Thu, 2 Nov 2023 11:12:15 +0000 |
On Thu, Nov 2, 2023 at 10:58 AM Eli Zaretskii <eliz@gnu.org> wrote:
> > After some rounds of benchmarking and discussion, Dmitry and I
> > think the latest version of the patch should be installed.
>
> Are there any problematic aspects of the patch that need to be
> discussed or considered before installing the patch?
>
> IOW, why are you soliciting our opinions, instead of just going ahead
> and installing?
No particular reason. Dmitry suggested that we do, and you
participated in this discussion a while back, and I know you
normally have some useful comment or two.
> > In a nutshell it solves the performance problem of overly eager
> > completion highlighting with minimal changes to the completion API.
>
> It looks to me like it adds a new feature, not just solves a
> performance problem?
>
> Some minor comments to the patch itself:
>
> > +(defvar-local completion-lazy-hilit nil
> > + "If non-nil, request completion lazy highlighting.
> > +
> > +Completion-presenting frontends may opt to bind this variable to
> > +non-nil value in the context of completion-producing calls (such
> > +as `completion-all-completions'). This hints the intervening
> > +completion styles that they do not need to
> > +fontify (i.e. propertize with the `face' property) completion
> > +strings with highlights of the matching parts.
>
> If this is intended to be bound by frontends, why is it defvar-local?
> I thought let-binding buffer-local variables is a tricky business that
> could have unexpected results?
Good catch! It shouldn't be defvar-local indeed, not with this latest
version. See, glad I called you ;-)
> Also, I think this doc string should reference
> completion-lazy-hilit-fn.
>
> > +(defvar completion-lazy-hilit-fn nil
> > + "Used by completions styles honoring `completion-lazy-hilit'.
>
> This should mention "function", since just "Used to..." doesn't convey
> that, and "-fn" could also mean "file name", not just "function".
Makes sense.
> > +(defun completion-lazy-hilit (str)
> > + "Return a copy of completion STR that is `face'-propertized.
> ^^^^^^^^^^^^^^^^^^
> Strange quoting. "face" is not a symbol that we want to have a link
> to, is it?
It's not a symbol we'll be referencing, but it's a symbol. I'll
rewrite it.
>
> I see a few more places in the doc strings that will "need work", but
> that can be done later.
>
> What did you want to say in NEWS about this? If it's just a
> performance improvement, we don't normally mention them in NEWS.
But it needs frontends to opt in, and that requires an non-breaking
addition to completion API.
João
- bug#47711: bug#48841: bug#47711: bug#48841: bug#47711: [PATCH VERSION 2] Add new `completion-filter-completions` API and deferred highlighting, João Távora, 2023/11/01
- bug#47711: bug#48841: bug#47711: bug#48841: bug#47711: [PATCH VERSION 2] Add new `completion-filter-completions` API and deferred highlighting, Dmitry Gutov, 2023/11/02
- bug#47711: bug#48841: bug#47711: bug#48841: bug#47711: [PATCH VERSION 2] Add new `completion-filter-completions` API and deferred highlighting, João Távora, 2023/11/02
- bug#47711: bug#48841: bug#47711: bug#48841: bug#47711: [PATCH VERSION 2] Add new `completion-filter-completions` API and deferred highlighting, Dmitry Gutov, 2023/11/02
- bug#47711: bug#48841: bug#47711: bug#48841: bug#47711: [PATCH VERSION 2] Add new `completion-filter-completions` API and deferred highlighting, João Távora, 2023/11/02
- bug#47711: bug#48841: bug#47711: bug#48841: bug#47711: [PATCH VERSION 2] Add new `completion-filter-completions` API and deferred highlighting, Dmitry Gutov, 2023/11/02
- bug#47711: bug#48841: bug#47711: bug#48841: bug#47711: [PATCH VERSION 2] Add new `completion-filter-completions` API and deferred highlighting, João Távora, 2023/11/02
- bug#47711: bug#48841: bug#47711: bug#48841: bug#47711: [PATCH VERSION 2] Add new `completion-filter-completions` API and deferred highlighting, Dmitry Gutov, 2023/11/02
- Message not available
- bug#47711: bug#48841: bug#47711: bug#48841: bug#47711: [PATCH VERSION 2] Add new `completion-filter-completions` API and deferred highlighting, Howard Melman, 2023/11/04
bug#47711: bug#48841: bug#47711: bug#48841: bug#47711: [PATCH VERSION 2] Add new `completion-filter-completions` API and deferred highlighting, João Távora, 2023/11/06