GNU bug report logs - #34420
[PATCH 2/2] smerge-mode: new function to go to next conflict

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 34420 in the body.
You can then email your comments to 34420 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: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH 2/2] smerge-mode: new function to go to next conflict
Date: Mon, 11 Feb 2019 00:14:53 +0300

* lisp/vc/smerge-mode.el (smerge-vc-next-conflict)
---
 lisp/vc/smerge-mode.el | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/lisp/vc/smerge-mode.el b/lisp/vc/smerge-mode.el
index 929cd85432a..8ac0b3ed370 100644
--- a/lisp/vc/smerge-mode.el
+++ b/lisp/vc/smerge-mode.el
@@ -1435,6 +1435,18 @@ smerge-start-session
         (smerge-next))
     (error (smerge-auto-leave))))
 
+(require 'vc)
+
+(defun smerge-vc-next-conflict ()
+  (interactive)
+  (let ((buffer (current-buffer)))
+    (when (not (smerge-goto-next-conflict))
+      (vc-find-conflicted-file)
+      (if (eq buffer (current-buffer))
+          (message "No conflicts found")
+        (goto-char 0)
+        (smerge-goto-next-conflict)))))
+
 (provide 'smerge-mode)
 
 ;;; smerge-mode.el ends here
-- 
2.20.1

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

From: Konstantin Kharlamov <Hi-Angel <at> yandex.ru>
To: 34420 <at> debbugs.gnu.org
Subject: [PATCH v2 2/2] smerge-mode: new function to go to next conflict
Date: Mon, 11 Feb 2019 01:30:13 +0300

* lisp/vc/smerge-mode.el (smerge-vc-next-conflict)
---

v2: added documentation for the function

 lisp/vc/smerge-mode.el | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/lisp/vc/smerge-mode.el b/lisp/vc/smerge-mode.el
index 929cd85432a..0bc11fee23c 100644
--- a/lisp/vc/smerge-mode.el
+++ b/lisp/vc/smerge-mode.el
@@ -1435,6 +1435,20 @@ smerge-start-session
         (smerge-next))
     (error (smerge-auto-leave))))
 
