From: "João Távora" <joaotavora@gmail.com>
To: Daniel Mendler <mail@daniel-mendler.de>
Cc: Stefan Monnier <monnier@iro.umontreal.ca>,
48841@debbugs.gnu.org, Dmitry Gutov <dgutov@yandex.ru>,
Lars Ingebrigtsen <larsi@gnus.org>,
47711@debbugs.gnu.org
Subject: bug#48841: bug#47711: bug#48841: bug#47711: [PATCH VERSION 2] Add new `completion-filter-completions` API and deferred highlighting
Date: Mon, 16 Aug 2021 13:17:56 +0100 [thread overview]
Message-ID: <CALDnm51UMr2ZRPbuNtKZnMPmWPRcOgNNFs1-PA=29_1s=y9cDw@mail.gmail.com> (raw)
In-Reply-To: <d10119c3-aac5-89c4-e6a0-f6655e53a5a2@daniel-mendler.de>
> João, please feel free to also present your closing arguments here.
Sorry, I don't "close" arguments like this.
I hope you can provide:
* the fixes to the regression identified
* the benchmarks you say you have
* the patches to icomplete.el that show how it uses your new API
* the demonstrations of the bugs you accuse my patch to suffer from
Thanks.
On Mon, Aug 16, 2021 at 1:05 PM Daniel Mendler <mail@daniel-mendler.de> wrote:
>
> João, the discussion is clearly not progressing. I propose that we both
> take a step back and let the Emacs maintainers, who participated in this
> discussion, decide on how to proceed. It seems to me that all arguments
> and data has been presented and there is no need for further
> reiterations in more and more colorful language. I would also like to
> point out that implying that I don't understand your language is
> borderline acceptable. I understand the discussion very well, but I
> don't understand why you are using these unfair and invalid means of
> discussion.
>
> For example there could be these decision outcomes:
>
> 1. The information presented up to now does not allow the maintainers to
> make a decision. For example the maintainers may ask for further
> clarification from you, João, or they may ask for benchmarks from me or
> a prove that my patch does not lead to performance regressions.
>
> 2. The maintainers decide that no patch should be merged.
>
> 2. The maintainers decide that your patch will be merged. I will accept
> this decision.
>
> 3. The maintainers decide that my patch will be merged. You will accept
> this decision.
>
> 4. The maintainers decide that both patches will be merged such that
> both approaches will be supported. We both will accept this decision.
>
> I want to summarize the situation in the following:
>
> The patches in question address a performance issue in the current
> completion machinery which is caused by over-eager copying of the
> completion candidate strings and over-eager application of the
> highlighting to all candidate strings. For incrementally updating UIs it
> would be sufficient to only copy and highlight the strings which are
> actually going to be displayed.
>
> My patch takes the approach to expose the existing two-step completion
> process, which consists of filtering and highlighting. By returning the
> filtered completion strings and a highlighting function this two-step
> process is decomposed and the caller of the API has the ability to call
> the highlighting function on only the displayed subset of completion
> candidates. I argue that exposing the filtering and highlighting
> procession steps is the logical and natural conclusion of the existing
> machinery.
>
> My patch is fully backward compatible and aims to not introduce any
> regressions (also no performance regressions) to the existing API.
> Furthermore my patch adheres to the current guarantees given by the
> existing 'completion-all-completions' API. The completion strings
> provided by the completion backend are not mutated in any way, no string
> properties are attached. Since the API 'completion-filter-completions'
> proposed in my patch does the minimal amount of work necessary (only
> filtering), if no highlighting is requested, I argue that the new API is
> the most efficient possible API, given the current constraints.
>
> Furthermore since I am introducing a new API, outstanding issues can be
> solved which could not be solved until now given the constraints of the
> existing 'completion-all-completions' API. In particular the new API
> 'completion-filter-completions' API returns additional data like the end
> position of the completion boundaries. Until now the end position was
> not made available and 'completion-base-position' just used the length
> of the input to guess the end position. In a strict sense this guess is
> incorrect and there is a FIXME in minibuffer.el, mentioning this issue.
>
> The downside of my patch is that it is a large patch. While it adds only
> 196 lines of code, which is not much and expected given that it only
> reshuffles the existing machinery, it is still a large patch in total.
>
> On the other side, João's patch avoids the complication of adhering to
> the existing guarantees of the APIs and takes the liberty of attaching
> "private" string properties to the completion strings of the completion
> table backend. I argue that attaching the string properties is a
> violation of the guarantees of the existing API and violates the
> expectations of the existing many completion tables. One very severe
> scenario is when obarray is used as completion table, since then each
> symbol name gets a private property attached. I argue that such global
> side effects like attaching string properties to all completion
> candidates should better be avoided. There is the issue that the
> attached string properties are a potential memory leak. When dumping the
> string representation of symbol names, the symbols will have additional
> properties which will complicate the debugging experience. Furthermore
> it will lead to confusion since the global side effect during completion
> will suddenly have an influence of symbols which don't have to do
> anything with completion. The big advantage of João's patch is that it
> is very limited in scope and very simple. However I argue that this
> simplicity is hard-won and we will regret this approach later due to the
> global side effects.
>
> Therefore I conclude that the two-step process proposed in my patch,
> which does not introduce problematic global side effects is the better
> approach forward. Furthermore a new API is needed such that more
> completion data can be returned, e.g., the completion end position. One
> could even return additional match data in the future given that the new
> API 'completion-filter-completions' is extensible thanks to its alist
> return value.
>
> João, please feel free to also present your closing arguments here.
>
> Daniel
--
João Távora
next prev parent reply other threads:[~2021-08-16 12:17 UTC|newest]
Thread overview: 174+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-05 1:39 bug#48841: fido-mode is slower than ido-mode with similar settings Dmitry Gutov
2021-06-05 9:35 ` João Távora
2021-06-05 23:02 ` Dmitry Gutov
2021-06-05 23:20 ` João Távora
2021-06-05 23:42 ` Dmitry Gutov
2021-06-06 0:25 ` Dmitry Gutov
2021-06-06 6:54 ` João Távora
2021-06-06 22:20 ` Dmitry Gutov
2021-06-06 23:49 ` João Távora
2021-06-07 0:11 ` Dmitry Gutov
2021-06-07 8:52 ` João Távora
2021-06-11 2:19 ` Dmitry Gutov
2021-06-11 17:09 ` João Távora
2021-06-11 22:34 ` Dmitry Gutov
2021-06-11 22:41 ` Dmitry Gutov
2021-06-13 14:55 ` João Távora
2021-06-17 2:36 ` Dmitry Gutov
2021-06-17 21:21 ` João Távora
2021-07-04 1:53 ` Dmitry Gutov
2021-07-07 8:56 ` bug#47711: " Daniel Mendler
2021-06-11 23:24 ` João Távora
2021-06-12 0:43 ` Dmitry Gutov
2021-06-13 14:29 ` João Távora
2021-06-14 0:08 ` Dmitry Gutov
2021-06-14 0:16 ` João Távora
2021-06-17 2:23 ` Dmitry Gutov
2021-06-17 21:29 ` João Távora
2021-07-04 1:42 ` Dmitry Gutov
2021-06-06 2:34 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-06-06 6:59 ` João Távora
2021-06-06 16:54 ` Dmitry Gutov
2021-06-06 18:37 ` João Távora
2021-06-06 22:21 ` Dmitry Gutov
2021-06-06 23:27 ` João Távora
2021-06-06 17:55 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-06-06 21:33 ` João Távora
2021-08-11 14:16 ` bug#48841: [PATCH] Add new `completion-filter-completions` API and deferred highlighting Daniel Mendler
2021-08-11 16:11 ` Daniel Mendler
2021-08-11 16:17 ` bug#47711: " João Távora
2021-08-12 9:24 ` Daniel Mendler
2021-08-13 10:38 ` bug#48841: [PATCH VERSION 2] " Daniel Mendler
2021-08-13 10:56 ` João Távora
2021-08-13 11:21 ` bug#48841: bug#47711: " Daniel Mendler
2021-08-13 12:05 ` João Távora
2021-08-13 12:22 ` Daniel Mendler
2021-08-13 12:37 ` bug#48841: " João Távora
2021-08-13 12:56 ` Daniel Mendler
2021-08-13 13:36 ` bug#48841: " João Távora
2021-08-13 14:03 ` Daniel Mendler
2021-08-13 14:11 ` bug#48841: " João Távora
2021-08-13 14:37 ` bug#47711: " Daniel Mendler
2021-08-14 2:47 ` Dmitry Gutov
2021-08-14 7:12 ` bug#47711: " Eli Zaretskii
2021-08-14 11:22 ` Dmitry Gutov
2021-08-16 8:48 ` Daniel Mendler
2021-08-16 11:57 ` bug#47711: " Eli Zaretskii
2021-08-16 12:02 ` João Távora
2021-08-16 12:19 ` Eli Zaretskii
2021-08-16 12:08 ` Daniel Mendler
2021-08-14 10:36 ` João Távora
2021-08-14 11:29 ` Eli Zaretskii
2021-08-14 12:12 ` bug#47711: " Lars Ingebrigtsen
2021-08-14 12:39 ` Eli Zaretskii
2021-08-14 13:29 ` Lars Ingebrigtsen
2021-08-16 3:21 ` Dmitry Gutov
2021-08-16 3:27 ` bug#47711: " João Távora
2021-08-16 3:31 ` Dmitry Gutov
2021-08-16 3:53 ` João Távora
2021-08-16 3:59 ` Dmitry Gutov
2021-08-16 4:25 ` bug#47711: " João Távora
2021-08-16 9:08 ` Daniel Mendler
2021-08-16 10:15 ` João Távora
2021-08-16 10:52 ` Daniel Mendler
2021-08-16 11:37 ` bug#48841: " João Távora
2021-08-16 12:05 ` Daniel Mendler
2021-08-16 12:17 ` João Távora [this message]
2021-08-16 12:43 ` Eli Zaretskii
2021-08-16 14:26 ` bug#48841: " Dmitry Gutov
2021-08-16 14:29 ` João Távora
2021-08-16 12:39 ` Eli Zaretskii
2021-08-16 12:49 ` bug#48841: " Daniel Mendler
2021-08-16 13:21 ` Eli Zaretskii
2021-08-16 14:00 ` Dmitry Gutov
2021-08-16 14:20 ` João Távora
2021-08-16 14:33 ` bug#48841: " Dmitry Gutov
2021-08-16 14:36 ` João Távora
2021-08-16 14:47 ` bug#47711: bug#48841: " Dmitry Gutov
2021-08-16 16:59 ` João Távora
2021-08-16 18:25 ` João Távora
2021-08-17 2:08 ` Dmitry Gutov
2021-08-17 8:59 ` João Távora
2021-08-17 11:48 ` bug#48841: " Eli Zaretskii
2021-08-17 11:52 ` bug#47711: " João Távora
2021-08-16 3:17 ` Dmitry Gutov
2021-08-16 11:46 ` Eli Zaretskii
2021-08-16 13:38 ` Dmitry Gutov
2021-08-16 13:41 ` João Távora
2021-08-16 14:14 ` bug#47711: " Dmitry Gutov
2021-08-15 18:32 ` bug#48841: [PATCH] Make fido-mode about as fast as ido-mode even with many completions João Távora
2021-08-25 15:42 ` João Távora
2021-08-14 7:01 ` bug#48841: bug#47711: [PATCH VERSION 2] Add new `completion-filter-completions` API and deferred highlighting Eli Zaretskii
2021-08-14 9:48 ` João Távora
2021-08-15 0:03 ` João Távora
2021-08-16 3:26 ` Dmitry Gutov
2021-08-16 11:48 ` bug#48841: " Eli Zaretskii
2021-08-16 8:47 ` Daniel Mendler
2021-08-14 2:55 ` bug#47711: bug#48841: " Dmitry Gutov
2021-08-14 7:16 ` bug#48841: " Eli Zaretskii
2023-10-24 22:25 ` bug#47711: " Dmitry Gutov
2023-10-25 17:52 ` João Távora
2023-10-25 20:50 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-10-25 21:02 ` João Távora
2023-10-25 22:12 ` João Távora
2023-10-26 21:49 ` João Távora
2023-10-26 23:10 ` Dmitry Gutov
2023-10-26 23:27 ` João Távora
2023-10-26 23:35 ` Dmitry Gutov
2023-10-26 23:52 ` João Távora
2023-10-26 23:25 ` Dmitry Gutov
2023-10-26 23:44 ` João Távora
2023-10-27 0:11 ` Dmitry Gutov
2023-10-27 0:26 ` João Távora
2023-10-27 13:29 ` Dmitry Gutov
2023-10-27 13:46 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-10-27 15:41 ` Dmitry Gutov
2023-10-27 16:19 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-10-27 17:06 ` Dmitry Gutov
2023-10-27 18:12 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-10-29 2:07 ` Dmitry Gutov
2023-10-29 4:41 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-11-03 0:16 ` Dmitry Gutov
2023-11-03 3:05 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-10-27 17:16 ` João Távora
2023-10-28 22:24 ` Dmitry Gutov
2023-10-29 23:12 ` João Távora
2023-10-31 3:20 ` Dmitry Gutov
2023-10-31 10:55 ` João Távora
2023-10-31 20:52 ` Dmitry Gutov
2023-11-01 18:47 ` João Távora
2023-11-01 22:45 ` Dmitry Gutov
2023-11-02 9:48 ` João Távora
2023-11-02 10:10 ` Eli Zaretskii
2023-11-02 10:39 ` João Távora
2023-11-02 10:58 ` Eli Zaretskii
2023-11-02 11:12 ` João Távora
2023-11-02 14:40 ` Dmitry Gutov
2023-11-02 15:24 ` João Távora
2023-11-02 15:36 ` Dmitry Gutov
2023-11-02 15:58 ` João Távora
2023-11-02 16:03 ` Dmitry Gutov
2023-11-02 16:09 ` João Távora
2023-11-02 16:15 ` Dmitry Gutov
2021-04-11 20:51 ` bug#47711: 27.1; Deferred highlighting support in `completion-all-completions', `vertico--all-completions` Daniel Mendler
[not found] ` <handler.47711.B.16181742862702.ack@debbugs.gnu.org>
2021-04-18 21:26 ` bug#47711: Acknowledgement (27.1; Deferred highlighting support in `completion-all-completions', `vertico--all-completions`) Daniel Mendler
2023-11-04 18:46 ` bug#47711: bug#48841: bug#47711: bug#48841: bug#47711: [PATCH VERSION 2] Add new `completion-filter-completions` API and deferred highlighting Howard Melman
2024-04-08 17:19 ` bug#47711: 27.1; Deferred highlighting support in `completion-all-completions', `vertico--all-completions` Dmitry Gutov
2023-11-06 16:20 ` 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 19:38 ` Dmitry Gutov
2023-11-07 12:13 ` João Távora
2023-11-08 1:06 ` Dmitry Gutov
2023-11-08 1:24 ` João Távora
2023-11-08 1:47 ` Dmitry Gutov
2023-10-27 0:14 ` João Távora
2021-08-14 8:23 ` João Távora
2021-08-16 3:48 ` Dmitry Gutov
2021-08-16 4:20 ` bug#48841: " João Távora
2021-08-16 8:53 ` Daniel Mendler
2021-08-14 6:45 ` Eli Zaretskii
2021-08-14 3:11 ` bug#47711: bug#48841: [PATCH] " Dmitry Gutov
2021-08-12 8:00 ` Eli Zaretskii
2021-08-12 8:47 ` Daniel Mendler
2021-08-14 6:27 ` Eli Zaretskii
2021-08-16 9:42 ` Daniel Mendler
2021-08-16 12:58 ` bug#47711: " Eli Zaretskii
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CALDnm51UMr2ZRPbuNtKZnMPmWPRcOgNNFs1-PA=29_1s=y9cDw@mail.gmail.com' \
--to=joaotavora@gmail.com \
--cc=47711@debbugs.gnu.org \
--cc=48841@debbugs.gnu.org \
--cc=dgutov@yandex.ru \
--cc=larsi@gnus.org \
--cc=mail@daniel-mendler.de \
--cc=monnier@iro.umontreal.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.