GNU bug report logs - #71419
30.0.50; Completion does not always highlight the "common part" corresponding to suffix

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 71419 in the body.
You can then email your comments to 71419 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: bug-gnu-emacs <at> gnu.org
Subject: 30.0.50; Completion does not always highlight the "common part"
 corresponding to suffix
Date: Sat, 8 Jun 2024 01:36:42 +0300

X-Debbugs-Cc: monnier <at> IRO.UMontreal.CA

For example:

1. Type

  (fo|-function

where | denotes the position of point.

2. C-M-i

You get 5 completions (the exact number is not important), where "fo" is
highlighted in all of them, but "function is not.

3. Move point to after "-" (meaning, just one character forward), press
C-M-i again (twice).

Completion expands the text to "(fo-|-function" (the addition might be
unimportant) and "function" is highlighted with the face
`completions-common-part' now, in all completions.

This seems inconsistent.

Also, highlighting is information. If it was more uniform, we could use
it to e.g. address the FIXME in completion-all-completions. Though
that's up for discussion, given that highlighting is inherently less
reliable than other methods we use. But it would be a non-breaking
change, OT2H.

WDYT?

In GNU Emacs 30.0.50 (build 28, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.18.0, Xaw scroll bars) of 2024-06-07 built on potemkin
Repository revision: b571c6571c8bc4c34569650104aee273c19cbfd4
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12302000
System Description: Ubuntu 23.10

Message #8 received at 71419 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: 71419 <at> debbugs.gnu.org
Cc: monnier <at> iro.umontreal.ca
Subject: Re: bug#71419: 30.0.50; Completion does not always highlight the
 "common part" corresponding to suffix
Date: Sat, 8 Jun 2024 01:47:46 +0300

On 08/06/2024 01:36, Dmitry Gutov wrote:
> If it was more uniform, we could use
> it to e.g. address the FIXME in completion-all-completions.

To address bug#70968, among other things.

Message #11 received at 71419 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 71419 <at> debbugs.gnu.org
Subject: Re: bug#71419: 30.0.50; Completion does not always highlight the
 "common part" corresponding to suffix
Date: Sun, 09 Jun 2024 09:52:56 -0400

> For example:
>
> 1. Type (fo|-function
> 2. C-M-i
>
> You get 5 completions (the exact number is not important), where "fo" is
> highlighted in all of them, but "function is not.

Looks like a bug in the `basic` completion style: it filters things that
match the glob pattern `fo*-function` yet it only highlights the prefix.

> Also, highlighting is information. If it was more uniform, we could use
> it to e.g. address the FIXME in completion-all-completions.

I don't think it could be trusted to always provide the needed data (and
even if it does, it would be sufficiently clunky to use that I'm not
sure we'd want to rely on it).


        Stefan

Message #14 received at 71419 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 71419 <at> debbugs.gnu.org
Subject: Re: bug#71419: 30.0.50; Completion does not always highlight the
 "common part" corresponding to suffix
Date: Sun, 9 Jun 2024 23:38:57 +0300

On 09/06/2024 16:52, Stefan Monnier wrote:
>> For example:
>>
>> 1. Type (fo|-function
>> 2. C-M-i
>>
>> You get 5 completions (the exact number is not important), where "fo" is
>> highlighted in all of them, but "function is not.
> 
> Looks like a bug in the `basic` completion style: it filters things that
> match the glob pattern `fo*-function` yet it only highlights the prefix.

Yes, thanks. Looks like this one-liner fixes it:

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index f62cb2566b2..144cda8cfdc 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -3816,7 +3816,7 @@ completion-basic-all-completions
                             'point
                             (substring afterpoint 0 (cdr bounds)))))
          (all (completion-pcm--all-completions prefix pattern table 
pred)))
-    (completion-hilit-commonality all point (car bounds))))
+    (completion-pcm--hilit-commonality pattern all)))

 ;;; Partial-completion-mode style completion.



>> Also, highlighting is information. If it was more uniform, we could use
>> it to e.g. address the FIXME in completion-all-completions.
> 
> I don't think it could be trusted to always provide the needed data (and
> even if it does, it would be sufficiently clunky to use that I'm not
> sure we'd want to rely on it).

You're probably right.

But it would have been nice to be able to use it in the "progressive 
improvement" kind of fashion: when the suffix and the other parts are 
highlighted correctly, we do the right thing; if the style missed the 
suffix highlighting, we do the other thing - and put the responsibility 
on the third party.

Message #17 received at 71419 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 71419 <at> debbugs.gnu.org
Subject: Re: bug#71419: 30.0.50; Completion does not always highlight the
 "common part" corresponding to suffix
Date: Sun, 09 Jun 2024 17:06:42 -0400

> diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
> index f62cb2566b2..144cda8cfdc 100644
> --- a/lisp/minibuffer.el
> +++ b/lisp/minibuffer.el
> @@ -3816,7 +3816,7 @@ completion-basic-all-completions
>                              'point
>                              (substring afterpoint 0 (cdr bounds)))))
>           (all (completion-pcm--all-completions prefix pattern table pred)))
> -    (completion-hilit-commonality all point (car bounds))))
> +    (completion-pcm--hilit-commonality pattern all)))
>
>  ;;; Partial-completion-mode style completion.

Thanks for tracking it down.  It matches my expectation.

> But it would have been nice to be able to use it in the "progressive
>  improvement" kind of fashion: when the suffix and the other parts are
>  highlighted correctly, we do the right thing; if the style missed the
>  suffix highlighting, we do the other thing - and put the responsibility on
> the third party.

🙂


        Stefan

Message #20 received at 71419 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 71419 <at> debbugs.gnu.org
Subject: Re: bug#71419: 30.0.50; Completion does not always highlight the
 "common part" corresponding to suffix
Date: Thu, 13 Jun 2024 02:57:43 +0300

[Message part 1 (text/plain, inline)]

On 10/06/2024 00:06, Stefan Monnier wrote:
>> diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
>> index f62cb2566b2..144cda8cfdc 100644
>> --- a/lisp/minibuffer.el
>> +++ b/lisp/minibuffer.el
>> @@ -3816,7 +3816,7 @@ completion-basic-all-completions
>>                               'point
>>                               (substring afterpoint 0 (cdr bounds)))))
>>            (all (completion-pcm--all-completions prefix pattern table pred)))
>> -    (completion-hilit-commonality all point (car bounds))))
>> +    (completion-pcm--hilit-commonality pattern all)))
>>
>>   ;;; Partial-completion-mode style completion.
> 
> Thanks for tracking it down.  It matches my expectation.

Thanks, now fixed on master.

>> But it would have been nice to be able to use it in the "progressive
>>   improvement" kind of fashion: when the suffix and the other parts are
>>   highlighted correctly, we do the right thing; if the style missed the
>>   suffix highlighting, we do the other thing - and put the responsibility on
>> the third party.
> 
> 🙂

It could work like in the attached. I agree that it's somewhat brittle, 
though.

If you're not in favor of installing this patch, what would be your 
preferred strategy for fixing bug#70968? We could try resurrecting the 
relevant part of Daniel's patch for completion-all-completions, but it 
means a fair amount of breakage. Or another dynamic variable similar to 
completion-lazy-hilit-fn...

FWIW my interest here is how to better implement the same step in 
company-mode, but the default UI is a good common ground.

[completions--count-common-spans.diff (text/x-patch, attachment)]

Message #25 received at 71419-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>, 71419-done <at> debbugs.gnu.org
Subject: Re: bug#71419: 30.0.50; Completion does not always highlight the
 "common part" corresponding to suffix
Date: Fri, 28 Feb 2025 18:06:01 -0800

Dmitry Gutov <dmitry <at> gutov.dev> writes:

> On 10/06/2024 00:06, Stefan Monnier wrote:
>>> diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
>>> index f62cb2566b2..144cda8cfdc 100644
>>> --- a/lisp/minibuffer.el
>>> +++ b/lisp/minibuffer.el
>>> @@ -3816,7 +3816,7 @@ completion-basic-all-completions
>>>                               'point
>>>                               (substring afterpoint 0 (cdr bounds)))))
>>>            (all (completion-pcm--all-completions prefix pattern table pred)))
>>> -    (completion-hilit-commonality all point (car bounds))))
>>> +    (completion-pcm--hilit-commonality pattern all)))
>>>
>>>   ;;; Partial-completion-mode style completion.
>> Thanks for tracking it down.  It matches my expectation.
>
> Thanks, now fixed on master.

I'm therefore closing this bug report.

>
>>> But it would have been nice to be able to use it in the "progressive
>>>   improvement" kind of fashion: when the suffix and the other parts are
>>>   highlighted correctly, we do the right thing; if the style missed the
>>>   suffix highlighting, we do the other thing - and put the responsibility on
>>> the third party.
>> 🙂
>
> It could work like in the attached. I agree that it's somewhat brittle, though.
>
> If you're not in favor of installing this patch, what would be your preferred
> strategy for fixing bug#70968? We could try resurrecting the relevant part of
> Daniel's patch for completion-all-completions, but it means a fair amount of
> breakage. Or another dynamic variable similar to completion-lazy-hilit-fn...
>
> FWIW my interest here is how to better implement the same step in company-mode,
> but the default UI is a good common ground.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.