bug#73544: smerge key bindings awkward

bug#73544: smerge key bindings awkward

[Daniel Colascione]

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

In smerge mode, C-c ^ n goes to the next merge conflict. Having resolved all merge conflicts, smerge turns itself off. Typing C-c ^ n again now puts a stray "n" into the buffer. Is there some tweak we can make to avoid this situation? Change the default of smerge-auto-leave?

[-- Attachment #2: Type: text/html, Size: 338 bytes --]

bug#73544: smerge key bindings awkward

[Eli Zaretskii]

> Date: Sat, 28 Sep 2024 23:30:13 -0400
> From: Daniel Colascione <dancol@dancol.org>
> 
> In smerge mode, C-c ^ n goes to the next merge conflict. Having resolved all merge conflicts, smerge turns
> itself off. Typing C-c ^ n again now puts a stray "n" into the buffer. Is there some tweak we can make to avoid
> this situation? Change the default of smerge-auto-leave?

Adding people who might have opinions or suggestions.

bug#73544: smerge key bindings awkward

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> In smerge mode, C-c ^ n goes to the next merge conflict. Having resolved
>> all merge conflicts, smerge turns
>> itself off. Typing C-c ^ n again now puts a stray "n" into the buffer.
>> Is there some tweak we can make to avoid
>> this situation?  Change the default of smerge-auto-leave?

FWIW, I use (setq smerge-command-prefix "\e") so for me the binding is
`M-n` which doesn't suffer from this problem.
Note: This setting has other downsides, of course, and can't be used as
a default.


        Stefan

bug#73544: smerge key bindings awkward

[Sean Whitton]

Hello,

On Sat 28 Sep 2024 at 11:30pm -04, Daniel Colascione wrote:

> In smerge mode, C-c ^ n goes to the next merge conflict. Having resolved all merge
> conflicts, smerge turns itself off. Typing C-c ^ n again now puts a stray "n" into the
> buffer. Is there some tweak we can make to avoid this situation? Change the default of
> smerge-auto-leave?

A related issue is that it would be nice if C-c ^ n wrapped.  Else you
have to both C-c ^ n and C-c ^ p to find the hunks to resolve.

-- 
Sean Whitton

bug#73544: smerge key bindings awkward

[Dmitry Gutov]

On 29/09/2024 06:30, Daniel Colascione wrote:
> In smerge mode, C-c ^ n goes to the next merge conflict. Having resolved 
> all merge conflicts, smerge turns itself off. Typing C-c ^ n again now 
> puts a stray "n" into the buffer. Is there some tweak we can make to 
> avoid this situation? Change the default of smerge-auto-leave?

Would it be better if 'C-c ^ n' jumped across files?

I.e. effectively called smerge-vc-next-conflict.

Not sure if it would always jump to the next file when reaching the end 
of the current one, or only when all hunks are resolved.

Anyway, that approach would require making the bindings global, I think. 
The prev/next ones, at least.

bug#73544: smerge key bindings awkward

[Daniel Colascione]

On September 29, 2024 5:33:02 PM PDT, Dmitry Gutov <dmitry@gutov.dev> wrote:
>On 29/09/2024 06:30, Daniel Colascione wrote:
>> In smerge mode, C-c ^ n goes to the next merge conflict. Having resolved all merge conflicts, smerge turns itself off. Typing C-c ^ n again now puts a stray "n" into the buffer. Is there some tweak we can make to avoid this situation? Change the default of smerge-auto-leave?
>
>Would it be better if 'C-c ^ n' jumped across files?
>
>I.e. effectively called smerge-vc-next-conflict.
>
>Not sure if it would always jump to the next file when reaching the end of the current one, or only when all hunks are resolved.
>
>Anyway, that approach would require making the bindings global, I think. The prev/next ones, at least.


Sure. If I had my druthers, we'd just enable the C-c ^ keymap globally everywhere all the time and be done with it. That would work whether or not we bound C-c ^ n to smerge-vc-next-conflict or left it as is. The way it is now, we optimize for a false economy of keymap minimalism (putting the smerge keymaps on a minor mode) and minor mode minimalism (disabling smerge mode ASAP gaining essentially nothing and in this vacuous exchange getting the problem I highlighted in the bug report and others.

bug#73544: smerge key bindings awkward

[Eli Zaretskii]

> Date: Mon, 30 Sep 2024 03:33:02 +0300
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> On 29/09/2024 06:30, Daniel Colascione wrote:
> > In smerge mode, C-c ^ n goes to the next merge conflict. Having resolved 
> > all merge conflicts, smerge turns itself off. Typing C-c ^ n again now 
> > puts a stray "n" into the buffer. Is there some tweak we can make to 
> > avoid this situation? Change the default of smerge-auto-leave?
> 
> Would it be better if 'C-c ^ n' jumped across files?
> 
> I.e. effectively called smerge-vc-next-conflict.
> 
> Not sure if it would always jump to the next file when reaching the end 
> of the current one, or only when all hunks are resolved.
> 
> Anyway, that approach would require making the bindings global, I think. 
> The prev/next ones, at least.

Why does smerge-mode have to turn itself off when all conflicts are
resolved?  Stefan?

bug#73544: smerge key bindings awkward

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Why does smerge-mode have to turn itself off when all conflicts are
> resolved?  Stefan?

It doesn't have to.  It's just useless when all conflicts are resolved,
so it's convenient for it to turn itself off in that case.  It also lets
you use the `SMerge` lighter in the mode line as an indication that
there are still merge conflicts to resolve in the buffer.


        Stefan

bug#73544: smerge key bindings awkward

[Daniel Colascione]

On September 30, 2024 5:43:09 AM PDT, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Stefan Monnier <monnier@iro.umontreal.ca>
>> Cc: Dmitry Gutov <dmitry@gutov.dev>,  dancol@dancol.org,  73544@debbugs.gnu.org
>> Date: Mon, 30 Sep 2024 08:28:33 -0400
>> 
>> > Why does smerge-mode have to turn itself off when all conflicts are
>> > resolved?  Stefan?
>> 
>> It doesn't have to.  It's just useless when all conflicts are resolved,
>> so it's convenient for it to turn itself off in that case.  It also lets
>> you use the `SMerge` lighter in the mode line as an indication that
>> there are still merge conflicts to resolve in the buffer.
>
>That's what I thought.
>
>So maybe a possible solution to Daniel's problem would be a (maybe
>optional) behavior, whereby when conflicts are resolved, smerge-mode
>doesn't turn itself off, but instead changes the lighter to indicate
>that there are no more conflicts?
>
>Daniel, would that solve your problem?  If it would, I think it's
>better than making the "C-x ^" keymap global, which would have a
>significant global effect, and might break someone's key bindings.


