VC state

VC state

[Stefan Monnier]

I tried to use vc-status with Bzr and found that VC still needs some
work to adjust it to current VCS realities:

- `vc-update' should do a `bzr pull'.
- `vc-merge' only operates on a single file.
- `vc-merge' takes revisions as arguments rather than a branch (plus
  optional revisions).

The above changes will require changing some of the backend operations,
since those operations will need to somehow return a list of files
changed.

The vc-status UI also needs more work:
- there should be a way to operate on (sub)directories.
- it should be possible to add an entry in the display (like PCL-CVS's
  `I') so as to then operate on it.
- indicate files with conflicts (probably requires changes in backends
  as well to provide that info).
- the modeline says "*VC status* from *vc-status*".


        Stefan

Re: VC state

[Dan Nicolaescu]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

  > I tried to use vc-status with Bzr and found that VC still needs some
  > work to adjust it to current VCS realities:

Please add to vc.el:Todo the items that are not already there.

  > - `vc-update' should do a `bzr pull'.
  > - `vc-merge' only operates on a single file.
  > - `vc-merge' takes revisions as arguments rather than a branch (plus
  >   optional revisions).
  > 
  > The above changes will require changing some of the backend operations,
  > since those operations will need to somehow return a list of files
  > changed.
  > 
  > The vc-status UI also needs more work:

The main question for vc-status is: should it be bound to C-x v d now?
It's already better than vc-dired. And it would be good if more people
use it/hack on it to shake the remaining bugs sooner rather than later.

  > - there should be a way to operate on (sub)directories.

