unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#70968: 29.2.50; choose-completion on an emacs22-style completion deletes text after point
@ 2024-05-15 20:26 Spencer Baugh
  2024-05-16  8:13 ` Eli Zaretskii
  2024-06-20 15:45 ` Spencer Baugh
  0 siblings, 2 replies; 16+ messages in thread
From: Spencer Baugh @ 2024-05-15 20:26 UTC (permalink / raw)
  To: 70968; +Cc: dmitry, juri


try-completion and choose-completion have different behavior; with the
emacs22 completion style, the former preserves the text after point
(while ignoring it), whereas the latter deletes the text after point.

1. emacs -Q

2. In scratch, type "inhibit-asdf" and move point to after the "-"

3. Type "q" and M-<tab>
   The buffer now contains "inhibit-quitasdf", because the emacs22 style
   ignored the text after point and completed on "inhibit-q".

4. C-/ C-/ to change the buffer back to "inhibit-asdf"

5. M-<tab>
   The *Completions* buffer will be displayed, containing among others
   "inhibit-quit" as a completion, because the emacs22 style ignored the
   text after point and completed on "inhibit-".

6. Use M-<down> and M-<ret> to select and choose "inhibit-quit"
   The buffer now contains "inhibit-quit".

The two ways of selecting completions should behave the same.

I suggest that the behavior of not deleting the text after point is
better.  The emacs22 style takes care to not delete the text after point
in try-completion and in completion cycling; we should take similar care
in choose-completion.

This is especially noticeable when using minibuffer-visible-completions,
which make it easier to use choose-completion.  It's also especially
noticeable with corfu-mode, which usually automatically calls
choose-completion when finishing completion.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#70968: 29.2.50; choose-completion on an emacs22-style completion deletes text after point
  2024-05-15 20:26 bug#70968: 29.2.50; choose-completion on an emacs22-style completion deletes text after point Spencer Baugh
@ 2024-05-16  8:13 ` Eli Zaretskii
  2024-05-16 17:26   ` Dmitry Gutov
  2024-05-16 17:40   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-06-20 15:45 ` Spencer Baugh
  1 sibling, 2 replies; 16+ messages in thread
From: Eli Zaretskii @ 2024-05-16  8:13 UTC (permalink / raw)
  To: Spencer Baugh; +Cc: dmitry, 70968, juri

> Cc: dmitry@gutov.dev, juri@linkov.net
> From: Spencer Baugh <sbaugh@janestreet.com>
> Date: Wed, 15 May 2024 16:26:50 -0400
> 
> 
> try-completion and choose-completion have different behavior; with the
> emacs22 completion style, the former preserves the text after point
> (while ignoring it), whereas the latter deletes the text after point.
> 
> 1. emacs -Q
> 
> 2. In scratch, type "inhibit-asdf" and move point to after the "-"
> 
> 3. Type "q" and M-<tab>
>    The buffer now contains "inhibit-quitasdf", because the emacs22 style
>    ignored the text after point and completed on "inhibit-q".
> 
> 4. C-/ C-/ to change the buffer back to "inhibit-asdf"
> 
> 5. M-<tab>
>    The *Completions* buffer will be displayed, containing among others
>    "inhibit-quit" as a completion, because the emacs22 style ignored the
>    text after point and completed on "inhibit-".
> 
> 6. Use M-<down> and M-<ret> to select and choose "inhibit-quit"
>    The buffer now contains "inhibit-quit".
> 
> The two ways of selecting completions should behave the same.
> 
> I suggest that the behavior of not deleting the text after point is
> better.  The emacs22 style takes care to not delete the text after point
> in try-completion and in completion cycling; we should take similar care
> in choose-completion.

If making these two ways consistent means we need to change the
behavior of emacs22 style of completion, then I'm sorry, but we cannot
do that.  This style is a legacy style, and is there to provide the
legacy behavior for those who need it.

So for making the decisions in this case we need to know how did Emacs
22 and later versions behave in the above scenario, and take it from
there.  If they all behaved like you show, then this behavior must
stay unchanged, unfortunately, for backward-compatibility reasons.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#70968: 29.2.50; choose-completion on an emacs22-style completion deletes text after point
  2024-05-16  8:13 ` Eli Zaretskii
@ 2024-05-16 17:26   ` Dmitry Gutov
  2024-05-16 18:25     ` Eli Zaretskii
  2024-05-16 17:40   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 16+ messages in thread
From: Dmitry Gutov @ 2024-05-16 17:26 UTC (permalink / raw)
  To: Eli Zaretskii, Spencer Baugh; +Cc: 70968, Stefan Monnier, juri

On 16/05/2024 11:13, Eli Zaretskii wrote:
>> Cc: dmitry@gutov.dev, juri@linkov.net
>> From: Spencer Baugh <sbaugh@janestreet.com>
>> Date: Wed, 15 May 2024 16:26:50 -0400
>>
>>
>> try-completion and choose-completion have different behavior; with the
>> emacs22 completion style, the former preserves the text after point
>> (while ignoring it), whereas the latter deletes the text after point.
>>
>> 1. emacs -Q
>>
>> 2. In scratch, type "inhibit-asdf" and move point to after the "-"
>>
>> 3. Type "q" and M-<tab>
>>     The buffer now contains "inhibit-quitasdf", because the emacs22 style
>>     ignored the text after point and completed on "inhibit-q".
>>
>> 4. C-/ C-/ to change the buffer back to "inhibit-asdf"
>>
>> 5. M-<tab>
>>     The *Completions* buffer will be displayed, containing among others
>>     "inhibit-quit" as a completion, because the emacs22 style ignored the
>>     text after point and completed on "inhibit-".
>>
>> 6. Use M-<down> and M-<ret> to select and choose "inhibit-quit"
>>     The buffer now contains "inhibit-quit".
>>
>> The two ways of selecting completions should behave the same.
>>
>> I suggest that the behavior of not deleting the text after point is
>> better.  The emacs22 style takes care to not delete the text after point
>> in try-completion and in completion cycling; we should take similar care
>> in choose-completion.
> 
> If making these two ways consistent means we need to change the
> behavior of emacs22 style of completion, then I'm sorry, but we cannot
> do that.  This style is a legacy style, and is there to provide the
> legacy behavior for those who need it.