That's a solution, sure. We could also have the lighter contain a visual indication of the number of conflict regions in the buffer, like flymake.

bug#73544: smerge key bindings awkward

[Eli Zaretskii]

> Date: Mon, 30 Sep 2024 07:50:50 -0700
> From: Daniel Colascione <dancol@dancol.org>
> CC: dmitry@gutov.dev, 73544@debbugs.gnu.org
> 
> 
> 
> On September 30, 2024 5:43:09 AM PDT, Eli Zaretskii <eliz@gnu.org> wrote:
> >> From: Stefan Monnier <monnier@iro.umontreal.ca>
> >> Cc: Dmitry Gutov <dmitry@gutov.dev>,  dancol@dancol.org,  73544@debbugs.gnu.org
> >> Date: Mon, 30 Sep 2024 08:28:33 -0400
> >> 
> >> > Why does smerge-mode have to turn itself off when all conflicts are
> >> > resolved?  Stefan?
> >> 
> >> It doesn't have to.  It's just useless when all conflicts are resolved,
> >> so it's convenient for it to turn itself off in that case.  It also lets
> >> you use the `SMerge` lighter in the mode line as an indication that
> >> there are still merge conflicts to resolve in the buffer.
> >
> >That's what I thought.
> >
> >So maybe a possible solution to Daniel's problem would be a (maybe
> >optional) behavior, whereby when conflicts are resolved, smerge-mode
> >doesn't turn itself off, but instead changes the lighter to indicate
> >that there are no more conflicts?
> >
> >Daniel, would that solve your problem?  If it would, I think it's
> >better than making the "C-x ^" keymap global, which would have a
> >significant global effect, and might break someone's key bindings.
> 
> 
> That's a solution, sure. We could also have the lighter contain a visual indication of the number of conflict regions in the buffer, like flymake. 