You mean something more than M-x vc-status DIR/SUBDIR RET ?

  > - it should be possible to add an entry in the display (like PCL-CVS's
  >   `I') so as to then operate on it.

Interesting, didn't know about this feature.  I'll implement this.

  > - indicate files with conflicts (probably requires changes in backends
  >   as well to provide that info).

This has been in Todo for a while, displaying this stuff is trivial, VC
needs just to provide the information.  Someone just needs to sit down
and figure out how it's supposed to work in VC...

  > - the modeline says "*VC status* from *vc-status*".

Fixed.

Re: VC state

[Stefan Monnier]

>> - there should be a way to operate on (sub)directories.

> You mean something more than M-x vc-status DIR/SUBDIR RET ?

Yes.  There's sometimes an important difference between passing "src/"
as the argument vs passing all the files in "src/" that appear in the
vc-status (beside the fact that the former does not require you to
first mark each one of those files).

>> - it should be possible to add an entry in the display (like PCL-CVS's
>> `I') so as to then operate on it.

> Interesting, didn't know about this feature.  I'll implement this.

I already mentioned it several times when we were talking about which
files should be shown: without `I', any choice other than "all" is
occasionally problematic.

>> - indicate files with conflicts (probably requires changes in backends
>> as well to provide that info).

> This has been in Todo for a while, displaying this stuff is trivial, VC
> needs just to provide the information.  Someone just needs to sit down
> and figure out how it's supposed to work in VC...

With some backends (E.g. Svn and Bzr), it would make a lot of sense to
make it a new vc-state, since you need to run `(svn|bzr) resolve' to
switch from that state to `edited' and you can't commit before.

In other bakends, it's less clear.  Maybe you should try to simply add
`conflict' as a new `vc-state' and see how it works out.  This will
require checking all uses of `vc-state' to adjust them to the new state.


        Stefan

conflict state (was Re: VC state)

[Dan Nicolaescu]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

  > > This has been in Todo for a while, displaying this stuff is trivial, VC
  > > needs just to provide the information.  Someone just needs to sit down
  > > and figure out how it's supposed to work in VC...
  > 
  > With some backends (E.g. Svn and Bzr), it would make a lot of sense to
  > make it a new vc-state, since you need to run `(svn|bzr) resolve' to
  > switch from that state to `edited' and you can't commit before.
  > 
  > In other bakends, it's less clear.  Maybe you should try to simply add
  > `conflict' as a new `vc-state' and see how it works out.  This will
  > require checking all uses of `vc-state' to adjust them to the new state.

A patch to implement the new conflict state is below.  Can you please
change the magic that runs the resolve command and turns on
smerge-mode to use this state instead?
Not sure what to do about the new `mark-resolved' backend function for
CVS, it probably should be just a no-op, it should be easy to see once
the smerge logic is in place.


Index: vc-hooks.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/vc-hooks.el,v
retrieving revision 1.234
diff -u -3 -p -r1.234 vc-hooks.el
--- vc-hooks.el	1 Apr 2008 02:58:15 -0000	1.234
+++ vc-hooks.el	8 Apr 2008 06:29:00 -0000
@@ -517,6 +517,8 @@ For registered files, the value returned
 
   'removed           Scheduled to be deleted from the repository on next commit.
 
+  'conflict          The file contains conflicts as the result of a merge.
+
   'missing           The file is not present in the file system, but the VC 
                      system still tracks it.
 
@@ -775,10 +777,10 @@ Before doing that, check if there are an
          (eq (vc-checkout-model file) 'implicit)
          (vc-file-setprop file 'vc-state 'edited)
 	 (vc-mode-line file)
-	 (if (featurep 'vc)
-	     ;; If VC is not loaded, then there can't be
-	     ;; any VC Dired buffer to synchronize.
-	     (vc-dired-resynch-file file)))))
+	 (when (featurep 'vc)
+	   ;; If VC is not loaded, then there can't be
+	   ;; any VC Dired buffer to synchronize.
+	   (vc-dired-resynch-file file)))))
 
 (defvar vc-menu-entry
   '(menu-item "Version Control" vc-menu-map
@@ -861,6 +863,9 @@ This function assumes that the file is r
            ((eq state 'added)
             (setq state-echo "Locally added file")
             (concat backend "@" rev))
+           ((eq state 'conflict)
+            (setq state-echo "File contains conflicts after the last merge")
+            (concat backend "!!" rev))
            ((eq state 'removed)
             (setq state-echo "File removed from the VC system")
             (concat backend "!" rev))
Index: vc.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/vc.el,v
retrieving revision 1.573
diff -u -3 -p -r1.573 vc.el
--- vc.el	6 Apr 2008 17:30:39 -0000	1.573
+++ vc.el	8 Apr 2008 06:29:04 -0000
@@ -362,6 +362,11 @@
 ;;   Modify the change comments associated with the files at the
 ;;   given revision.  This is optional, many backends do not support it.
 ;;
+;; - mark-resolved (files)
+;;
+;;   The the VCS that conflicts have been resolved.  Not all systems
+;;   need to do this.
+;;
 ;; HISTORY FUNCTIONS
 ;;
 ;; * print-log (files &optional buffer)
@@ -1478,7 +1483,7 @@ Otherwise, throw an error."
 (defsubst vc-editable-p (file)
   "Return non-nil if FILE can be edited."
   (or (eq (vc-checkout-model file) 'implicit)
-      (memq (vc-state file) '(edited needs-merge))))
+      (memq (vc-state file) '(edited needs-merge conflict))))
 
 (defun vc-revert-buffer-internal (&optional arg no-confirm)
   "Revert buffer, keeping point and mark where user expects them.
@@ -1667,6 +1672,9 @@ merge in the changes into your working c
                   (read-string (format "%s revision to steal: " file))
                 (vc-working-revision file))
          state)))
+     ;; conflict
+     ((eq state 'conflict)
+      (vc-mark-resolved files))
      ;; needs-patch
      ((eq state 'needs-patch)
       (dolist (file files)
@@ -1901,6 +1909,13 @@ After check-out, runs the normal hook `v
   (vc-resynch-buffer file t t)
   (run-hooks 'vc-checkout-hook))
 
+(defun vc-mark-resolved (files)
+  (with-vc-properties
+   files
+   (vc-call mark-resolved files)
+   ;; XXX: Is this TRTD?
+   `((vc-state . edited))))
+
 (defun vc-steal-lock (file rev owner)
   "Steal the lock on FILE."
   (let (file-description)
@@ -2722,7 +2737,7 @@ specific headers."
      (propertize
       (format "%-20s" state)
       'face (cond ((eq state 'up-to-date) 'font-lock-builtin-face)
-		  ((eq state 'missing) 'font-lock-warning-face)
+		  ((memq state '(missing conflict)) 'font-lock-warning-face)
 		  (t 'font-lock-variable-name-face))
       'mouse-face 'highlight)
      " "
@@ -3898,6 +3913,10 @@ to provide the `find-revision' operation
   (with-current-buffer (find-file-noselect new)
     (vc-register)))
 
+(defun vc-default-mark-resolved (backend files)
+  ;; XXX: For testing.
+  (error "Backend implements the conflict state, but it does not implement a `mark-resolved' function"))
+
 (defalias 'vc-default-logentry-check 'ignore)
 (defalias 'vc-default-check-headers 'ignore)
 
@@ -3909,11 +3928,11 @@ to provide the `find-revision' operation
 
 (defun vc-default-comment-history (backend file)
   "Return a string with all log entries stored in BACKEND for FILE."
-  (if (vc-find-backend-function backend 'print-log)
-      (with-current-buffer "*vc*"
-	(vc-call print-log (list file))
-	(vc-call-backend backend 'wash-log)
-	(buffer-string))))
+  (when (vc-find-backend-function backend 'print-log)
+    (with-current-buffer "*vc*"
+      (vc-call print-log (list file))
+      (vc-call-backend backend 'wash-log)
+      (buffer-string))))
 
 (defun vc-default-receive-file (backend file rev)
   "Let BACKEND receive FILE from another version control system."
Index: vc-svn.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/vc-svn.el,v
retrieving revision 1.75
diff -u -3 -p -r1.75 vc-svn.el
--- vc-svn.el	31 Mar 2008 15:36:54 -0000	1.75
+++ vc-svn.el	8 Apr 2008 14:50:29 -0000
@@ -160,7 +160,7 @@ If you want to force an empty list of ar
 
 (defun vc-svn-after-dir-status (callback buffer)
   (let ((state-map '((?A . added)
-                    (?C . edited)
+                    (?C . conflict)
                     (?D . removed)
                     (?I . ignored)
                     (?M . edited)
@@ -327,12 +327,16 @@ The changes are between FIRST-VERSION an
 		 "-r" (if second-version
 			(concat first-version ":" second-version)
 		      first-version))
-  (vc-file-setprop file 'vc-state 'edited)
   (with-current-buffer (get-buffer "*vc*")
     (goto-char (point-min))
     (if (looking-at "C  ")
-        1				; signal conflict
-      0)))				; signal success
+	(progn
+	  (vc-file-setprop file 'vc-state 'conflict)
+	  ;; signal conflict
+	  1)
+      ;; signal success
+      (vc-file-setprop file 'vc-state 'edited)
+      0)))
 
 (defun vc-svn-merge-news (file)
   "Merge in any new changes made to FILE."
@@ -376,7 +380,7 @@ The changes are between FIRST-VERSION an
                 0);; indicate success to the caller
                ;; Conflicts detected!
                (t
-                (vc-file-setprop file 'vc-state 'edited)
+                (vc-file-setprop file 'vc-state 'conflict)
                 1);; signal the error to the caller
                )
             (pop-to-buffer "*vc*")
@@ -621,7 +625,9 @@ information about FILENAME and return it
 	   (vc-file-setprop file 'vc-working-revision "0")
 	   (vc-file-setprop file 'vc-checkout-time 0)
 	   'added)
-	  ((memq status '(?M ?C))
+	  ((eq status ?C)
+	   (vc-file-setprop file 'vc-state 'conflict))
+	  ((eq status '?M)
 	   (if (eq (char-after (match-beginning 1)) ?*)
 	       'needs-merge
 	     'edited))
Index: vc-cvs.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/vc-cvs.el,v
retrieving revision 1.114
diff -u -3 -p -r1.114 vc-cvs.el
--- vc-cvs.el	31 Mar 2008 15:36:55 -0000	1.114
+++ vc-cvs.el	8 Apr 2008 14:50:30 -0000
@@ -432,12 +432,16 @@ The changes are between FIRST-REVISION a
                  "update" "-kk"
                  (concat "-j" first-revision)
                  (concat "-j" second-revision))
-  (vc-file-setprop file 'vc-state 'edited)
   (with-current-buffer (get-buffer "*vc*")
     (goto-char (point-min))
     (if (re-search-forward "conflicts during merge" nil t)
-        1				; signal error
-      0)))				; signal success
+	(progn 
+	  (vc-file-setprop file 'vc-state 'conflict)
+	  ;; signal error
+	  1)
+      (vc-file-setprop file 'vc-state 'edited)
+      ;; signal success
+      0)))
 
 (defun vc-cvs-merge-news (file)
   "Merge in any new changes made to FILE."
@@ -478,7 +482,7 @@ The changes are between FIRST-REVISION a
                 0);; indicate success to the caller
                ;; Conflicts detected!
                (t
-                (vc-file-setprop file 'vc-state 'edited)
+                (vc-file-setprop file 'vc-state 'conflict)
                 1);; signal the error to the caller
                )
             (pop-to-buffer "*vc*")
@@ -839,11 +843,11 @@ state."
 	(if (not (re-search-forward "\\=[ \t]+Status: \\(.*\\)" nil t))
 	    (setq status "Unknown")
 	  (setq status (match-string 1)))
-	(if (and full
-		 (re-search-forward
-		  "\\(RCS Version\\|RCS Revision\\|Repository revision\\):\
+	(when (and full
+		   (re-search-forward
+		    "\\(RCS Version\\|RCS Revision\\|Repository revision\\):\
 \[\t ]+\\([0-9.]+\\)"
-		  nil t))
+		    nil t))
 	    (vc-file-setprop file 'vc-latest-revision (match-string 2)))
 	(vc-file-setprop
 	 file 'vc-state
@@ -858,6 +862,7 @@ state."
 	   (if missing 'missing 'needs-patch))
 	  ((string-match "Locally Added" status)                'added)
 	  ((string-match "Locally Removed" status)              'removed)
+	  ((string-match "File had conflicts " status)          'conflict)
 	  (t 'edited))))))))
 
 (defun vc-cvs-dir-state-heuristic (dir)
@@ -922,6 +927,7 @@ state."
 		    (if missing 'missing 'needs-patch))
 		   ((string-match "Locally Added" status-str) 'added)
 		   ((string-match "Locally Removed" status-str) 'removed)
+		   ((string-match "File had conflicts " status-str) 'conflict)
 		   (t 'edited)))
 	    (unless (eq status 'up-to-date)
 	      (push (list file status) result))))))

Re: VC state

[Dan Nicolaescu]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

  > >> - there should be a way to operate on (sub)directories.
  > 
  > > You mean something more than M-x vc-status DIR/SUBDIR RET ?
  > 
  > Yes.  There's sometimes an important difference between passing "src/"
  > as the argument vs passing all the files in "src/" that appear in the
  > vc-status (beside the fact that the former does not require you to
  > first mark each one of those files).

Many VC commands choke when given a directory parameter, so this does
not seem particularly useful at this time.

More, given that vc-status is still "experimental code", and there is no
clear path/criteria on getting it out of that state, I am not inclined
to put more effort in adding more features to it.

Re: VC state

[Stefan Monnier]

>> >> - there should be a way to operate on (sub)directories.
>> 
>> > You mean something more than M-x vc-status DIR/SUBDIR RET ?
>> 
>> Yes.  There's sometimes an important difference between passing "src/"
>> as the argument vs passing all the files in "src/" that appear in the
>> vc-status (beside the fact that the former does not require you to
>> first mark each one of those files).

> Many VC commands choke when given a directory parameter, so this does
> not seem particularly useful at this time.

> More, given that vc-status is still "experimental code", and there is no
> clear path/criteria on getting it out of that state, I am not inclined
> to put more effort in adding more features to it.

I don't see it as particularly experimental: it's what I now use as
a replacement for PCL-CVS when I use Bzr.


        Stefan

Re: VC state

[Dan Nicolaescu]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

  > >> >> - there should be a way to operate on (sub)directories.
  > >> 
  > >> > You mean something more than M-x vc-status DIR/SUBDIR RET ?
  > >> 
  > >> Yes.  There's sometimes an important difference between passing "src/"
  > >> as the argument vs passing all the files in "src/" that appear in the
  > >> vc-status (beside the fact that the former does not require you to
  > >> first mark each one of those files).
  > 
  > > Many VC commands choke when given a directory parameter, so this does
  > > not seem particularly useful at this time.
  > 
  > > More, given that vc-status is still "experimental code", and there is no
  > > clear path/criteria on getting it out of that state, I am not inclined
  > > to put more effort in adding more features to it.
  > 
  > I don't see it as particularly experimental: it's what I now use as
  > a replacement for PCL-CVS when I use Bzr.

So can the experimental label be removed then?
How about binding vc-status to C-x v d so that it gets more exposure?
What is the plan for vc-dired?
When can it be removed?
IMO it does not make sense to keep code for two different
implementations of the same thing, it just adds complexity to the code.

Re: VC state

[Nick Roberts]

 >   > I don't see it as particularly experimental: it's what I now use as
 >   > a replacement for PCL-CVS when I use Bzr.
 > 
 > So can the experimental label be removed then?
 > How about binding vc-status to C-x v d so that it gets more exposure?
 > What is the plan for vc-dired?
 > When can it be removed?
 > IMO it does not make sense to keep code for two different
 > implementations of the same thing, it just adds complexity to the code.

Any "experimental label" is in the mind, so consider it already removed.
If vc-status is meant to be an improved replacement for vc-dired then
it seems reasonable to me that it takes the binding 'C-x v d'.  However,
I still use vc-dired so I think it's unreasonable to remove vc-dired
until vc-status does everything better.

I've only briefly looked at vc-status but here are a few things that I
noticed:

* vc-status doesn't seem to heed vc-stay-local or vc-cvs-stay-local.

* The filenames are highlighted by the mouse but clicking mouse-2 doesn't
  visit the file in another buffer

* The statuses are highlighted by the mouse but clicking mouse-2 doesn't
  do anything special

* The bindings vc-status-previous/next-line are on the menubar as menu-items
  but no-one would ever use them from there.

* I'm not sure if the keybindings are based on Dired or Pcl-cvs but they
  seem to be an unmemorable mix of upper and lowercase letters and other
  characters.

* If Emacs can't connect with the server there is no message in the buffer
  or elsewhere to explain what has happened.

* There is no description in the manual for vc-status.  Indeed the node
  "VC Status" is about Log-View mode.

* The mode name is "*VC Status*".  I think the convention of enclosing
  names in asterisks is for buffer names only.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: VC state

[Stefan Monnier]

>> >> >> - there should be a way to operate on (sub)directories.
>> >> 
>> >> > You mean something more than M-x vc-status DIR/SUBDIR RET ?
>> >> 
>> >> Yes.  There's sometimes an important difference between passing "src/"
>> >> as the argument vs passing all the files in "src/" that appear in the
>> >> vc-status (beside the fact that the former does not require you to
>> >> first mark each one of those files).
>> 
>> > Many VC commands choke when given a directory parameter, so this does
>> > not seem particularly useful at this time.
>> 
>> > More, given that vc-status is still "experimental code", and there is no
>> > clear path/criteria on getting it out of that state, I am not inclined
>> > to put more effort in adding more features to it.
>> 
>> I don't see it as particularly experimental: it's what I now use as
>> a replacement for PCL-CVS when I use Bzr.

> So can the experimental label be removed then?

Yes.

> How about binding vc-status to C-x v d so that it gets more exposure?
> What is the plan for vc-dired?

I'd suggest we leave vc-dired alone for now.  You can remove vc-dired
from the menu bar, tho, and replace it by an entry for vc-status.
You may want to put it more preeminently at the beginning of
the submenu.

As for a key binding, I don't find C-x v d particularly good anyway, so
we may as well choose a better one.

You may also want to add a vc-dired-noselect (which calls vc-status) to
find-directory-functions, like PCL-CVS does (it's what I always use to
invoke PCL-CVS).

> When can it be removed?

Just pretend it's not there.


        Stefan

Re: conflict state

[Stefan Monnier]

>> > This has been in Todo for a while, displaying this stuff is trivial, VC
>> > needs just to provide the information.  Someone just needs to sit down
>> > and figure out how it's supposed to work in VC...
>> 
>> With some backends (E.g. Svn and Bzr), it would make a lot of sense to
>> make it a new vc-state, since you need to run `(svn|bzr) resolve' to
>> switch from that state to `edited' and you can't commit before.
>> 
>> In other bakends, it's less clear.  Maybe you should try to simply add
>> `conflict' as a new `vc-state' and see how it works out.  This will
>> require checking all uses of `vc-state' to adjust them to the new state.

> A patch to implement the new conflict state is below.  Can you please
> change the magic that runs the resolve command and turns on
> smerge-mode to use this state instead?

I do not have time to do that right now.

> Not sure what to do about the new `mark-resolved' backend function for
> CVS, it probably should be just a no-op, it should be easy to see once
> the smerge logic is in place.

Yes, it should be a no-op.

> @@ -775,10 +777,10 @@ Before doing that, check if there are an
>           (eq (vc-checkout-model file) 'implicit)
>           (vc-file-setprop file 'vc-state 'edited)
>  	 (vc-mode-line file)
> -	 (if (featurep 'vc)
> -	     ;; If VC is not loaded, then there can't be
> -	     ;; any VC Dired buffer to synchronize.
> -	     (vc-dired-resynch-file file)))))
> +	 (when (featurep 'vc)
> +	   ;; If VC is not loaded, then there can't be
> +	   ;; any VC Dired buffer to synchronize.
> +	   (vc-dired-resynch-file file)))))
 
>  (defvar vc-menu-entry
>    '(menu-item "Version Control" vc-menu-map

Please use "diff -b" when sending patches that are intended for review
rather than for installation.

> @@ -861,6 +863,9 @@ This function assumes that the file is r
>             ((eq state 'added)
>              (setq state-echo "Locally added file")
>              (concat backend "@" rev))
> +           ((eq state 'conflict)
> +            (setq state-echo "File contains conflicts after the last merge")
> +            (concat backend "!!" rev))
>             ((eq state 'removed)
>              (setq state-echo "File removed from the VC system")
>              (concat backend "!" rev))

I'm not really happy with ! for removed and !! for conflict.
I'd rather just use the same for edited and conflict if we can't come up
with a good one.

This reminds me: we have to be careful that `conflict' is for conflicts
in the file's contents.  Not for conflicts about meta-info (e.g. when
a file is moved to the same location as some other file, or when a file
is renamed in two conflicting ways, ...).

> +;; - mark-resolved (files)
> +;;
> +;;   The the VCS that conflicts have been resolved.  Not all systems
> +;;   need to do this.

I don't know enough about The The to know if they're an appropriate
reference here, but I guess not.  We can probably provide a

  (defalias 'vc-default-mark-resolved 'ignore)

> +(defun vc-mark-resolved (files)
> +  (with-vc-properties
> +   files
> +   (vc-call mark-resolved files)
> +   ;; XXX: Is this TRTD?
> +   `((vc-state . edited))))

I'd rather we just call vc-state-heuristic to get the new state.

> @@ -3898,6 +3913,10 @@ to provide the `find-revision' operation
>    (with-current-buffer (find-file-noselect new)
>      (vc-register)))
 
> +(defun vc-default-mark-resolved (backend files)
> +  ;; XXX: For testing.
> +  (error "Backend implements the conflict state, but it does not implement a `mark-resolved' function"))

If we don't want to just do nothing, then let's just not provide any
default: the vc-call mechanism will then signal an error for us.

We should probably arrange to call mark-resolved from vc-after-save
(after checking that all the conflicts were indeed resolved).


        Stefan

Re: conflict state

[Dan Nicolaescu]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

  > >> > This has been in Todo for a while, displaying this stuff is trivial, VC
  > >> > needs just to provide the information.  Someone just needs to sit down
  > >> > and figure out how it's supposed to work in VC...
  > >> 
  > >> With some backends (E.g. Svn and Bzr), it would make a lot of sense to
  > >> make it a new vc-state, since you need to run `(svn|bzr) resolve' to
  > >> switch from that state to `edited' and you can't commit before.
  > >> 
  > >> In other bakends, it's less clear.  Maybe you should try to simply add
  > >> `conflict' as a new `vc-state' and see how it works out.  This will
  > >> require checking all uses of `vc-state' to adjust them to the new state.
  > 
  > > A patch to implement the new conflict state is below.  Can you please
  > > change the magic that runs the resolve command and turns on
  > > smerge-mode to use this state instead?
  > 
  > I do not have time to do that right now.

In that case should this code be installed?  I don't have time to do
this either.  Maybe someone else can pick it up...

  > > Not sure what to do about the new `mark-resolved' backend function for
  > > CVS, it probably should be just a no-op, it should be easy to see once
  > > the smerge logic is in place.
  > 
  > Yes, it should be a no-op.
  > 
  > > @@ -775,10 +777,10 @@ Before doing that, check if there are an
  > >           (eq (vc-checkout-model file) 'implicit)
  > >           (vc-file-setprop file 'vc-state 'edited)
  > >  	 (vc-mode-line file)
  > > -	 (if (featurep 'vc)
  > > -	     ;; If VC is not loaded, then there can't be
  > > -	     ;; any VC Dired buffer to synchronize.
  > > -	     (vc-dired-resynch-file file)))))
  > > +	 (when (featurep 'vc)
  > > +	   ;; If VC is not loaded, then there can't be
  > > +	   ;; any VC Dired buffer to synchronize.
  > > +	   (vc-dired-resynch-file file)))))
  >  
  > >  (defvar vc-menu-entry
  > >    '(menu-item "Version Control" vc-menu-map
  > 
  > Please use "diff -b" when sending patches that are intended for review
  > rather than for installation.
  > 
  > > @@ -861,6 +863,9 @@ This function assumes that the file is r
  > >             ((eq state 'added)
  > >              (setq state-echo "Locally added file")
  > >              (concat backend "@" rev))
  > > +           ((eq state 'conflict)
  > > +            (setq state-echo "File contains conflicts after the last merge")
  > > +            (concat backend "!!" rev))
  > >             ((eq state 'removed)
  > >              (setq state-echo "File removed from the VC system")
  > >              (concat backend "!" rev))
  > 
  > I'm not really happy with ! for removed and !! for conflict.

No preference here, feel free to change all off them.

  > I'd rather just use the same for edited and conflict if we can't come up
  > with a good one.

I'd prefer to have different things for conflict and edited so that they
can be easily distinguished.

  > This reminds me: we have to be careful that `conflict' is for conflicts
  > in the file's contents.  Not for conflicts about meta-info (e.g. when
  > a file is moved to the same location as some other file, or when a file
  > is renamed in two conflicting ways, ...).

Are you sure that is a good idea?  For systems that require a "resolve"
command to be run after resolving a conflict, there needs to be some
convenient way run that command.  One can resolve the metadata conflict
from within emacs, but then won't be able to have emacs run the resolve
command.

  > > +;; - mark-resolved (files)
  > > +;;
  > > +;;   The the VCS that conflicts have been resolved.  Not all systems
  > > +;;   need to do this.
  > 
  > I don't know enough about The The to know if they're an appropriate
  > reference here, but I guess not.  We can probably provide a
  > 
  >   (defalias 'vc-default-mark-resolved 'ignore)
  > 
  > > +(defun vc-mark-resolved (files)
  > > +  (with-vc-properties
  > > +   files
  > > +   (vc-call mark-resolved files)
  > > +   ;; XXX: Is this TRTD?
  > > +   `((vc-state . edited))))
  > 
  > I'd rather we just call vc-state-heuristic to get the new state.

OK.

  > > @@ -3898,6 +3913,10 @@ to provide the `find-revision' operation
  > >    (with-current-buffer (find-file-noselect new)
  > >      (vc-register)))
  >  
  > > +(defun vc-default-mark-resolved (backend files)
  > > +  ;; XXX: For testing.
  > > +  (error "Backend implements the conflict state, but it does not implement a `mark-resolved' function"))
  > 
  > If we don't want to just do nothing, then let's just not provide any
  > default: the vc-call mechanism will then signal an error for us.

OK.

  > We should probably arrange to call mark-resolved from vc-after-save
  > (after checking that all the conflicts were indeed resolved).

Let's see how this turns out in real life.  It seems that there might be
many corner cases to consider...

Re: VC state

[Dan Nicolaescu]

Nick Roberts <nickrob@snap.net.nz> writes:

  >  >   > I don't see it as particularly experimental: it's what I now use as
  >  >   > a replacement for PCL-CVS when I use Bzr.
  >  > 
  >  > So can the experimental label be removed then?
  >  > How about binding vc-status to C-x v d so that it gets more exposure?
  >  > What is the plan for vc-dired?
  >  > When can it be removed?
  >  > IMO it does not make sense to keep code for two different
  >  > implementations of the same thing, it just adds complexity to the code.
  > 
  > Any "experimental label" is in the mind, so consider it already removed.
  > If vc-status is meant to be an improved replacement for vc-dired then
  > it seems reasonable to me that it takes the binding 'C-x v d'.  However,
  > I still use vc-dired so I think it's unreasonable to remove vc-dired
  > until vc-status does everything better.
  > 
  > I've only briefly looked at vc-status but here are a few things that I
  > noticed:

Sorry you went through the trouble to write this, most of this stuff is
well known, some is already present in the Todo section.  But none of
this is a bug per se, just missing features that were not implemented
yet because there were more important things to deal with.

If more people join the development, they can be implemented faster,
nothing here is complicated, and (except for the stay-local support)
does not require knowledge of VC.

So this is an open invitation for anyone to join the effort.

  > * vc-status doesn't seem to heed vc-stay-local or vc-cvs-stay-local.

That is fine, vc-status does not care about those, it just displays
whatever the backend tells it to display.  The backends are the ones
that are supposed to take care of this.

  > * The filenames are highlighted by the mouse but clicking mouse-2 doesn't
  >   visit the file in another buffer
  > 
  > * The statuses are highlighted by the mouse but clicking mouse-2 doesn't
  >   do anything special
  > 
  > * The bindings vc-status-previous/next-line are on the menubar as menu-items
  >   but no-one would ever use them from there.

They were added when the menu was almost empty, they might get removed
later when more useful stuff is added, but they don't hurt anything for now.

  > * I'm not sure if the keybindings are based on Dired or Pcl-cvs but they
  >   seem to be an unmemorable mix of upper and lowercase letters and other
  >   characters.

Most key bindings are explained in the comments.  But I don't really
want to discuss them on the mailing list, this type of discussion tends
to be never ending.

  > * If Emacs can't connect with the server there is no message in the buffer
  >   or elsewhere to explain what has happened.
  > 
  > * There is no description in the manual for vc-status.  Indeed the node
  >   "VC Status" is about Log-View mode.
  > 
  > * The mode name is "*VC Status*".  I think the convention of enclosing
  >   names in asterisks is for buffer names only.

I keep forgetting to change that one, I'll fix it when I get a chance.

Re: conflict state

[Stefan Monnier]

>> > A patch to implement the new conflict state is below.  Can you please
>> > change the magic that runs the resolve command and turns on
>> > smerge-mode to use this state instead?
>> 
>> I do not have time to do that right now.

> In that case should this code be installed?

Yes.

>> I'd rather just use the same for edited and conflict if we can't come up
>> with a good one.
> I'd prefer to have different things for conflict and edited so that they
> can be easily distinguished.

Agreed, but I think that "no difference" is better than "bad
choice" here.  Maybe ! should be used for conflicts and remove should
use something else.

>> This reminds me: we have to be careful that `conflict' is for conflicts
>> in the file's contents.  Not for conflicts about meta-info (e.g. when
>> a file is moved to the same location as some other file, or when a file
>> is renamed in two conflicting ways, ...).

> Are you sure that is a good idea?  For systems that require a "resolve"
> command to be run after resolving a conflict, there needs to be some
> convenient way run that command.  One can resolve the metadata conflict
> from within emacs, but then won't be able to have emacs run the resolve
> command.

I think you read a bit more into my words, than I intended to put.
I just want to make sure we keep the distinction between those two
in mind.  Sometimes they can be treated identically, other times not.
The code is currently used and written for "conflicts in the file's
content", so we should clearly say that, so that if we introduce code
that uses it for metadata-conflicts we hopefully won't forget to add
enough comments justifying why it's OK to use it here.

>> We should probably arrange to call mark-resolved from vc-after-save
>> (after checking that all the conflicts were indeed resolved).

> Let's see how this turns out in real life.  It seems that there might be
> many corner cases to consider...

Sure,


        Stefan

VC development [was Re: VC state]

[Nick Roberts]

 > If more people join the development, they can be implemented faster,
 > nothing here is complicated, and (except for the stay-local support)
 > does not require knowledge of VC.
 > 
 > So this is an open invitation for anyone to join the effort.

Given the long discussions on this list about the pros and cons of different
version control systems, it is indeed disappointing that there is not more
interest in developing VC in Emacs.  Individuals could presumably develop Emacs
just for the backend that they use.  I would think this would be a good way to
get into Emacs development.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: VC state

[Nick Roberts]

 >   > * vc-status doesn't seem to heed vc-stay-local or vc-cvs-stay-local.
 > 
 > That is fine, vc-status does not care about those, it just displays
 > whatever the backend tells it to display.  The backends are the ones
 > that are supposed to take care of this.

I don't understand.  After vc-dired has gone, how do I find which files under
version control are modified without consulting the (remote) repository?
Do you just mean that this functionality is yet to be implemented but when
it is it won't be done in vc-status?

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: VC state

[Dan Nicolaescu]

Nick Roberts <nickrob@snap.net.nz> writes:

  >  >   > * vc-status doesn't seem to heed vc-stay-local or vc-cvs-stay-local.
  >  > 
  >  > That is fine, vc-status does not care about those, it just displays
  >  > whatever the backend tells it to display.  The backends are the ones
  >  > that are supposed to take care of this.
  > 
  > I don't understand.  After vc-dired has gone, how do I find which files under
  > version control are modified without consulting the (remote) repository?
  > Do you just mean that this functionality is yet to be implemented but when
  > it is it won't be done in vc-status?

Exactly.  This is similar to what vc-dired does.  The backends are
responsible for the information that vc-status displays.
If you actually care about such functionality you might want to
volunteer to implement, it's pretty low on the priority list...

Re: VC state

[Dan Nicolaescu]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

  > As for a key binding, I don't find C-x v d particularly good anyway, so
  > we may as well choose a better one.

Couldn't think of something better, so I left it on C-x v d 
Suggestions are welcome...  We could change the `vc-status' names to
match the key binding... 

  > You may also want to add a vc-dired-noselect (which calls vc-status) to
  > find-directory-functions, like PCL-CVS does (it's what I always use to
  > invoke PCL-CVS).

I'll look into that.

Re: VC state

[Stefan Monnier]

>> > * vc-status doesn't seem to heed vc-stay-local or vc-cvs-stay-local.
>> 
>> That is fine, vc-status does not care about those, it just displays
>> whatever the backend tells it to display.  The backends are the ones
>> that are supposed to take care of this.

> I don't understand.  After vc-dired has gone, how do I find which files under
> version control are modified without consulting the (remote) repository?
> Do you just mean that this functionality is yet to be implemented but when
> it is it won't be done in vc-status?

The way I see it is: `vc-status' should work in "stay-local" mode.
Always.  Then we add a `vc-pull' which may contact the
remote repository.


        Stefan

Re: VC state

[Nick Roberts]

 >   > I don't understand.  After vc-dired has gone, how do I find which files
 >   > under version control are modified without consulting the (remote)
 >   > repository?  Do you just mean that this functionality is yet to be
 >   > implemented but when it is it won't be done in vc-status?
 > 
 > Exactly.  This is similar to what vc-dired does.  The backends are
 > responsible for the information that vc-status displays.
 > If you actually care about such functionality you might want to
 > volunteer to implement, it's pretty low on the priority list...

I'll probably only ever offer simple bugfixes for VC.  If you are adding
features then it's your prerogative to prioritise them.  It's a different
matter if features are being removed and, in that respect, I think it's
important that vc-dired isn't disabled/dismantled.

-- 
Nick                                           http://www.inet.net.nz/~nickrob