vc-checkin: Check whether the fileset or patches have changed

* lisp/vc/vc-dispatcher.el (vc-finish-logentry): Delay popping
to vc-parent-buffer until after calling the log operation.
That way if the log operation exits early, the current buffer
remains *vc-log*.
(vc-dir-marked-files, dired-get-marked-files): Declare.
(vc-dispatcher--explicit-marks-p): New function.
* lisp/vc/vc.el (vc-checkin): Check the user isn't likely to be
surprised by what is included in the checkin.  Specifically,
check whether the fileset or patches implied by vc-parent-buffer
are unchanged.
* doc/emacs/maintaining.texi (VC With A Merging VCS): Explain
how the fileset or patch string is fixed once *vc-log* pops up.

3 files changed, 72 insertions, 8 deletions

diff --git a/doc/emacs/maintaining.texi b/doc/emacs/maintaining.texi
index 12770f218ca..ea7b8794bbc 100644
--- a/doc/emacs/maintaining.texi
+++ b/doc/emacs/maintaining.texi
@@ -572,6 +572,23 @@ then applies the changes to the respective files and commits the changes
 after popping up the @file{*vc-log*} buffer to allow you to type a
 suitable commit log message.
 
+Once you type @kbd{C-x v v}, the fileset or patches cannot be changed
+without first cancelling the commit by typing @kbd{C-c C-k} in the
+@file{*vc-log*} buffer.  For example, if you change which files are
+marked in the @file{*vc-dir*} buffer after Emacs has already popped up
+the @file{*vc-log*} buffer, the old fileset will remain in effect for
+this commit.  (This is in contrast to changes made to the
+@emph{contents} of files in the fileset: all such changes will be
+included in the commit even if they are made after Emacs has popped up
+the @file{*vc-dir*} buffer.)
+
+When you cancel a commit, Emacs saves your log message.  This means that
+if you need to adjust the fileset or patches, it is easy to restart the
+commit operation again: type @w{@kbd{C-c C-k C-x v v M-p}}.  Here
+@kbd{C-c C-k} cancels the commit, @kbd{C-x v v} initiates another with
+the new fileset or patches, and finally @kbd{M-p} recalls your previous
+log message.
+
 With modern decentralized version control systems (Git, Mercurial,
 etc.), the changes are committed locally and not automatically
 propagated to the upstream repository (which is usually on a remote
diff --git a/lisp/vc/vc-dispatcher.el b/lisp/vc/vc-dispatcher.el
index be50e51ed5f..97f1971dd1d 100644
--- a/lisp/vc/vc-dispatcher.el
+++ b/lisp/vc/vc-dispatcher.el
@@ -864,18 +864,13 @@ the buffer contents as a comment."
   ;; save the parameters held in buffer-local variables
   (let ((logbuf (current-buffer))
         (log-operation vc-log-operation)
-        ;; FIXME: When coming from VC-Dir, we should check that the
-        ;; set of selected files is still equal to vc-log-fileset,
-        ;; to avoid surprises.
         (log-fileset vc-log-fileset)
         (log-entry (buffer-string))
         (after-hook vc-log-after-operation-hook))
-    (pop-to-buffer vc-parent-buffer)
     ;; OK, do it to it
-    (save-excursion
-      (funcall log-operation
-               log-fileset
-               log-entry))
+    (with-current-buffer vc-parent-buffer
+      (funcall log-operation log-fileset log-entry))
+    (pop-to-buffer vc-parent-buffer)
     (setq vc-log-operation nil)
 
     ;; Quit windows on logbuf.
@@ -896,6 +891,16 @@ the buffer contents as a comment."
       (derived-mode-p 'diff-mode)
       (derived-mode-p 'log-view-mode)))
 
+(declare-function vc-dir-marked-files "vc-dir")
+(declare-function dired-get-marked-files "dired")
+
+(defun vc-dispatcher--explicit-marks-p ()
+  "Are any files in the directory browser explicitly marked?"
+  (or (and (derived-mode-p 'vc-dir-mode)
+           (vc-dir-marked-files))
+      (and (derived-mode-p 'dired-mode)
+           (length> (dired-get-marked-files nil nil nil t) 1))))
+
 ;; These are unused.
 ;; (defun vc-dispatcher-in-fileset-p (fileset)
 ;;   (let ((member nil))
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 824959fc231..a6a4aa50579 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1953,6 +1953,48 @@ Runs the normal hooks `vc-before-checkin-hook' and `vc-checkin-hook'."
     (lambda ()
       (vc-call-backend backend 'log-edit-mode))
     (lambda (files comment)
+      ;; Check the user isn't likely to be surprised by what is included
+      ;; in the checkin.  Once a log operation is started, the fileset
+      ;; or patch string is locked in.  In particular, it's probably too
+      ;; late to offer to change it now -- checks in hooks and/or the
+      ;; backend's Log Edit derived mode have all already okayed the
+      ;; checkin.  Restarting with the new fileset or patch is easy.
+      (let* ((start-again
+              (substitute-command-keys "\\[vc-next-action] to check in again"))
+             (instructions
+              (substitute-command-keys
+               (string-join
+                (list "type \\<log-edit-mode-map>\\[log-edit-kill-buffer] to cancel"
+                      start-again
+                      "\\[log-edit-previous-comment] to recall your message")
+                ", "))))
+        (cond (patch-string
+               (unless (or (not (derived-mode-p 'diff-mode))
+                           (equal patch-string (buffer-string))
+                           (yes-or-no-p
+                            (format-message "Patch in buffer \"%s\" \
+has changed; continue with old patch?" (current-buffer))))
+                 (user-error "%s %s"
+                             "To check in the new patch" instructions)))
+              ((vc-dispatcher-browsing)
+               (unless (or (and (length= files 1)
+                                ;; If no files in the dispatcher were
+                                ;; marked and it was just that point
+                                ;; moved to a different line, we don't
+                                ;; want to bother the user.  This isn't
+                                ;; foolproof because we don't know
+                                ;; whether FILES was selected by means
+                                ;; of marking a single file or the
+                                ;; implicit selection of the file at
+                                ;; point in the absence of any marks.
+                                (not (vc-dispatcher--explicit-marks-p)))
+                           (equal files (cadr (vc-deduce-fileset)))
+                           (yes-or-no-p
+                            (format-message "Selected file(s) in buffer \"%s\" \
+have changed; continue with old fileset?" (current-buffer))))
+                 (user-error "%s %s"
+                             "To use the new fileset" instructions)))))
+
       ;; "This log message intentionally left almost blank".
       ;; RCS 5.7 gripes about whitespace-only comments too.
       (unless (and comment (string-match "[^\t\n ]" comment))