Actually, I see that the first part of this is already there: we have
a defcustom smerge-auto-leave, whose default is t.  So does it mean we
can consider this bug fixed?

bug#73544: smerge key bindings awkward

[Daniel Colascione]

On October 13, 2024 7:06:11 AM EDT, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Mon, 30 Sep 2024 07:50:50 -0700
>> From: Daniel Colascione <dancol@dancol.org>
>> CC: dmitry@gutov.dev, 73544@debbugs.gnu.org
>> 
>> 
>> 
>> On September 30, 2024 5:43:09 AM PDT, Eli Zaretskii <eliz@gnu.org> wrote:
>> >> From: Stefan Monnier <monnier@iro.umontreal.ca>
>> >> Cc: Dmitry Gutov <dmitry@gutov.dev>,  dancol@dancol.org,  73544@debbugs.gnu.org
>> >> Date: Mon, 30 Sep 2024 08:28:33 -0400
>> >> 
>> >> > Why does smerge-mode have to turn itself off when all conflicts are
>> >> > resolved?  Stefan?
>> >> 
>> >> It doesn't have to.  It's just useless when all conflicts are resolved,
>> >> so it's convenient for it to turn itself off in that case.  It also lets
>> >> you use the `SMerge` lighter in the mode line as an indication that
>> >> there are still merge conflicts to resolve in the buffer.
>> >
>> >That's what I thought.
>> >
>> >So maybe a possible solution to Daniel's problem would be a (maybe
>> >optional) behavior, whereby when conflicts are resolved, smerge-mode
>> >doesn't turn itself off, but instead changes the lighter to indicate
>> >that there are no more conflicts?
>> >
>> >Daniel, would that solve your problem?  If it would, I think it's
>> >better than making the "C-x ^" keymap global, which would have a
>> >significant global effect, and might break someone's key bindings.
>> 
>> 
>> That's a solution, sure. We could also have the lighter contain a visual indication of the number of conflict regions in the buffer, like flymake. 
>
>Actually, I see that the first part of this is already there: we have
>a defcustom smerge-auto-leave, whose default is t.  So does it mean we
>can consider this bug fixed?

The default is what produces the problem.

bug#73544: smerge key bindings awkward

[Eli Zaretskii]

> Date: Sun, 13 Oct 2024 12:01:33 -0400
> From: Daniel Colascione <dancol@dancol.org>
> CC: monnier@iro.umontreal.ca, dmitry@gutov.dev, 73544@debbugs.gnu.org
> 
> 
> 
> On October 13, 2024 7:06:11 AM EDT, Eli Zaretskii <eliz@gnu.org> wrote:
> >> Date: Mon, 30 Sep 2024 07:50:50 -0700
> >> From: Daniel Colascione <dancol@dancol.org>
> >> CC: dmitry@gutov.dev, 73544@debbugs.gnu.org
> >> 
> >Actually, I see that the first part of this is already there: we have
> >a defcustom smerge-auto-leave, whose default is t.  So does it mean we
> >can consider this bug fixed?
> 
> The default is what produces the problem.

Sure, but you can customize it according to your needs.  It's been the
default for the last 24 years, so I think reversing it after such a
long time would need a lot of complaints.

bug#73544: smerge key bindings awkward

[Daniel Colascione]

