Re: [PATCH] Completion preview common

[Ergus <spacibba@aol.com>]

Hi:

On Tue, Apr 09, 2024 at 08:30:16PM +0200, Eshel Yaron wrote:
>Hi,
>
>Ergus <spacibba@aol.com> writes:
>
>> Your idea sounds very sensible. But I don't want to add unneeded
>> complexity to the package. To get a complete candidates list I prefer
>> to rely on other tools like company or Corfu. And getting the first
>> entry in my use case is not very useful.
>
>The patch you proposed adds an option to show only the longest common
>prefix instead of a whole candidate, but the whole candidate includes
>the longest common prefix, and so ISTM that it provides strictly more
>information.  That's why an option that makes Completion Preview mode
>only show the longest common prefix strikes me as less useful than
>letting you complete only up to that common prefix on demand, while
>still previewing a full candidate.  If that doesn't fit your bill,
>perhaps you can describe the use case with some more detail?
>

I don't think common preview it is less useful. Indeed what I find
totally useless is the insertion of first candidate instead of the
common prefix. Why the first? How far we need to rotate until we find
the right one? isn't it faster just continue typing instead of
next-next-next or insert the first and remove the last letters and
re-write the ones I need?

Inserting the common prefix is actually a good trick that asserts that
the tab-insertion will be correct (but maybe incomplete), because when
some candidate is shown it means that it is 100% sure what is valid
after point in the whole completion list; but and also matches the
muscular memory for bash users.

In the minibuffer (and in bash and zsh) the behavior is
similar. Generally we only need preview+tab-insert the common part,
because the first candidate is not good in most of the cases; actually
it is as good and bad than any other candidate (that's why packages that
track and sort based on history, and context exist) OTOH to browse
candidates is generally better things like corfu, company or
auto-complete, so, a list/tooltip to navigate with extra information (a
preview of 5-10 candidates, a counter of the total number, a counter
with the relative position in the list, similar candidates grouped and
sorted in the tooltip).

Inserting the common part is useful when typing, to safe time, specially
when working with long names prefixes packages (when all the functions
and variables have the same prefix). Inserting the first candidate for
those is actually annoying because there will be many candidates with
same prefix, but different suffix. And rotating candidate is usually
slower than just type some more letter. 

OTOH com<Tab>-pr<Tab>-somesufix is faster and intuitive than
com<M-n><M-n><M-n><M-n>..<TAB> specially because the user does not even
know if the actual candidates list is long or short and how far the
candidate may be.

>> Certainly we can add many other features, properties, bindings and
>> colors, but every feature implies complexity I'll like to avoid
>> (specially in a completion feature code). The current package is
>> simple, with few bindings and a predictable behavior. Please keep
>> that.
>>
>> I added this change because it didn't imply more than 10 extra lines
>> of execution code (ignoring comments); otherwise I would prefer to
>> live with it as is.
>
>While I agree that brevity and simplicity are some of the nice
>properties of this library, IMO providing useful features is far more
>important then keeping the line count to a minimum.  So if there's a
>strong use case that we don't currently support, I wouldn't mind adding
>10, 100 or 1000 lines of code to accommodate that.
>
I don't mean that having a more elaborated preview is not useful; what I
mean is that it doesn't worth it due to the code complexity and
performance impact it generally implies. Many packages have tried this
before.

The package is useful as is, and in Emacs generally every new feature
finds 100 requests for more and more details and configs hard to
maintain and sadly mostly under-tested due to the high number of
different config combinations needed. So I recommend KISS better and
just add more features if they are actually requested by someone.

But of course, you are free to implement the whole completion feature.
Now you have the equivalents to company-preview-frontend and
company-preview-if-just-one-frontend... my patch actually tried to mimic
the missing company-preview-common-frontend

If you allow me a couple of code comments:

1. Considering how the package works maybe it is useful to let-set:

(completion-styles '(basic)) before calling completion-all-completions.

Because you only care and handle the candidates where the matches are in
the beginning, but with substring, partial-completion, etc the matches
could be anywhere, giving a longer list (with many useless) candidates.

2. The `completion-all-completions`' output is a list with propertized
string and information of the match location within the string... maybe
it worth taking advantage of that information for the insertion or
preview.

Otherwise it is probably better to use `all-completions` instead. It
will be faster and you don't need to de-propertize the substrings to add
your own fonts and properties.

>
>Best,
>
>Eshel
>
Best,
Ergus