Subject: Re: bug#34949: 27.0.50; Docstring of `vc-deduce-fileset' incomplete

From: Juri Linkov <[email-masked]>
Date: Tue, 18 Feb 2020 01:42:13 +0200

>> What I don't fully understand, however, is why prohibit state-changing
>> operations in Dired buffers. Is it just because there's no VC-Dir buffer
>> easily at hand, to update the visible file statuses?
>
> Exactly the same question I ask myself every time I forget about this limitation
> and type 'C-x v v' in Dired.

I see now why it's difficult to support state changing VC operations in dired.

The difficultly comes from the need to get a VC state from every marked file
and to confirm that all marked files are in the same state.

So I implemented this in vc-dired-deduce-fileset whose basic
algorithm was taken from vc-dir-deduce-fileset.

Now with this patch state changing VC operations are supported on marked
_files_ in dired.

However, the need for the OBSERVER arg still remains because this patch
doesn't support state changing VC operations on marked _directories_ in dired.
It raises the error in this case:

  State changing VC operations on directories not supported in `dired-mode'

State changing VC operations could be supported on directories as well,
but there are several possibilities to choose from, and I can't decide:

1. On a marked directory get a state from all its files
   (probably this variant makes no sense)

2. On a marked directory get only files with a "modified" state
   (i.e. edited/added/removed)

But the latter is not easy to implement because the needed functionality
is missing in VC, because the only VC method that is suitable for this is
'dir-status-files' to get VC files from a directory but it's too tightly
integrated with vc-dir and can't be easily separated, i.e. there is
no easy way to do something like:

  (with-temp-buffer
    (vc-call-backend
     (vc-responsible-backend default-directory) 'dir-status-files default-directory nil
     (lambda (entries &optional more-to-come)
       (message "! %S" entries))))

Once we decide how to support marked directories in dired, the arg OBSERVER
can be deprecated.

diff --git a/lisp/vc/vc-dispatcher.el b/lisp/vc/vc-dispatcher.el
index 5ae300bf09..4a04c9365a 100644
--- a/lisp/vc/vc-dispatcher.el
+++ b/lisp/vc/vc-dispatcher.el
@@ -746,7 +746,8 @@ vc-finish-logentry
 
 (defun vc-dispatcher-browsing ()
   "Are we in a directory browser buffer?"
-  (derived-mode-p 'vc-dir-mode))
+  (or (derived-mode-p 'vc-dir-mode)
+      (derived-mode-p 'dired-mode)))
 
 ;; These are unused.
 ;; (defun vc-dispatcher-in-fileset-p (fileset)
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index ec252b74d4..bba1fb5919 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1032,9 +1032,7 @@ vc-deduce-fileset
      ((derived-mode-p 'vc-dir-mode)
       (vc-dir-deduce-fileset state-model-only-files))
      ((derived-mode-p 'dired-mode)
-      (if observer
-	  (vc-dired-deduce-fileset)
-	(error "State changing VC operations not supported in `dired-mode'")))
+      (vc-dired-deduce-fileset state-model-only-files observer))
      ((setq backend (vc-backend buffer-file-name))
       (if state-model-only-files
 	(list backend (list buffer-file-name)
@@ -1046,7 +1044,8 @@ vc-deduce-fileset
            ;; FIXME: Why this test?  --Stef
            (or (buffer-file-name vc-parent-buffer)
 				(with-current-buffer vc-parent-buffer
-				  (derived-mode-p 'vc-dir-mode))))
+				  (or (derived-mode-p 'vc-dir-mode)
+				      (derived-mode-p 'dired-mode)))))
       (progn                  ;FIXME: Why not `with-current-buffer'? --Stef.
 	(set-buffer vc-parent-buffer)
 	(vc-deduce-fileset observer allow-unregistered state-model-only-files)))
@@ -1066,9 +1065,31 @@ vc-deduce-fileset
 	      (list buffer-file-name))))
      (t (error "File is not under version control")))))
 
-(defun vc-dired-deduce-fileset ()
-  (list (vc-responsible-backend default-directory)
-        (dired-map-over-marks (dired-get-filename nil t) nil)))
+(declare-function dired-get-marked-files "dired"
+                  (&optional localp arg filter distinguish-one-marked error))
+
+(defun vc-dired-deduce-fileset (&optional state-model-only-files observer)
+  (let ((backend (vc-responsible-backend default-directory))
+        (files (dired-get-marked-files nil nil nil nil t))
+	only-files-list
+	state
+	model)
+    (when (and (not observer) (cl-some #'file-directory-p files))
+      (error "State changing VC operations on directories not supported in `dired-mode'"))
+
+    (when state-model-only-files
+      (setq only-files-list (mapcar (lambda (file) (cons file (vc-state file))) files))
+      (setq state (cdar only-files-list))
+      ;; Check that all files are in a consistent state, since we use that
+      ;; state to decide which operation to perform.
+      (dolist (crt (cdr only-files-list))
+	(unless (vc-compatible-state (cdr crt) state)
+	  (error "When applying VC operations to multiple files, the files are required\nto  be in similar VC states.\n%s in state %s clashes with %s in state %s"
+		 (car crt) (cdr crt) (caar only-files-list) state)))
+      (setq only-files-list (mapcar 'car only-files-list))
+      (when (and state (not (eq state 'unregistered)))
+	(setq model (vc-checkout-model backend only-files-list))))
+    (list backend files only-files-list state model)))
 
 (defun vc-ensure-vc-buffer ()
   "Make sure that the current buffer visits a version-controlled file."