Subject: bug#70968: 29.2.50; choose-completion on an emacs22-style completion deletes text after point

From: Dmitry Gutov <[email-masked]>
Date: Thu, 17 Oct 2024 03:19:48 +0300

On 15/10/2024 21:53, Spencer Baugh wrote:

> I think this is preferable, though, and even if it isn't, it should
> still be supported.
> 
> This keeps it explicit to the user exactly what text will be inserted
> into the buffer upon accepting a completion.
> 
> Consider these two cases of completion (| representing point):
> 
> A. switch-to-|asdf
> 
> B. switch-to-|buffer
> 
> In both cases, switch-to-next-buffer is a potential completion, provided
> by "emacs22" in the case A and "basic" in case B.
> 
> But in case A, if switch-to-next-buffer is chosen, the "asdf" should be
> preserved, and in case B, if switch-to-next-buffer, the "buffer" should
> be deleted.
> 
> Because they have different behavior, they should appear differently to
> the user.  That's why I think case A should show
> "switch-to-next-bufferasdf" in the *Completions* buffer, with "asdf"
> grayed out via a "completions-ignored" face.
> 
> If a user decides they don't want "asdf" text to be shown in case A,
> then they can customize that.  If a frontend decides it doesn't want to
> show that text, it can omit the text.
> 
> But it should at least be *possible* for the text to be shown.
> Otherwise there's no way to distinguish the two cases.

I suppose we should ask what makes sense as the default behavior, and if 
it's going to be different between frontends, how difficult is the 
"other" approach would be to support.

Since company-mode supports the "proper" behavior for emacs22, I've had 
some time to get used to it and see if anything looks off. And IME the 
current indicators seem enough. For Elisp anyway, we a) have the 
completions themselves, b) the highlighting inside the completions for 
the characters matching the input. When the style matches the prefix 
only, all the completions will only have that many characters 
highlighted, not the chars from the suffix.

See the second attached pic for an example. It seems okay to me, but I 
suppose the experience might vary between completion types and 
programming languages, etc.

>> Finally, the use of 'completion-position-after-insert' seems like it
>> could be used separately from the "ignored text", meaning the spans
>> don't have to match. So it could be a separate feature, one that's
>> easy enough to implement on its own.
> 
> Yes, and I indeed think this feature is useful on its own, because it
> allows choose-completion to be fully equivalent to
> completion-try-completion.  Right now completion-try-completion can
> change point, but choose-completion can't.  That's limiting for a bunch
> of reasons, and the inability to fix this bug purely in a completion
> style is one of them.

Makes a certain sense. Just to make a note, I think try-completion moves 
point specifically to help position before the next char which would 
help disambiguate completions. choose-completion happens when no other 
completions are relevant, so it couldn't move point similarly.

>> None of the above would be insurmountable, but here's what I think
>> avoids it using the 'completion--adjust-metadata' thingy that was
>> originally added for 'flex' a few releases ago: adding a metadata key
>> 'completion-ignore-after-point'.
>>
>> The attached patch does not make a distinction for file name
>> completion - it just covers the core problem - but I think any UI
>> could use the addition and likewise lookup the 'file' category, and
>> print the "hidden" suffix in the Completions, and decide to drop the
>> suffix in your first scenario (file name completion with exit
>> imminent).
>>
>> Spencer, Stefan, WDYT?
> 
> Your patch is simple, and it works for default completion, but two
> issues:
> 
> - Your patch doesn't distinguish the two cases A and B that I described above.
>    
> - Your patch will require company-mode and other frontends to change.
>    My patch does not strictly require that.

In practice, company-mode will require a small change anyway, since it 
doesn't do pass-through for any of the faces now. See the first attachment.

But to slice off the suffixes in the popup, which I'm currently inclined 
to do, might require more (and more complex) code than to print them 
selectively in the frontends that choose to (the "opt-in" approach). 
This is a part that gives me pause.

>    But if we're already requiring that, I think we should take the
>    opportunity to add the more powerful feature
>    completion-position-after-insert.
> 
> Here's a simplified version of my earlier patch, which modifies the
> emacs22 style to make it easier to discuss.  I think this is equally
> simple as your patch, but it:
> 
> - Distinguishes the cases A and B by including the ignored text in the
>    completion, just grayed out.
> 
> - Fixes the bug for other frontends without requiring them to change
>    (they get additional benefit when they change to support
>    completion-position-after-insert)
> 
> Note that due to a separate bug in completion--twq-all, used in filename
> completion, the graying-out face is dropped from the completion
> candidates before they reach *Completions*; so if you try this, try it
> by e.g. completing Lisp symbols.

Thanks. If I were to do a test-drive to look for edge cases, is Lisp 
symbols the only kind of completion that I should try out?