Subject: Re: bug#3860: vc-svn calls "svn resolved" without permission

From: Mauro Aranda <[email-masked]>
Date: Thu, 13 Mar 2025 12:11:21 -0300

Eli Zaretskii <[email-masked]> writes:

 >> Date: Thu, 13 Mar 2025 09:04:52 -0300
 >> Cc: Eli Zaretskii <[email-masked]>, Dmitry Gutov <[email-masked]>,
 >>  Lars Ingebrigtsen <[email-masked]>, [email-masked], [email-masked]
 >> From: Mauro Aranda <[email-masked]>
 >>
 >> Eli Zaretskii <[email-masked]> writes:
 >>
 >>  >> Cc: [email-masked], [email-masked], [email-masked]
 >>  >> From: Dmitry Gutov <[email-masked]>
 >>  >> Date: Fri, 22 Jan 2021 00:35:07 +0200
 >>  >>
 >>  >> On 21.01.2021 21:33, Eli Zaretskii wrote:
 >>  >> > I think if we are going to make this optional with SVN, we 
should make
 >>  >> > it optional with the other back-ends as well.  VC always 
presented a
 >>  >> > uniform UI and user experience with all back-ends, as far as 
possible
 >>  >> > and reasonable.  In this case, I think automatically marking the
 >>  >> > conflicts as resolved makes a lot of sense, but I won't object 
to make
 >>  >> > it opt-out, as long as it's done for every VCS that can 
support it.
 >>  >>
 >>  >> So someone might want to generalize the Git-specific variable:
 >>  >> vc-git-resolve-conflicts.
 >>  >
 >>  > Yes, I think so.
 >>  >
 >>  >> But they'd have to consider what to do with the special value
 >>  >> 'unstage-maybe', which doesn't really make sense for other backends.
 >>  >
 >>  > Right.
 >>
 >> I took a shot at it.  Please review the attached patch.
 >
 > Thanks.
 >
 >> +(defcustom vc-bzr-resolve-conflicts 'default
 >> +  "When non-nil, mark conflicted file as resolved upon saving.
 >
 > "If non-nil" is better, IMO.

I was going to change it to that, but I realized the text was
misleading.  So I rephrased it.

 >> +If this is non-nil and there are no more conflict markers in the file,
 >> +VC will mark the saved file as resolved.
 >    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 > "VC will mark the conflicts in the saved file as resolved."
 >
 > Same comments for doc strings of all the other options, since this
 > text is repeated in all of them.

Thanks.  Revised patch attached.
From 664cb84de9fa756b75d869fdac2b1b8c8eda892d Mon Sep 17 00:00:00 2001
From: Mauro Aranda <[email-masked]>
Date: Wed, 12 Mar 2025 07:26:13 -0300
Subject: [PATCH] Make marking conflicted files as resolved upon saving opt-out

This fixes Bug#3860.

* lisp/vc/vc.el (vc-resolve-conflicts): New user option.
* lisp/vc/vc-bzr.el (vc-bzr-resolve-conflicts): New user option.
(vc-bzr-find-file-hook): Use it.
* lisp/vc/vc-hg.el (vc-hg-resolve-conflicts): New user option.
(vc-hg-find-file-hook): Use it.
* lisp/vc/vc-svn.el (vc-svn-resolve-conflicts): New user option.
(vc-svn-find-file-hook): Use it.
* lisp/vc/vc-git.el (vc-git-resolve-conflicts): Support
'default' as an option.  Adjust docstring and version.
(vc-git-find-file-hook): Respect vc-resolve-conflicts.
* etc/NEWS: Announce the new options.
---
 etc/NEWS          | 12 ++++++++++++
 lisp/vc/vc-bzr.el | 15 ++++++++++++++-
 lisp/vc/vc-git.el | 21 +++++++++++++++------
 lisp/vc/vc-hg.el  | 15 ++++++++++++++-
 lisp/vc/vc-svn.el | 15 ++++++++++++++-
 lisp/vc/vc.el     | 15 +++++++++++++++
 6 files changed, 84 insertions(+), 9 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 39b2199564d..604ed865329 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1364,6 +1364,18 @@ It is intended to be added to the 'log-edit-done-hook' so that
 'vc-cvs-checkin' behaves like invoking "cvs commit [files...]" from the
 command line.
 
+---
+*** New user options 'vc-resolve-conflicts', 'vc-*-resolve-conflicts'
+Control whether to mark a conflicted file as resolved when saving.
+You can now control it globally, with 'vc-resolve-conflicts' or for
+specific backends with 'vc-bzr-resolve-conflicts',
+'vc-hg-resolve-conflicts' and 'vc-svn-resolve-conflicts'.
+---
+*** New value for 'vc-git-resolve-conflicts'
+The option now accepts the symbol 'default' as a value, which is
+its default value.  Effectively, the default value hasn't changed,
+since 'vc-resolve-conflicts' defaults to t, the previous default value
+for 'vc-git-resolve-conflicts'.
 ** Diff mode
 
 +++
diff --git a/lisp/vc/vc-bzr.el b/lisp/vc/vc-bzr.el
index 0907eaee448..1ba4450cf5d 100644
--- a/lisp/vc/vc-bzr.el
+++ b/lisp/vc/vc-bzr.el
@@ -110,6 +110,16 @@ vc-bzr-status-switches
                  (repeat :tag "Argument List" :value ("") string))
   :version "24.1")
 
