all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#46538: Patch: wrap around smerge-vc-next-conflict if current file still has conflicts
@ 2021-02-15 18:51 Konstantin Kharlamov
  2021-02-15 19:15 ` Basil L. Contovounesios
  0 siblings, 1 reply; 5+ messages in thread
From: Konstantin Kharlamov @ 2021-02-15 18:51 UTC (permalink / raw)
  To: 46538

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

Currently, if the buffer has conflict markers above the (point) and it's the last conflicted file, we bail out. A user pointed out¹ that it is a good idea to try to find conflicts in the current file as well. Indeed, it seems like more correct behaviour: going to the next conflict is whole purpose of the function, and current file might still have them.

Attached patch implements additional check for conflicts just for the case when the buffer is the last conflicted file and all conflicts are above current point.

1: https://emacs.stackexchange.com/questions/63413/finding-git-conflict-in-the-same-buffer-if-cursor-is-at-end-of-the-buffer

[-- Attachment #2: 0001-vc-make-smerge-vc-next-conflict-wrap-around.patch --]
[-- Type: text/x-patch, Size: 1963 bytes --]

From f8d8f1cbdb94f75765165acacd089f1e3cc4a763 Mon Sep 17 00:00:00 2001
From: Konstantin Kharlamov <Hi-Angel@yandex.ru>
Date: Mon, 15 Feb 2021 21:37:33 +0300
Subject: [PATCH] vc: make smerge-vc-next-conflict wrap around

lisp/vc/smerge-mode.el:
(smerge-next-safe): a wrapper around smerge-next that doesn't throw
an error.
(smerge-vc-next-conflict): make it wrap around a file if it's the last
conflicted file, and it still has conflicts
---
 lisp/vc/smerge-mode.el | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/lisp/vc/smerge-mode.el b/lisp/vc/smerge-mode.el
index 3b09dfe5d2..0558240120 100644
--- a/lisp/vc/smerge-mode.el
+++ b/lisp/vc/smerge-mode.el
@@ -1446,6 +1446,14 @@ smerge-change-buffer-confirm
   "If non-nil, request confirmation before moving to another buffer."
   :type 'boolean)
 
+(defun smerge-next-safe ()
+    "Tries to move to next conflict, returns t on success, nil
+otherwise"
+  (condition-case err
+      (not (smerge-next))
+    ('error
+     nil)))
+
 (defun smerge-vc-next-conflict ()
   "Go to next conflict, possibly in another file.
 First tries to go to the next conflict in the current buffer, and if not
@@ -1469,12 +1477,12 @@ smerge-vc-next-conflict
          (if (and (buffer-modified-p) buffer-file-name)
              (save-buffer))
          (vc-find-conflicted-file)
-         (if (eq buffer (current-buffer))
-             ;; Do nothing: presumably `vc-find-conflicted-file' already
-             ;; emitted a message explaining there aren't any more conflicts.
-             nil
-           (goto-char (point-min))
-           (smerge-next)))))))
+         (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))
+             (when (not (smerge-next-safe))
+               (goto-char prev-pos)))))))))
 
 (provide 'smerge-mode)
 
-- 
2.30.1


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

* bug#46538: Patch: wrap around smerge-vc-next-conflict if current file still has conflicts
  2021-02-15 18:51 bug#46538: Patch: wrap around smerge-vc-next-conflict if current file still has conflicts Konstantin Kharlamov
@ 2021-02-15 19:15 ` Basil L. Contovounesios
  2021-02-15 22:00   ` Konstantin Kharlamov
  2021-02-15 22:25   ` bug#46538: [PATCH] vc: make smerge-vc-next-conflict wrap around Konstantin Kharlamov
  0 siblings, 2 replies; 5+ messages in thread
From: Basil L. Contovounesios @ 2021-02-15 19:15 UTC (permalink / raw)
  To: Konstantin Kharlamov; +Cc: 46538

tags 46538 + patch
quit

Konstantin Kharlamov <hi-angel@yandex.ru> writes:

> Attached patch implements additional check for conflicts just for the case when
> the buffer is the last conflicted file and all conflicts are above current
> point.

Thanks, just a few nits from me.

> From f8d8f1cbdb94f75765165acacd089f1e3cc4a763 Mon Sep 17 00:00:00 2001
> From: Konstantin Kharlamov <Hi-Angel@yandex.ru>
> Date: Mon, 15 Feb 2021 21:37:33 +0300
> Subject: [PATCH] vc: make smerge-vc-next-conflict wrap around
>
> lisp/vc/smerge-mode.el:
> (smerge-next-safe): a wrapper around smerge-next that doesn't throw
> an error.
> (smerge-vc-next-conflict): make it wrap around a file if it's the last
> conflicted file, and it still has conflicts

Please use proper sentences starting with a capital letter and ending in
a full stop.

> ---
>  lisp/vc/smerge-mode.el | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/lisp/vc/smerge-mode.el b/lisp/vc/smerge-mode.el
> index 3b09dfe5d2..0558240120 100644
> --- a/lisp/vc/smerge-mode.el
> +++ b/lisp/vc/smerge-mode.el
> @@ -1446,6 +1446,14 @@ smerge-change-buffer-confirm
>    "If non-nil, request confirmation before moving to another buffer."
>    :type 'boolean)
>  
> +(defun smerge-next-safe ()
> +    "Tries to move to next conflict, returns t on success, nil
   ^^^^
Excess indentation.

> +otherwise"

The first line should contain a whole sentence, usually in the
imperative; see (info "(elisp) Documentation Tips").  E.g.:

    "Try moving to next conflict.
  Return non-nil on success; nil otherwise."

> +  (condition-case err
                     ^^^
This variable is unused, so just pass nil instead.

> +      (not (smerge-next))
> +    ('error
        ^^
Conditions are usually unquoted.

> +     nil)))

You can reuse the macro ignore-errors for this:

  (ignore-errors (not (smerge-next)))

>  (defun smerge-vc-next-conflict ()
>    "Go to next conflict, possibly in another file.
>  First tries to go to the next conflict in the current buffer, and if not
> @@ -1469,12 +1477,12 @@ smerge-vc-next-conflict
>           (if (and (buffer-modified-p) buffer-file-name)
>               (save-buffer))
>           (vc-find-conflicted-file)
> -         (if (eq buffer (current-buffer))
> -             ;; Do nothing: presumably `vc-find-conflicted-file' already
> -             ;; emitted a message explaining there aren't any more conflicts.
> -             nil
> -           (goto-char (point-min))
> -           (smerge-next)))))))
> +         (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))
> +             (when (not (smerge-next-safe))
                ^^^^^^^^^^
Nit: when + not = unless

> +               (goto-char prev-pos)))))))))