I don't think that would be required exactly.

The problem here (IIUC) is that completion behaves differently with the 
emacs22 style depending on whether the execution path went through 
choose-completion (which is not a method of completion style but a 
common subroutine) or not (when completion--do-completion performed 
expansion).





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#70968: 29.2.50; choose-completion on an emacs22-style completion deletes text after point
  2024-05-16  8:13 ` Eli Zaretskii
  2024-05-16 17:26   ` Dmitry Gutov
@ 2024-05-16 17:40   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-05-16 18:28     ` Eli Zaretskii
  1 sibling, 1 reply; 16+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-05-16 17:40 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Spencer Baugh, juri, 70968, dmitry

>> I suggest that the behavior of not deleting the text after point is
>> better.  The emacs22 style takes care to not delete the text after point
>> in try-completion and in completion cycling; we should take similar care
>> in choose-completion.

I agree this inconsistency is a bug.
I suspect it may have been a result of implementation limitations.

A typical use case is

    C-x C-w bar
    ... realize you're not in the right subdir ...
    M-b ?
    ... choose some subdirectory in which you want to save `bar`
    ... Oops, oh no, `bar` has disappeared!

> If making these two ways consistent means we need to change the
> behavior of emacs22 style of completion, then I'm sorry, but we cannot
> do that.  This style is a legacy style, and is there to provide the
> legacy behavior for those who need it.

I'm not sure it's terribly important to preserve this detail of the
behavior of `Emacs-22`.  The `emacs22` style does not aim to provide the
illusion you're running an old Emacs.  I named it that way because
I couldn't come up with a good descriptive name for it.  If it
misbehaves, I don't see a need to be bug-compatible, especially since
this doesn't affect ELisp code but users.


        Stefan






^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#70968: 29.2.50; choose-completion on an emacs22-style completion deletes text after point
  2024-05-16 17:26   ` Dmitry Gutov
@ 2024-05-16 18:25     ` Eli Zaretskii
  2024-08-26  0:08       ` Dmitry Gutov
  0 siblings, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2024-05-16 18:25 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: sbaugh, 70968, monnier, juri

> Date: Thu, 16 May 2024 20:26:31 +0300
> Cc: 70968@debbugs.gnu.org, juri@linkov.net,
>  Stefan Monnier <monnier@IRO.UMontreal.CA>
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> On 16/05/2024 11:13, Eli Zaretskii wrote:
> >> Cc: dmitry@gutov.dev, juri@linkov.net
> >> From: Spencer Baugh <sbaugh@janestreet.com>
> >> Date: Wed, 15 May 2024 16:26:50 -0400
> >>
> >>
> >> try-completion and choose-completion have different behavior; with the
> >> emacs22 completion style, the former preserves the text after point
> >> (while ignoring it), whereas the latter deletes the text after point.
> >>
> >> 1. emacs -Q
> >>
> >> 2. In scratch, type "inhibit-asdf" and move point to after the "-"
> >>
> >> 3. Type "q" and M-<tab>
> >>     The buffer now contains "inhibit-quitasdf", because the emacs22 style
> >>     ignored the text after point and completed on "inhibit-q".
> >>
> >> 4. C-/ C-/ to change the buffer back to "inhibit-asdf"
> >>
> >> 5. M-<tab>
> >>     The *Completions* buffer will be displayed, containing among others
> >>     "inhibit-quit" as a completion, because the emacs22 style ignored the
> >>     text after point and completed on "inhibit-".
> >>
> >> 6. Use M-<down> and M-<ret> to select and choose "inhibit-quit"
> >>     The buffer now contains "inhibit-quit".
> >>
> >> The two ways of selecting completions should behave the same.
> >>
> >> I suggest that the behavior of not deleting the text after point is
> >> better.  The emacs22 style takes care to not delete the text after point
> >> in try-completion and in completion cycling; we should take similar care
> >> in choose-completion.
> > 
> > If making these two ways consistent means we need to change the
> > behavior of emacs22 style of completion, then I'm sorry, but we cannot
> > do that.  This style is a legacy style, and is there to provide the
> > legacy behavior for those who need it.
> 
> I don't think that would be required exactly.
> 
> The problem here (IIUC) is that completion behaves differently with the 
> emacs22 style depending on whether the execution path went through 
> choose-completion (which is not a method of completion style but a 
> common subroutine) or not (when completion--do-completion performed 
> expansion).