+(defcustom vc-bzr-resolve-conflicts 'default
+  "Whether to mark conflicted file as resolved upon saving.
+If this is t and there are no more conflict markers in the file,
+VC will mark the conflicts in the saved file as resolved.
+A value of `default' means to use the value of `vc-resolve-conflicts'."
+  :type '(choice (const :tag "Don't resolve" nil)
+                 (const :tag "Resolve" t)
+                 (const :tag "Use vc-resolve-conflicts" default))
+  :version "31.1")
+
 ;; since v0.9, bzr supports removing the progress indicators
 ;; by setting environment variable BZR_PROGRESS_BAR to "none".
 (defun vc-bzr-command (bzr-command buffer okstatus file-or-list &rest args)
@@ -531,7 +541,10 @@ vc-bzr-find-file-hook
     ;; but the one in `bzr pull' isn't, so it would be good to provide an
     ;; elisp function to remerge from the .BASE/OTHER/THIS files.
     (smerge-start-session)
-    (add-hook 'after-save-hook #'vc-bzr-resolve-when-done nil t)
+    (when (or (eq vc-bzr-resolve-conflicts t)
+              (and (eq vc-bzr-resolve-conflicts 'default)
+                   vc-resolve-conflicts))
+      (add-hook 'after-save-hook #'vc-bzr-resolve-when-done nil t))
     (vc-message-unresolved-conflicts buffer-file-name)))
 
 (defun vc-bzr-clone (remote directory rev)
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 78827c74691..932dddaefbb 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -166,17 +166,24 @@ vc-git-file-name-changes-switches
                  (repeat :tag "Argument List" :value ("") string))
   :version "30.1")
 
-(defcustom vc-git-resolve-conflicts t
-  "When non-nil, mark conflicted file as resolved upon saving.
+(defcustom vc-git-resolve-conflicts 'default
+  "Whether to mark conflicted file as resolved upon saving.
 That is performed after all conflict markers in it have been removed.
+
+If this is t and there are no more conflict markers in the file,
+VC will mark the conflicts in the saved file as resolved.
+
 If the value is `unstage-maybe', and no merge, rebase or similar
 operation is in progress, then after the last conflict is resolved, also
-clear the staging area."
+clear the staging area.
+
+A value of `default' means to use the value of `vc-resolve-conflicts'."
   :type '(choice (const :tag "Don't resolve" nil)
                  (const :tag "Resolve" t)
                  (const :tag "Resolve and maybe unstage all files"
-                        unstage-maybe))
-  :version "25.1")
+                        unstage-maybe)
+                 (const :tag "Use vc-resolve-conflicts" default))
+  :version "31.1")
 
 (defcustom vc-git-program "git"
   "Name of the Git executable (excluding any arguments)."
@@ -1445,7 +1452,9 @@ vc-git-find-file-hook
                (goto-char (point-min))
                (re-search-forward "^<<<<<<< " nil 'noerror)))
     (smerge-start-session)
-    (when vc-git-resolve-conflicts
+    (unless (or (null vc-git-resolve-conflicts)
+                (and (eq vc-git-resolve-conflicts 'default)
+                     (not vc-resolve-conflicts)))
       (add-hook 'after-save-hook #'vc-git-resolve-when-done nil 'local))
     (vc-message-unresolved-conflicts buffer-file-name)))
 
diff --git a/lisp/vc/vc-hg.el b/lisp/vc/vc-hg.el
index b4d7844013a..b7200da0914 100644
--- a/lisp/vc/vc-hg.el
+++ b/lisp/vc/vc-hg.el
@@ -183,6 +183,16 @@ vc-hg-create-bookmark
                  (const :tag "Ask" ask))
   :version "28.1")
 
+(defcustom vc-hg-resolve-conflicts 'default
+  "Whether to mark conflicted file as resolved upon saving.
+If this is t and there are no more conflict markers in the file,
+VC will mark the conflicts in the saved file as resolved.
+A value of `default' means to use the value of `vc-resolve-conflicts'."
+  :type '(choice (const :tag "Don't resolve" nil)
+                 (const :tag "Resolve" t)
+                 (const :tag "Use vc-resolve-conflicts" default))
+  :version "31.1")
+
 
 ;; Clear up the cache to force vc-call to check again and discover
 ;; new functions when we reload this file.
@@ -1263,7 +1273,10 @@ vc-hg-find-file-hook
     ;; Hg may not recognize "conflict" as a state, but we can do better.
     (vc-file-setprop buffer-file-name 'vc-state 'conflict)
     (smerge-start-session)
-    (add-hook 'after-save-hook #'vc-hg-resolve-when-done nil t)
+    (when (or (eq vc-hg-resolve-conflicts t)
+              (and (eq vc-hg-resolve-conflicts 'default)
+                   vc-resolve-conflicts))
+      (add-hook 'after-save-hook #'vc-hg-resolve-when-done nil t))
     (vc-message-unresolved-conflicts buffer-file-name)))
 
 (defun vc-hg-clone (remote directory rev)
diff --git a/lisp/vc/vc-svn.el b/lisp/vc/vc-svn.el
index 422ef48f8c0..e81636552b5 100644
--- a/lisp/vc/vc-svn.el
+++ b/lisp/vc/vc-svn.el
@@ -106,6 +106,16 @@ vc-svn-header
   :version "24.1"     ; no longer consult the obsolete vc-header-alist
   :type '(repeat string))
 
+(defcustom vc-svn-resolve-conflicts 'default
+  "Whether to mark conflicted file as resolved upon saving.
+If this is t and there are no more conflict markers in the file,
+VC will mark the conflicts in the saved file as resolved.
+A value of `default' means to use the value of `vc-resolve-conflicts'."
+  :type '(choice (const :tag "Don't resolve" nil)
+                 (const :tag "Resolve" t)
+                 (const :tag "Use vc-resolve-conflicts" default))
+  :version "31.1")
+
 ;; We want to autoload it for use by the autoloaded version of
 ;; vc-svn-registered, but we want the value to be compiled at startup, not
 ;; at dump time.
