Subject: bug#33650: 27.0.50; Root diffs between revisions

From: Eli Zaretskii <[email-masked]>
Date: Sun, 09 Dec 2018 10:22:01 +0200

> From: Juri Linkov <[email-masked]>
> Cc: [email-masked]
> Date: Sun, 09 Dec 2018 01:32:47 +0200
> 
> > I also wonder what would this mean for a VCS such as CVS or RCS, where
> > there's no meaning of a "tree revision".  I'm guessing this is why the
> > old code deduced files to diff.  See also the description of vc-diff
> > in the Emacs manual.
> 
> `vc-root-diff' is not applicable to VCS without tree revisions, and
> the manual says nothing about this.  Maybe the manual should say this
> explicitly for `vc-root-diff' as it says it for `vc-print-root-log'
> in (info "(emacs) VC Change Log")

Yes, we should say that.

> > Finally, we need documentation changes to go with this change of
> > user-visible behavior.
> 
> This feature is already documented in (info "(emacs) Old Revisions")
> 
>   ‘C-x v D’
>        Compare the entire working tree to the revision you started from
>        (‘vc-root-diff’).  With a prefix argument, prompt for two revisions
>        and compare their trees.
> 
> It didn't work as documented before properly implemented by the patch.

Isn't the result user-visible change in behavior?

>  (defun vc-version-diff (_files rev1 rev2)
> -  "Report diffs between revisions of the fileset in the repository history."
> +  "Report diffs between revisions of the fileset in the repository history.
> +REV1 is an older revision, REV2 is a newer revision."

We usually mention the arguments in the first line of the doc string.
Something like this:

  Report diffs between revisions REV1 and REV2 in the repository history.

I wonder whether "fileset" needs to be mentioned at all, given your
changes.  When does any "fileset" come into play here?

> +(defun vc-root-version-diff (_files rev1 rev2)
> +  "Report diffs between revisions of the whole tree in the repository history.
> +REV1 is an older revision, REV2 is a newer revision."

Same here: please mention the arguments in the first sentence.

> +    (if backend
> +	(setq rootdir (vc-call-backend backend 'root default-directory))

There's a TAB at the beginning of this line which should be spaces.

Thanks.