[Emacs-diffs] master 1face76: Revert the disputed VC change and update the tests

[Dmitry Gutov]

branch: master
commit 1face76ba6d19b269310ddbb0a6a618a3bfe54a2
Author: Dmitry Gutov <address@hidden>
Commit: Dmitry Gutov <address@hidden>

    Revert the disputed VC change and update the tests
    
    * lisp/vc/vc-hooks.el (vc-working-revision):
    Remove the previous change.
    (vc-state): Same.  And update the old, incorrect comment about
    unregistered files
    (http://lists.gnu.org/archive/html/emacs-devel/2016-04/msg00526.html).
    
    * test/lisp/vc/vc-tests.el (vc-test--state): Remove the check
    calling `vc-state' on default-directory (VC state is undefined
    for directories).  Check that `vc-state' returns nil where it
    returned `unregistered' before.  Remove all checks comparing
    invocations with the backend passed in explictly and without.
    (vc-test--working-revision): Remove all checks comparing
    invocations with the backend passed in explictly and without.
    Update comments, and add a new one.
---
 lisp/vc/vc-hooks.el      |   30 +++++++++++++------------
 test/lisp/vc/vc-tests.el |   55 ++++++++++++++--------------------------------
 2 files changed, 32 insertions(+), 53 deletions(-)

diff --git a/lisp/vc/vc-hooks.el b/lisp/vc/vc-hooks.el
index 4047bca..6b4cd6a 100644
--- a/lisp/vc/vc-hooks.el
+++ b/lisp/vc/vc-hooks.el
@@ -468,18 +468,21 @@ status of this file.  Otherwise, the value returned is 
one of:
 
   `unregistered'     The file is not under version control."
 
-  ;; Note: in Emacs 22 and older, return of nil meant the file was
-  ;; unregistered.  This is potentially a source of
-  ;; backward-compatibility bugs.
+  ;; Note: we usually return nil here for unregistered files anyway
+  ;; when called with only one argument.  This doesn't seem to cause
+  ;; any problems.  But if we wanted to change that, we should
+  ;; probably opt for redefining the `registered' command to return
+  ;; non-nil even for unregistered files (maybe also rename it), and
+  ;; then make sure that all `state' implementations handle
+  ;; unregistered file appropriately.
 
   ;; FIXME: New (sub)states needed (?):
   ;; - `copied' and `moved' (might be handled by `removed' and `added')
   (or (vc-file-getprop file 'vc-state)
-      (and (not (vc-registered file)) 'unregistered)
       (when (> (length file) 0)         ;Why??  --Stef
-       (setq backend (or backend (vc-responsible-backend file)))
-       (when backend
-         (vc-state-refresh file backend)))))
+        (setq backend (or backend (vc-backend file)))
+        (when backend
+          (vc-state-refresh file backend)))))
 
 (defun vc-state-refresh (file backend)
   "Quickly recompute the `state' of FILE."
@@ -495,13 +498,12 @@ status of this file.  Otherwise, the value returned is 
one of:
   "Return the repository version from which FILE was checked out.
 If FILE is not registered, this function always returns nil."
   (or (vc-file-getprop file 'vc-working-revision)
-      (and (vc-registered file)
-          (progn
-            (setq backend (or backend (vc-responsible-backend file)))
-            (when backend
-              (vc-file-setprop file 'vc-working-revision
-                               (vc-call-backend
-                                 backend 'working-revision file)))))))
+      (progn
+        (setq backend (or backend (vc-backend file)))
+        (when backend
+          (vc-file-setprop file 'vc-working-revision
+                           (vc-call-backend
+                            backend 'working-revision file))))))
 
 ;; Backward compatibility.
 (define-obsolete-function-alias
diff --git a/test/lisp/vc/vc-tests.el b/test/lisp/vc/vc-tests.el
index 793ad82..ac10ce2 100644
--- a/test/lisp/vc/vc-tests.el
+++ b/test/lisp/vc/vc-tests.el
@@ -316,46 +316,31 @@ This checks also `vc-backend' and 
`vc-responsible-backend'."
           'vc-test--cleanup-hook
           `(lambda () (delete-directory ,default-directory 'recursive)))
 
-         ;; Create empty repository.  Check repository state.
+         ;; Create empty repository.
          (make-directory default-directory)
          (vc-test--create-repo-function backend)
 
-          ;; FIXME: The state shall be unregistered only.
-         ;; nil: RCS
-          ;; unregistered: Bzr CVS Git Hg Mtn SCCS SRC
-         ;; up-to-date: SVN
-          (message "vc-state1 %s" (vc-state default-directory))
-         (should (eq (vc-state default-directory)
-                     (vc-state default-directory backend)))
-         (should (memq (vc-state default-directory)
-                       '(nil unregistered up-to-date)))
-
          (let ((tmp-name (expand-file-name "foo" default-directory)))
            ;; Check state of a nonexistent file.
 
-           ;; unregistered: Bzr CVS Git Hg Mtn RCS SCCS SRC SVN
             (message "vc-state2 %s" (vc-state tmp-name))
-           (should (eq (vc-state tmp-name) (vc-state tmp-name backend)))
-           (should (eq (vc-state tmp-name) 'unregistered))
+           (should (null (vc-state tmp-name)))
 
            ;; Write a new file.  Check state.
            (write-region "foo" nil tmp-name nil 'nomessage)
 
-            ;; unregistered: Bzr CVS Git Hg Mtn RCS SCCS SRC SVN
             (message "vc-state3 %s" (vc-state tmp-name))
-           (should (eq (vc-state tmp-name) (vc-state tmp-name backend)))
-           (should (eq (vc-state tmp-name) 'unregistered))
+           (should (null (vc-state tmp-name)))
 
            ;; Register a file.  Check state.
            (vc-register
             (list backend (list (file-name-nondirectory tmp-name))))
 
-            ;; FIXME: nil seems to be wrong.
+            ;; FIXME: nil is definitely wrong.
            ;; nil: SRC
             ;; added: Bzr CVS Git Hg Mtn SVN
            ;; up-to-date: RCS SCCS
             (message "vc-state4 %s" (vc-state tmp-name))
-           (should (eq (vc-state tmp-name) (vc-state tmp-name backend)))
            (should (memq (vc-state tmp-name) '(nil added up-to-date)))
 
            ;; Unregister the file.  Check state.
@@ -363,11 +348,10 @@ This checks also `vc-backend' and 
`vc-responsible-backend'."
                      'vc-test--unregister-function backend tmp-name)
                     'vc-not-supported)
                 (message "vc-state5 unsupported")
-              ;; unregistered: Bzr Git Hg RCS
+              ;; nil: Bzr Git Hg RCS
               ;; unsupported: CVS Mtn SCCS SRC SVN
               (message "vc-state5 %s" (vc-state tmp-name))
-              (should (eq (vc-state tmp-name) (vc-state tmp-name backend)))
-              (should (memq (vc-state tmp-name) '(unregistered))))))
+              (should (null (vc-state tmp-name))))))
 
       ;; Save exit.
       (ignore-errors (run-hooks 'vc-test--cleanup-hook)))))
@@ -399,41 +383,36 @@ This checks also `vc-backend' and 
`vc-responsible-backend'."
          ;; "0": SVN
           (message
            "vc-working-revision1 %s" (vc-working-revision default-directory))
-         (should (eq (vc-working-revision default-directory)
-                     (vc-working-revision default-directory backend)))
-         (should (member (vc-working-revision default-directory) '(nil "0")))
+          (should (member (vc-working-revision default-directory) '(nil "0")))
 
          (let ((tmp-name (expand-file-name "foo" default-directory)))
            ;; Check initial working revision, should be nil until
             ;; it's registered.
 
-           ;; nil: Bzr CVS Git Hg Mtn RCS SCCS SRC SVN
             (message "vc-working-revision2 %s" (vc-working-revision tmp-name))
-           (should (eq (vc-working-revision tmp-name)
-                       (vc-working-revision tmp-name backend)))
-           (should-not (vc-working-revision tmp-name))
+            (should-not (vc-working-revision tmp-name))
 
            ;; Write a new file.  Check working revision.
            (write-region "foo" nil tmp-name nil 'nomessage)
 
-           ;; nil: Bzr CVS Git Hg Mtn RCS SCCS SRC SVN
             (message "vc-working-revision3 %s" (vc-working-revision tmp-name))
-           (should (eq (vc-working-revision tmp-name)
-                       (vc-working-revision tmp-name backend)))
-           (should-not (vc-working-revision tmp-name))
+            (should-not (vc-working-revision tmp-name))
 
            ;; Register a file.  Check working revision.
            (vc-register
             (list backend (list (file-name-nondirectory tmp-name))))
 
-            ;; FIXME: nil doesn't seem to be proper.
+            ;; XXX: nil is fine, at least in Git's case, because
+           ;; `vc-register' only makes the file `added' in this case.
            ;; nil: Git Mtn
            ;; "0": Bzr CVS Hg SRC SVN
            ;; "1.1": RCS SCCS
             (message "vc-working-revision4 %s" (vc-working-revision tmp-name))
-           (should (eq (vc-working-revision tmp-name)
-                       (vc-working-revision tmp-name backend)))
-           (should (member (vc-working-revision tmp-name) '(nil "0" "1.1")))
+            (should (member (vc-working-revision tmp-name) '(nil "0" "1.1")))
+
+            ;; TODO: Call `vc-checkin', and check the resulting
+            ;; working revision.  None of the return values should be
+            ;; nil then.
 
            ;; Unregister the file.  Check working revision.
             (if (eq (vc-test--run-maybe-unsupported-function
@@ -443,8 +422,6 @@ This checks also `vc-backend' and `vc-responsible-backend'."
               ;; nil: Bzr Git Hg RCS
               ;; unsupported: CVS Mtn SCCS SRC SVN
               (message "vc-working-revision5 %s" (vc-working-revision 
tmp-name))
-              (should (eq (vc-working-revision tmp-name)
-                          (vc-working-revision tmp-name backend)))
               (should-not (vc-working-revision tmp-name)))))
 
       ;; Save exit.