+(require 'vc)
+
+(defun smerge-vc-next-conflict ()
+  "Tries to go to next conflict in current file, otherwise tries
+to open next conflicted file version-control-system wise"
+  (interactive)
+  (let ((buffer (current-buffer)))
+    (when (not (smerge-goto-next-conflict))
+      (vc-find-conflicted-file)
+      (if (eq buffer (current-buffer))
+          (message "No conflicts found")
+        (goto-char 0)
+        (smerge-goto-next-conflict)))))
+
 (provide 'smerge-mode)
 
 ;;; smerge-mode.el ends here
-- 
2.20.1

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

From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
To: 34420 <at> debbugs.gnu.org
Subject: Re: [PATCH 2/2] smerge-mode: new function to go to next conflict
Date: Thu, 14 Feb 2019 00:44:03 +0300

To give some context: these patches follow this thread 
http://lists.gnu.org/archive/html/help-gnu-emacs/2019-01/msg00111.html 
(note: the thread continues the next month, i.e. February, too).

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Konstantin Kharlamov <hi-angel <at> yandex.ru>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 34420 <at> debbugs.gnu.org
Subject: Re: bug#34420: [PATCH 2/2] smerge-mode: new function to go to next
 conflict
Date: Sat, 16 Feb 2019 09:32:00 +0200

> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
> Date: Thu, 14 Feb 2019 00:44:03 +0300
> 
> To give some context: these patches follow this thread 
> http://lists.gnu.org/archive/html/help-gnu-emacs/2019-01/msg00111.html 
> (note: the thread continues the next month, i.e. February, too).

I hope Stafen (CC'ed) could comment on those.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Konstantin Kharlamov <hi-angel <at> yandex.ru>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 34420 <at> debbugs.gnu.org
Subject: Re: bug#34420: [PATCH 2/2] smerge-mode: new function to go to next
 conflict
Date: Sat, 16 Feb 2019 09:32:15 +0200

> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
> Date: Thu, 14 Feb 2019 00:44:03 +0300
> 
> To give some context: these patches follow this thread 
> http://lists.gnu.org/archive/html/help-gnu-emacs/2019-01/msg00111.html 
> (note: the thread continues the next month, i.e. February, too).

I hope Stefan (CC'ed) could comment on those.

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 34420 <at> debbugs.gnu.org, Konstantin Kharlamov <hi-angel <at> yandex.ru>
Subject: Re: bug#34420: [PATCH 2/2] smerge-mode: new function to go to next
 conflict
Date: Sat, 16 Feb 2019 08:41:58 -0500

>> To give some context: these patches follow this thread 
>> http://lists.gnu.org/archive/html/help-gnu-emacs/2019-01/msg00111.html 
>> (note: the thread continues the next month, i.e. February, too).
> I hope Stefan (CC'ed) could comment on those.

Hmm...

In terms of implementation, there's nothing very deep and it looks OK
(tho maybe it points to the need to autoload vc-find-conflicted-file,
and obviously I'd use (point-min) rather than a hardcoded 0 and change
the docstring to use the imperative rather than present tense).

In terms of UI it's a question of taste: I didn't provide such a command
because I never felt the need for it (I think I like to be in control of
changing buffer).  Maybe I'd change it to first emit a warning "no more
conflicts in this buffer" and only go to the next buffer if you repeat
the command (as a form of confirmation that you're done with the current
buffer).  Also, I'd save the file before switching to the next buffer.


        Stefan

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

From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 34420 <at> debbugs.gnu.org
Subject: Re: bug#34420: [PATCH 2/2] smerge-mode: new function to go to next
 conflict
Date: Sun, 17 Feb 2019 12:18:53 +0300

On 16.02.2019 16:41, Stefan Monnier wrote:
>>> To give some context: these patches follow this thread
>>> http://lists.gnu.org/archive/html/help-gnu-emacs/2019-01/msg00111.html
>>> (note: the thread continues the next month, i.e. February, too).
>> I hope Stefan (CC'ed) could comment on those.
> 
> Hmm...
> 
> In terms of implementation, there's nothing very deep and it looks OK
> (tho maybe it points to the need to autoload vc-find-conflicted-file,
> and obviously I'd use (point-min) rather than a hardcoded 0 and change
> the docstring to use the imperative rather than present tense).

Thanks for your comments!

> In terms of UI it's a question of taste: I didn't provide such a command
> because I never felt the need for it (I think I like to be in control of
> changing buffer).  Maybe I'd change it to first emit a warning "no more
> conflicts in this buffer" and only go to the next buffer if you repeat
> the command (as a form of confirmation that you're done with the current
> buffer).

Well, I wouldn't call changing a buffer upon explicit command from a 
user a "taking control from them". The command name being explicit (at 
least should be — I'm not a native English speaker, please tell me if 
it's not) about going to the next conflict in the repository, and 
conflicts are markers in different files.

Emitting a warning in this case would be annoying, because the user 
expects to be taken to the next conflict if there's any. Imagine the 
case with many files having one small conflict, and that the user have 
to press "take me to the next conflict" twice every time.

> Also, I'd save the file before switching to the next buffer.
Okay, I can add that. I can't really comment anything about usability in 
this case, because I have a habit of saving files myself way too often :)

------

Since this discussion may take time, and you even may not even like the 
change — please, could you push the first patch in the series? When I 
first started looking at smerge-mode.el, I was confused about how going 
to the next conflict is implemented, and why the code does not call 
`smerge-next` anywhere. Extracting the code to an inline function, I 
think, should make it a bit easier for future contributors.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Konstantin Kharlamov <hi-angel <at> yandex.ru>
Cc: monnier <at> IRO.UMontreal.CA, 34420 <at> debbugs.gnu.org
Subject: Re: bug#34420: [PATCH 2/2] smerge-mode: new function to go to next
 conflict
Date: Sun, 17 Feb 2019 17:37:56 +0200

> Cc: 34420 <at> debbugs.gnu.org
> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
> Date: Sun, 17 Feb 2019 12:18:53 +0300
> 
> > In terms of UI it's a question of taste: I didn't provide such a command
> > because I never felt the need for it (I think I like to be in control of
> > changing buffer).  Maybe I'd change it to first emit a warning "no more
> > conflicts in this buffer" and only go to the next buffer if you repeat
> > the command (as a form of confirmation that you're done with the current
> > buffer).
> 
> Well, I wouldn't call changing a buffer upon explicit command from a 
> user a "taking control from them". The command name being explicit (at 
> least should be — I'm not a native English speaker, please tell me if 
> it's not) about going to the next conflict in the repository, and 
> conflicts are markers in different files.

I'm not sure this is always the case.  I can certainly envision a user
who might be surprised by being switched to another buffer.  Asking a
question like "No more conflicts in this file; go to next one?" might
in that case be a good idea.

> Emitting a warning in this case would be annoying, because the user 
> expects to be taken to the next conflict if there's any.

We could have an option to control this behavior.

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

From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: monnier <at> IRO.UMontreal.CA, 34420 <at> debbugs.gnu.org
Subject: Re: bug#34420: [PATCH 2/2] smerge-mode: new function to go to next
 conflict
Date: Sun, 17 Feb 2019 18:59:24 +0300

On 17.02.2019 18:37, Eli Zaretskii wrote:
>> Cc: 34420 <at> debbugs.gnu.org
>> From: Konstantin Kharlamov <hi-angel <at> yandex.ru>
>> Date: Sun, 17 Feb 2019 12:18:53 +0300
>>
>>> In terms of UI it's a question of taste: I didn't provide such a command
>>> because I never felt the need for it (I think I like to be in control of
>>> changing buffer).  Maybe I'd change it to first emit a warning "no more
>>> conflicts in this buffer" and only go to the next buffer if you repeat
>>> the command (as a form of confirmation that you're done with the current
>>> buffer).
>>
>> Well, I wouldn't call changing a buffer upon explicit command from a
>> user a "taking control from them". The command name being explicit (at
>> least should be — I'm not a native English speaker, please tell me if
>> it's not) about going to the next conflict in the repository, and
>> conflicts are markers in different files.
> 
> I'm not sure this is always the case.  I can certainly envision a user
> who might be surprised by being switched to another buffer.  Asking a
> question like "No more conflicts in this file; go to next one?" might
> in that case be a good idea.

The thing is, we already have a function that only works within focused 
buffer, it is smerge-next.

This patch is about adding a more "global" function, one that works 
within the focused repository instead of just the focused buffer.

>> Emitting a warning in this case would be annoying, because the user
>> expects to be taken to the next conflict if there's any.
> 
> We could have an option to control this behavior.
> 

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Konstantin Kharlamov <hi-angel <at> yandex.ru>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 34420 <at> debbugs.gnu.org
Subject: Re: bug#34420: [PATCH 2/2] smerge-mode: new function to go to next
 conflict
Date: Sun, 17 Feb 2019 22:22:09 -0500

>> In terms of UI it's a question of taste: I didn't provide such a command
>> because I never felt the need for it (I think I like to be in control of
>> changing buffer).  Maybe I'd change it to first emit a warning "no more
>> conflicts in this buffer" and only go to the next buffer if you repeat
>> the command (as a form of confirmation that you're done with the current
>> buffer).
> Well, I wouldn't call changing a buffer upon explicit command from a user
> a "taking control from them".

I didn't mean it in the sense that the command misbehaves.  Just that
the command does 2 things (go to the next conflict in the same file
sometimes, and go the next conflict in another file other times) and as
a user I'd prefer to know which one's going to happen before
it happens.  As I said it's a question of taste.

I think I could be tempted to use your command (in place of smerge-next)
if it requested confirmation before going to the next file.

I'll install your code as a first step,


        Stefan

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Konstantin Kharlamov <hi-angel <at> yandex.ru>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 34420 <at> debbugs.gnu.org
Subject: Re: bug#34420: [PATCH 2/2] smerge-mode: new function to go to next
 conflict
Date: Mon, 18 Feb 2019 12:49:54 -0500

> I'll install your code as a first step,

Done, as well as a subsequent patch which makes it save the buffer and
request confirmation.  Try it out and let me know of any
remaining problems.


        Stefan

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

From: hi-angel <at> yandex.ru
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 34420 <at> debbugs.gnu.org
Subject: Re: bug#34420: [PATCH 2/2] smerge-mode: new function to go to next
 conflict
Date: Tue, 19 Feb 2019 00:12:06 +0300

В Пн, 18 фев. 2019 в 8:49 ПП (PM), Stefan Monnier 
<monnier <at> IRO.UMontreal.CA> написал:
>>  I'll install your code as a first step,
> 
> Done, as well as a subsequent patch which makes it save the buffer and
> request confirmation.  Try it out and let me know of any
> remaining problems.
> 
> 
>         Stefan

Ok, thanks, so, I'm trying the latest smerge-mode.el out, and two 
issues I see:

1. On the first call of the function I get "smerge-vc-next-conflict: 
Symbol’s function definition is void: vc-find-conflicted-file". This 
starts working once I execute (require 'vc)
2. Although I didn't set smerge-change-buffer-confirm to nil, there's 
no confirmation before going to another buffer.

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: hi-angel <at> yandex.ru
Cc: Eli Zaretskii <eliz <at> gnu.org>, 34420 <at> debbugs.gnu.org
Subject: Re: bug#34420: [PATCH 2/2] smerge-mode: new function to go to next
 conflict
Date: Mon, 18 Feb 2019 16:51:05 -0500

> 1. On the first call of the function I get "smerge-vc-next-conflict:
> Symbol’s function definition is void: vc-find-conflicted-file". This starts
> working once I execute (require 'vc)

Did you take only smerge-mode.el ?

I changed vc.el so vc-find-conflicted-file is now autoloaded in the
`master` branch, so you'll only see it if you use the `master` branch.

> 2. Although I didn't set smerge-change-buffer-confirm to nil, there's no
> confirmation before going to another buffer.

I assume it's because you run it via `M-x`.
The confirmation only kicks in if you run it via a keybinding.


        Stefan

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

From: hi-angel <at> yandex.ru
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 34420 <at> debbugs.gnu.org
Subject: Re: bug#34420: [PATCH 2/2] smerge-mode: new function to go to next
 conflict
Date: Tue, 19 Feb 2019 01:01:52 +0300

В Вт, 19 фев. 2019 в 12:51 ДП (AM), Stefan Monnier 
<monnier <at> IRO.UMontreal.CA> написал:
>>  1. On the first call of the function I get "smerge-vc-next-conflict:
>>  Symbol’s function definition is void: vc-find-conflicted-file". 
>> This starts
>>  working once I execute (require 'vc)
> 
> Did you take only smerge-mode.el ?
> 
> I changed vc.el so vc-find-conflicted-file is now autoloaded in the
> `master` branch, so you'll only see it if you use the `master` branch.
> 
>>  2. Although I didn't set smerge-change-buffer-confirm to nil, 
>> there's no
>>  confirmation before going to another buffer.
> 
> I assume it's because you run it via `M-x`.
> The confirmation only kicks in if you run it via a keybinding.
> 
> 
>         Stefan

Ah, works for me, thanks! I didn't want to upgrade to latest master 
because I've been using harfbuzz branch, but I just installed and 
tested master — everything is okay.

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: hi-angel <at> yandex.ru
Cc: 34420-done <at> debbugs.gnu.org
Subject: Re: bug#34420: [PATCH 2/2] smerge-mode: new function to go to next
 conflict
Date: Mon, 18 Feb 2019 18:28:33 -0500

> Ah, works for me, thanks! I didn't want to upgrade to latest master because
> I've been using harfbuzz branch, but I just installed and tested master —
> everything is okay.

Thanks, closing,


        Stefan

Previous Next

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