On October 13, 2024 12:13:49 PM EDT, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Sun, 13 Oct 2024 12:01:33 -0400
>> From: Daniel Colascione <dancol@dancol.org>
>> CC: monnier@iro.umontreal.ca, dmitry@gutov.dev, 73544@debbugs.gnu.org
>> 
>> 
>> 
>> On October 13, 2024 7:06:11 AM EDT, Eli Zaretskii <eliz@gnu.org> wrote:
>> >> Date: Mon, 30 Sep 2024 07:50:50 -0700
>> >> From: Daniel Colascione <dancol@dancol.org>
>> >> CC: dmitry@gutov.dev, 73544@debbugs.gnu.org
>> >> 
>> >Actually, I see that the first part of this is already there: we have
>> >a defcustom smerge-auto-leave, whose default is t.  So does it mean we
>> >can consider this bug fixed?
>> 
>> The default is what produces the problem.
>
>Sure, but you can customize it according to your needs.  It's been the
>default for the last 24 years, so I think reversing it after such a
>long time would need a lot of complaints.

It's a defect in the user interface. Going NIMBY on the core is no way to make great software. We need to be able to add new key bindings, new features, and sand down high friction parts of the UI, even if they've been bumpy for a long time. 

After a certain point, customizing the program to work around all the terrible inherited defaults the maintainers refuse to change becomes tantamount to maintaining a fork. I guess that's why Doom exists.

bug#73544: smerge key bindings awkward

[Eli Zaretskii]

> Date: Sun, 13 Oct 2024 12:57:01 -0400
> From: Daniel Colascione <dancol@dancol.org>
> CC: monnier@iro.umontreal.ca, dmitry@gutov.dev, 73544@debbugs.gnu.org
> 
> >Sure, but you can customize it according to your needs.  It's been the
> >default for the last 24 years, so I think reversing it after such a
> >long time would need a lot of complaints.
> 
> It's a defect in the user interface. Going NIMBY on the core is no way to make great software. We need to be able to add new key bindings, new features, and sand down high friction parts of the UI, even if they've been bumpy for a long time. 
> 
> After a certain point, customizing the program to work around all the terrible inherited defaults the maintainers refuse to change becomes tantamount to maintaining a fork. I guess that's why Doom exists.

How do we know that this is indeed a defect in the UI?  The only way I
know of is to hear the same complaints from enough users.  Without
that, it could be your personal opinion -- to which you are entitled,
of course, and maybe it is even correct in some sense, but flipping
the default based on your single opinion runs the risk of annoying a
lot of people who maybe happen to like the current default.

That said, I don't use SMerge enough to object to this change too
strenuously, so if others (CC'ed) don't mind to make the change, I
won't mind.

bug#73544: smerge key bindings awkward

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> It's a defect in the user interface.

IIUC the original scenario is:

    C-c ^ n
    <fix the conflict>
    C-c ^ n

and here being annoyed that `n` was inserted (after flashing the buffer
because `C-c ^` is not bound to anything) because that was the last
conflict and `smerge-mode` turned itself off automatically.

I'm not completely sure what is the exact defect you see here and how
you suggest to fix it.  E.g. do you also care to "fix" a case like

    emacs foo
    C-c ^ n

where the user forgot to enable `smerge-mode`?

How 'bout the patch below, which will end up preventing `read-key-sequence`
from stopping at `C-c ^`?


        Stefan


diff --git a/lisp/vc/smerge-mode.el b/lisp/vc/smerge-mode.el
index aad3e302826..fa52286fee7 100644
--- a/lisp/vc/smerge-mode.el
+++ b/lisp/vc/smerge-mode.el
@@ -168,6 +168,10 @@ smerge-command-prefix
 		 (const :tag "none"  "")
 		 string))
 
+;; Make it so `C-x ^ n' doesn't insert `n' but just signals an error
+;; when SMerge mode is not enabled.
+;;;###autoload (global-set-key "\C-c^" (make-sparse-keymap))
+
 (defvar-keymap smerge-mode-map
   (key-description smerge-command-prefix) smerge-basic-map)
 

bug#73544: smerge key bindings awkward