I understand that much.  But what did these two (or their
then-equivalents) do in Emacs 22 and Emacs 23?





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#70968: 29.2.50; choose-completion on an emacs22-style completion deletes text after point
  2024-05-16 17:40   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-05-16 18:28     ` Eli Zaretskii
  0 siblings, 0 replies; 16+ messages in thread
From: Eli Zaretskii @ 2024-05-16 18:28 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: sbaugh, juri, 70968, dmitry

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Spencer Baugh <sbaugh@janestreet.com>,  dmitry@gutov.dev,
>   70968@debbugs.gnu.org,  juri@linkov.net
> Date: Thu, 16 May 2024 13:40:35 -0400
> 
> > If making these two ways consistent means we need to change the
> > behavior of emacs22 style of completion, then I'm sorry, but we cannot
> > do that.  This style is a legacy style, and is there to provide the
> > legacy behavior for those who need it.
> 
> I'm not sure it's terribly important to preserve this detail of the
> behavior of `Emacs-22`.  The `emacs22` style does not aim to provide the
> illusion you're running an old Emacs.  I named it that way because
> I couldn't come up with a good descriptive name for it.  If it
> misbehaves, I don't see a need to be bug-compatible, especially since
> this doesn't affect ELisp code but users.

I think it does, sorry.  Suchj old behavior is a de-facto standard.
If we change that, we should at least have a knob to get back the old
behavior.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#70968: 29.2.50; choose-completion on an emacs22-style completion deletes text after point
  2024-05-15 20:26 bug#70968: 29.2.50; choose-completion on an emacs22-style completion deletes text after point Spencer Baugh
  2024-05-16  8:13 ` Eli Zaretskii
@ 2024-06-20 15:45 ` Spencer Baugh
  1 sibling, 0 replies; 16+ messages in thread
From: Spencer Baugh @ 2024-06-20 15:45 UTC (permalink / raw)
  To: 70968; +Cc: dmitry, Stefan Monnier, juri


Here is an idea:

Currently emacs22 is the only style that ignores the text after point
when completing.  But, this is often useful behavior, and I'd like to
support it in other completion styles as a fallback.  Specifically, I
think it would be nice if completion always:

1. Try to complete including the suffix after point (like the basic,
partial-completion, substring styles do)

2. If no completion matches, try to complete again without the suffix
after point

What if we implemented this in the completion machinery itself?  If no
style matches, run through all the styles again without the suffix after
point.  This would allow e.g. partial-completion behavior when
completing in the middle of a symbol, which currently doesn't work.

This would basically remove the emacs22 style, since it would be a
behavior implemented at a higher level.  Then each completion frontend
could implement this "first with suffix then without suffix" behavior
separately, which would make choose-completion and similar calls easier
to make work.

Stefan, I wonder if you have thoughts on this idea?





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#70968: 29.2.50; choose-completion on an emacs22-style completion deletes text after point
  2024-05-16 18:25     ` Eli Zaretskii
@ 2024-08-26  0:08       ` Dmitry Gutov
  2024-09-07  7:30         ` Eli Zaretskii
  0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Gutov @ 2024-08-26  0:08 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: sbaugh, 70968, monnier, juri

On 16/05/2024 21:25, Eli Zaretskii wrote:
>> I don't think that would be required exactly.
>>
>> The problem here (IIUC) is that completion behaves differently with the
>> emacs22 style depending on whether the execution path went through
>> choose-completion (which is not a method of completion style but a
>> common subroutine) or not (when completion--do-completion performed
>> expansion).
> I understand that much.  But what did these two (or their
> then-equivalents) do in Emacs 22 and Emacs 23?

I'm guessing they behaved incorrectly (or however we want to call the 
inconsistent behavior), but I don't have a compiled Emacs 22/23 around, 
and they might be difficult to build.

Note that we fixed bug#48356 not too long ago, which is from the same 
general area, and it probably originated from before Emacs 22/23 too.