-- 
Basil





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

* bug#46538: Patch: wrap around smerge-vc-next-conflict if current file still has conflicts
  2021-02-15 19:15 ` Basil L. Contovounesios
@ 2021-02-15 22:00   ` Konstantin Kharlamov
  2021-02-15 22:25   ` bug#46538: [PATCH] vc: make smerge-vc-next-conflict wrap around Konstantin Kharlamov
  1 sibling, 0 replies; 5+ messages in thread
From: Konstantin Kharlamov @ 2021-02-15 22:00 UTC (permalink / raw)
  To: Basil L. Contovounesios; +Cc: 46538

Thank you! I will fix it up.

Nice hint on the `ignore-errors` macro, I didn't know about it

On Mon, 2021-02-15 at 19:15 +0000, Basil L. Contovounesios wrote:
> tags 46538 + patch
> quit
> 
> Konstantin Kharlamov <hi-angel@yandex.ru> writes:
> 
> > Attached patch implements additional check for conflicts just for the case
> > when
> > the buffer is the last conflicted file and all conflicts are above current
> > point.
> 
> Thanks, just a few nits from me.
> 
> > From f8d8f1cbdb94f75765165acacd089f1e3cc4a763 Mon Sep 17 00:00:00 2001
> > From: Konstantin Kharlamov <Hi-Angel@yandex.ru>
> > Date: Mon, 15 Feb 2021 21:37:33 +0300
> > Subject: [PATCH] vc: make smerge-vc-next-conflict wrap around
> > 
> > lisp/vc/smerge-mode.el:
> > (smerge-next-safe): a wrapper around smerge-next that doesn't throw
> > an error.
> > (smerge-vc-next-conflict): make it wrap around a file if it's the last
> > conflicted file, and it still has conflicts
> 
> Please use proper sentences starting with a capital letter and ending in
> a full stop.
> 
> > ---
> >  lisp/vc/smerge-mode.el | 20 ++++++++++++++------
> >  1 file changed, 14 insertions(+), 6 deletions(-)
> > 
> > diff --git a/lisp/vc/smerge-mode.el b/lisp/vc/smerge-mode.el
> > index 3b09dfe5d2..0558240120 100644
> > --- a/lisp/vc/smerge-mode.el
> > +++ b/lisp/vc/smerge-mode.el
> > @@ -1446,6 +1446,14 @@ smerge-change-buffer-confirm
> >    "If non-nil, request confirmation before moving to another buffer."
> >    :type 'boolean)
> >  
> > +(defun smerge-next-safe ()
> > +    "Tries to move to next conflict, returns t on success, nil
>    ^^^^
> Excess indentation.
> 
> > +otherwise"
> 
> The first line should contain a whole sentence, usually in the
> imperative; see (info "(elisp) Documentation Tips").  E.g.:
> 
>     "Try moving to next conflict.
>   Return non-nil on success; nil otherwise."
> 
> > +  (condition-case err
>                      ^^^
> This variable is unused, so just pass nil instead.
> 
> > +      (not (smerge-next))
> > +    ('error
>         ^^
> Conditions are usually unquoted.
> 
> > +     nil)))
> 
> You can reuse the macro ignore-errors for this:
> 
>   (ignore-errors (not (smerge-next)))
> 
> >  (defun smerge-vc-next-conflict ()
> >    "Go to next conflict, possibly in another file.
> >  First tries to go to the next conflict in the current buffer, and if not
> > @@ -1469,12 +1477,12 @@ smerge-vc-next-conflict
> >           (if (and (buffer-modified-p) buffer-file-name)
> >               (save-buffer))
> >           (vc-find-conflicted-file)
> > -         (if (eq buffer (current-buffer))
> > -             ;; Do nothing: presumably `vc-find-conflicted-file' already
> > -             ;; emitted a message explaining there aren't any more
> > conflicts.
> > -             nil
> > -           (goto-char (point-min))
> > -           (smerge-next)))))))
> > +         (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))
> > +             (when (not (smerge-next-safe))
>                 ^^^^^^^^^^
> Nit: when + not = unless
> 
> > +               (goto-char prev-pos)))))))))
> 







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

