GNU bug report logs -
#33650
27.0.50; Root diffs between revisions
Previous Next
Reported by: Juri Linkov <juri <at> linkov.net>
Date: Thu, 6 Dec 2018 22:33:01 UTC
Severity: wishlist
Found in version 27.0.50
Done: Juri Linkov <juri <at> linkov.net>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 33650 in the body.
You can then email your comments to 33650 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#33650
; Package
emacs
.
(Thu, 06 Dec 2018 22:33:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Juri Linkov <juri <at> linkov.net>
:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org
.
(Thu, 06 Dec 2018 22:33:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
A VC command that still doesn't allow using diff-mode automatically
thus preventing proper fontification and all other related features
is `vc-root-diff' with a prefix argument HISTORIC. It has such comment:
;; FIXME: this does not work right, `vc-version-diff' ends up
;; calling `vc-deduce-fileset' to find the files to diff, and
;; that's not what we want here, we want the diff for the VC root dir.
This patch fixes this omission by implementing `vc-root-version-diff'
that is able to compare two revisions even on separate branches from
their roots:
[vc-root-version-diff.patch (text/x-diff, inline)]
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index de43544864..74979e7381 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1827,6 +1827,28 @@ vc-version-diff
(vc-diff-internal t (vc-deduce-fileset t) rev1 rev2
(called-interactively-p 'interactive)))
+;;;###autoload
+(defun vc-root-version-diff (_files rev1 rev2)
+ "Report diffs between revisions of the whole tree in the repository history."
+ (interactive (vc-diff-build-argument-list-internal))
+ ;; This is a mix of `vc-root-diff' and `vc-version-diff'
+ (when (and (not rev1) rev2)
+ (error "Not a valid revision range"))
+ (let ((backend (vc-deduce-backend))
+ (default-directory default-directory)
+ rootdir)
+ (if backend
+ (setq rootdir (vc-call-backend backend 'root default-directory))
+ (setq rootdir (read-directory-name "Directory for VC root-diff: "))
+ (setq backend (vc-responsible-backend rootdir))
+ (if backend
+ (setq default-directory rootdir)
+ (error "Directory is not version controlled")))
+ (let ((default-directory rootdir))
+ (vc-diff-internal
+ t (list backend (list rootdir)) rev1 rev2
+ (called-interactively-p 'interactive)))))
+
;;;###autoload
(defun vc-diff (&optional historic not-urgent)
"Display diffs between file revisions.
@@ -1900,10 +1922,8 @@ vc-root-diff
saving the buffer."
(interactive (list current-prefix-arg t))
(if historic
- ;; FIXME: this does not work right, `vc-version-diff' ends up
- ;; calling `vc-deduce-fileset' to find the files to diff, and
- ;; that's not what we want here, we want the diff for the VC root dir.
- (call-interactively 'vc-version-diff)
+ ;; We want the diff for the VC root dir.
+ (call-interactively 'vc-root-version-diff)
(when buffer-file-name (vc-buffer-sync not-urgent))
(let ((backend (vc-deduce-backend))
(default-directory default-directory)
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#33650
; Package
emacs
.
(Fri, 07 Dec 2018 06:42:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 33650 <at> debbugs.gnu.org (full text, mbox):
> From: Juri Linkov <juri <at> linkov.net>
> Date: Thu, 06 Dec 2018 23:54:16 +0200
>
> This patch fixes this omission by implementing `vc-root-version-diff'
> that is able to compare two revisions even on separate branches from
> their roots:
Thanks. A few comments:
> +(defun vc-root-version-diff (_files rev1 rev2)
> + "Report diffs between revisions of the whole tree in the repository history."
This doc string doesn't describe the arguments, and doesn't tell how
those arguments are populated in interactive invocations.
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.
Finally, we need documentation changes to go with this change of
user-visible behavior.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#33650
; Package
emacs
.
(Sun, 09 Dec 2018 00:14:03 GMT)
Full text and
rfc822 format available.
Message #11 received at 33650 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
>> +(defun vc-root-version-diff (_files rev1 rev2)
>> + "Report diffs between revisions of the whole tree in the repository history."
>
> This doc string doesn't describe the arguments, and doesn't tell how
> those arguments are populated in interactive invocations.
I copied the doc string from `vc-version-diff' that doesn't describe its
arguments. Now I fixed the doc string of `vc-version-diff' as well below.
> 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")
‘C-x v L’ (‘vc-print-root-log’) displays a ‘*vc-change-log*’ buffer
showing the history of the entire version-controlled directory tree
(RCS, SCCS, CVS, and SRC do not support this feature).
> 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.
[vc-root-version-diff.2.patch (text/x-diff, inline)]
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index dbbc3e2038..ba410dca0e 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1817,7 +1817,8 @@ vc-diff-build-argument-list-internal
;;;###autoload
(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."
(interactive (vc-diff-build-argument-list-internal))
;; All that was just so we could do argument completion!
(when (and (not rev1) rev2)
@@ -1827,6 +1828,29 @@ vc-version-diff
(vc-diff-internal t (vc-deduce-fileset t) rev1 rev2
(called-interactively-p 'interactive)))
+;;;###autoload
+(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."
+ (interactive (vc-diff-build-argument-list-internal))
+ ;; This is a mix of `vc-root-diff' and `vc-version-diff'
+ (when (and (not rev1) rev2)
+ (error "Not a valid revision range"))
+ (let ((backend (vc-deduce-backend))
+ (default-directory default-directory)
+ rootdir)
+ (if backend
+ (setq rootdir (vc-call-backend backend 'root default-directory))
+ (setq rootdir (read-directory-name "Directory for VC root-diff: "))
+ (setq backend (vc-responsible-backend rootdir))
+ (if backend
+ (setq default-directory rootdir)
+ (error "Directory is not version controlled")))
+ (let ((default-directory rootdir))
+ (vc-diff-internal
+ t (list backend (list rootdir)) rev1 rev2
+ (called-interactively-p 'interactive)))))
+
;;;###autoload
(defun vc-diff (&optional historic not-urgent)
"Display diffs between file revisions.
@@ -1900,10 +1924,8 @@ vc-root-diff
saving the buffer."
(interactive (list current-prefix-arg t))
(if historic
- ;; FIXME: this does not work right, `vc-version-diff' ends up
- ;; calling `vc-deduce-fileset' to find the files to diff, and
- ;; that's not what we want here, we want the diff for the VC root dir.
- (call-interactively 'vc-version-diff)
+ ;; We want the diff for the VC root dir.
+ (call-interactively 'vc-root-version-diff)
(when buffer-file-name (vc-buffer-sync not-urgent))
(let ((backend (vc-deduce-backend))
(default-directory default-directory)
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#33650
; Package
emacs
.
(Sun, 09 Dec 2018 08:23:02 GMT)
Full text and
rfc822 format available.
Message #14 received at 33650 <at> debbugs.gnu.org (full text, mbox):
> From: Juri Linkov <juri <at> linkov.net>
> Cc: 33650 <at> debbugs.gnu.org
> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#33650
; Package
emacs
.
(Mon, 10 Dec 2018 00:38:03 GMT)
Full text and
rfc822 format available.
Message #17 received at 33650 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
>> > 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?
Yes, this fix changes user-visible behavior, so perhaps it should be
mentioned in NEWS.
>> (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?
"fileset" is essential to be mentioned in `vc-version-diff'
whereas the text "in the repository history" is unnecessary.
>> +(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.
As "fileset" is essential for `vc-version-diff',
"whole tree" is essential for `vc-root-version-diff':
[vc-root-version-diff.3.patch (text/x-diff, inline)]
diff --git a/doc/emacs/maintaining.texi b/doc/emacs/maintaining.texi
index 4527c23d9e..e92db9e419 100644
--- a/doc/emacs/maintaining.texi
+++ b/doc/emacs/maintaining.texi
@@ -774,8 +774,7 @@ Old Revisions
@item C-x v D
Compare the entire working tree to the revision you started from
-(@code{vc-root-diff}). With a prefix argument, prompt for two
-revisions and compare their trees.
+(@code{vc-root-diff}).
@item C-x v ~
Prompt for a revision of the current file, and visit it in a separate
@@ -829,7 +828,9 @@ Old Revisions
it displays the changes in the entire current working tree (i.e., the
working tree containing the current VC fileset). If you invoke this
command from a Dired buffer, it applies to the working tree containing
-the directory.
+the directory. With a prefix argument, prompt for two revisions and
+compare the entire version-controlled directory trees (RCS, SCCS, CVS,
+and SRC do not support this feature).
@vindex vc-diff-switches
You can customize the @command{diff} options that @kbd{C-x v =} and
@@ -963,6 +964,7 @@ VC Change Log
Directory Mode}) or a Dired buffer (@pxref{Dired}), it applies to the
file listed on the current line.
+@kindex C-x v L
@findex vc-print-root-log
@findex log-view-toggle-entry-display
@kbd{C-x v L} (@code{vc-print-root-log}) displays a
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index dbbc3e2038..5ff9f4d5be 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1817,7 +1817,7 @@ vc-diff-build-argument-list-internal
;;;###autoload
(defun vc-version-diff (_files rev1 rev2)
- "Report diffs between revisions of the fileset in the repository history."
+ "Report diffs between REV1 and REV2 revisions of the fileset."
(interactive (vc-diff-build-argument-list-internal))
;; All that was just so we could do argument completion!
(when (and (not rev1) rev2)
@@ -1827,6 +1827,28 @@ vc-version-diff
(vc-diff-internal t (vc-deduce-fileset t) rev1 rev2
(called-interactively-p 'interactive)))
+;;;###autoload
+(defun vc-root-version-diff (_files rev1 rev2)
+ "Report diffs between REV1 and REV2 revisions of the whole tree."
+ (interactive (vc-diff-build-argument-list-internal))
+ ;; This is a mix of `vc-root-diff' and `vc-version-diff'
+ (when (and (not rev1) rev2)
+ (error "Not a valid revision range"))
+ (let ((backend (vc-deduce-backend))
+ (default-directory default-directory)
+ rootdir)
+ (if backend
+ (setq rootdir (vc-call-backend backend 'root default-directory))
+ (setq rootdir (read-directory-name "Directory for VC root-diff: "))
+ (setq backend (vc-responsible-backend rootdir))
+ (if backend
+ (setq default-directory rootdir)
+ (error "Directory is not version controlled")))
+ (let ((default-directory rootdir))
+ (vc-diff-internal
+ t (list backend (list rootdir)) rev1 rev2
+ (called-interactively-p 'interactive)))))
+
;;;###autoload
(defun vc-diff (&optional historic not-urgent)
"Display diffs between file revisions.
@@ -1900,10 +1922,8 @@ vc-root-diff
saving the buffer."
(interactive (list current-prefix-arg t))
(if historic
- ;; FIXME: this does not work right, `vc-version-diff' ends up
- ;; calling `vc-deduce-fileset' to find the files to diff, and
- ;; that's not what we want here, we want the diff for the VC root dir.
- (call-interactively 'vc-version-diff)
+ ;; We want the diff for the VC root dir.
+ (call-interactively 'vc-root-version-diff)
(when buffer-file-name (vc-buffer-sync not-urgent))
(let ((backend (vc-deduce-backend))
(default-directory default-directory)
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#33650
; Package
emacs
.
(Mon, 10 Dec 2018 01:00:02 GMT)
Full text and
rfc822 format available.
Message #20 received at 33650 <at> debbugs.gnu.org (full text, mbox):
On 10.12.2018 2:04, Juri Linkov wrote:
> @item C-x v D
> Compare the entire working tree to the revision you started from
> -(@code{vc-root-diff}). With a prefix argument, prompt for two
> -revisions and compare their trees.
> +(@code{vc-root-diff}).
Why remove this part? Isn't it fixed in this patch?
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#33650
; Package
emacs
.
(Mon, 10 Dec 2018 06:30:02 GMT)
Full text and
rfc822 format available.
Message #23 received at 33650 <at> debbugs.gnu.org (full text, mbox):
> From: Juri Linkov <juri <at> linkov.net>
> Cc: 33650 <at> debbugs.gnu.org
> Date: Mon, 10 Dec 2018 02:04:30 +0200
>
> >> It didn't work as documented before properly implemented by the patch.
> >
> > Isn't the result user-visible change in behavior?
>
> Yes, this fix changes user-visible behavior, so perhaps it should be
> mentioned in NEWS.
I think so too.
> > I wonder whether "fileset" needs to be mentioned at all, given your
> > changes. When does any "fileset" come into play here?
>
> "fileset" is essential to be mentioned in `vc-version-diff'
> whereas the text "in the repository history" is unnecessary.
>
> >> +(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.
>
> As "fileset" is essential for `vc-version-diff',
> "whole tree" is essential for `vc-root-version-diff':
OK, thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#33650
; Package
emacs
.
(Mon, 10 Dec 2018 06:32:02 GMT)
Full text and
rfc822 format available.
Message #26 received at 33650 <at> debbugs.gnu.org (full text, mbox):
> Cc: 33650 <at> debbugs.gnu.org
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Mon, 10 Dec 2018 02:59:08 +0200
>
> On 10.12.2018 2:04, Juri Linkov wrote:
> > @item C-x v D
> > Compare the entire working tree to the revision you started from
> > -(@code{vc-root-diff}). With a prefix argument, prompt for two
> > -revisions and compare their trees.
> > +(@code{vc-root-diff}).
>
> Why remove this part? Isn't it fixed in this patch?
The above is the short summary of the command. The longer description
does mention this feature. So I think this is in line with our style
in describing commands in the manual.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#33650
; Package
emacs
.
(Mon, 10 Dec 2018 23:58:02 GMT)
Full text and
rfc822 format available.
Message #29 received at 33650 <at> debbugs.gnu.org (full text, mbox):
>> @item C-x v D
>> Compare the entire working tree to the revision you started from
>> -(@code{vc-root-diff}). With a prefix argument, prompt for two
>> -revisions and compare their trees.
>> +(@code{vc-root-diff}).
>
> Why remove this part? Isn't it fixed in this patch?
For consistency with ‘C-x v L’ (‘vc-print-root-log’) whose
prefix argument is described in the longer description
in (info "(emacs) VC Change Log")
But since you asked I looked closer and see that e.g. a prefix argument
of ‘C-x v =’ is described in the short summary in the same node
(info "(emacs) Old Revisions")
‘C-x v =’
Compare the work files in the current VC fileset with the versions
you started from (‘vc-diff’). With a prefix argument, prompt for
two revisions of the current VC fileset and compare them.
So I installed something better.
Reply sent
to
Juri Linkov <juri <at> linkov.net>
:
You have taken responsibility.
(Mon, 10 Dec 2018 23:58:03 GMT)
Full text and
rfc822 format available.
Notification sent
to
Juri Linkov <juri <at> linkov.net>
:
bug acknowledged by developer.
(Mon, 10 Dec 2018 23:58:03 GMT)
Full text and
rfc822 format available.
Message #34 received at 33650-done <at> debbugs.gnu.org (full text, mbox):
>> > Isn't the result user-visible change in behavior?
>>
>> Yes, this fix changes user-visible behavior, so perhaps it should be
>> mentioned in NEWS.
>
> I think so too.
>
>> As "fileset" is essential for `vc-version-diff',
>> "whole tree" is essential for `vc-root-version-diff':
>
> OK, thanks.
Pushed to master and closed.
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org
.
(Tue, 08 Jan 2019 12:24:04 GMT)
Full text and
rfc822 format available.
This bug report was last modified 6 years and 214 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.