It's worth looking for edge cases where we'd strongly prefer the current 
behavior, and they might exist, but so far I only know of situations 
where the change would be for the better, or the user might be okay with 
either (example at the end of https://debbugs.gnu.org/72705#35).





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#70968: 29.2.50; choose-completion on an emacs22-style completion deletes text after point
  2024-08-26  0:08       ` Dmitry Gutov
@ 2024-09-07  7:30         ` Eli Zaretskii
  2024-09-08  2:02           ` Dmitry Gutov
  2024-09-08 11:12           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 2 replies; 16+ messages in thread
From: Eli Zaretskii @ 2024-09-07  7:30 UTC (permalink / raw)
  To: sbaugh, Dmitry Gutov, monnier; +Cc: 70968, juri

> Date: Mon, 26 Aug 2024 03:08:56 +0300
> Cc: sbaugh@janestreet.com, 70968@debbugs.gnu.org, juri@linkov.net,
>  monnier@IRO.UMontreal.CA
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> On 16/05/2024 21:25, Eli Zaretskii wrote:
> >> I don't think that would be required exactly.
> >>
> >> The problem here (IIUC) is that completion behaves differently with the
> >> emacs22 style depending on whether the execution path went through
> >> choose-completion (which is not a method of completion style but a
> >> common subroutine) or not (when completion--do-completion performed
> >> expansion).
> > I understand that much.  But what did these two (or their
> > then-equivalents) do in Emacs 22 and Emacs 23?
> 
> I'm guessing they behaved incorrectly (or however we want to call the 
> inconsistent behavior), but I don't have a compiled Emacs 22/23 around, 
> and they might be difficult to build.
> 
> Note that we fixed bug#48356 not too long ago, which is from the same 
> general area, and it probably originated from before Emacs 22/23 too.
> 
> It's worth looking for edge cases where we'd strongly prefer the current 
> behavior, and they might exist, but so far I only know of situations 
> where the change would be for the better, or the user might be okay with 
> either (example at the end of https://debbugs.gnu.org/72705#35).

Ping!  How should we proceed with this bug report?





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#70968: 29.2.50; choose-completion on an emacs22-style completion deletes text after point
  2024-09-07  7:30         ` Eli Zaretskii
@ 2024-09-08  2:02           ` Dmitry Gutov
  2024-09-08 11:12           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 0 replies; 16+ messages in thread
From: Dmitry Gutov @ 2024-09-08  2:02 UTC (permalink / raw)
  To: Eli Zaretskii, sbaugh, monnier; +Cc: 70968, juri

On 07/09/2024 10:30, Eli Zaretskii wrote:
>> Date: Mon, 26 Aug 2024 03:08:56 +0300
>> Cc:sbaugh@janestreet.com,70968@debbugs.gnu.org,juri@linkov.net,
>>   monnier@IRO.UMontreal.CA
>> From: Dmitry Gutov<dmitry@gutov.dev>
>>
>> On 16/05/2024 21:25, Eli Zaretskii wrote:
>>>> I don't think that would be required exactly.
>>>>
>>>> The problem here (IIUC) is that completion behaves differently with the
>>>> emacs22 style depending on whether the execution path went through
>>>> choose-completion (which is not a method of completion style but a
>>>> common subroutine) or not (when completion--do-completion performed
>>>> expansion).
>>> I understand that much.  But what did these two (or their
>>> then-equivalents) do in Emacs 22 and Emacs 23?
>> I'm guessing they behaved incorrectly (or however we want to call the
>> inconsistent behavior), but I don't have a compiled Emacs 22/23 around,
>> and they might be difficult to build.
>>
>> Note that we fixed bug#48356 not too long ago, which is from the same
>> general area, and it probably originated from before Emacs 22/23 too.
>>
>> It's worth looking for edge cases where we'd strongly prefer the current
>> behavior, and they might exist, but so far I only know of situations
>> where the change would be for the better, or the user might be okay with
>> either (example at the end ofhttps://debbugs.gnu.org/72705#35).
> Ping!  How should we proceed with this bug report?

I suggest we come up with a fix (which Stefan might have some ideas 
for), then see which reasonable scenarios get broken, if any. The one 
edge case in Eglot could be fixed in the same Emacs release, I believe.

If any larger scope problems, we could add a variable/option to switch 
to the previous behavior.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#70968: 29.2.50; choose-completion on an emacs22-style completion deletes text after point
  2024-09-07  7:30         ` Eli Zaretskii
  2024-09-08  2:02           ` Dmitry Gutov
@ 2024-09-08 11:12           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-09-10 16:54             ` Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 16+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-09-08 11:12 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: sbaugh, juri, 70968, Dmitry Gutov

> Ping!  How should we proceed with this bug report?

I don't quite understand the question.

After:

Eli wrote:
> Stefan wrote:
> > I'm not sure it's terribly important to preserve this detail of the
> > behavior of `Emacs-22`.  The `emacs22` style does not aim to provide the
> > illusion you're running an old Emacs.  I named it that way because
> > I couldn't come up with a good descriptive name for it.  If it
> > misbehaves, I don't see a need to be bug-compatible, especially since
> > this doesn't affect ELisp code but users.
>
> I think it does, sorry.  Suchj old behavior is a de-facto standard.
> If we change that, we should at least have a knob to get back the old
> behavior.

I thought you had decided that the current behavior is not a bug.
I'm fine with this choice and we can close it as such.
Tho maybe we'd want to deprecate that `emacs22` style because of
those odd cases.


        Stefan






^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#70968: 29.2.50; choose-completion on an emacs22-style completion deletes text after point
  2024-09-08 11:12           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-09-10 16:54             ` Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-09-14  9:17               ` Eli Zaretskii
  0 siblings, 1 reply; 16+ messages in thread
From: Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-09-10 16:54 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Dmitry Gutov, Eli Zaretskii, 70968, juri

Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> Ping!  How should we proceed with this bug report?
>
> I don't quite understand the question.
>
> After:
>
> Eli wrote:
>> Stefan wrote:
>> > I'm not sure it's terribly important to preserve this detail of the
>> > behavior of `Emacs-22`.  The `emacs22` style does not aim to provide the
>> > illusion you're running an old Emacs.  I named it that way because
>> > I couldn't come up with a good descriptive name for it.  If it
>> > misbehaves, I don't see a need to be bug-compatible, especially since
>> > this doesn't affect ELisp code but users.
>>
>> I think it does, sorry.  Suchj old behavior is a de-facto standard.
>> If we change that, we should at least have a knob to get back the old
>> behavior.
>
> I thought you had decided that the current behavior is not a bug.
> I'm fine with this choice and we can close it as such.
> Tho maybe we'd want to deprecate that `emacs22` style because of
> those odd cases.

I definitely don't want to just close this bug.  I often get user
complaints about this behavior.  In my experience, for novice users,
it's a fairly significant inconsistency in the default Emacs completion
experience.

I see a few good ways to fix this in a backwards-compatible way:

A. Fix it in emacs22 with a defcustom to get back the old behavior.

B. Deprecate the emacs22 style and replace it with a new style called
   `ignore-suffix` which has this bug fixed, and which replaces emacs22
   in the default value of completion-styles.

C. Follow this idea I suggested earlier:

   Currently emacs22 is the only style that ignores the text after point
   when completing.  But, this is often useful behavior, and I'd like to
   support it in other completion styles.  Specifically, I think it
   would be nice if completion always:
   
   1. First, run the completion styles on the literal text in the
   completion region or minibuffer.
   
   2. If that returned nil, run the completion styles again, but without
   the text after point in the region or minibuffer.
   
   Step 2 when run with the basic style is equivalent to the emacs22
   style.  So, emacs22 could be removed from the default
   completion-styles, since a completion-styles of '(basic) would be
   equivalent to '(basic emacs22).

   I think this would make it straightforward to then fix
   choose-completion to behave correctly when the completion was
   generated through step 2.
   
I personally like the option C best, because I already want to do this
generalization, so that the ignore-suffix behavior also works for other
completion styles (e.g. partial-completion or substring).

But I would like to get some feedback on this idea from others first.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#70968: 29.2.50; choose-completion on an emacs22-style completion deletes text after point
  2024-09-10 16:54             ` Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-09-14  9:17               ` Eli Zaretskii
  2024-09-14 15:18                 ` Dmitry Gutov
  2024-09-16 19:54                 ` Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 2 replies; 16+ messages in thread
From: Eli Zaretskii @ 2024-09-14  9:17 UTC (permalink / raw)
  To: Spencer Baugh; +Cc: dmitry, 70968, monnier, juri

> From: Spencer Baugh <sbaugh@janestreet.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  juri@linkov.net,  70968@debbugs.gnu.org,
>    Dmitry Gutov <dmitry@gutov.dev>
> Date: Tue, 10 Sep 2024 12:54:05 -0400
> 
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> >> Ping!  How should we proceed with this bug report?
> >
> > I don't quite understand the question.
> >
> > After:
> >
> > Eli wrote:
> >> Stefan wrote:
> >> > I'm not sure it's terribly important to preserve this detail of the
> >> > behavior of `Emacs-22`.  The `emacs22` style does not aim to provide the
> >> > illusion you're running an old Emacs.  I named it that way because
> >> > I couldn't come up with a good descriptive name for it.  If it
> >> > misbehaves, I don't see a need to be bug-compatible, especially since
> >> > this doesn't affect ELisp code but users.
> >>
> >> I think it does, sorry.  Suchj old behavior is a de-facto standard.
> >> If we change that, we should at least have a knob to get back the old
> >> behavior.
> >
> > I thought you had decided that the current behavior is not a bug.
> > I'm fine with this choice and we can close it as such.
> > Tho maybe we'd want to deprecate that `emacs22` style because of
> > those odd cases.
> 
> I definitely don't want to just close this bug.  I often get user
> complaints about this behavior.  In my experience, for novice users,
> it's a fairly significant inconsistency in the default Emacs completion
> experience.
> 
> I see a few good ways to fix this in a backwards-compatible way:
> 
> A. Fix it in emacs22 with a defcustom to get back the old behavior.
> 
> B. Deprecate the emacs22 style and replace it with a new style called
>    `ignore-suffix` which has this bug fixed, and which replaces emacs22
>    in the default value of completion-styles.
> 
> C. Follow this idea I suggested earlier:
> 
>    Currently emacs22 is the only style that ignores the text after point
>    when completing.  But, this is often useful behavior, and I'd like to
>    support it in other completion styles.  Specifically, I think it
>    would be nice if completion always:
>    
>    1. First, run the completion styles on the literal text in the
>    completion region or minibuffer.
>    
>    2. If that returned nil, run the completion styles again, but without
>    the text after point in the region or minibuffer.
>    
>    Step 2 when run with the basic style is equivalent to the emacs22
>    style.  So, emacs22 could be removed from the default
>    completion-styles, since a completion-styles of '(basic) would be
>    equivalent to '(basic emacs22).
> 
>    I think this would make it straightforward to then fix
>    choose-completion to behave correctly when the completion was
>    generated through step 2.
>    
> I personally like the option C best, because I already want to do this
> generalization, so that the ignore-suffix behavior also works for other
> completion styles (e.g. partial-completion or substring).
> 
> But I would like to get some feedback on this idea from others first.

Option C is from my POV the least desirable one: it adds some
complicated logic, which will almost certainly produce unintended
results, as everything in this completion-related mess we have.

I'm okay with adding a new style, per B, but why do we need to
deprecate emacs22 at the same time?  Let users who want this new
behavior customize their completion styles to use this new style
instead of emacs22.  That'd be fully backward compatible, and will
solve the problem for those who don't like the current behavior.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#70968: 29.2.50; choose-completion on an emacs22-style completion deletes text after point
  2024-09-14  9:17               ` Eli Zaretskii
@ 2024-09-14 15:18                 ` Dmitry Gutov
  2024-09-14 16:00                   ` Eli Zaretskii
  2024-09-16 19:54                 ` Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 16+ messages in thread
From: Dmitry Gutov @ 2024-09-14 15:18 UTC (permalink / raw)
  To: Eli Zaretskii, Spencer Baugh; +Cc: 70968, monnier, juri

On 14/09/2024 12:17, Eli Zaretskii wrote:
> B. Deprecate the emacs22 style and replace it with a new style called
>     `ignore-suffix` which has this bug fixed, and which replaces emacs22
>     in the default value of completion-styles.

Note that while this is a viable option, we're still lacking a mechanism 
which would fix the problem for the new completion style.

And then we'd have two code paths for this behavior that somehow need to 
distinguish between these styles.

With the original proposal we would delegate to the style's completion 
logic, but it would apparently behave the same between emacs22 and the 
new style.

> Option C is from my POV the least desirable one: it adds some
> complicated logic, which will almost certainly produce unintended
> results, as everything in this completion-related mess we have.

That's probably true, but whether it will make things more complicated 
or not would also depend on the overall design. With some effort, it 
could be the opposite - but with more changes across the board, overall 
not backward compatible ones.

> I'm okay with adding a new style, per B, but why do we need to
> deprecate emacs22 at the same time?  Let users who want this new
> behavior customize their completion styles to use this new style
> instead of emacs22.

I don't envy the person who's going to write documentation and has to 
describe the distinction between these two styles, that they differ in 
this particular place of the UI, and are otherwise the same.

Deprecating the old style would at least say that this complication is a 
wart which would someday go away, and we could take a shortcut in the 
implementation (e.g. hardcode the value `emacs22` in some check).





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#70968: 29.2.50; choose-completion on an emacs22-style completion deletes text after point
  2024-09-14 15:18                 ` Dmitry Gutov
@ 2024-09-14 16:00                   ` Eli Zaretskii
  0 siblings, 0 replies; 16+ messages in thread
From: Eli Zaretskii @ 2024-09-14 16:00 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: sbaugh, 70968, monnier, juri

> Date: Sat, 14 Sep 2024 18:18:46 +0300
> Cc: monnier@iro.umontreal.ca, juri@linkov.net, 70968@debbugs.gnu.org
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> > I'm okay with adding a new style, per B, but why do we need to
> > deprecate emacs22 at the same time?  Let users who want this new
> > behavior customize their completion styles to use this new style
> > instead of emacs22.
> 
> I don't envy the person who's going to write documentation and has to 
> describe the distinction between these two styles, that they differ in 
> this particular place of the UI, and are otherwise the same.
> 
> Deprecating the old style would at least say that this complication is a 
> wart which would someday go away, and we could take a shortcut in the 
> implementation (e.g. hardcode the value `emacs22` in some check).

If documentation difficulties are the main issue with my proposal,
then you can stop worrying.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#70968: 29.2.50; choose-completion on an emacs22-style completion deletes text after point
  2024-09-14  9:17               ` Eli Zaretskii
  2024-09-14 15:18                 ` Dmitry Gutov
@ 2024-09-16 19:54                 ` Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 0 replies; 16+ messages in thread
From: Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-09-16 19:54 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: dmitry, 70968, monnier, juri

[-- Attachment #1: Type: text/plain, Size: 2612 bytes --]

Eli Zaretskii <eliz@gnu.org> writes:
> I'm okay with adding a new style, per B, but why do we need to
> deprecate emacs22 at the same time?  Let users who want this new
> behavior customize their completion styles to use this new style
> instead of emacs22.  That'd be fully backward compatible, and will
> solve the problem for those who don't like the current behavior.

That's fair enough, we don't need to deprecate emacs22 at the same time,
we can wait until the new style has been battle-tested.

I think a new style should replace emacs22 in the default
completion-styles eventually, but that can wait.

And after working on this bug for a while, I now am convinced that the
new style approach is straightforward, and is the best way.  (Although
it would also work to make these changes in the emacs22 style)

Attached is a patch which adds a new ignore-after-point style.  The fix
for this bug is entirely contained in the differences between
completion-ignore-after-point-all-completions (c-iap-a-c) and
completion-emacs22-all-completions (c-e22-a-c).

c-e22-a-c always omits the text after point, which means
choose-completion always deletes that text, causing this bug.

c-iap-a-c includes the text after point in some cases.  Whenever the
text after point is included, it's grayed out with a new
completions-ignored face to indicate it was ignored.

The cases where c-iap-a-c includes the text after point are those where
the user will have further chances to edit or complete with that text:

- When we're doing minibuffer completion and choose-completion will
  immediately exit the minibuffer, the text after point is not included,
  since the user won't get any further changes to use it, and it's
  probably garbage.

- When we're doing completion-at-point (or certain kinds of minibuffer
  completion, e.g. completing a directory in filename completion), the
  text after point is included.  In such cases, the user can always
  delete it themselves later, or might want to actually use it.

To make this work consistently with completion-try-completion (which
keeps point before the ignored suffix in both the ignore-after-point and
emacs22 styles), choose-completion-string now checks a new
'completion-position-after-insert text property, and moves point to that
position.

(There are two other changes which are general improvements unrelated to
this bug:

- The behavior of keeping point before the ignored suffix is more
  consistent.

- Instead of hardcoding try-completion and all-completion,
  ignore-after-point uses the configured completion styles.)

Stefan, Dmitry, any comments?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-ignore-after-point-completion-style.patch --]
[-- Type: text/x-patch, Size: 12976 bytes --]

From cdc7ce4b9958a0ef36e911066ecce82a6da09c02 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@janestreet.com>
Date: Mon, 16 Sep 2024 15:15:57 -0400
Subject: [PATCH] Add ignore-after-point completion style

* lisp/minibuffer.el (completion--twq-all): Use the original
completion faces where possible.
(completion-styles-alist, completion-ignore-after-point--force-nil)
(completions-ignored, completion-ignore-after-point-try-completion)
(completion-ignore-after-point-all-completions): Add
ignore-after-point completion style.  (bug#70968)
* lisp/simple.el (choose-completion-string--should-exit):
Add. (choose-completion-string): Call
choose-completion-string--should-exit.
---
 lisp/minibuffer.el | 142 ++++++++++++++++++++++++++++++++++++---------
 lisp/simple.el     |  47 +++++++++------
 2 files changed, 142 insertions(+), 47 deletions(-)

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 72ee5d02002..e5d85ed6fc8 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -674,34 +674,42 @@ completion--twq-all
       ;; requote them, so that *Completions* can show nicer unquoted values
       ;; which only get quoted when needed by choose-completion.
       (nconc
-       (mapcar (lambda (completion)
-                 (cl-assert (string-prefix-p prefix completion 'ignore-case) t)
-                 (let* ((new (substring completion (length prefix)))
-                        (qnew (funcall qfun new))
- 			(qprefix
-                         (if (not completion-ignore-case)
-                             qprefix
-                           ;; Make qprefix inherit the case from `completion'.
-                           (let* ((rest (substring completion
-                                                   0 (length prefix)))
-                                  (qrest (funcall qfun rest)))
-                             (if (string-equal-ignore-case qprefix qrest)
-                                 (propertize qrest 'face
-                                             'completions-common-part)
-                               qprefix))))
-                        (qcompletion (concat qprefix qnew)))
-		   ;; FIXME: Similarly here, Cygwin's mapping trips this
-		   ;; assertion.
-                   ;;(cl-assert
-                   ;; (string-equal-ignore-case
-		   ;;  (funcall unquote
-		   ;;           (concat (substring string 0 qboundary)
-		   ;;                   qcompletion))
-		   ;;  (concat (substring ustring 0 boundary)
-		   ;;          completion))
-		   ;; t)
-                   qcompletion))
-               completions)
+       (mapcar
+        (if (string-equal qprefix prefix)
+            ;; There's no quoting in the prefix; quoting in the completions
+            ;; can be simpler and preserve the existing faces.
+            (lambda (completion)
+              (concat
+               (substring completion 0 (length prefix))
+               (funcall qfun (substring completion (length prefix)))))
+          (lambda (completion)
+            (cl-assert (string-prefix-p prefix completion 'ignore-case) t)
+            (let* ((new (substring completion (length prefix)))
+                   (qnew (funcall qfun new))
+                   (qprefix
+                    (if (not completion-ignore-case)
+                        qprefix
+                      ;; Make qprefix inherit the case from `completion'.
+                      (let* ((rest (substring completion
+                                              0 (length prefix)))
+                             (qrest (funcall qfun rest)))
+                        (if (string-equal-ignore-case qprefix qrest)
+                            (propertize qrest 'face
+                                        'completions-common-part)
+                          qprefix))))
+                   (qcompletion (concat qprefix qnew)))
+              ;; FIXME: Similarly here, Cygwin's mapping trips this
+              ;; assertion.
+              ;;(cl-assert
+              ;; (string-equal-ignore-case
+              ;;  (funcall unquote
+              ;;           (concat (substring string 0 qboundary)
+              ;;                   qcompletion))
+              ;;  (concat (substring ustring 0 boundary)
+              ;;          completion))
+              ;; t)
+              qcompletion)))
+        completions)
        qboundary))))
 
 ;;; Minibuffer completion
@@ -1038,6 +1046,12 @@ completion-styles-alist
      "Prefix completion that only operates on the text before point.
 I.e. when completing \"foo_bar\" (where _ is the position of point),
 it will consider all completions candidates matching the glob
+pattern \"foo*\" and will add back \"bar\" to the end of it.")
+    (ignore-after-point
+     completion-ignore-after-point-try-completion completion-ignore-after-point-all-completions
+     "Prefix completion that only operates on the text before point.
+I.e. when completing \"foo_bar\" (where _ is the position of point),
+it will consider all completions candidates matching the glob
 pattern \"foo*\" and will add back \"bar\" to the end of it.")
     (basic
      completion-basic-try-completion completion-basic-all-completions
@@ -3692,6 +3706,78 @@ completion-emacs22-all-completions
      point
      (car (completion-boundaries beforepoint table pred "")))))
 
+;;; ignore-after-point completion style.
+
+(defvar completion-ignore-after-point--force-nil nil
+  "When non-nil, the ignore-after-point style always returns nil.")
+
+(defface completions-ignored
+  '((t (:inherit shadow)))
+  "Face for text which was ignored by the completion style.")
+
+(defun completion-ignore-after-point-try-completion (string table pred point)
+  "Run `completion-try-completion' ignoring the part of STRING after POINT.
+
+We add the part of STRING after POINT back to the result."
+  (let ((prefix (substring string 0 point))
+        (suffix (substring string point)))
+    (when-let ((completion
+                (unless completion-ignore-after-point--force-nil
+                  (let ((completion-ignore-after-point--force-nil t))
+                    (completion-try-completion prefix table pred point)))))
+      ;; Add SUFFIX back to COMPLETION.  However, previous completion styles failed and
+      ;; this one succeeded by ignoring SUFFIX.  The success of future completion depends
+      ;; on ignoring SUFFIX.  We mostly do that by keeping point right before SUFFIX.
+      (if (eq completion t)
+          ;; Keep point in the same place, right before SUFFIX.
+          (cons string point)
+        (let ((newstring (car completion))
+              (newpoint (cdr completion)))
+          (cond
+           ((= (length newstring) newpoint)
+            ;; NEWPOINT is already right before SUFFIX.
+            (cons (concat newstring suffix) newpoint))
+           ((minibufferp completion-reference-buffer)
+            ;; Don't allow moving point, keep it right before SUFFIX.
+            (cons (concat newstring suffix) (length newstring)))
+           (t
+            ;; If we're not in a minibuffer, then we're using `completion-at-point', which
+            ;; calculates a completion region to complete over.  We can allow point to
+            ;; move and still cause SUFFIX to be omitted from the completion region, by
+            ;; including a space right before SUFFIX.
+            (cons (concat newstring
+                          ;; Don't add another space if SUFFIX already starts with one.
+                          (when (/= (aref suffix 0) ? ) " ") suffix)
+                  newpoint))))))))
+
+(defun completion-ignore-after-point-all-completions (string table pred point)
+  "Run `completion-all-completions' ignoring the part of STRING after POINT."
+  (let ((prefix (substring string 0 point))
+        (suffix (propertize (substring string point) 'face 'completions-ignored)))
+    (when-let ((completions
+                (unless completion-ignore-after-point--force-nil
+                  (let ((completion-ignore-after-point--force-nil t))
+                    (completion-all-completions prefix table pred point)))))
+      ;; Add SUFFIX back to each completion.  COMPLETIONS may be an improper list (with
+      ;; the base position in its last cdr) so we can't use `mapcar'.
+      (let ((tail completions))
+        (while (consp tail)
+          (let* ((completion (car tail))
+                 (choose-completion-will-exit
+                  (and (minibufferp completion-reference-buffer)
+                       (choose-completion-string--should-exit completion))))
+            ;; Include the suffix if, after `choose-completion' runs on COMPLETION, the
+            ;; user is still able to use and edit the suffix.
+            (unless choose-completion-will-exit
+              (let ((end-of-real-completion (length completion)))
+                (setcar tail (concat completion suffix))
+                ;; When chosen, point should go before SUFFIX.
+                (put-text-property
+                 0 1 'completion-position-after-insert end-of-real-completion
+                 (car tail)))))
+          (setq tail (cdr tail))))
+      completions)))
+
 ;;; Basic completion.
 
 (defun completion--merge-suffix (completion point suffix)
diff --git a/lisp/simple.el b/lisp/simple.el
index 1dd6bfe5b22..fe68f23c4da 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -10083,6 +10083,20 @@ choose-completion-string-functions
 If all functions in the list return nil, that means to use
 the default method of inserting the completion in BUFFER.")
 
+(defun choose-completion-string--should-exit (result)
+  "Should `choose-completion-string' exit the minibuffer if RESULT is chosen?"
+  (and
+   (not completion-no-auto-exit)
+   minibuffer-completion-table
+   ;; If this is reading a file name, and the file name chosen
+   ;; is a directory, don't exit the minibuffer.
+   (let ((bounds (completion-boundaries
+                  result minibuffer-completion-table
+                  minibuffer-completion-predicate "")))
+     ;; The completion chosen leads to a new set of completions
+     ;; (e.g. it's a directory): don't exit the minibuffer yet.
+     (not (eq (car bounds) (length result))))))
+
 (defun choose-completion-string (choice &optional
                                         buffer base-position insert-function)
   "Switch to BUFFER and insert the completion choice CHOICE.
@@ -10116,10 +10130,13 @@ choose-completion-string
         ;; comes from buffer-substring-no-properties.
         ;;(remove-text-properties 0 (length choice) '(mouse-face nil) choice)
 	;; Insert the completion into the buffer where it was requested.
-        (funcall (or insert-function completion-list-insert-choice-function)
-                 (or (car base-position) (point))
-                 (or (cadr base-position) (point))
-                 choice)
+        (let ((beg (or (car base-position) (point)))
+              (end (or (cadr base-position) (point))))
+          (funcall (or insert-function completion-list-insert-choice-function)
+                   beg end choice)
+          (unless (string-empty-p choice)
+            (when-let ((pos (get-text-property 0 'completion-position-after-insert choice)))
+              (goto-char (+ pos beg)))))
         ;; Update point in the window that BUFFER is showing in.
 	(let ((window (get-buffer-window buffer t)))
 	  (set-window-point window (point)))
@@ -10127,21 +10144,13 @@ choose-completion-string
 	(and (not completion-no-auto-exit)
              (minibufferp buffer)
 	     minibuffer-completion-table
-	     ;; If this is reading a file name, and the file name chosen
-	     ;; is a directory, don't exit the minibuffer.
-             (let* ((result (buffer-substring (field-beginning) (point)))
-                    (bounds
-                     (completion-boundaries result minibuffer-completion-table
-                                            minibuffer-completion-predicate
-                                            "")))
-               (if (eq (car bounds) (length result))
-                   ;; The completion chosen leads to a new set of completions
-                   ;; (e.g. it's a directory): don't exit the minibuffer yet.
-                   (let ((mini (active-minibuffer-window)))
-                     (select-window mini)
-                     (when minibuffer-auto-raise
-                       (raise-frame (window-frame mini))))
-                 (exit-minibuffer))))))))
+             (if (choose-completion-string--should-exit
+                  (buffer-substring (field-beginning) (point)))
+                 (exit-minibuffer)
+               (let ((mini (active-minibuffer-window)))
+                 (select-window mini)
+                 (when minibuffer-auto-raise
+                   (raise-frame (window-frame mini))))))))))
 
 (define-derived-mode completion-list-mode nil "Completion List"
   "Major mode for buffers showing lists of possible completions.
-- 
2.39.3


^ permalink raw reply related	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2024-09-16 19:54 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-15 20:26 bug#70968: 29.2.50; choose-completion on an emacs22-style completion deletes text after point Spencer Baugh
2024-05-16  8:13 ` Eli Zaretskii
2024-05-16 17:26   ` Dmitry Gutov
2024-05-16 18:25     ` Eli Zaretskii
2024-08-26  0:08       ` Dmitry Gutov
2024-09-07  7:30         ` Eli Zaretskii
2024-09-08  2:02           ` Dmitry Gutov
2024-09-08 11:12           ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-09-10 16:54             ` Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-09-14  9:17               ` Eli Zaretskii
2024-09-14 15:18                 ` Dmitry Gutov
2024-09-14 16:00                   ` Eli Zaretskii
2024-09-16 19:54                 ` Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-16 17:40   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-16 18:28     ` Eli Zaretskii
2024-06-20 15:45 ` Spencer Baugh

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).