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

[Basil L. Contovounesios]

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