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