bug#48841: bug#47711: bug#48841: bug#47711: [PATCH VERSION 2] Add new `completion-filter-completions` API and deferred highlighting

[Daniel Mendler <mail@daniel-mendler.de>]

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