@@ -688,7 +698,10 @@ vc-svn-find-file-hook
         ;; There are conflict markers.
         (progn
           (smerge-start-session)
-          (add-hook 'after-save-hook #'vc-svn-resolve-when-done nil t))
+          (when (or (eq vc-svn-resolve-conflicts t)
+              (and (eq vc-svn-resolve-conflicts 'default)
+                   vc-resolve-conflicts))
+            (add-hook 'after-save-hook #'vc-svn-resolve-when-done nil t)))
       ;; There are no conflict markers.  This is problematic: maybe it means
       ;; the conflict has been resolved and we should immediately call "svn
       ;; resolved", or it means that the file's type does not allow Svn to
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index bc96173d198..ff61a9e7501 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -999,6 +999,21 @@ vc-clone-heuristic-alist
                 :value-type ,vc-cloneable-backends-custom-type)
   :version "31.1")
 
+(defcustom vc-resolve-conflicts t
+  "Whether to mark conflicted file as resolved upon saving.
+
+If this is non-nil and there are no more conflict markers in the file,
+VC will mark the conflicts in the saved file as resolved.  This is
+only meaningful for VCS that handle conflicts by inserting conflict
+markers in a conflicted file.
+
+When saving a conflicted file, VC first tries to use the value
+of `vc-BACKEND-resolve-conflicts', for handling backend-specific
+settings.  It defaults to this option if that option has the special
+value `default'."
+  :type 'boolean
+  :version "31.1")
+
 
 ;; File property caching
 
-- 
2.34.1