[Daniel Colascione]

On October 17, 2024 1:37:43 PM EDT, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>> It's a defect in the user interface.
>
>IIUC the original scenario is:
>
>    C-c ^ n
>    <fix the conflict>
>    C-c ^ n
>
>and here being annoyed that `n` was inserted (after flashing the buffer
>because `C-c ^` is not bound to anything) because that was the last
>conflict and `smerge-mode` turned itself off automatically.
>
>I'm not completely sure what is the exact defect you see here and how
>you suggest to fix it.  E.g. do you also care to "fix" a case like
>
>    emacs foo
>    C-c ^ n
>
>where the user forgot to enable `smerge-mode`?
>
>How 'bout the patch below, which will end up preventing `read-key-sequence`
>from stopping at `C-c ^`?
>
>
>        Stefan
>
>
>diff --git a/lisp/vc/smerge-mode.el b/lisp/vc/smerge-mode.el
>index aad3e302826..fa52286fee7 100644
>--- a/lisp/vc/smerge-mode.el
>+++ b/lisp/vc/smerge-mode.el
>@@ -168,6 +168,10 @@ smerge-command-prefix
> 		 (const :tag "none"  "")
> 		 string))
> 
>+;; Make it so `C-x ^ n' doesn't insert `n' but just signals an error
>+;; when SMerge mode is not enabled.
>+;;;###autoload (global-set-key "\C-c^" (make-sparse-keymap))
>+
> (defvar-keymap smerge-mode-map
>   (key-description smerge-command-prefix) smerge-basic-map)
> 
>

WFM

bug#73544: smerge key bindings awkward

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>>How 'bout the patch below, which will end up preventing `read-key-sequence`
>>from stopping at `C-c ^`?
>>
>>
>>        Stefan
>>
>>
>>diff --git a/lisp/vc/smerge-mode.el b/lisp/vc/smerge-mode.el
>>index aad3e302826..fa52286fee7 100644
>>--- a/lisp/vc/smerge-mode.el
>>+++ b/lisp/vc/smerge-mode.el
>>@@ -168,6 +168,10 @@ smerge-command-prefix
>> 		 (const :tag "none"  "")
>> 		 string))
>> 
>>+;; Make it so `C-x ^ n' doesn't insert `n' but just signals an error
>>+;; when SMerge mode is not enabled.
>>+;;;###autoload (global-set-key "\C-c^" (make-sparse-keymap))
>>+
>> (defvar-keymap smerge-mode-map
>>   (key-description smerge-command-prefix) smerge-basic-map)
>
> WFM

Maintainers?  Any objections?


        Stefan

bug#73544: smerge key bindings awkward

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  dmitry@gutov.dev,  73544@debbugs.gnu.org
> Date: Mon, 21 Oct 2024 19:56:12 -0400
> 
> >>How 'bout the patch below, which will end up preventing `read-key-sequence`
> >>from stopping at `C-c ^`?
> >>
> >>
> >>        Stefan
> >>
> >>
> >>diff --git a/lisp/vc/smerge-mode.el b/lisp/vc/smerge-mode.el
> >>index aad3e302826..fa52286fee7 100644
> >>--- a/lisp/vc/smerge-mode.el
> >>+++ b/lisp/vc/smerge-mode.el
> >>@@ -168,6 +168,10 @@ smerge-command-prefix
> >> 		 (const :tag "none"  "")
> >> 		 string))
> >> 
> >>+;; Make it so `C-x ^ n' doesn't insert `n' but just signals an error
> >>+;; when SMerge mode is not enabled.
> >>+;;;###autoload (global-set-key "\C-c^" (make-sparse-keymap))
> >>+
> >> (defvar-keymap smerge-mode-map
> >>   (key-description smerge-command-prefix) smerge-basic-map)
> >
> > WFM
> 
> Maintainers?  Any objections?

Not here.

bug#73544: smerge key bindings awkward

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> Maintainers?  Any objections?
> Not here.

Thanks, pushed to `master`.


        Stefan