* bug#46538: [PATCH] vc: make smerge-vc-next-conflict wrap around
  2021-02-15 19:15 ` Basil L. Contovounesios
  2021-02-15 22:00   ` Konstantin Kharlamov
@ 2021-02-15 22:25   ` Konstantin Kharlamov
  2021-02-16 11:50     ` Lars Ingebrigtsen
  1 sibling, 1 reply; 5+ messages in thread
From: Konstantin Kharlamov @ 2021-02-15 22:25 UTC (permalink / raw)
  To: 46538; +Cc: contovob

* lisp/vc/smerge-mode.el:
(smerge-vc-next-conflict): While searching for conflict markers, wrap
search around if current file is the last one with conflicts.
---
 lisp/vc/smerge-mode.el | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lisp/vc/smerge-mode.el b/lisp/vc/smerge-mode.el
index 3b09dfe5d2..b825fd9837 100644
--- a/lisp/vc/smerge-mode.el
+++ b/lisp/vc/smerge-mode.el
@@ -1469,12 +1469,12 @@ smerge-vc-next-conflict
          (if (and (buffer-modified-p) buffer-file-name)
              (save-buffer))
          (vc-find-conflicted-file)
-         (if (eq buffer (current-buffer))
-             ;; Do nothing: presumably `vc-find-conflicted-file' already
-             ;; emitted a message explaining there aren't any more conflicts.
-             nil
-           (goto-char (point-min))
-           (smerge-next)))))))
+         (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)))))))))
 
 (provide 'smerge-mode)
 
-- 
2.30.1






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

* bug#46538: [PATCH] vc: make smerge-vc-next-conflict wrap around
  2021-02-15 22:25   ` bug#46538: [PATCH] vc: make smerge-vc-next-conflict wrap around Konstantin Kharlamov
@ 2021-02-16 11:50     ` Lars Ingebrigtsen
  0 siblings, 0 replies; 5+ messages in thread
From: Lars Ingebrigtsen @ 2021-02-16 11:50 UTC (permalink / raw)
  To: Konstantin Kharlamov; +Cc: contovob, 46538

Konstantin Kharlamov <Hi-Angel@yandex.ru> writes:

> * lisp/vc/smerge-mode.el:
> (smerge-vc-next-conflict): While searching for conflict markers, wrap
> search around if current file is the last one with conflicts.

Looks good to me, so I've applied your patch to Emacs 28.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

end of thread, other threads:[~2021-02-16 11:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-02-15 18:51 bug#46538: Patch: wrap around smerge-vc-next-conflict if current file still has conflicts Konstantin Kharlamov
2021-02-15 19:15 ` Basil L. Contovounesios
2021-02-15 22:00   ` Konstantin Kharlamov
2021-02-15 22:25   ` bug#46538: [PATCH] vc: make smerge-vc-next-conflict wrap around Konstantin Kharlamov
2021-02-16 11:50     ` Lars Ingebrigtsen

Code repositories for project(s) associated with this external index

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.