Subject: bug#46538: Patch: wrap around smerge-vc-next-conflict if current file still has conflicts

From: Konstantin Kharlamov <[email-masked]>
Date: Tue, 16 Feb 2021 01:00:49 +0300

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 <[email-masked]> 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 <[email-masked]>
> > 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)))))))))
>