* bug#33650: 27.0.50; Root diffs between revisions
@ 2018-12-06 21:54 Juri Linkov
2018-12-07 6:41 ` Eli Zaretskii
0 siblings, 1 reply; 10+ messages in thread
From: Juri Linkov @ 2018-12-06 21:54 UTC (permalink / raw)
To: 33650
[-- Attachment #1: Type: text/plain, Size: 577 bytes --]
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:
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vc-root-version-diff.patch --]
[-- Type: text/x-diff, Size: 1911 bytes --]
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)
^ permalink raw reply related [flat|nested] 10+ messages in thread
* bug#33650: 27.0.50; Root diffs between revisions
2018-12-06 21:54 bug#33650: 27.0.50; Root diffs between revisions Juri Linkov
@ 2018-12-07 6:41 ` Eli Zaretskii
2018-12-08 23:32 ` Juri Linkov
0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2018-12-07 6:41 UTC (permalink / raw)
To: Juri Linkov; +Cc: 33650
> From: Juri Linkov <juri@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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#33650: 27.0.50; Root diffs between revisions
2018-12-07 6:41 ` Eli Zaretskii
@ 2018-12-08 23:32 ` Juri Linkov
2018-12-09 8:22 ` Eli Zaretskii
0 siblings, 1 reply; 10+ messages in thread
From: Juri Linkov @ 2018-12-08 23:32 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 33650
[-- Attachment #1: Type: text/plain, Size: 1583 bytes --]
>> +(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.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vc-root-version-diff.2.patch --]
[-- Type: text/x-diff, Size: 2442 bytes --]
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)
^ permalink raw reply related [flat|nested] 10+ messages in thread
* bug#33650: 27.0.50; Root diffs between revisions
2018-12-08 23:32 ` Juri Linkov
@ 2018-12-09 8:22 ` Eli Zaretskii
2018-12-10 0:04 ` Juri Linkov
0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2018-12-09 8:22 UTC (permalink / raw)
To: Juri Linkov; +Cc: 33650
> From: Juri Linkov <juri@linkov.net>
> Cc: 33650@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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#33650: 27.0.50; Root diffs between revisions
2018-12-09 8:22 ` Eli Zaretskii
@ 2018-12-10 0:04 ` Juri Linkov
2018-12-10 0:59 ` Dmitry Gutov
2018-12-10 6:29 ` Eli Zaretskii
0 siblings, 2 replies; 10+ messages in thread
From: Juri Linkov @ 2018-12-10 0:04 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 33650
[-- Attachment #1: Type: text/plain, Size: 1706 bytes --]
>> > 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':
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vc-root-version-diff.3.patch --]
[-- Type: text/x-diff, Size: 3653 bytes --]
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)
^ permalink raw reply related [flat|nested] 10+ messages in thread
* bug#33650: 27.0.50; Root diffs between revisions
2018-12-10 0:04 ` Juri Linkov
@ 2018-12-10 0:59 ` Dmitry Gutov
2018-12-10 6:30 ` Eli Zaretskii
2018-12-10 23:55 ` Juri Linkov
2018-12-10 6:29 ` Eli Zaretskii
1 sibling, 2 replies; 10+ messages in thread
From: Dmitry Gutov @ 2018-12-10 0:59 UTC (permalink / raw)
To: Juri Linkov, Eli Zaretskii; +Cc: 33650
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?
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#33650: 27.0.50; Root diffs between revisions
2018-12-10 0:04 ` Juri Linkov
2018-12-10 0:59 ` Dmitry Gutov
@ 2018-12-10 6:29 ` Eli Zaretskii
2018-12-10 23:55 ` Juri Linkov
1 sibling, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2018-12-10 6:29 UTC (permalink / raw)
To: Juri Linkov; +Cc: 33650
> From: Juri Linkov <juri@linkov.net>
> Cc: 33650@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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#33650: 27.0.50; Root diffs between revisions
2018-12-10 0:59 ` Dmitry Gutov
@ 2018-12-10 6:30 ` Eli Zaretskii
2018-12-10 23:55 ` Juri Linkov
1 sibling, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2018-12-10 6:30 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 33650, juri
> Cc: 33650@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#33650: 27.0.50; Root diffs between revisions
2018-12-10 0:59 ` Dmitry Gutov
2018-12-10 6:30 ` Eli Zaretskii
@ 2018-12-10 23:55 ` Juri Linkov
1 sibling, 0 replies; 10+ messages in thread
From: Juri Linkov @ 2018-12-10 23:55 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 33650
>> @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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#33650: 27.0.50; Root diffs between revisions
2018-12-10 6:29 ` Eli Zaretskii
@ 2018-12-10 23:55 ` Juri Linkov
0 siblings, 0 replies; 10+ messages in thread
From: Juri Linkov @ 2018-12-10 23:55 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 33650-done
>> > 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.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-12-10 23:55 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-12-06 21:54 bug#33650: 27.0.50; Root diffs between revisions Juri Linkov
2018-12-07 6:41 ` Eli Zaretskii
2018-12-08 23:32 ` Juri Linkov
2018-12-09 8:22 ` Eli Zaretskii
2018-12-10 0:04 ` Juri Linkov
2018-12-10 0:59 ` Dmitry Gutov
2018-12-10 6:30 ` Eli Zaretskii
2018-12-10 23:55 ` Juri Linkov
2018-12-10 6:29 ` Eli Zaretskii
2018-12-10 23:55 ` Juri Linkov
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).