bug#46876: [PATCH] Find conflict markers in opened buffers as well

bug#46876: [PATCH] Find conflict markers in opened buffers as well

[Konstantin Kharlamov]

Call to (vc-find-conflicted-file) will only result in jump to a conflict
marker when file is a newly opened one. When a file is already open in
Emacs, (vc-find-conflicted-file) only switches to that buffer, so we
need to explicitly jump to a conflict marker.

* lisp/vc/smerge-mode.el (smerge-vc-next-conflict): Search for a
conflict marker if call to (vc-find-conflicted-file) haven't resulted in
a jump to one.
---
 lisp/vc/smerge-mode.el | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lisp/vc/smerge-mode.el b/lisp/vc/smerge-mode.el
index 782c799273..383f8435f5 100644
--- a/lisp/vc/smerge-mode.el
+++ b/lisp/vc/smerge-mode.el
@@ -1468,8 +1468,10 @@ smerge-vc-next-conflict
          (if (and (buffer-modified-p) buffer-file-name)
              (save-buffer))
          (vc-find-conflicted-file)
-         (when (eq buffer (current-buffer))
-           ;; Try to find a conflict marker in current file above the point.
+         ;; At this point, the caret will only be at a conflict marker
+         ;; if the file did not correspond to an opened
+         ;; buffer. Otherwise, we need to jump to a marker explicitly.
+         (unless (looking-at "^<<<<<<<")
            (let ((prev-pos (point)))
              (goto-char (point-min))
              (unless (ignore-errors (not (smerge-next)))
-- 
2.30.1

bug#46876: [PATCH] Find conflict markers in opened buffers as well

[Konstantin Kharlamov]

Hello! Sorry, it seems like debbugs notifications got broken, at least for me. I never got a notifications about this report creation; and it seems not a coincidence because while working with bug #46400 at some point notifications stopped coming in.

I'll create a separate report on this, but please add me to CC for now because otherwise I can't know if someone is replying.

With that said, right after I sent the patch I figured there an unused variable appeared after my changes, so I'll sent in a moment the newer version of the changes, with it removed.

bug#46876: [PATCH] Find conflict markers in opened buffers as well

[Konstantin Kharlamov]

On Tue, 2021-03-02 at 22:37 +0300, Konstantin Kharlamov wrote:
> Hello! Sorry, it seems like debbugs notifications got broken, at least for me.
> I never got a notifications about this report creation; and it seems not a
> coincidence because while working with bug #46400 at some point notifications
> stopped coming in.
> 
> I'll create a separate report on this, but please add me to CC for now because
> otherwise I can't know if someone is replying.

Ah, nvm, it seems it was a coincidence, combined with odd timings of various events. If anyone is curious: when I sent the patch, I did not get a notification: not immediately, nor after 10 minutes, not after 20 minutes… I checked the spam folder, and there was nothing as well. This looked very suspicious, so when after 3 hours I still didn't get anything, I thought it is the problem as in #46400, apparently something is happening. Well, it turned out I did get the notification, and it got into spam folder. But I did check spam folder, you remember, right? Well, the notification was only sent 43 minutes after my patch, so no surprise at this point I did not look at spam folder anymore (also, this is the only false-positive by my email-provider in, like, years, I think).

bug#46876: [PATCH] Find conflict markers in opened buffers as well

[Dmitry Gutov]

On 02.03.2021 21:52, Konstantin Kharlamov wrote:
> Well, the notification was only sent 43 minutes after my patch, so no surprise at this point I did not look at spam folder anymore (also, this is the only false-positive by my email-provider in, like, years, I think).

Yeah, the bug tracker can be slow with the notifications. Mailing lists 
deliver some messages pretty late too.

bug#46876: [PATCH] Find conflict markers in opened buffers as well

[Konstantin Kharlamov]

Call to (vc-find-conflicted-file) will only result in jump to a conflict
marker when file is a newly opened one. When a file is already open in
Emacs, (vc-find-conflicted-file) only switches to that buffer, so we
need to explicitly jump to a conflict marker.

* lisp/vc/smerge-mode.el (smerge-vc-next-conflict): Search for a
conflict marker if call to (vc-find-conflicted-file) haven't resulted in
a jump to one. And remove `buffer` variable that becomes unused.
---
 lisp/vc/smerge-mode.el | 49 +++++++++++++++++++++---------------------
 1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/lisp/vc/smerge-mode.el b/lisp/vc/smerge-mode.el
index 782c799273..694d4529b9 100644
--- a/lisp/vc/smerge-mode.el
+++ b/lisp/vc/smerge-mode.el
@@ -1450,30 +1450,31 @@ smerge-vc-next-conflict
 First tries to go to the next conflict in the current buffer, and if not
 found, uses VC to try and find the next file with conflict."
   (interactive)
-  (let ((buffer (current-buffer)))
-    (condition-case nil
-        ;; FIXME: Try again from BOB before moving to the next file.
-        (smerge-next)
-      (error
-       (if (and (or smerge-change-buffer-confirm
-                    (and (buffer-modified-p) buffer-file-name))
-                (not (or (eq last-command this-command)
-                         (eq ?\r last-command-event)))) ;Called via M-x!?
-           ;; FIXME: Don't emit this message if `vc-find-conflicted-file' won't
-           ;; go to another file anyway (because there are no more conflicted
-           ;; files).
-           (message (if (buffer-modified-p)
-                        "No more conflicts here.  Repeat to save and go to next buffer"
-                      "No more conflicts here.  Repeat to go to next buffer"))
-         (if (and (buffer-modified-p) buffer-file-name)
-             (save-buffer))
-         (vc-find-conflicted-file)
-         (when (eq buffer (current-buffer))
-           ;; Try to find a conflict marker in current file above the point.
-           (let ((prev-pos (point)))
-             (goto-char (point-min))
-             (unless (ignore-errors (not (smerge-next)))
-               (goto-char prev-pos)))))))))
+  (condition-case nil
+      ;; FIXME: Try again from BOB before moving to the next file.
+      (smerge-next)
+    (error
+     (if (and (or smerge-change-buffer-confirm
+                  (and (buffer-modified-p) buffer-file-name))
+              (not (or (eq last-command this-command)
+                       (eq ?\r last-command-event)))) ;Called via M-x!?
+         ;; FIXME: Don't emit this message if `vc-find-conflicted-file' won't
+         ;; go to another file anyway (because there are no more conflicted
+         ;; files).
+         (message (if (buffer-modified-p)
+                      "No more conflicts here.  Repeat to save and go to next buffer"
+                    "No more conflicts here.  Repeat to go to next buffer"))
+       (if (and (buffer-modified-p) buffer-file-name)
+           (save-buffer))
+       (vc-find-conflicted-file)
+       ;; At this point, the caret will only be at a conflict marker
+       ;; if the file did not correspond to an opened
+       ;; buffer. Otherwise we need to jump to a marker explicitly.
+       (unless (looking-at "^<<<<<<<")
+         (let ((prev-pos (point)))
+           (goto-char (point-min))
+           (unless (ignore-errors (not (smerge-next)))
+             (goto-char prev-pos))))))))
 
 (provide 'smerge-mode)
 
-- 
2.30.1

bug#46876: [PATCH] Find conflict markers in opened buffers as well

[Dmitry Gutov]

On 02.03.2021 21:40, Konstantin Kharlamov wrote:
> Call to (vc-find-conflicted-file) will only result in jump to a conflict
> marker when file is a newly opened one. When a file is already open in
> Emacs, (vc-find-conflicted-file) only switches to that buffer, so we
> need to explicitly jump to a conflict marker.
> 
> * lisp/vc/smerge-mode.el (smerge-vc-next-conflict): Search for a
> conflict marker if call to (vc-find-conflicted-file) haven't resulted in
> a jump to one. And remove `buffer` variable that becomes unused.

Thank you, this makes sense. Applied and pushed to master.

This part is suboptimal:

 > When a file is already open in
 > Emacs, (vc-find-conflicted-file) only switches to that buffer

...and I had to spend some time figuring out why that happens (hint: 
vc-git-find-file-hook), and that kind of unpredictable behavior is Not 
Good(tm).

I don't have an alternative fix to propose, though. At least not at this 
time.

bug#46876: [PATCH] Find conflict markers in opened buffers as well

[Konstantin Kharlamov]

On Tue, 2021-03-09 at 04:51 +0200, Dmitry Gutov wrote:
> On 02.03.2021 21:40, Konstantin Kharlamov wrote:
> > Call to (vc-find-conflicted-file) will only result in jump to a conflict
> > marker when file is a newly opened one. When a file is already open in
> > Emacs, (vc-find-conflicted-file) only switches to that buffer, so we
> > need to explicitly jump to a conflict marker.
> > 
> > * lisp/vc/smerge-mode.el (smerge-vc-next-conflict): Search for a
> > conflict marker if call to (vc-find-conflicted-file) haven't resulted in
> > a jump to one. And remove `buffer` variable that becomes unused.
> 
> Thank you, this makes sense. Applied and pushed to master.

Thank you!

> This part is suboptimal:
> 
>  > When a file is already open in
>  > Emacs, (vc-find-conflicted-file) only switches to that buffer
> 
> ...and I had to spend some time figuring out why that happens (hint: 
> vc-git-find-file-hook), and that kind of unpredictable behavior is Not 
> Good(tm).

Back when I stumbled upon this behaviour, I didn't research into it because I thought it could be deliberate. The reasoning might have been: if you didn't have a file opened, it doesn't really matter where your point would be once it is. So it shouldn't hurt to just jump to a conflict marker, and so it does. On the other hand, if you did have the file opened, you might not want to lose position of your point (for example, you could have a selection, which you don't want to lose for some reason), IOW initial point position in this case might matter.

I'm just speculating though, I do not know if it's true, neither I remember having a usecase as the one I imagine it's trying to cover. FWIW, usually when I want to save positions in a buffer, I use (evil-set-marker) from Evil package.

bug#46876: [PATCH] Find conflict markers in opened buffers as well

[Dmitry Gutov]

On 09.03.2021 08:32, Konstantin Kharlamov wrote:

>> This part is suboptimal:
>>
>>   > When a file is already open in
>>   > Emacs, (vc-find-conflicted-file) only switches to that buffer
>>
>> ...and I had to spend some time figuring out why that happens (hint:
>> vc-git-find-file-hook), and that kind of unpredictable behavior is Not
>> Good(tm).
> 
> Back when I stumbled upon this behaviour, I didn't research into it because I thought it could be deliberate. The reasoning might have been: if you didn't have a file opened, it doesn't really matter where your point would be once it is. So it shouldn't hurt to just jump to a conflict marker, and so it does. On the other hand, if you did have the file opened, you might not want to lose position of your point (for example, you could have a selection, which you don't want to lose for some reason), IOW initial point position in this case might matter.
> 
> I'm just speculating though, I do not know if it's true, neither I remember having a usecase as the one I imagine it's trying to cover. FWIW, usually when I want to save positions in a buffer, I use (evil-set-marker) from Evil package.

I don't use Evil, but set-mark should work.

In any case, when the user is calling vc-find-conflicted-file, they 
probably want to either always go to the conflict markers (whether the 
file has been visited or not), or never. And "always" probably makes 
more sense.

It's not urgent, though, especially with neither command having a 
default binding.