bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Lars Ingebrigtsen]

Often when discussing code changes, people will send an email saying
something like "this was fixed by <id>", but Emacs doesn't have a
convenient way to display that.

For that case, Emacs should have a command that prompts for an ID
(defaulting to the ID under point), and then (unless default-directory
is already in a vc-controlled directory), prompts for the directory to
look for that ID, and then display the commit.

Stefan M also commented:

> It's worse: if the commit is not in the current branch it won't be
> listed at all, and `C-x v L` doesn't let you show the log corresponding
> to another branch.
> 
> If `C-x v l` accepted a C-u to specify the "revision" for which to show
> the log, then it would solve both problems: you could get the log
> of other branches and you could see the commit message and the diff of
> a specific REVision with:
> 
> C-u C-x v l <REV> RET
> followed by hitting `D` on the first element to see the diff

This would be a separate change, because it can also be useful to see a
commit in context.


In GNU Emacs 27.0.50 (build 6, x86_64-pc-linux-gnu, GTK+ Version 3.24.5)
 of 2019-11-01 built on marnie
Repository revision: eda98211e31ed969823c1048b3cde635e08eebe5
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12004000
System Description: Debian GNU/Linux 10 (buster)


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Stephen Berman]

On Sun, 03 Nov 2019 16:17:05 +0100 Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Often when discussing code changes, people will send an email saying
> something like "this was fixed by <id>", but Emacs doesn't have a
> convenient way to display that.
>
> For that case, Emacs should have a command that prompts for an ID
> (defaulting to the ID under point), and then (unless default-directory
> is already in a vc-controlled directory), prompts for the directory to
> look for that ID, and then display the commit.

I wrote such a command (appended below) for my own use, but it's
git-specific and I don't know the innards of VC well enough to adapt it,
which is why I haven't proposed it for inclusion in Emacs.  Maybe some
of it could be used for a VC command.

Steve Berman


(defun srb-git-log (&optional repo commit)
  "Check REPO for COMMIT and if it exists, display its commit message.
Interactively, prompt for REPO, defaulting to emacs-master, and
for COMMIT, defaulting to the commit hash at point.  If called
with a prefix argument `C-u', show the commit diff in addition to
the commit message."
  (interactive "P")
  (let* ((show (equal current-prefix-arg '(4)))
	 (git-dir (if repo
		      (read-directory-name "Repo: " "~/src/emacs/"
					   nil t "emacs-master")
		    "~/src/emacs/emacs-master"))
	 (commit0 (substring-no-properties
		   (or commit
		       (read-string "Commit: " nil nil (word-at-point)))))
	 (default-directory git-dir)
	 (output-buffer (get-buffer-create "*git log*"))
	 (args (split-string (mapconcat #'concat
					(if show
					    `("show" ,commit0)
					  `("log" "-1" ,commit0)) " ")))
	 ;; FIXME: output of `git branch --contains' can be ambiguous (even
	 ;; when `git log isn't, because one hash is for a commit, one for a
	 ;; tree?).  Can use `git rev-parse --disambiguate=' to find matching
	 ;; full hashes.
	 (proc (progn
		 (with-current-buffer output-buffer (erase-buffer))
		 (call-process "git" nil output-buffer nil
			       "branch" "--contains" commit0))))
    (when proc
      (with-current-buffer output-buffer
	(goto-char (point-min))
	(unless (looking-at "[ *]")
	  (user-error "%s is not on branch %s" commit0
		      (file-name-base git-dir)))
	(insert "Branches:\n")
	(goto-char (point-max))
	(apply #'call-process "git" nil output-buffer nil args)
	(when show
	  (with-current-buffer output-buffer
	    (diff-mode)))
	(pop-to-buffer output-buffer)))))

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Juri Linkov]

>> For that case, Emacs should have a command that prompts for an ID
>> (defaulting to the ID under point), and then (unless default-directory
>> is already in a vc-controlled directory), prompts for the directory to
>> look for that ID, and then display the commit.
>
> I wrote such a command (appended below) for my own use, but it's
> git-specific and I don't know the innards of VC well enough to adapt it,
> which is why I haven't proposed it for inclusion in Emacs.  Maybe some
> of it could be used for a VC command.

It should be easy to add a command properly integrated into the innards of VC
by just grepping for "log-search" in lisp/vc/vc*.el (there are only 7 matches),
and after copying to replace "log-search" with "log-show", and the new command
is ready.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Juri Linkov]

>>> For that case, Emacs should have a command that prompts for an ID
>>> (defaulting to the ID under point), and then (unless default-directory
>>> is already in a vc-controlled directory), prompts for the directory to
>>> look for that ID, and then display the commit.
>>
>> I wrote such a command (appended below) for my own use, but it's
>> git-specific and I don't know the innards of VC well enough to adapt it,
>> which is why I haven't proposed it for inclusion in Emacs.  Maybe some
>> of it could be used for a VC command.
>
> It should be easy to add a command properly integrated into the innards of VC
> by just grepping for "log-search" in lisp/vc/vc*.el (there are only 7 matches),
> and after copying to replace "log-search" with "log-show", and the new command
> is ready.

Actually "log-show" is a bad name.  It's too git-specific OT1H,
and OTOH it's too general since its output varies that doesn't fit
to log-view-mode.

I hoped git would be able to search both sha and commit message
in one command, something like:

  git log -1 5761a1a393 --grep=5761a1a393

to output the log of sha 5761a1a393, and logs of matching commit messages,
but this is not possible in git.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Juri Linkov]

> I hoped git would be able to search both sha and commit message
> in one command, something like:
>
>   git log -1 5761a1a393 --grep=5761a1a393
>
> to output the log of sha 5761a1a393, and logs of matching commit messages,
> but this is not possible in git.

We could support a special syntax for sha search, e.g.

  M-x vc-log-search RET sha:5761a1a393 RET

or using quoted strings as sha search instead of text search:

  M-x vc-log-search RET "5761a1a393" RET

or add a rule that if the search string matches only hex numbers
with a regexp like /[0-9A-F]+/, then run sha search:

  M-x vc-log-search RET 5761a1a393 RET

and any other text will run text search:

  M-x vc-log-search RET text RET

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 13.11.2019 23:03, Juri Linkov wrote:

> Actually "log-show" is a bad name.  It's too git-specific OT1H,
> and OTOH it's too general since its output varies that doesn't fit
> to log-view-mode.

Git's log output is flexible, isn't it? It can be changed using a template.

> I hoped git would be able to search both sha and commit message
> in one command, something like:
> 
>    git log -1 5761a1a393 --grep=5761a1a393
> 
> to output the log of sha 5761a1a393, and logs of matching commit messages,
> but this is not possible in git.

It's feasible: just call Git twice and concat the results.

But I don't think it's a good idea to mix the results of two different 
searches. I think there should be two of them. But there could be an 
extra -dwim- command that makes the choice based on the word under point 
(and whether it's hexinumeric).

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Juri Linkov]

> But I don't think it's a good idea to mix the results of two different
> searches. I think there should be two of them. But there could be an
> extra -dwim- command

I agree than dwim would be better.  But what a hint to use for deciding
on showing a commit by sha instead of matching it in the commit messages?

> that makes the choice based on the word under point
> (and whether it's hexinumeric).

Making the choice whether the word is hexinumeric doesn't look too
unambiguous.  What if the user wants to search a hexinumeric word
in the text of commit messages?  For example, searching for "5761a1a393"
to find messages that contain "Revert commit 5761a1a393 ..."

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 17.11.2019 23:20, Juri Linkov wrote:
> Making the choice whether the word is hexinumeric doesn't look too
> unambiguous.  What if the user wants to search a hexinumeric word
> in the text of commit messages?  For example, searching for "5761a1a393"
> to find messages that contain "Revert commit 5761a1a393 ..."

Then they would call one of the non-dwim commands.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Juri Linkov]

>> Making the choice whether the word is hexinumeric doesn't look too
>> unambiguous.  What if the user wants to search a hexinumeric word
>> in the text of commit messages?  For example, searching for "5761a1a393"
>> to find messages that contain "Revert commit 5761a1a393 ..."
>
> Then they would call one of the non-dwim commands.

Then no dwim command is needed.  We'll just add vc-log-revision and done.
The user will decide whether to use vc-log-search to search in commit
messages, or vc-log-revision to show the log message of one commit.

If adding a new command is not desirable, then add a prefix to existing.
'C-x v L' already uses a prefix arg to prompt for LIMIT, that can be 1.
Another prefix arg could ask for a revision, e.g. 'C-u C-u C-x L 5761a1a393'.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 18.11.2019 0:36, Juri Linkov wrote:
> Then no dwim command is needed.  We'll just add vc-log-revision and done.
> The user will decide whether to use vc-log-search to search in commit
> messages, or vc-log-revision to show the log message of one commit.

I'm fine with having two commands. Having a '-dwim-' one could save on 
key bindings, but we don't have one for vc-log-search anyway.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 405 bytes --]

>> Then no dwim command is needed.  We'll just add vc-log-revision and done.
>> The user will decide whether to use vc-log-search to search in commit
>> messages, or vc-log-revision to show the log message of one commit.
>
> I'm fine with having two commands. Having a '-dwim-' one could save on key
> bindings, but we don't have one for vc-log-search anyway.

Ok, here is a new command vc-log-revision:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vc-log-revision.patch --]
[-- Type: text/x-diff, Size: 3026 bytes --]

diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 5ab8e7ec53..f379c3d890 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -1189,7 +1189,7 @@ vc-git-log-incoming
 			"@{upstream}"
 		      remote-location))))
 
-(defun vc-git-log-search (buffer pattern)
+(defun vc-git-log-search (buffer pattern &optional limit)
   "Search the log of changes for PATTERN and output results into BUFFER.
 
 PATTERN is a basic regular expression by default in Git.
@@ -1197,8 +1197,10 @@ vc-git-log-search
 Display all entries that match log messages in long format.
 With a prefix argument, ask for a command to run that will output
 log entries."
-  (let ((args `("log" "--no-color" "-i"
-                ,(format "--grep=%s" (or pattern "")))))
+  (let ((args (if limit
+                  `("log" "--no-color" "-n" "1" ,(or pattern ""))
+                `("log" "--no-color" "-i"
+                  ,(format "--grep=%s" (or pattern ""))))))
     (when current-prefix-arg
       (setq args (cdr (split-string
 		       (read-shell-command
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 0d29c80d02..92faa59502 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -2542,6 +2542,16 @@ vc-log-outgoing
     (vc-incoming-outgoing-internal backend (or remote-location "")
                                    "*vc-outgoing*" 'log-outgoing)))
 
+(defun vc-log-search-internal (backend buffer-name type pattern &optional limit)
+  (vc-log-internal-common
+   backend buffer-name nil type
+   (lambda (bk buf type-arg _files)
+     (vc-call-backend bk type-arg buf pattern limit))
+   (lambda (_bk _files-arg _ret) nil)
+   nil ;; Don't move point.
+   (lambda (_ignore-auto _noconfirm)
+     (vc-log-search-internal backend pattern buffer-name type))))
+
 ;;;###autoload
 (defun vc-log-search (pattern)
   "Search the log of changes for PATTERN.
@@ -2558,8 +2568,25 @@ vc-log-search
   (let ((backend (vc-deduce-backend)))
     (unless backend
       (error "Buffer is not version controlled"))
-    (vc-incoming-outgoing-internal backend pattern
-                                   "*vc-search-log*" 'log-search)))
+    (vc-log-search-internal backend "*vc-search-log*" 'log-search pattern)))
+
+;;;###autoload
+(defun vc-log-revision (revision)
+  "Search the log of changes for REVISION.
+Display the REVISION log entry in long format."
+  (interactive (list (unless current-prefix-arg
+                       (let ((default (thing-at-point 'word)))
+                         (vc-read-revision
+                          (if default
+                              (format "Revision to log (default %s): " default)
+                            "Revision to log: ")
+                          nil nil default)))))
+  (when (equal revision "")
+    (error "No revision specified"))
+  (let ((backend (vc-deduce-backend)))
+    (unless backend
+      (error "Buffer is not version controlled"))
+    (vc-log-search-internal backend "*vc-search-log*" 'log-search revision 1)))
 
 ;;;###autoload
 (defun vc-log-mergebase (_files rev1 rev2)

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Mon, 18 Nov 2019 23:31:50 +0200
> Cc: Lars Ingebrigtsen <larsi@gnus.org>, Stephen Berman <stephen.berman@gmx.net>,
>  38044@debbugs.gnu.org
> 
> +(defun vc-log-revision (revision)
> +  "Search the log of changes for REVISION.
> +Display the REVISION log entry in long format."

The first sentence says that it "searches the log", but the second
says that it shows the log.  I think the first sentence is wrong and
the second is right: there's no "search" here, is there?

Even if the implementation does search, the doc string should describe
the result, not the implementation.

Thanks.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 18.11.2019 23:31, Juri Linkov wrote:
> +(defun vc-git-log-search (buffer pattern &optional limit)
> 
> +  (let ((args (if limit
> +                  `("log" "--no-color" "-n" "1" ,(or pattern ""))
> +                `("log" "--no-color" "-i"
> +                  ,(format "--grep=%s" (or pattern ""))))))

Why would we hardcode that if LIMIT is passed then PATTERN is a 
revision, not a search string?

Let's please make it another backend command instead of piggy-backing on 
'log-search'.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> Why would we hardcode that if LIMIT is passed then PATTERN is a 
> revision, not a search string?
> 
> Let's please make it another backend command instead of piggy-backing on 
> 'log-search'.

I agree.

Stepping a notch back, wasn't the original request to have a command
that would display a specific commit?  If so, that's not a "log"
command, that's closer to a "diff" command.  And don't we already have
a "diff" command which shows diffs for a specific revision?  If not,
let's provide that.  The fact that Git can show diffs in some
sub-commands of 'log' shouldn't dupe us into thinking that this is
about "log" in any way.

A command that searches the log entries for a string/pattern should be
a separate command, which could be a "log" command.

Thanks.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 19.11.2019 18:12, Eli Zaretskii wrote:
> If so, that's not a "log"
> command, that's closer to a "diff" command.

It's kind of both. 'git show HEAD' is the format that I'd personally 
expect: some meta info, including the commit message, followed by the 
diff contents.

> And don't we already have
> a "diff" command which shows diffs for a specific revision?

It *can* do that. And we also have a command that shows the revision log 
message and stuff: vc-annotate-show-log-revision-at-line. We could reuse 
its logic.

And either add a diff output at the botton (making it a different 
command an dealing with major mode having to support both the headers 
and the diff), or rely on log-view-mode's bindings (the user can press 
'd' or 'D' there).

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> Cc: larsi@gnus.org, stephen.berman@gmx.net, 38044@debbugs.gnu.org,
>  juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Tue, 19 Nov 2019 18:59:36 +0200
> 
> On 19.11.2019 18:12, Eli Zaretskii wrote:
> > If so, that's not a "log"
> > command, that's closer to a "diff" command.
> 
> It's kind of both. 'git show HEAD' is the format that I'd personally 
> expect: some meta info, including the commit message, followed by the 
> diff contents.

That's true, but I'd hesitate to introduce a new class of "show"
commands just because Git has it.  I see no problem showing the
meta-data with diffs (when Git is the back-end), perhaps with an
option to disable that.  My rationale is that other VCSes have a diff
command that shows only the diffs, and that command is used "to show"
a revision with those VCSes.

> > And don't we already have
> > a "diff" command which shows diffs for a specific revision?
> 
> It *can* do that. And we also have a command that shows the revision log 
> message and stuff: vc-annotate-show-log-revision-at-line. We could reuse 
> its logic.
> 
> And either add a diff output at the botton (making it a different 
> command an dealing with major mode having to support both the headers 
> and the diff), or rely on log-view-mode's bindings (the user can press 
> 'd' or 'D' there).

I think we should disconnect this "show" command from "log".  It is
conceptually wrong to make them related.  Of course a vc-log buffer
could have (and already has, AFAIK) a binding to a command that shows
the revision at point's line.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 19.11.2019 19:43, Eli Zaretskii wrote:

>> It's kind of both. 'git show HEAD' is the format that I'd personally
>> expect: some meta info, including the commit message, followed by the
>> diff contents.
> 
> That's true, but I'd hesitate to introduce a new class of "show"
> commands just because Git has it.  I see no problem showing the
> meta-data with diffs (when Git is the back-end), perhaps with an
> option to disable that.

These two sentences seem to contradict each other. To do the latter, we 
need to "introduce a new class of show commands". Because vc-git-diff 
won't print any metadata.

> My rationale is that other VCSes have a diff
> command that shows only the diffs, and that command is used "to show"
> a revision with those VCSes.

IMO the log message is more important because it describes and justifies 
what happened. Showing the diff is good as well.

Maybe the other VCSes don't have a simple command to do the same, but 
they can either be called twice, or use special formatting. For 
instance, Hg can use this command:

hg log -r <REV> -p

>>> And don't we already have
>>> a "diff" command which shows diffs for a specific revision?
>>
>> It *can* do that. And we also have a command that shows the revision log
>> message and stuff: vc-annotate-show-log-revision-at-line. We could reuse
>> its logic.
>>
>> And either add a diff output at the botton (making it a different
>> command an dealing with major mode having to support both the headers
>> and the diff), or rely on log-view-mode's bindings (the user can press
>> 'd' or 'D' there).
> 
> I think we should disconnect this "show" command from "log".  It is
> conceptually wrong to make them related.  Of course a vc-log buffer
> could have (and already has, AFAIK) a binding to a command that shows
> the revision at point's line.

Linking to a commit message from a diff buffer wouldn't really work. But 
we can "link" to a diff buffer from a common message buffer.

Of course, it's better to just display both.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> Cc: larsi@gnus.org, stephen.berman@gmx.net, 38044@debbugs.gnu.org,
>  juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 20 Nov 2019 01:17:18 +0200
> 
> > That's true, but I'd hesitate to introduce a new class of "show"
> > commands just because Git has it.  I see no problem showing the
> > meta-data with diffs (when Git is the back-end), perhaps with an
> > option to disable that.
> 
> These two sentences seem to contradict each other. To do the latter, we 
> need to "introduce a new class of show commands". Because vc-git-diff 
> won't print any metadata.

I was talking about the VC level, not the vc-git level.  vc-git could
have a show command, but the user of VC would still invoke a variant
of vc-diff.

> > My rationale is that other VCSes have a diff
> > command that shows only the diffs, and that command is used "to show"
> > a revision with those VCSes.
> 
> IMO the log message is more important because it describes and justifies 
> what happened. Showing the diff is good as well.

That's not relevant to the issue at hand.  Like it or not, VCSes other
than Git describe a revision by the diffs alone.

> Maybe the other VCSes don't have a simple command to do the same, but 
> they can either be called twice, or use special formatting. For 
> instance, Hg can use this command:
> 
> hg log -r <REV> -p

IMO, this is over-engineering.  If the VCS developers don't see the
need to have a commands which shows meta-data together with the diffs,
we should not force that on that VCS.  IOW, what Git does is not
necessarily the only game in town.

The VC level should be clean and uniform.  Any VCS-specific mindset
should not enter it too far, or else VC will not do its job of
insulating the user from the VCS differences well.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 20.11.2019 5:43, Eli Zaretskii wrote:

>>> That's true, but I'd hesitate to introduce a new class of "show"
>>> commands just because Git has it.  I see no problem showing the
>>> meta-data with diffs (when Git is the back-end), perhaps with an
>>> option to disable that.
>>
>> These two sentences seem to contradict each other. To do the latter, we
>> need to "introduce a new class of show commands". Because vc-git-diff
>> won't print any metadata.
> 
> I was talking about the VC level, not the vc-git level.  vc-git could
> have a show command, but the user of VC would still invoke a variant
> of vc-diff.

How would that even work? vc-diff will always delegate to vc-git-diff.

>>> My rationale is that other VCSes have a diff
>>> command that shows only the diffs, and that command is used "to show"
>>> a revision with those VCSes.
>>
>> IMO the log message is more important because it describes and justifies
>> what happened. Showing the diff is good as well.
> 
> That's not relevant to the issue at hand.  Like it or not, VCSes other
> than Git describe a revision by the diffs alone.

It's 100% relevant, and the fact that certain older VCSes can't do this 
should have no bearing on whether we implement a satisfactory UI in VC 
or not. That's the whole purpose of VC: make interacting with different 
VS systems easier.

>> Maybe the other VCSes don't have a simple command to do the same, but
>> they can either be called twice, or use special formatting. For
>> instance, Hg can use this command:
>>
>> hg log -r <REV> -p
> 
> IMO, this is over-engineering.  If the VCS developers don't see the
> need to have a commands which shows meta-data together with the diffs,
> we should not force that on that VCS.

They added the '-p' flag. So apparently they did see the need.

And this kind of invocation is certainly in demand (I found it in 
"answers" on StackOverflow).

> The VC level should be clean and uniform.  Any VCS-specific mindset
> should not enter it too far, or else VC will not do its job of
> insulating the user from the VCS differences well.

There's nothing VCS-specific in what I'm proposing. Any of them should 
be able to do that (though some will only manage it in two invocations).

In any case, the solution you're suggesting is trivial to implement 
indeed, but that's not what I would expect from the feature, nor, 
apparently, what Lars wanted either.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> Cc: larsi@gnus.org, stephen.berman@gmx.net, 38044@debbugs.gnu.org,
>  juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 20 Nov 2019 14:19:47 +0200
> 
> > I was talking about the VC level, not the vc-git level.  vc-git could
> > have a show command, but the user of VC would still invoke a variant
> > of vc-diff.
> 
> How would that even work? vc-diff will always delegate to vc-git-diff.

It will work if we program either vc-diff or vc-git-diff to call "git
show" under some specific circumstances.

> >> IMO the log message is more important because it describes and justifies
> >> what happened. Showing the diff is good as well.
> > 
> > That's not relevant to the issue at hand.  Like it or not, VCSes other
> > than Git describe a revision by the diffs alone.
> 
> It's 100% relevant, and the fact that certain older VCSes can't do this 
> should have no bearing on whether we implement a satisfactory UI in VC 
> or not. That's the whole purpose of VC: make interacting with different 
> VS systems easier.

Easier, yes.  But also present the results in a familiar enough form.
If users are accustomed to seeing a revision described by diffs, then
this is what they should by default see in VC, IMO.

> >> Maybe the other VCSes don't have a simple command to do the same, but
> >> they can either be called twice, or use special formatting. For
> >> instance, Hg can use this command:
> >>
> >> hg log -r <REV> -p
> > 
> > IMO, this is over-engineering.  If the VCS developers don't see the
> > need to have a commands which shows meta-data together with the diffs,
> > we should not force that on that VCS.
> 
> They added the '-p' flag. So apparently they did see the need.

Then maybe the hg back-end should indeed call "log -r -p", if that's
what hg users are used to (I don't use hg).  What I mean is that we
should show a revision like users are accustomed to see it with the
particular back-end; jumping through hoops to produce Git-like display
where users don't really expect it is IMO over-engineering.

And I'm also saying that conceptually a revision's description is a
kind of "diff" operation, so it should preferably be a sub-command of
"C-x v =".

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 20.11.2019 18:34, Eli Zaretskii wrote:

>> How would that even work? vc-diff will always delegate to vc-git-diff.
> 
> It will work if we program either vc-diff or vc-git-diff to call "git
> show" under some specific circumstances.

That makes little sense to me, sorry.

>>>> IMO the log message is more important because it describes and justifies
>>>> what happened. Showing the diff is good as well.
>>>
>>> That's not relevant to the issue at hand.  Like it or not, VCSes other
>>> than Git describe a revision by the diffs alone.
>>
>> It's 100% relevant, and the fact that certain older VCSes can't do this
>> should have no bearing on whether we implement a satisfactory UI in VC
>> or not. That's the whole purpose of VC: make interacting with different
>> VS systems easier.
> 
> Easier, yes.  But also present the results in a familiar enough form.
> If users are accustomed to seeing a revision described by diffs, then
> this is what they should by default see in VC, IMO.

There's nothing unfamiliar about also seeing the author name and the 
commit message.

Also: most of our users are Git users. Hence, users are accustomed to 
'git show'.

>>>> Maybe the other VCSes don't have a simple command to do the same, but
>>>> they can either be called twice, or use special formatting. For
>>>> instance, Hg can use this command:
>>>>
>>>> hg log -r <REV> -p
>>>
>>> IMO, this is over-engineering.  If the VCS developers don't see the
>>> need to have a commands which shows meta-data together with the diffs,
>>> we should not force that on that VCS.
>>
>> They added the '-p' flag. So apparently they did see the need.
> 
> Then maybe the hg back-end should indeed call "log -r -p", if that's
> what hg users are used to (I don't use hg).  What I mean is that we
> should show a revision like users are accustomed to see it with the
> particular back-end; jumping through hoops to produce Git-like display
> where users don't really expect it is IMO over-engineering.

I disagree. I think the possible arguments are exhausted at this point.

Are you going to invoke the privilege of the Emacs maintainer? All I got 
to say to this is "lisp/vc/*" is near my name in admin/MAINTAINERS.

> And I'm also saying that conceptually a revision's description is a
> kind of "diff" operation, so it should preferably be a sub-command of
> "C-x v =".

That's not how I think about it either. Again: I think the metadata is 
just as important. And we can't get to that metadata from the diff output.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 20.11.2019 21:40, Dmitry Gutov wrote:
>>> How would that even work? vc-diff will always delegate to vc-git-diff.
>>
>> It will work if we program either vc-diff or vc-git-diff to call "git
>> show" under some specific circumstances.
> 
> That makes little sense to me, sorry.

What we can try instead:

A new command: vc-describe-revision (for example; call it however you 
want) which first invokes vc-print-log-internal like 
vc-annotate-show-log-revision-at-line does, followed by calling 
vc-diff-internal like vc-annotate-show-diff-revision-at-line-internal 
does. The only difficulty there is coloring diff output properly. Maybe 
just take the fontified buffer strings from the above commands and copy 
them over into a new buffer.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> From: Dmitry Gutov <dgutov@yandex.ru>
> Cc: larsi@gnus.org, stephen.berman@gmx.net, 38044@debbugs.gnu.org,
>  juri@linkov.net
> Date: Wed, 20 Nov 2019 21:57:11 +0200
> 
> On 20.11.2019 21:40, Dmitry Gutov wrote:
> >>> How would that even work? vc-diff will always delegate to vc-git-diff.
> >>
> >> It will work if we program either vc-diff or vc-git-diff to call "git
> >> show" under some specific circumstances.
> > 
> > That makes little sense to me, sorry.
> 
> What we can try instead:
> 
> A new command: vc-describe-revision

Sorry, I'm not interested in bending VC to Git's twisted mindset.  The
description of a revision is its diffs, perhaps accompanied by some
meta-data, as per the back-end users' expectations.  But inventing a
new VC command class because Git has it makes no sense to me.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> Cc: larsi@gnus.org, stephen.berman@gmx.net, 38044@debbugs.gnu.org,
>  juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 20 Nov 2019 21:40:32 +0200
> 
> On 20.11.2019 18:34, Eli Zaretskii wrote:
> 
> >> How would that even work? vc-diff will always delegate to vc-git-diff.
> > 
> > It will work if we program either vc-diff or vc-git-diff to call "git
> > show" under some specific circumstances.
> 
> That makes little sense to me, sorry.

I'm sorry, I'm not interested in continuing this discussion with you.
Maybe in the future I should simply avoid chiming into discussions you
are part of.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Lars Ingebrigtsen]

Dmitry Gutov <dgutov@yandex.ru> writes:

> It's kind of both. 'git show HEAD' is the format that I'd personally
> expect: some meta info, including the commit message, followed by the 
> diff contents.

Yup; that is indeed what I wanted when I opened this bug report.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Eli Zaretskii <eliz@gnu.org>,  stephen.berman@gmx.net,
>   38044@debbugs.gnu.org,  juri@linkov.net
> Date: Wed, 20 Nov 2019 12:04:51 +0100
> 
> Dmitry Gutov <dgutov@yandex.ru> writes:
> 
> > It's kind of both. 'git show HEAD' is the format that I'd personally
> > expect: some meta info, including the commit message, followed by the 
> > diff contents.
> 
> Yup; that is indeed what I wanted when I opened this bug report.

And you will, with Git (and probably hg as well).  I was talking about
other backends, where users are used to describing a revision by diffs
alone.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Juri Linkov]

>> Let's please make it another backend command instead of piggy-backing on
>> 'log-search'.
>
> I agree.
>
> Stepping a notch back, wasn't the original request to have a command
> that would display a specific commit?  If so, that's not a "log"
> command, that's closer to a "diff" command.  And don't we already have
> a "diff" command which shows diffs for a specific revision?

Then we could use something like the existing backend 'region-history'
that for vc-git uses 'git log', but displays both the log and diff
in one output buffer.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Dmitry Gutov <dgutov@yandex.ru>,  larsi@gnus.org,
>   stephen.berman@gmx.net,  38044@debbugs.gnu.org
> Date: Wed, 20 Nov 2019 00:20:08 +0200
> 
> > Stepping a notch back, wasn't the original request to have a command
> > that would display a specific commit?  If so, that's not a "log"
> > command, that's closer to a "diff" command.  And don't we already have
> > a "diff" command which shows diffs for a specific revision?
> 
> Then we could use something like the existing backend 'region-history'
> that for vc-git uses 'git log', but displays both the log and diff
> in one output buffer.

region-history is slow, so I'm not sure it is a good starting point
for this feature.  A diff command is usually very fats with every VCS.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 855 bytes --]

>> > Stepping a notch back, wasn't the original request to have a command
>> > that would display a specific commit?  If so, that's not a "log"
>> > command, that's closer to a "diff" command.  And don't we already have
>> > a "diff" command which shows diffs for a specific revision?
>>
>> Then we could use something like the existing backend 'region-history'
>> that for vc-git uses 'git log', but displays both the log and diff
>> in one output buffer.
>
> region-history is slow, so I'm not sure it is a good starting point
> for this feature.  A diff command is usually very fats with every VCS.

It doesn't use 'region-history'.  I meant using 'region-history-mode'
on the output buffer.  Everything works perfectly with this patch
that also implements defaulting to the revision under point and
prompting for the directory like Lars asked to do:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vc-print-revision.patch --]
[-- Type: text/x-diff, Size: 4272 bytes --]

diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 0d29c80d02..fedc30e932 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -359,6 +359,10 @@
 ;;   and make sure it is displayed in the buffer's window.  The default
 ;;   implementation of this function works for RCS-style logs.
 ;;
+;; - print-revision (revision)
+;;
+;;   Show details of REVISION.
+;;
 ;; - comment-history (file)
 ;;
 ;;   Return a string containing all log entries that were made for FILE.
@@ -2516,6 +2520,31 @@ vc-print-branch-log
                          (list default-directory) branch t
                          (when (> vc-log-show-limit 0) vc-log-show-limit)))
 
+;;;###autoload
+(defun vc-print-revision (revision)
+  "Show the details of the revision REVISION."
+  (interactive (list (unless current-prefix-arg
+                       (let ((default (thing-at-point 'word)))
+                         (vc-read-revision
+                          (if default
+                              (format "Revision to show (default %s): " default)
+                            "Revision to show: ")
+                          nil nil default)))))
+  (when (equal revision "")
+    (error "No revision specified"))
+  (let ((backend (vc-deduce-backend))
+        rootdir)
+    (if backend
+        (setq rootdir (vc-call-backend backend 'root default-directory))
+      (setq rootdir (read-directory-name "Directory for VC print-revision: "))
+      (setq backend (vc-responsible-backend rootdir))
+      (unless backend
+        (error "Directory is not version controlled")))
+    (setq default-directory rootdir)
+    (vc-incoming-outgoing-internal backend revision
+                                   "*vc-revision*" 'print-revision)
+    (vc-call-backend backend 'region-history-mode)))
+
 ;;;###autoload
 (defun vc-log-incoming (&optional remote-location)
   "Show a log of changes that will be received with a pull operation from REMOTE-LOCATION.
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 5ab8e7ec53..a330adaa52 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -80,6 +80,7 @@
 ;; - log-search (buffer pattern)                   OK
 ;; - log-view-mode ()                              OK
 ;; - show-log-entry (revision)                     OK
+;; - print-revision (revision)                     OK
 ;; - comment-history (file)                        ??
 ;; - update-changelog (files)                      COULD BE SUPPORTED
 ;; * diff (file &optional rev1 rev2 buffer async)  OK
@@ -1163,6 +1164,22 @@ vc-git-print-log
                     (list start-revision)))
 		'("--")))))))
 
+(defun vc-git-print-revision (buffer revision)
+  "Show the details of REVISION with output in BUFFER.
+With a prefix argument, ask for a command to run that will output
+the revision information."
+  (let ((args `("show" "--no-color" ,(or revision ""))))
+    (when current-prefix-arg
+      (setq args (cdr (split-string
+		       (read-shell-command
+                        "Show revision with command: "
+                        (format "%s %s" vc-git-program
+                                (mapconcat 'identity args " "))
+                        'vc-git-history)
+		       " " t))))
+    (vc-setup-buffer buffer)
+    (apply 'vc-git-command buffer 'async nil args)))
+
 (defun vc-git-log-outgoing (buffer remote-location)
   (vc-setup-buffer buffer)
   (vc-git-command
@@ -1226,7 +1243,7 @@ vc-git-log-view-mode
   (set (make-local-variable 'log-view-file-re) regexp-unmatchable)
   (set (make-local-variable 'log-view-per-file-logs) nil)
   (set (make-local-variable 'log-view-message-re)
-       (if (not (memq vc-log-view-type '(long log-search)))
+       (if (not (memq vc-log-view-type '(long log-search print-revision)))
 	   (cadr vc-git-root-log-format)
 	 "^commit *\\([0-9a-z]+\\)"))
   ;; Allow expanding short log entries.
@@ -1235,7 +1252,7 @@ vc-git-log-view-mode
     (set (make-local-variable 'log-view-expanded-log-entry-function)
 	 'vc-git-expanded-log-entry))
   (set (make-local-variable 'log-view-font-lock-keywords)
-       (if (not (memq vc-log-view-type '(long log-search)))
+       (if (not (memq vc-log-view-type '(long log-search print-revision)))
 	   (list (cons (nth 1 vc-git-root-log-format)
 		       (nth 2 vc-git-root-log-format)))
 	 (append

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

Hi Juri,

The approach LGTM. I'd have used "describe" or "show", but it's not 
important. Some minor nits:

On 20.11.2019 23:50, Juri Linkov wrote:
>      (setq default-directory rootdir)

This probably needs a saving (let ((default-directory 
default-directory)) ...) above it.

> +With a prefix argument, ask for a command to run that will output
> +the revision information."

This sentence should be into the docstring of the new command, not in 
the backend implementation.

> +		       (read-shell-command
> +                        "Show revision with command: "
> +                        (format "%s %s" vc-git-program
> +                                (mapconcat 'identity args " "))
> +                        'vc-git-history)
> +		       " " t))))

If wonder if some user someday is going to try and input there something 
that doesn't start with 'git '.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Juri Linkov]

> The approach LGTM. I'd have used "describe" or "show", but it's not
> important.

The name vc-print-revision was modeled after vc-print-log,
vc-print-root-log, vc-print-branch-log that are not the best names
but at least consistent for their prefix.

>>      (setq default-directory rootdir)
>
> This probably needs a saving (let ((default-directory default-directory))
> ...) above it.

This is intentional because on testing I discovered that RET
on the diff part of the buffer can't visit source files
when default-directory is not root.

>> +		       (read-shell-command
>> +                        "Show revision with command: "
>> +                        (format "%s %s" vc-git-program
>> +                                (mapconcat 'identity args " "))
>> +                        'vc-git-history)
>> +		       " " t))))
>
> If wonder if some user someday is going to try and input there something
> that doesn't start with 'git '.

Someone might want to use a wrapper script.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 22.11.2019 1:08, Juri Linkov wrote:
>> The approach LGTM. I'd have used "describe" or "show", but it's not
>> important.
> 
> The name vc-print-revision was modeled after vc-print-log,
> vc-print-root-log, vc-print-branch-log that are not the best names
> but at least consistent for their prefix.

Yeah, OK.

>>>       (setq default-directory rootdir)
>>
>> This probably needs a saving (let ((default-directory default-directory))
>> ...) above it.
> 
> This is intentional because on testing I discovered that RET
> on the diff part of the buffer can't visit source files
> when default-directory is not root.

I'm saying your code modifies the value of default-directory in the 
original buffer as well. Hence the need for a (let ((...)) before that.

Just like vc-print-root-log does it.

BTW, vc-print-branch-log skips this step, so maybe it also exhibits the 
bug you mentioned. If so, I wonder if we should move the logic changing 
default-directory inside vc-print-log-internal.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: dgutov@yandex.ru,  larsi@gnus.org,  stephen.berman@gmx.net,
>   38044@debbugs.gnu.org
> Date: Wed, 20 Nov 2019 23:50:00 +0200
> 
> >> Then we could use something like the existing backend 'region-history'
> >> that for vc-git uses 'git log', but displays both the log and diff
> >> in one output buffer.
> >
> > region-history is slow, so I'm not sure it is a good starting point
> > for this feature.  A diff command is usually very fats with every VCS.
> 
> It doesn't use 'region-history'.  I meant using 'region-history-mode'
> on the output buffer.

Ah, okay.  Btw, region-history-mode is a Git-only feature, so I don't
think I understand why it is in vc.el and not in vc-git.el.

> Everything works perfectly with this patch that also implements
> defaulting to the revision under point and prompting for the
> directory like Lars asked to do:

Thanks.  A few comments:

> +;; - print-revision (revision)
> +;;
> +;;   Show details of REVISION.
> +;;

I'd prefer this to be a variant of vc-diff, and bound to "C-x v =".
The "C-u C-x v =" form is already taken, and tweaking it to accept a
single revision would not be easy.  So how about "C-u C-u C-x v ="?

The reason I think this should be a vc-diff subcommand is that most
kinds of VCS describe a revision as diffs.  Where a revision's
description is expected to include meta-data, i.e. the VCS backend
provides a command to show a revision in that format, we should use
that backend command, of course.  But conceptually we just show diffs,
so inventing a whole new class of VC commands for a minor variant of
diffs display sounds sub-optimal to me.

Also, why only implement this for a single backend?  The corresponding
diff commands exist in every VCS we support, so let's implement this
for all of them, okay?  Let us know if you need help with other VCSes.

Finally, this needs documentation in NEWS and in the user manual.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 21.11.2019 16:59, Eli Zaretskii wrote:
> The reason I think this should be a vc-diff subcommand is that most
> kinds of VCS describe a revision as diffs.

I strongly disagree with that idea.

When we create a revision, we specify both the changes, *and* a commit 
message. And some other metadata, like author, etc.

That happens in *every* VCS I had worked with, starting with CVS.

So no, a revision can't be described simply by a diff. The info should 
include all of the above.

Any VCS that doesn't support commit messages, etc, can simply show a 
diff. But we shouldn't cater to the lowest commit denominator in the API.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 21.11.2019 16:59, Eli Zaretskii wrote:
> I'd prefer this to be a variant of vc-diff, and bound to "C-x v =".
> The "C-u C-x v =" form is already taken, and tweaking it to accept a
> single revision would not be easy.  So how about "C-u C-u C-x v ="?

Okay, I see what you mean now: you're basically suggesting to tackle the 
new behavior (the one everybody wants apparently) on top the 'diff' 
backend action. Which can kind of work, but I don't see why we would 
make that choice.

Adding a new backend command is relatively cheap, and we won't force the 
backend implementation to try to reconcile incompatible arguments (e.g. 
REV1 that is not a parent of REV2 and SHOW-METADATA=t).

I also think the current patch proposed by Juri is cleaner than the one 
that is required to implement your idea.

Finally, "C-u C-u C-x v =" doesn't look semantic enough for me (revision 
!= diff in my mind, at least not entirely). I think it would be nicer to 
have a new command, but opinions welcome on this.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> Cc: larsi@gnus.org, stephen.berman@gmx.net, 38044@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 21 Nov 2019 17:50:10 +0200
> 
> Okay, I see what you mean now: you're basically suggesting to tackle the 
> new behavior (the one everybody wants apparently) on top the 'diff' 
> backend action. Which can kind of work, but I don't see why we would 
> make that choice.

I think it's the logical place for such a command, because, as I said
before, in many VCSes a description of a revision _is_ the diffs of
that revision against its parent.  That is how we always presented a
revision before Git.  And "git show" also presents diffs, it just
prepends some meta-data to it.  So it's actually a minor variation of
"diff".

> Adding a new backend command is relatively cheap, and we won't force the 
> backend implementation to try to reconcile incompatible arguments (e.g. 
> REV1 that is not a parent of REV2 and SHOW-METADATA=t).

I agree that adding a command is cheap.  But it makes the system more
complex and harder to remember and make sense of.  So IMO we should
only add a new class of commands if the command is radically different
from others.

> I also think the current patch proposed by Juri is cleaner than the one 
> that is required to implement your idea.

I think the difference is very small, because the function Juri wrote
can simply be called from vc-diff given a special value of prefix arg.

> Finally, "C-u C-u C-x v =" doesn't look semantic enough for me (revision 
> != diff in my mind, at least not entirely). I think it would be nicer to 
> have a new command, but opinions welcome on this.

I think that's because you keep the command issued by the backend in
mind all the time, and that command is not "diff" for Git and
Mercurial.  But the output is almost exactly that of "diff", so IMO
the mental model is simple and easy to remember.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 21.11.2019 20:33, Eli Zaretskii wrote:
>> Cc: larsi@gnus.org, stephen.berman@gmx.net, 38044@debbugs.gnu.org
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> Date: Thu, 21 Nov 2019 17:50:10 +0200
>>
>> Okay, I see what you mean now: you're basically suggesting to tackle the
>> new behavior (the one everybody wants apparently) on top the 'diff'
>> backend action. Which can kind of work, but I don't see why we would
>> make that choice.
> 
> I think it's the logical place for such a command, because, as I said
> before, in many VCSes a description of a revision _is_ the diffs of
> that revision against its parent.  That is how we always presented a
> revision before Git.  And "git show" also presents diffs, it just
> prepends some meta-data to it.  So it's actually a minor variation of
> "diff".

"As I said before", when a revision is created, we fill in a number of 
different fields, most importantly, the commit message. That's in every 
VCS except some ancient ones. So to show a revision means to show all 
that stuff.

The fact that some VCS's command line doesn't provide an easy way to do 
this is incidental.

>> Adding a new backend command is relatively cheap, and we won't force the
>> backend implementation to try to reconcile incompatible arguments (e.g.
>> REV1 that is not a parent of REV2 and SHOW-METADATA=t).
> 
> I agree that adding a command is cheap.  But it makes the system more
> complex and harder to remember and make sense of.  So IMO we should
> only add a new class of commands if the command is radically different
> from others.

An awkward implementation is even harder to make sense of. And creating 
a function that does something different based on an optional argument 
is bad programming.

Anyway, we could implement this new command using *zero* new backend 
actions. Even without calling 'git show'.

>> I also think the current patch proposed by Juri is cleaner than the one
>> that is required to implement your idea.
> 
> I think the difference is very small, because the function Juri wrote
> can simply be called from vc-diff given a special value of prefix arg.

Does this make sense for anybody else here?

For me, the diff command, even VCS diff, is about showing differences 
between file trees, or states of the file system. Not about describing 
one revision.

>> Finally, "C-u C-u C-x v =" doesn't look semantic enough for me (revision
>> != diff in my mind, at least not entirely). I think it would be nicer to
>> have a new command, but opinions welcome on this.
> 
> I think that's because you keep the command issued by the backend in
> mind all the time, and that command is not "diff" for Git and
> Mercurial.

Not necessarily. And see above.

> But the output is almost exactly that of "diff", so IMO
> the mental model is simple and easy to remember.

We all have our biases. You apparently dislike Git (VCS used by most of 
everybody these days, including our users) and prefer the way command 
line interfaces looked in previous systems. That's a valid preference, 
but it's unlikely to reflect the expectations of most of our users.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> Cc: juri@linkov.net, larsi@gnus.org, stephen.berman@gmx.net,
>  38044@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 21 Nov 2019 21:08:49 +0200
> 
> "As I said before", when a revision is created, we fill in a number of 
> different fields, most importantly, the commit message. That's in every 
> VCS except some ancient ones. So to show a revision means to show all 
> that stuff.

Here's an alternative proposal.  It seems like almost all VCS backends
we support provide a variant of a "log" command that shows the diffs
together with the usual meta-data shown by "log".  Only RCS and CVS
don't have such an option of "log", all the rest do (most of them via
"log -p").

So we could make this a subcommand of vc-log, more accurately
vc-print-root-log, such that "C-u C-u C-x v L" will prompt for a
revision ID, and display the information produced by such a "log -p"
command (and fall back to displaying just the diffs for RCS and CVS).

Does this sound better?

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 21.11.2019 22:19, Eli Zaretskii wrote:

>> "As I said before", when a revision is created, we fill in a number of
>> different fields, most importantly, the commit message. That's in every
>> VCS except some ancient ones. So to show a revision means to show all
>> that stuff.
> 
> Here's an alternative proposal.  It seems like almost all VCS backends
> we support provide a variant of a "log" command that shows the diffs
> together with the usual meta-data shown by "log".  Only RCS and CVS
> don't have such an option of "log", all the rest do (most of them via
> "log -p").

Somewhat better, but all backends would have to be updated anyway, for 
this to work. So the change in VC backend API is comparable to adding a 
new action.

I don't mind this too much (asking vc-git-print-log to include the diffs 
makes sense, at least), but doing it this way loses out on the 
opportunity to support all backends in one fell swoop. And we can do 
that by adding a default 'print-revision' implementation that calls two 
other backend actions (print-log and diff).

Or, again, we can do that for all backends and forego a new backend 
action altogether.

> So we could make this a subcommand of vc-log, more accurately
> vc-print-root-log, such that "C-u C-u C-x v L" will prompt for a
> revision ID, and display the information produced by such a "log -p"
> command (and fall back to displaying just the diffs for RCS and CVS).
> 
> Does this sound better?

I hardly see myself ever choosing 'C-u C-u C-x v L' instead of 'M-x 
vc-print-revision'. Simply because I'll never remember the former.

Are you really that against a new command?

Considering we have vc-print-branch-log for simply showing the same 
thing for a different branch, that feels... inconsistent.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> Cc: larsi@gnus.org, stephen.berman@gmx.net, 38044@debbugs.gnu.org,
>  juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 21 Nov 2019 23:05:07 +0200
> 
> On 21.11.2019 22:19, Eli Zaretskii wrote:
> 
> > Here's an alternative proposal.  It seems like almost all VCS backends
> > we support provide a variant of a "log" command that shows the diffs
> > together with the usual meta-data shown by "log".  Only RCS and CVS
> > don't have such an option of "log", all the rest do (most of them via
> > "log -p").
> 
> Somewhat better, but all backends would have to be updated anyway, for 
> this to work.

It's new functionality, so that goes without saying.  Or did you mean
something specific when you said "updated"?

> So the change in VC backend API is comparable to adding a new
> action.

We don't necessarily need a change in the API: vc-print-log-internal
has enough arguments to pass this new meaning to it and to the
backends.  But even if there's a change in the API, it isn't a
catastrophe from my POV.

> I don't mind this too much (asking vc-git-print-log to include the diffs 
> makes sense, at least), but doing it this way loses out on the 
> opportunity to support all backends in one fell swoop.

I don't understand why would we lose that opportunity.  We will have
to write new code for each backend in any alternative, and the code to
add is really quite simple, so I'm probably missing something here,
but what?

> I hardly see myself ever choosing 'C-u C-u C-x v L' instead of 'M-x 
> vc-print-revision'. Simply because I'll never remember the former.

That's fine.

> Are you really that against a new command?

Yes, more or less.  IMO, we already have too many of them.  VC used to
be simple and elegant, and this proliferation of too many high-level
commands makes it more and more complex, and inevitably causes us
tweak the user level and UI to this or that particular VCS, which is
wrong in the long run.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

[-- Attachment #1: Type: text/plain, Size: 2209 bytes --]

On 22.11.2019 9:20, Eli Zaretskii wrote:

>> Somewhat better, but all backends would have to be updated anyway, for
>> this to work.
> 
> It's new functionality, so that goes without saying.  Or did you mean
> something specific when you said "updated"?

Not every new functionality needs explicit support from backends.

>> So the change in VC backend API is comparable to adding a new
>> action.
> 
> We don't necessarily need a change in the API: vc-print-log-internal
> has enough arguments to pass this new meaning to it and to the
> backends.  But even if there's a change in the API, it isn't a
> catastrophe from my POV.

The backend API. It also has certain backward compatibility expectations.

Anyway, I'm saying the change you are proposing is roughly the same 
magnitude in complexity as adding a new backend action.

>> I don't mind this too much (asking vc-git-print-log to include the diffs
>> makes sense, at least), but doing it this way loses out on the
>> opportunity to support all backends in one fell swoop.
> 
> I don't understand why would we lose that opportunity.  We will have
> to write new code for each backend in any alternative,

Not necessarily. See the attached patch (it's a modification of Juri's).

Since we don't have a way to combine async process invocation, there is 
some complexity there with accept-process-output. But from what I see, 
the diff operation is considerably more resource-intensive, at least for 
big Hg repos.

There is a catch, however: it requires an implementation of 
region-history-mode. But, as you remarked, it can be extracted to be 
more backend-independent.

Consequently, for now this "default" implementation only adds 
print-revision support to Hg.

> VC used to
> be simple and elegant, and this proliferation of too many high-level
> commands makes it more and more complex,

I hardly see any complexity in the presence of a command. There's more 
more of it in tiny details of implementation of the main ones.

 > and inevitably causes us
 > tweak the user level and UI to this or that particular VCS, which is
 > wrong in the long run.

When was the last time we did that? And how adding a new command would 
cause it in this case?

[-- Attachment #2: vc-print-revision-with-default.diff --]
[-- Type: text/x-patch, Size: 6090 bytes --]

diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index ca4c66a06d..729d98f143 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -80,6 +80,7 @@
 ;; - log-search (buffer pattern)                   OK
 ;; - log-view-mode ()                              OK
 ;; - show-log-entry (revision)                     OK
+;; - print-revision (revision)                     OK
 ;; - comment-history (file)                        ??
 ;; - update-changelog (files)                      COULD BE SUPPORTED
 ;; * diff (file &optional rev1 rev2 buffer async)  OK
@@ -1163,6 +1164,22 @@ vc-git-print-log
                     (list start-revision)))
 		'("--")))))))
 
+(defun vc-git-print-revision (buffer revision)
+  "Show the details of REVISION with output in BUFFER.
+With a prefix argument, ask for a command to run that will output
+the revision information."
+  (let ((args `("show" "--no-color" ,(or revision ""))))
+    (when current-prefix-arg
+      (setq args (cdr (split-string
+		       (read-shell-command
+                        "Show revision with command: "
+                        (format "%s %s" vc-git-program
+                                (mapconcat 'identity args " "))
+                        'vc-git-history)
+		       " " t))))
+    (vc-setup-buffer buffer)
+    (apply 'vc-git-command buffer 'async nil args)))
+
 (defun vc-git-log-outgoing (buffer remote-location)
   (vc-setup-buffer buffer)
   (vc-git-command
@@ -1226,7 +1243,7 @@ vc-git-log-view-mode
   (set (make-local-variable 'log-view-file-re) regexp-unmatchable)
   (set (make-local-variable 'log-view-per-file-logs) nil)
   (set (make-local-variable 'log-view-message-re)
-       (if (not (memq vc-log-view-type '(long log-search)))
+       (if (not (memq vc-log-view-type '(long log-search print-revision)))
 	   (cadr vc-git-root-log-format)
 	 "^commit *\\([0-9a-z]+\\)"))
   ;; Allow expanding short log entries.
@@ -1235,7 +1252,7 @@ vc-git-log-view-mode
     (set (make-local-variable 'log-view-expanded-log-entry-function)
 	 'vc-git-expanded-log-entry))
   (set (make-local-variable 'log-view-font-lock-keywords)
-       (if (not (memq vc-log-view-type '(long log-search)))
+       (if (not (memq vc-log-view-type '(long log-search print-revision)))
 	   (list (cons (nth 1 vc-git-root-log-format)
 		       (nth 2 vc-git-root-log-format)))
 	 (append
diff --git a/lisp/vc/vc-hg.el b/lisp/vc/vc-hg.el
index 17d38fa400..ae0c93bf9d 100644
--- a/lisp/vc/vc-hg.el
+++ b/lisp/vc/vc-hg.el
@@ -564,7 +564,8 @@ vc-hg-expanded-log-entry
 (defun vc-hg-revision-table (files)
   (let ((default-directory (file-name-directory (car files))))
     (with-temp-buffer
-      (vc-hg-command t nil files "log" "--template" "{rev} ")
+      (vc-hg-command t nil nil "branches" "--template" "{branch}\n")
+      (vc-hg-command t nil nil "tags" "--template" "{tag}\n")
       (split-string
        (buffer-substring-no-properties (point-min) (point-max))))))
 
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 0d29c80d02..be4ed95fe3 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -359,6 +359,10 @@
 ;;   and make sure it is displayed in the buffer's window.  The default
 ;;   implementation of this function works for RCS-style logs.
 ;;
+;; - print-revision (revision)
+;;
+;;   Show details of REVISION.
+;;
 ;; - comment-history (file)
 ;;
 ;;   Return a string containing all log entries that were made for FILE.
@@ -2516,6 +2520,31 @@ vc-print-branch-log
                          (list default-directory) branch t
                          (when (> vc-log-show-limit 0) vc-log-show-limit)))
 
+;;;###autoload
+(defun vc-print-revision (revision)
+  "Show the details of the revision REVISION."
+  (interactive (list (unless current-prefix-arg
+                       (let ((default (thing-at-point 'word)))
+                         (vc-read-revision
+                          (if default
+                              (format "Revision to show (default %s): " default)
+                            "Revision to show: ")
+                          nil nil default)))))
+  (when (equal revision "")
+    (error "No revision specified"))
+  (let ((backend (vc-deduce-backend))
+        rootdir)
+    (if backend
+        (setq rootdir (vc-call-backend backend 'root default-directory))
+      (setq rootdir (read-directory-name "Directory for VC print-revision: "))
+      (setq backend (vc-responsible-backend rootdir))
+      (unless backend
+        (error "Directory is not version controlled")))
+    (setq default-directory rootdir)
+    (vc-incoming-outgoing-internal backend revision
+                                   "*vc-revision*" 'print-revision)
+    (vc-call-backend backend 'region-history-mode)))
+
 ;;;###autoload
 (defun vc-log-incoming (&optional remote-location)
   "Show a log of changes that will be received with a pull operation from REMOTE-LOCATION.
@@ -3091,6 +3120,30 @@ vc-default-dir-status-files
   (funcall update-function
            (mapcar (lambda (file) (list file 'up-to-date)) files)))
 
+(defun vc-default-print-revision (backend buffer revision)
+  (let* ((buffer-name " *vc-revision-log*")
+         (buf (get-buffer-create buffer-name))
+         proc)
+    (vc-call-backend backend 'print-log (list default-directory) buf nil revision 1)
+    (unwind-protect
+        (progn
+          (setq proc (get-buffer-process buf))
+          (while (process-live-p proc)
+            (accept-process-output proc 0.1))
+          (save-current-buffer
+            (vc-setup-buffer buffer)
+            (let ((inhibit-read-only t))
+              (insert-buffer-substring buf)
+              (insert "\n"))
+            (vc-call-backend backend 'diff (list default-directory)
+                             (vc-call-backend backend 'previous-revision nil revision)
+                             revision
+                             buffer
+                             t)
+            (vc-run-delayed
+              (goto-char (point-min)))))
+      (kill-buffer buf))))
+
 (defun vc-check-headers ()
   "Check if the current file has any headers in it."
   (interactive)

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> Cc: larsi@gnus.org, stephen.berman@gmx.net, 38044@debbugs.gnu.org,
>  juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 22 Nov 2019 21:59:14 +0200
> 
> >> I don't mind this too much (asking vc-git-print-log to include the diffs
> >> makes sense, at least), but doing it this way loses out on the
> >> opportunity to support all backends in one fell swoop.
> > 
> > I don't understand why would we lose that opportunity.  We will have
> > to write new code for each backend in any alternative,
> 
> Not necessarily. See the attached patch (it's a modification of Juri's).
> 
> Since we don't have a way to combine async process invocation, there is 
> some complexity there with accept-process-output. But from what I see, 
> the diff operation is considerably more resource-intensive, at least for 
> big Hg repos.
> 
> There is a catch, however: it requires an implementation of 
> region-history-mode. But, as you remarked, it can be extracted to be 
> more backend-independent.
> 
> Consequently, for now this "default" implementation only adds 
> print-revision support to Hg.

I'd prefer to use "log -p" in the backend ("log --diff" for svn),
because that yields a single command and will work for all the
backends except RCS and CVS.  The latter two can use "diff".

Thanks.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 22.11.2019 23:03, Eli Zaretskii wrote:
> I'd prefer to use "log -p" in the backend ("log --diff" for svn),
> because that yields a single command and will work for all the
> backends except RCS and CVS.

Would it?

For Git, we need 'git log -p -n 1', for Hg, 'hg log -p -l 1'. So it's 
hard to do the default implementation.

If you still mean to update the print-log action (which will mean 
updating all backends), I'm okay with that essentially, except we'd 
likely document the extra argument as "also print diffs for each 
revision". It would be weird for it to mean "show diffs INSTEAD of the 
revision log" only in CVS and RCS. But if someone wants to implement it 
that way (with ample commentary), I won't stop them.

I do think this should be better invoked via a new user-visible command, 
though.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> Cc: larsi@gnus.org, stephen.berman@gmx.net, 38044@debbugs.gnu.org,
>  juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 22 Nov 2019 23:43:08 +0200
> 
> On 22.11.2019 23:03, Eli Zaretskii wrote:
> > I'd prefer to use "log -p" in the backend ("log --diff" for svn),
> > because that yields a single command and will work for all the
> > backends except RCS and CVS.
> 
> Would it?
> 
> For Git, we need 'git log -p -n 1', for Hg, 'hg log -p -l 1'. So it's 
> hard to do the default implementation.

I didn't mean to say that we should have a default implementation, I
meant to have an implementation in each backend.  So small differences
in the commands to be issued are not an issue.

> If you still mean to update the print-log action (which will mean 
> updating all backends), I'm okay with that essentially, except we'd 
> likely document the extra argument as "also print diffs for each 
> revision". It would be weird for it to mean "show diffs INSTEAD of the 
> revision log" only in CVS and RCS. But if someone wants to implement it 
> that way (with ample commentary), I won't stop them.

I'd prefer it that way, yes.

> I do think this should be better invoked via a new user-visible command, 
> though.

I don't object to having a separate command as well, but not instead
of "C-x v L".

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> Cc: larsi@gnus.org, stephen.berman@gmx.net, 38044@debbugs.gnu.org,
>  juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 22 Nov 2019 23:43:08 +0200
> 
> For Git, we need 'git log -p -n 1', for Hg, 'hg log -p -l 1'.

Btw, it looks like only Git and Src need to explicitly limit the
output of 'log' to just a single revision, others have a way of
specifying a single revision for 'log'.  They all use "-r REVISION"
for that.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 23.11.2019 15:59, Eli Zaretskii wrote:
>> Cc: larsi@gnus.org, stephen.berman@gmx.net, 38044@debbugs.gnu.org,
>>   juri@linkov.net
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> Date: Fri, 22 Nov 2019 23:43:08 +0200
>>
>> For Git, we need 'git log -p -n 1', for Hg, 'hg log -p -l 1'.
> 
> Btw, it looks like only Git and Src need to explicitly limit the
> output of 'log' to just a single revision, others have a way of
> specifying a single revision for 'log'.  They all use "-r REVISION"
> for that.

I don't think we are going to use this new fact because the print-log 
implementations already have a start-revision argument, and it's 
implemented in a way that shows both the given revision and the previous 
ones. And there is an argument to limit the number of shows revisions.

We'll want the new argument to be as orthogonal to the existing ones as 
possible.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Stephen Berman]

On Thu, 21 Nov 2019 22:19:03 +0200 Eli Zaretskii <eliz@gnu.org> wrote:

>> Cc: juri@linkov.net, larsi@gnus.org, stephen.berman@gmx.net,
>>  38044@debbugs.gnu.org
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> Date: Thu, 21 Nov 2019 21:08:49 +0200
>>
>> "As I said before", when a revision is created, we fill in a number of
>> different fields, most importantly, the commit message. That's in every
>> VCS except some ancient ones. So to show a revision means to show all
>> that stuff.
>
> Here's an alternative proposal.  It seems like almost all VCS backends
> we support provide a variant of a "log" command that shows the diffs
> together with the usual meta-data shown by "log".  Only RCS and CVS
> don't have such an option of "log", all the rest do (most of them via
> "log -p").
>
> So we could make this a subcommand of vc-log, more accurately
> vc-print-root-log, such that "C-u C-u C-x v L" will prompt for a
> revision ID, and display the information produced by such a "log -p"
> command (and fall back to displaying just the diffs for RCS and CVS).
>
> Does this sound better?

If it's to be assigned to an existing VC command and key binding, I
think `log' is better than `diff' (FWIW, I named the git-specific
command I wrote for myself, posted near the top of this thread,
srb-git-log).  However, I'm not thrilled with the thought of having to
type two prefix keys to invoke it.  Since one of the desiderata of this
command, perhaps even the main one, is that it should act on the
revision ID at point, how about making just `C-x v L' do that if it
recognizes the word at point as a revision ID?  If this is deemed to
unreliable, it could be conditioned by a user option, or perhaps (though
more annoying) by asking for confirmation.

Steve Berman

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: Dmitry Gutov <dgutov@yandex.ru>,  juri@linkov.net,  larsi@gnus.org,
>   38044@debbugs.gnu.org
> Date: Thu, 21 Nov 2019 22:15:21 +0100
> 
> However, I'm not thrilled with the thought of having to type two
> prefix keys to invoke it.

AFAIU, this command is not something that will be invoked too
frequently, and typing C-u twice is not much harder than doing it
once.

> Since one of the desiderata of this command, perhaps even the main
> one, is that it should act on the revision ID at point, how about
> making just `C-x v L' do that if it recognizes the word at point as
> a revision ID?

We could do that, yes.

> If this is deemed to unreliable, it could be conditioned by a user
> option, or perhaps (though more annoying) by asking for
> confirmation.

Fine with me, assuming the rest of the proposal is accepted.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Stephen Berman]

On Fri, 22 Nov 2019 09:24:41 +0200 Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Stephen Berman <stephen.berman@gmx.net>
>> Cc: Dmitry Gutov <dgutov@yandex.ru>,  juri@linkov.net,  larsi@gnus.org,
>>   38044@debbugs.gnu.org
>> Date: Thu, 21 Nov 2019 22:15:21 +0100
>>
>> However, I'm not thrilled with the thought of having to type two
>> prefix keys to invoke it.
>
> AFAIU, this command is not something that will be invoked too
> frequently,

That's not true for me, I use my srb-git-log command much more often
than I do `C-x v L' precisely because the emacs-bug and -devel lists
make many references to commits by means of the commit hash, and I find
it much more convenient to go directly from that to the commit message
(and maybe then the diff too), rather than first calling up the whole
commit log and searching for the hash in it.

>             and typing C-u twice is not much harder than doing it
> once.

I find even just typing one C-u less convenient, hence my suggestion
below, which I'm glad you find worth considering.

>> Since one of the desiderata of this command, perhaps even the main
>> one, is that it should act on the revision ID at point, how about
>> making just `C-x v L' do that if it recognizes the word at point as
>> a revision ID?
>
> We could do that, yes.
>
>> If this is deemed to unreliable, it could be conditioned by a user
>> option, or perhaps (though more annoying) by asking for
>> confirmation.
>
> Fine with me, assuming the rest of the proposal is accepted.

Thanks,

Steve Berman

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: dgutov@yandex.ru,  juri@linkov.net,  larsi@gnus.org,  38044@debbugs.gnu.org
> Date: Fri, 22 Nov 2019 10:25:15 +0100
> 
> I find even just typing one C-u less convenient, hence my suggestion
> below, which I'm glad you find worth considering.
> 
> >> Since one of the desiderata of this command, perhaps even the main
> >> one, is that it should act on the revision ID at point, how about
> >> making just `C-x v L' do that if it recognizes the word at point as
> >> a revision ID?
> >
> > We could do that, yes.
> >
> >> If this is deemed to unreliable, it could be conditioned by a user
> >> option, or perhaps (though more annoying) by asking for
> >> confirmation.
> >
> > Fine with me, assuming the rest of the proposal is accepted.

Btw, in the above scenario, I guess it would be more convenient if the
revision was displayed in "the other" window, so that the user could
continue reading/studying whatever he/she was looking at before
invoking the command, is that right?

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Stephen Berman]

On Sat, 23 Nov 2019 14:13:44 +0200 Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Stephen Berman <stephen.berman@gmx.net>
>> Cc: dgutov@yandex.ru,  juri@linkov.net,  larsi@gnus.org,  38044@debbugs.gnu.org
>> Date: Fri, 22 Nov 2019 10:25:15 +0100
>>
>> I find even just typing one C-u less convenient, hence my suggestion
>> below, which I'm glad you find worth considering.
>>
>> >> Since one of the desiderata of this command, perhaps even the main
>> >> one, is that it should act on the revision ID at point, how about
>> >> making just `C-x v L' do that if it recognizes the word at point as
>> >> a revision ID?
>> >
>> > We could do that, yes.
>> >
>> >> If this is deemed to unreliable, it could be conditioned by a user
>> >> option, or perhaps (though more annoying) by asking for
>> >> confirmation.
>> >
>> > Fine with me, assuming the rest of the proposal is accepted.
>
> Btw, in the above scenario, I guess it would be more convenient if the
> revision was displayed in "the other" window, so that the user could
> continue reading/studying whatever he/she was looking at before
> invoking the command, is that right?

Yes, I think so.  That's what my home-grown command does, and it's also
the way `C-x v L' already works, if I'm not misunderstanding you.

Steve Berman

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 22.11.2019 9:24, Eli Zaretskii wrote:
>> If this is deemed to unreliable, it could be conditioned by a user
>> option, or perhaps (though more annoying) by asking for
>> confirmation.
> Fine with me, assuming the rest of the proposal is accepted.

I fail to see how this is less complicated that just adding a new command.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Lars Ingebrigtsen]

Dmitry Gutov <dgutov@yandex.ru> writes:

> I fail to see how this is less complicated that just adding a new command.

I'd rather have a new command.  Makes it easier to rebind, which is
something that's popular for commands that have long key bindings by
default.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> Cc: larsi@gnus.org, 38044@debbugs.gnu.org, juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 22 Nov 2019 13:19:08 +0200
> 
> On 22.11.2019 9:24, Eli Zaretskii wrote:
> >> If this is deemed to unreliable, it could be conditioned by a user
> >> option, or perhaps (though more annoying) by asking for
> >> confirmation.
> > Fine with me, assuming the rest of the proposal is accepted.
> 
> I fail to see how this is less complicated that just adding a new command.

It isn't, not from the implementation POV.  But it's one less command
to remember.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 22.11.2019 15:04, Eli Zaretskii wrote:
> But it's one less command
> to remember.

The command name will be mnemonic.

M-x vc--revision TAB, and there you go.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Stephen Berman]

[-- Attachment #1: Type: text/plain, Size: 1810 bytes --]

On Fri, 22 Nov 2019 09:24:41 +0200 Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Stephen Berman <stephen.berman@gmx.net>
>> Cc: Dmitry Gutov <dgutov@yandex.ru>,  juri@linkov.net,  larsi@gnus.org,
>>   38044@debbugs.gnu.org
>> Date: Thu, 21 Nov 2019 22:15:21 +0100
[...]
>> Since one of the desiderata of this command, perhaps even the main
>> one, is that it should act on the revision ID at point, how about
>> making just `C-x v L' do that if it recognizes the word at point as
>> a revision ID?
>
> We could do that, yes.

I haven't found a useful way to do this.

>> If this is deemed to unreliable, it could be conditioned by a user
>> option, or perhaps (though more annoying) by asking for
>> confirmation.
>
> Fine with me, assuming the rest of the proposal is accepted.

On reflection, both of these alternatives don't seem convenient enough
to justify them.

There is, however, another option that is convenient (at least to me)
and easy to implement.  Most of the time of I want to look up the log
entry and/or diff of the revision ID at point the current buffer is not
a version-controlled buffer (typically it's a Gnus buffer), so the
default directory is wrong and I have to manually change to the
version-controlled root directory.  For me this is almost always the
Emacs master branch, so it would be convenient if typing `C-1 C-x v L'
offered this directory as a default, as a user option.  The patch below
does this.

The way vc-print-root-log sets the root directory is also done exactly
the same by vc-root-version-diff, vc-diff-mergebase, vc-root-diff and
vc-log-mergebase, so perhaps these commands should also be changed to
offer a user-specified default.  I haven't used any of these yet, so I
don't know if this is as convenient for them as it is for
vc-print-root-log.

Steve Berman


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: VC patch --]
[-- Type: text/x-patch, Size: 1054 bytes --]

diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index d0d2c39ac3..06a3e90837 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -848,6 +848,14 @@ vc-revert-show-diff
   :type 'boolean
   :version "24.1")

+(defcustom vc-default-root-dir nil
+  "User-specified root directory for VC operations.
+Offered as default when the VC backend fails to identify a root
+directory."
+  :type '(choice (const :tag "None" nil)
+                 (file :must-match t :tag "File"))
+  :version "27.1")
+
 ;; Header-insertion hair

 (defcustom vc-static-header-alist
@@ -2517,7 +2525,8 @@ vc-print-root-log
 	 rootdir)
     (if backend
 	(setq rootdir (vc-call-backend backend 'root default-directory))
-      (setq rootdir (read-directory-name "Directory for VC root-log: "))
+      (setq rootdir (read-directory-name "Directory for VC root-log: "
+                                         vc-default-root-dir))
       (setq backend (vc-responsible-backend rootdir))
       (unless backend
         (error "Directory is not version controlled")))

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: larsi@gnus.org,  juri@linkov.net,  38044@debbugs.gnu.org,  dgutov@yandex.ru
> Date: Sun, 01 Dec 2019 14:13:06 +0100
> 
> >> Since one of the desiderata of this command, perhaps even the main
> >> one, is that it should act on the revision ID at point, how about
> >> making just `C-x v L' do that if it recognizes the word at point as
> >> a revision ID?
> >
> > We could do that, yes.
> 
> I haven't found a useful way to do this.

??? The current master AFAICS already does, but only if you invoke
"C-x v L" with an argument of 1.  Or am I missing something?

(It does so quite naively, though: it just picks up the word at point,
so I've seen it prompting with default being something like
"Copyright".  Perhaps we should use number-at-point instead?)

> There is, however, another option that is convenient (at least to me)
> and easy to implement.  Most of the time of I want to look up the log
> entry and/or diff of the revision ID at point the current buffer is not
> a version-controlled buffer (typically it's a Gnus buffer), so the
> default directory is wrong and I have to manually change to the
> version-controlled root directory.  For me this is almost always the
> Emacs master branch, so it would be convenient if typing `C-1 C-x v L'
> offered this directory as a default, as a user option.  The patch below
> does this.

The command prompts for a directory, so you can type whatever you
want.  Having a fixed default doesn't sound very wise to me (do you
never have to work with several repositories at the same time?), but
I'll defer to Dmitry's opinion here.

Thanks.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Stephen Berman]

On Sun, 01 Dec 2019 19:53:51 +0200 Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Stephen Berman <stephen.berman@gmx.net>
>> Cc: larsi@gnus.org,  juri@linkov.net,  38044@debbugs.gnu.org,  dgutov@yandex.ru
>> Date: Sun, 01 Dec 2019 14:13:06 +0100
>>
>> >> Since one of the desiderata of this command, perhaps even the main
>> >> one, is that it should act on the revision ID at point, how about
>> >> making just `C-x v L' do that if it recognizes the word at point as
>> >> a revision ID?
>> >
>> > We could do that, yes.
>>
>> I haven't found a useful way to do this.
>
> ??? The current master AFAICS already does, but only if you invoke
> "C-x v L" with an argument of 1.  Or am I missing something?

My proposal was for `C-x v L' to do double duty, i.e., that if point is
on a revision ID, just typing `C-x v L' should show the corresponding
commit data and diff, so this would save having to type `C-u 1' or `C-1'
first, but ...

> (It does so quite naively, though: it just picks up the word at point,
> so I've seen it prompting with default being something like
> "Copyright".

... this is exactly the problem: if whatever is at point is taken to be
a revision ID, then there's no way for `C-x v L' to know when it should
(try to) show the commit data and diff as opposed to showing the commit
log.

>               Perhaps we should use number-at-point instead?)

I think number-at-point would also get too many false positives to make
it really useful.

>> There is, however, another option that is convenient (at least to me)
>> and easy to implement.  Most of the time of I want to look up the log
>> entry and/or diff of the revision ID at point the current buffer is not
>> a version-controlled buffer (typically it's a Gnus buffer), so the
>> default directory is wrong and I have to manually change to the
>> version-controlled root directory.  For me this is almost always the
>> Emacs master branch, so it would be convenient if typing `C-1 C-x v L'
>> offered this directory as a default, as a user option.  The patch below
>> does this.
>
> The command prompts for a directory, so you can type whatever you
> want.

This is precisely the extra effort I want to avoid.

>        Having a fixed default doesn't sound very wise to me (do you
> never have to work with several repositories at the same time?), but
> I'll defer to Dmitry's opinion here.

Since the default value of the proposed user option is nil, those who
don't set it will just get the current behavior, and even those who do
set it will still get the prompt, but only have to type RET instead of
directory name; and of course, if they don't want to use that default,
they can type in the directory name they want, just as now.  (At first I
made the default be immediately used without prompting; this is the most
convenient UI if you never use any other repository, but it's obviously
too restrictive.)

I did consider another idea: instead of having one default root
directory, have a list, so users could have all (or the most common)
root directories they need, and the one to use would be chosen by
read-multiple-choice.  But I thought this was probably too complicated a
UI to be really convenient for this command.

Steve Berman

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 01.12.2019 19:53, Eli Zaretskii wrote:
> The command prompts for a directory, so you can type whatever you
> want.  Having a fixed default doesn't sound very wise to me (do you
> never have to work with several repositories at the same time?), but
> I'll defer to Dmitry's opinion here.

I don't like to add new user options without seeing concrete demand. For 
now, only one user wants it and I don't see myself using it, thus far.

In cases like this, I usually ask the requester (Stephen!) to write 
their own custom command and bind to a key combination of their liking 
(rebind C-x v L, in this particular case). If this turns out to be less 
than trivial, then we look into changing something. It should be simple 
in this case, I think.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Stephen Berman]

On Sun, 1 Dec 2019 23:29:37 +0200 Dmitry Gutov <dgutov@yandex.ru> wrote:

> On 01.12.2019 19:53, Eli Zaretskii wrote:
>> The command prompts for a directory, so you can type whatever you
>> want.  Having a fixed default doesn't sound very wise to me (do you
>> never have to work with several repositories at the same time?), but
>> I'll defer to Dmitry's opinion here.
>
> I don't like to add new user options without seeing concrete demand. For now,
> only one user wants it and I don't see myself using it, thus far.
>
> In cases like this, I usually ask the requester (Stephen!) to write their own
> custom command and bind to a key combination of their liking (rebind C-x v L,
> in this particular case). If this turns out to be less than trivial, then we
> look into changing something. It should be simple in this case, I think.

As mentioned near the start of this bug, I had already written my own
command to do essentially what `C-1 C-x v L' now does, but also
including the option I proposed.  So I'll just keep using that command.

Steve Berman

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 01.12.2019 23:39, Stephen Berman wrote:
> As mentioned near the start of this bug, I had already written my own
> command to do essentially what `C-1 C-x v L' now does, but also
> including the option I proposed.  So I'll just keep using that command.

Then we should keep this as is, for now. But thank you for the request, 
and others who need this as well should feel free to speak up.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 2 Dec 2019 00:30:09 +0200
> Cc: larsi@gnus.org, 38044@debbugs.gnu.org, juri@linkov.net
> 
> On 01.12.2019 23:39, Stephen Berman wrote:
> > As mentioned near the start of this bug, I had already written my own
> > command to do essentially what `C-1 C-x v L' now does, but also
> > including the option I proposed.  So I'll just keep using that command.
> 
> Then we should keep this as is, for now. But thank you for the request, 
> and others who need this as well should feel free to speak up.

What do you think about replacing the call to thing-at-point with
number-at-point, where we look at the revision ID?

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 02.12.2019 5:34, Eli Zaretskii wrote:
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> Date: Mon, 2 Dec 2019 00:30:09 +0200
>> Cc: larsi@gnus.org, 38044@debbugs.gnu.org, juri@linkov.net
>>
>> On 01.12.2019 23:39, Stephen Berman wrote:
>>> As mentioned near the start of this bug, I had already written my own
>>> command to do essentially what `C-1 C-x v L' now does, but also
>>> including the option I proposed.  So I'll just keep using that command.
>>
>> Then we should keep this as is, for now. But thank you for the request,
>> and others who need this as well should feel free to speak up.
> 
> What do you think about replacing the call to thing-at-point with
> number-at-point, where we look at the revision ID?

Probably not:

- Git revisions are SHA hashes, not numbers. Hg also has commit hashes, 
though they are less visible.

- I'm pretty sure this command will be just as often used with symbolic 
refs such as branch names. So I'd rather use (thing-at-point 'symbol t) 
instead. Or a dedicated "thing" that does not depend on the buffer's 
syntax (and always includes slashes, hyphens and underscores in addition 
to alphanumerics).

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Andreas Schwab]

On Dez 02 2019, Dmitry Gutov wrote:

> - I'm pretty sure this command will be just as often used with symbolic
> refs such as branch names. So I'd rather use (thing-at-point 'symbol t)
> instead. Or a dedicated "thing" that does not depend on the buffer's
> syntax (and always includes slashes, hyphens and underscores in addition
> to alphanumerics).

A ref in git basically has filename syntax.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> Cc: stephen.berman@gmx.net, larsi@gnus.org, 38044@debbugs.gnu.org,
>  juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 2 Dec 2019 12:39:47 +0200
> 
> > What do you think about replacing the call to thing-at-point with
> > number-at-point, where we look at the revision ID?
> 
> Probably not:
> 
> - Git revisions are SHA hashes, not numbers. Hg also has commit hashes, 
> though they are less visible.

They are both hex numbers.  But I failed to notice that
number-at-point needs the 0x prefix to detect hex numbers, sorry.  (We
could have a new function that accepted hex numbers without 0x.)

> - I'm pretty sure this command will be just as often used with symbolic 
> refs such as branch names. So I'd rather use (thing-at-point 'symbol t) 
> instead. Or a dedicated "thing" that does not depend on the buffer's 
> syntax (and always includes slashes, hyphens and underscores in addition 
> to alphanumerics).

We are talking about heuristic guesswork to suggest a reasonable
default, not about what forms are accepted.  I thought that suggesting
only numbers, either decimal or hex, as such a guess should be enough,
as I almost never see any other revision IDs in practice.  (I do use
other types of refs with Git, like reflog and stash refs, but those
are local and are extremely unlikely to appear in a discussion.)

But if you don't feel that's a good idea, it's fine with me.  It just
strike me as a mild misfeature that we offer completely unrelated
words as "revision IDs".

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 02.12.2019 17:58, Eli Zaretskii wrote:
> (I do use
> other types of refs with Git, like reflog and stash refs, but those
> are local and are extremely unlikely to appear in a discussion.)

We do see branch names often enough. The current code will frequently 
fail to pick them up, and I think that's unfortunate.

> But if you don't feel that's a good idea, it's fine with me.  It just
> strike me as a mild misfeature that we offer completely unrelated
> words as "revision IDs".

It's fine if we offer something unrelated as the default because 
otherwise we'd simply offer nothing (nil). The user can type over it anyway.

Not offering something useful when we could is more problematic, IMO.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 722 bytes --]

> Here's an alternative proposal.  It seems like almost all VCS backends
> we support provide a variant of a "log" command that shows the diffs
> together with the usual meta-data shown by "log".  Only RCS and CVS
> don't have such an option of "log", all the rest do (most of them via
> "log -p").
>
> So we could make this a subcommand of vc-log, more accurately
> vc-print-root-log, such that "C-u C-u C-x v L" will prompt for a
> revision ID, and display the information produced by such a "log -p"
> command (and fall back to displaying just the diffs for RCS and CVS).
>
> Does this sound better?

This is fine.  Here's a new patch that implements 'M-1 C-x v L'
to limit the log to one revision expanded with diff:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vc-print-root-log-revision.patch --]
[-- Type: text/x-diff, Size: 2784 bytes --]

diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index ca4c66a06d..3b977aa1f4 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -1160,7 +1159,9 @@ vc-git-print-log
                       (list (concat start-revision ".." (if (equal limit "")
                                                             "HEAD"
                                                           limit)))
-                    (list start-revision)))
+                    (if (eq limit 1)
+                        (list "-p" start-revision)
+                      (list start-revision))))
 		'("--")))))))
 
 (defun vc-git-log-outgoing (buffer remote-location)
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 0d29c80d02..90603541b5 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -2475,13 +2475,26 @@ vc-print-log
     (vc-print-log-internal backend files working-revision nil limit)))
 
 ;;;###autoload
-(defun vc-print-root-log (&optional limit)
+(defun vc-print-root-log (&optional limit revision)
   "List the change log for the current VC controlled tree in a window.
 If LIMIT is non-nil, it should be a number specifying the maximum
 number of revisions to show; the default is `vc-log-show-limit'.
-When called interactively with a prefix argument, prompt for LIMIT."
+When called interactively with a prefix argument, prompt for LIMIT.
+When the prefix argument is a number, use it as LIMIT.
+A special case is when the prefix argument is 1, in this case
+it asks for the revision and shows it with its diff."
   (interactive
    (cond
+    ((eq current-prefix-arg 1)
+     (let* ((default (thing-at-point 'word))
+	    (revision (vc-read-revision
+		       (if default
+			   (format "Revision to show (default %s): " default)
+			 "Revision to show: ")
+		       nil nil default)))
+       (list 1 revision)))
+    ((numberp current-prefix-arg)
+     (list current-prefix-arg))
     (current-prefix-arg
      (let ((lim (string-to-number
 		 (read-from-minibuffer
@@ -2494,6 +2507,8 @@ vc-print-root-log
      (list (when (> vc-log-show-limit 0) vc-log-show-limit)))))
   (let ((backend (vc-deduce-backend))
 	(default-directory default-directory)
+	(vc-log-short-style (unless (and (eq limit 1) revision)
+                              vc-log-short-style))
 	rootdir)
     (if backend
 	(setq rootdir (vc-call-backend backend 'root default-directory))
@@ -2502,7 +2517,9 @@ vc-print-root-log
       (unless backend
         (error "Directory is not version controlled")))
     (setq default-directory rootdir)
-    (vc-print-log-internal backend (list rootdir) nil nil limit)))
+    (vc-print-log-internal backend (list rootdir) revision revision limit)
+    (when (and (eq limit 1) revision)
+      (vc-git-region-history-mode))))
 
 ;;;###autoload
 (defun vc-print-branch-log (branch)

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 23.11.2019 20:50, Juri Linkov wrote:
> -                    (list start-revision)))
> +                    (if (eq limit 1)
> +                        (list "-p" start-revision)
> +                      (list start-revision))))
> 
> +A special case is when the prefix argument is 1, in this case
> +it asks for the revision and shows it with its diff."

I think that's rather ugly (see my comment about having the arguments to 
be orthogonal), and it precludes the use of this backend action to print 
a more extensive commit log (with diffs, that is).

> +    (when (and (eq limit 1) revision)
> +      (vc-git-region-history-mode))))

A Git-specific function in vc.el?

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 921 bytes --]

>> -                    (list start-revision)))
>> +                    (if (eq limit 1)
>> +                        (list "-p" start-revision)
>> +                      (list start-revision))))
>> +A special case is when the prefix argument is 1, in this case
>> +it asks for the revision and shows it with its diff."
>
> I think that's rather ugly (see my comment about having the arguments to be
> orthogonal), and it precludes the use of this backend action to print
> a more extensive commit log (with diffs, that is).

I rewrote the patch to make it more flexible, so now the function
can check 'vc-log-view-type' for the value 'with-diff'.

>> +    (when (and (eq limit 1) revision)
>> +      (vc-git-region-history-mode))))
>
> A Git-specific function in vc.el?

Thanks for noticing, this was intended to call a backend function.
But now I rewrote this part as well to call region-history-mode
from other place:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vc-print-root-log-with-diff.patch --]
[-- Type: text/x-diff, Size: 5702 bytes --]

diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index ca4c66a06d..02a8a3a525 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -1161,6 +1161,8 @@ vc-git-print-log
                                                             "HEAD"
                                                           limit)))
                     (list start-revision)))
+                (when (eq vc-log-view-type 'with-diff)
+                  (list "-p"))
 		'("--")))))))
 
 (defun vc-git-log-outgoing (buffer remote-location)
@@ -1226,7 +1228,7 @@ vc-git-log-view-mode
   (set (make-local-variable 'log-view-file-re) regexp-unmatchable)
   (set (make-local-variable 'log-view-per-file-logs) nil)
   (set (make-local-variable 'log-view-message-re)
-       (if (not (memq vc-log-view-type '(long log-search)))
+       (if (not (memq vc-log-view-type '(long log-search with-diff)))
 	   (cadr vc-git-root-log-format)
 	 "^commit *\\([0-9a-z]+\\)"))
   ;; Allow expanding short log entries.
@@ -1235,7 +1237,7 @@ vc-git-log-view-mode
     (set (make-local-variable 'log-view-expanded-log-entry-function)
 	 'vc-git-expanded-log-entry))
   (set (make-local-variable 'log-view-font-lock-keywords)
-       (if (not (memq vc-log-view-type '(long log-search)))
+       (if (not (memq vc-log-view-type '(long log-search with-diff)))
 	   (list (cons (nth 1 vc-git-root-log-format)
 		       (nth 2 vc-git-root-log-format)))
 	 (append
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 0d29c80d02..5cca3c6dd6 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -2361,7 +2361,7 @@ vc-print-log-setup-buttons
                         'help-echo "Show the log again, including all entries")))
 
 (defun vc-print-log-internal (backend files working-revision
-                                      &optional is-start-revision limit)
+                                      &optional is-start-revision limit type)
   "For specified BACKEND and FILES, show the VC log.
 Leave point at WORKING-REVISION, if it is non-nil.
 If IS-START-REVISION is non-nil, start the log from WORKING-REVISION
@@ -2377,7 +2377,7 @@ vc-print-log-internal
          (shortlog (not (null (memq (if dir-present 'directory 'file)
                                     vc-log-short-style))))
 	(buffer-name "*vc-change-log*")
-         (type (if shortlog 'short 'long)))
+         (type (or type (if shortlog 'short 'long))))
       (vc-log-internal-common
        backend buffer-name files type
        (lambda (bk buf _type-arg files-arg)
@@ -2393,7 +2393,7 @@ vc-print-log-internal
            (vc-call-backend bk 'show-log-entry working-revision)))
        (lambda (_ignore-auto _noconfirm)
 	 (vc-print-log-internal backend files working-revision
-                              is-start-revision limit)))))
+                              is-start-revision limit type)))))
 
 (defvar vc-log-view-type nil
   "Set this to differentiate the different types of logs.")
@@ -2416,7 +2416,10 @@ vc-log-internal-common
       (let ((inhibit-read-only t))
 	;; log-view-mode used to be called with inhibit-read-only bound
 	;; to t, so let's keep doing it, just in case.
-	(vc-call-backend backend 'log-view-mode)
+	(vc-call-backend backend
+                         (if (eq type 'with-diff)
+                             'region-history-mode
+                           'log-view-mode))
 	(set (make-local-variable 'log-view-vc-backend) backend)
 	(set (make-local-variable 'log-view-vc-fileset) files)
 	(set (make-local-variable 'revert-buffer-function)
@@ -2475,13 +2478,26 @@ vc-print-log
     (vc-print-log-internal backend files working-revision nil limit)))
 
 ;;;###autoload
-(defun vc-print-root-log (&optional limit)
+(defun vc-print-root-log (&optional limit revision)
   "List the change log for the current VC controlled tree in a window.
 If LIMIT is non-nil, it should be a number specifying the maximum
 number of revisions to show; the default is `vc-log-show-limit'.
-When called interactively with a prefix argument, prompt for LIMIT."
+When called interactively with a prefix argument, prompt for LIMIT.
+When the prefix argument is a number, use it as LIMIT.
+A special case is when the prefix argument is 1, in this case
+it asks for the revision and shows it with its diff."
   (interactive
    (cond
+    ((eq current-prefix-arg 1)
+     (let* ((default (thing-at-point 'word))
+	    (revision (vc-read-revision
+		       (if default
+			   (format "Revision to show (default %s): " default)
+			 "Revision to show: ")
+		       nil nil default)))
+       (list 1 revision)))
+    ((numberp current-prefix-arg)
+     (list current-prefix-arg))
     (current-prefix-arg
      (let ((lim (string-to-number
 		 (read-from-minibuffer
@@ -2492,9 +2508,11 @@ vc-print-root-log
        (list lim)))
     (t
      (list (when (> vc-log-show-limit 0) vc-log-show-limit)))))
-  (let ((backend (vc-deduce-backend))
-	(default-directory default-directory)
-	rootdir)
+  (let* ((backend (vc-deduce-backend))
+	 (default-directory default-directory)
+         (with-diff (and (eq limit 1) revision))
+         (vc-log-short-style (unless with-diff vc-log-short-style))
+	 rootdir)
     (if backend
 	(setq rootdir (vc-call-backend backend 'root default-directory))
       (setq rootdir (read-directory-name "Directory for VC root-log: "))
@@ -2502,7 +2520,8 @@ vc-print-root-log
       (unless backend
         (error "Directory is not version controlled")))
     (setq default-directory rootdir)
-    (vc-print-log-internal backend (list rootdir) nil nil limit)))
+    (vc-print-log-internal backend (list rootdir) revision revision limit
+                           (when with-diff 'with-diff))))
 
 ;;;###autoload
 (defun vc-print-branch-log (branch)

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  larsi@gnus.org,  stephen.berman@gmx.net,
>   38044@debbugs.gnu.org
> Date: Sun, 24 Nov 2019 00:43:37 +0200
> 
> @@ -2416,7 +2416,10 @@ vc-log-internal-common
>        (let ((inhibit-read-only t))
>  	;; log-view-mode used to be called with inhibit-read-only bound
>  	;; to t, so let's keep doing it, just in case.
> -	(vc-call-backend backend 'log-view-mode)
> +	(vc-call-backend backend
> +                         (if (eq type 'with-diff)
> +                             'region-history-mode
> +                           'log-view-mode))

Can we avoid calling region-history-mode if the backend doesn't
implement it?  Or do you plan on adding that to all backends?

Thanks.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 659 bytes --]

>> @@ -2416,7 +2416,10 @@ vc-log-internal-common
>>        (let ((inhibit-read-only t))
>>  	;; log-view-mode used to be called with inhibit-read-only bound
>>  	;; to t, so let's keep doing it, just in case.
>> -	(vc-call-backend backend 'log-view-mode)
>> +	(vc-call-backend backend
>> +                         (if (eq type 'with-diff)
>> +                             'region-history-mode
>> +                           'log-view-mode))
>
> Can we avoid calling region-history-mode if the backend doesn't
> implement it?  Or do you plan on adding that to all backends?

A new patch avoids calling region-history-mode
if the backend doesn't implement it:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vc-print-root-log-with-diff-2.patch --]
[-- Type: text/x-diff, Size: 5816 bytes --]

diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index ca4c66a06d..71307cdffd 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -1161,6 +1160,8 @@ vc-git-print-log
                                                             "HEAD"
                                                           limit)))
                     (list start-revision)))
+                (when (eq vc-log-view-type 'with-diff)
+                  (list "-p"))
 		'("--")))))))
 
 (defun vc-git-log-outgoing (buffer remote-location)
@@ -1226,7 +1227,7 @@ vc-git-log-view-mode
   (set (make-local-variable 'log-view-file-re) regexp-unmatchable)
   (set (make-local-variable 'log-view-per-file-logs) nil)
   (set (make-local-variable 'log-view-message-re)
-       (if (not (memq vc-log-view-type '(long log-search)))
+       (if (not (memq vc-log-view-type '(long log-search with-diff)))
 	   (cadr vc-git-root-log-format)
 	 "^commit *\\([0-9a-z]+\\)"))
   ;; Allow expanding short log entries.
@@ -1235,7 +1236,7 @@ vc-git-log-view-mode
     (set (make-local-variable 'log-view-expanded-log-entry-function)
 	 'vc-git-expanded-log-entry))
   (set (make-local-variable 'log-view-font-lock-keywords)
-       (if (not (memq vc-log-view-type '(long log-search)))
+       (if (not (memq vc-log-view-type '(long log-search with-diff)))
 	   (list (cons (nth 1 vc-git-root-log-format)
 		       (nth 2 vc-git-root-log-format)))
 	 (append
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 0d29c80d02..d0d2c39ac3 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -2361,7 +2361,7 @@ vc-print-log-setup-buttons
                         'help-echo "Show the log again, including all entries")))
 
 (defun vc-print-log-internal (backend files working-revision
-                                      &optional is-start-revision limit)
+                                      &optional is-start-revision limit type)
   "For specified BACKEND and FILES, show the VC log.
 Leave point at WORKING-REVISION, if it is non-nil.
 If IS-START-REVISION is non-nil, start the log from WORKING-REVISION
@@ -2377,7 +2377,7 @@ vc-print-log-internal
          (shortlog (not (null (memq (if dir-present 'directory 'file)
                                     vc-log-short-style))))
 	(buffer-name "*vc-change-log*")
-         (type (if shortlog 'short 'long)))
+         (type (or type (if shortlog 'short 'long))))
       (vc-log-internal-common
        backend buffer-name files type
        (lambda (bk buf _type-arg files-arg)
@@ -2393,7 +2393,7 @@ vc-print-log-internal
            (vc-call-backend bk 'show-log-entry working-revision)))
        (lambda (_ignore-auto _noconfirm)
 	 (vc-print-log-internal backend files working-revision
-                              is-start-revision limit)))))
+                              is-start-revision limit type)))))
 
 (defvar vc-log-view-type nil
   "Set this to differentiate the different types of logs.")
@@ -2416,7 +2416,12 @@ vc-log-internal-common
       (let ((inhibit-read-only t))
 	;; log-view-mode used to be called with inhibit-read-only bound
 	;; to t, so let's keep doing it, just in case.
-	(vc-call-backend backend 'log-view-mode)
+	(vc-call-backend backend
+                         (if (and (eq type 'with-diff)
+                                  (vc-find-backend-function
+                                   backend 'region-history-mode))
+                             'region-history-mode
+                           'log-view-mode))
 	(set (make-local-variable 'log-view-vc-backend) backend)
 	(set (make-local-variable 'log-view-vc-fileset) files)
 	(set (make-local-variable 'revert-buffer-function)
@@ -2475,13 +2480,26 @@ vc-print-log
     (vc-print-log-internal backend files working-revision nil limit)))
 
 ;;;###autoload
-(defun vc-print-root-log (&optional limit)
+(defun vc-print-root-log (&optional limit revision)
   "List the change log for the current VC controlled tree in a window.
 If LIMIT is non-nil, it should be a number specifying the maximum
 number of revisions to show; the default is `vc-log-show-limit'.
-When called interactively with a prefix argument, prompt for LIMIT."
+When called interactively with a prefix argument, prompt for LIMIT.
+When the prefix argument is a number, use it as LIMIT.
+A special case is when the prefix argument is 1, in this case
+it asks for the revision and shows it with its diff."
   (interactive
    (cond
+    ((eq current-prefix-arg 1)
+     (let* ((default (thing-at-point 'word))
+	    (revision (read-string
+		       (if default
+			   (format "Revision to show (default %s): " default)
+			 "Revision to show: ")
+		       nil nil default)))
+       (list 1 revision)))
+    ((numberp current-prefix-arg)
+     (list current-prefix-arg))
     (current-prefix-arg
      (let ((lim (string-to-number
 		 (read-from-minibuffer
@@ -2492,9 +2510,11 @@ vc-print-root-log
        (list lim)))
     (t
      (list (when (> vc-log-show-limit 0) vc-log-show-limit)))))
-  (let ((backend (vc-deduce-backend))
-	(default-directory default-directory)
-	rootdir)
+  (let* ((backend (vc-deduce-backend))
+	 (default-directory default-directory)
+	 (with-diff (and (eq limit 1) revision))
+	 (vc-log-short-style (unless with-diff vc-log-short-style))
+	 rootdir)
     (if backend
 	(setq rootdir (vc-call-backend backend 'root default-directory))
       (setq rootdir (read-directory-name "Directory for VC root-log: "))
@@ -2502,7 +2522,8 @@ vc-print-root-log
       (unless backend
         (error "Directory is not version controlled")))
     (setq default-directory rootdir)
-    (vc-print-log-internal backend (list rootdir) nil nil limit)))
+    (vc-print-log-internal backend (list rootdir) revision revision limit
+                           (when with-diff 'with-diff))))
 
 ;;;###autoload
 (defun vc-print-branch-log (branch)

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: dgutov@yandex.ru,  larsi@gnus.org,  stephen.berman@gmx.net,
>   38044@debbugs.gnu.org
> Date: Wed, 27 Nov 2019 01:01:20 +0200
> 
> A new patch avoids calling region-history-mode
> if the backend doesn't implement it:

Thanks.

This seems to implement the command only for Git, though?  Or am I
missing something?

Also, we need to document this.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Juri Linkov]

>> A new patch avoids calling region-history-mode
>> if the backend doesn't implement it:
>
> Thanks.
>
> This seems to implement the command only for Git, though?  Or am I
> missing something?

Sorry, can't help with other backends.  But it should be easy
to do the same with other backends for anyone who uses them.

> Also, we need to document this.

Now installed with documentation.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 24.11.2019 0:43, Juri Linkov wrote:
> I rewrote the patch to make it more flexible, so now the function
> can check 'vc-log-view-type' for the value 'with-diff'.

Generally, an explicit argument is better than an implicit one.

So what's the idea here? Gracefully degrading to just showing a log 
entry when the backend doesn't support the WITH-DIFF feature? (BTW, the 
print-revision approach seems better for graceful degradation, but I'm 
tired of arguing for that).

I think it would work, but for that all backends need support for 
region-history-mode anyway. The caller function doesn't have a way to 
check the lack of support.

So I guess the "proper" choice here is to extract region-history-mode is 
a way that doesn't need redefinition by all backends.

Or a shortcut: catch the vc-not-supported error and call backend's 
log-view-mode instead.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Juri Linkov]

>> I rewrote the patch to make it more flexible, so now the function
>> can check 'vc-log-view-type' for the value 'with-diff'.
>
> Generally, an explicit argument is better than an implicit one.
>
> So what's the idea here? Gracefully degrading to just showing a log entry
> when the backend doesn't support the WITH-DIFF feature?

Yes, if a backend doesn't support WITH-DIFF, then do nothing special.

> (BTW, the print-revision approach seems better for graceful
> degradation, but I'm tired of arguing for that).

The print-log approach is not bad too:
'C-1 C-x v L' is mnemonic ("show me just 1 revision") and easy to type.

> I think it would work, but for that all backends need support for
> region-history-mode anyway. The caller function doesn't have a way to 
> check the lack of support.

The new patch I sent now in another message
checks if a backend supports region-history-mode.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 27.11.2019 1:06, Juri Linkov wrote:

>> (BTW, the print-revision approach seems better for graceful
>> degradation, but I'm tired of arguing for that).
> 
> The print-log approach is not bad too:
> 'C-1 C-x v L' is mnemonic ("show me just 1 revision") and easy to type.

I mean the print-revision backend action (which can have a default 
impl). The user command could be separate, or it could be this one.

Anyway...

>> I think it would work, but for that all backends need support for
>> region-history-mode anyway. The caller function doesn't have a way to
>> check the lack of support.
> 
> The new patch I sent now in another message
> checks if a backend supports region-history-mode.

Thanks, this looks better.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  larsi@gnus.org,  stephen.berman@gmx.net,
>   38044@debbugs.gnu.org
> Date: Wed, 27 Nov 2019 01:06:39 +0200
> 
> > So what's the idea here? Gracefully degrading to just showing a log entry
> > when the backend doesn't support the WITH-DIFF feature?
> 
> Yes, if a backend doesn't support WITH-DIFF, then do nothing special.

Is there a command in the log buffer to show the diffs if the user
wants that?

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Juri Linkov]

>> > So what's the idea here? Gracefully degrading to just showing a log entry
>> > when the backend doesn't support the WITH-DIFF feature?
>>
>> Yes, if a backend doesn't support WITH-DIFF, then do nothing special.
>
> Is there a command in the log buffer to show the diffs if the user
> wants that?

Yes, the command is log-view-diff.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: dgutov@yandex.ru,  larsi@gnus.org,  stephen.berman@gmx.net,
>   38044@debbugs.gnu.org
> Date: Wed, 27 Nov 2019 23:32:04 +0200
> 
> >> > So what's the idea here? Gracefully degrading to just showing a log entry
> >> > when the backend doesn't support the WITH-DIFF feature?
> >>
> >> Yes, if a backend doesn't support WITH-DIFF, then do nothing special.
> >
> > Is there a command in the log buffer to show the diffs if the user
> > wants that?
> 
> Yes, the command is log-view-diff.

Then I think we are okay here: if a backend doesn't support the likes
of "log -p", and the user wants to see the diffs, they can invoke that
command manually.

Thanks.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Juri Linkov]

>> It doesn't use 'region-history'.  I meant using 'region-history-mode'
>> on the output buffer.
>
> Ah, okay.  Btw, region-history-mode is a Git-only feature, so I don't
> think I understand why it is in vc.el and not in vc-git.el.

Maybe region-history-mode is not the best name, but at least
it provides fontification for revisions composed of both
the commit metadata and the diff of changes.

> The reason I think this should be a vc-diff subcommand is that most
> kinds of VCS describe a revision as diffs.  Where a revision's
> description is expected to include meta-data, i.e. the VCS backend
> provides a command to show a revision in that format, we should use
> that backend command, of course.  But conceptually we just show diffs,
> so inventing a whole new class of VC commands for a minor variant of
> diffs display sounds sub-optimal to me.

Similar to wave-particle duality, in revision's duality the revision
may be described as either a log metadata or a diff depending on the
observer's point of view.  A new separate command resolves such dilemma.

> Also, why only implement this for a single backend?  The corresponding
> diff commands exist in every VCS we support, so let's implement this
> for all of them, okay?  Let us know if you need help with other VCSes.

A new command was intended to produce the output similar to e.g.

http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=6f30642973975a317a9c94ceba737a4bafc89919

i.e. first comes the metadata, then the commit message, and finally the diff.

Do other VCSes or their interfaces produce a similar layout?

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: dgutov@yandex.ru,  larsi@gnus.org,  stephen.berman@gmx.net,
>   38044@debbugs.gnu.org
> Date: Fri, 22 Nov 2019 01:13:23 +0200
> 
> >> It doesn't use 'region-history'.  I meant using 'region-history-mode'
> >> on the output buffer.
> >
> > Ah, okay.  Btw, region-history-mode is a Git-only feature, so I don't
> > think I understand why it is in vc.el and not in vc-git.el.
> 
> Maybe region-history-mode is not the best name, but at least
> it provides fontification for revisions composed of both
> the commit metadata and the diff of changes.

My problem is that having region-history-mode in vc.el requires every
backend to implement that mode, if it implements region-history.  What
that results in can be seen in vc-hg.el, where I recently added this
command's implementation.  Do you like the result?

> > Also, why only implement this for a single backend?  The corresponding
> > diff commands exist in every VCS we support, so let's implement this
> > for all of them, okay?  Let us know if you need help with other VCSes.
> 
> A new command was intended to produce the output similar to e.g.
> 
> http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=6f30642973975a317a9c94ceba737a4bafc89919
> 
> i.e. first comes the metadata, then the commit message, and finally the diff.
> 
> Do other VCSes or their interfaces produce a similar layout?

Almost all of them do, see "log -p" command in each VCS ("log --diff"
for svn).

And the original request was for describing a revision, so if a VCS
doesn't support "log -p", we should simply display the diffs, because
that's the best description of the revision in those VCSes.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 22.11.2019 10:03, Eli Zaretskii wrote:
> And the original request was for describing a revision, so if a VCS
> doesn't support "log -p", we should simply display the diffs, because
> that's the best description of the revision in those VCSes.

Why? All VCSes support printing the commit message.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> Cc: larsi@gnus.org, stephen.berman@gmx.net, 38044@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 22 Nov 2019 13:20:26 +0200
> 
> On 22.11.2019 10:03, Eli Zaretskii wrote:
> > And the original request was for describing a revision, so if a VCS
> > doesn't support "log -p", we should simply display the diffs, because
> > that's the best description of the revision in those VCSes.
> 
> Why? All VCSes support printing the commit message.

Because (see above) that's the best description of a revision with
those VCSes.  And because adding what 'log' produces to that would
mean an additional delay with those VCSes, which in this case is not
justified, IMO.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 22.11.2019 15:06, Eli Zaretskii wrote:
>> Cc: larsi@gnus.org, stephen.berman@gmx.net, 38044@debbugs.gnu.org
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> Date: Fri, 22 Nov 2019 13:20:26 +0200
>>
>> On 22.11.2019 10:03, Eli Zaretskii wrote:
>>> And the original request was for describing a revision, so if a VCS
>>> doesn't support "log -p", we should simply display the diffs, because
>>> that's the best description of the revision in those VCSes.
>>
>> Why? All VCSes support printing the commit message.
> 
> Because (see above) that's the best description of a revision with
> those VCSes.  And because adding what 'log' produces to that would
> mean an additional delay with those VCSes, which in this case is not
> justified, IMO.

That's against the principles of VC, which is making a generic, unified 
interface.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> Cc: larsi@gnus.org, stephen.berman@gmx.net, 38044@debbugs.gnu.org,
>  juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 22 Nov 2019 18:26:42 +0200
> 
> >> Why? All VCSes support printing the commit message.
> > 
> > Because (see above) that's the best description of a revision with
> > those VCSes.  And because adding what 'log' produces to that would
> > mean an additional delay with those VCSes, which in this case is not
> > justified, IMO.
> 
> That's against the principles of VC, which is making a generic, unified 
> interface.

The interface is indeed the same, but the results aren't, because each
VCS produces different outputs.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Andrii Kolomoiets]

Hi Eli,

> Ah, okay.  Btw, region-history-mode is a Git-only feature, so I don't
> think I understand why it is in vc.el and not in vc-git.el.

Experimental option 'line-range' to command 'log' is added in mercurial 4.4
'region-history' is also implemented in vc-hgcmd.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> From: Andrii Kolomoiets <andreyk.mad@gmail.com>
> Date: Fri, 22 Nov 2019 00:36:18 +0200
> Cc: 38044@debbugs.gnu.org
> 
> > Ah, okay.  Btw, region-history-mode is a Git-only feature, so I don't
> > think I understand why it is in vc.el and not in vc-git.el.
> 
> Experimental option 'line-range' to command 'log' is added in mercurial 4.4
> 'region-history' is also implemented in vc-hgcmd.

I know, see commit 1110d14.  But I was talking about
region-history-mode, not vc-region-history.  By placing that mode in
vc.el, we now require _every_ backend to implement this mode, which
doesn't make a lot of sense to me, and you can see in that commit what
that causes for backends that do want to implement region-history.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> For that case, Emacs should have a command that prompts for an ID
> (defaulting to the ID under point), and then (unless default-directory
> is already in a vc-controlled directory), prompts for the directory to
> look for that ID, and then display the commit.

And this was, indeed, implemented (via `C-u 1 C-x v L' *phew*), so I'm
now closing this bug report.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 19.07.2020 18:31, Lars Ingebrigtsen wrote:
> And this was, indeed, implemented (via `C-u 1 C-x v L'*phew*), so I'm
> now closing this bug report.

Are you happy with the result?

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Lars Ingebrigtsen]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 19.07.2020 18:31, Lars Ingebrigtsen wrote:
>> And this was, indeed, implemented (via `C-u 1 C-x v L'*phew*), so I'm
>> now closing this bug report.
>
> Are you happy with the result?

Yes, looks perfect to me.

But I wish it'd have been on its own keystroke instead of being a
special-case "1" prefix for a command that's quite different
conceptually.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 19.07.2020 20:16, Lars Ingebrigtsen wrote:

>> Are you happy with the result?
> 
> Yes, looks perfect to me.
> 
> But I wish it'd have been on its own keystroke instead of being a
> special-case "1" prefix for a command that's quite different
> conceptually.

Indeed. And the implementation reflects this dissonance as well.

But the resulting buffer looks fine, I agree.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Lars Ingebrigtsen]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 19.07.2020 20:16, Lars Ingebrigtsen wrote:
>
>>> Are you happy with the result?
>> Yes, looks perfect to me.
>> But I wish it'd have been on its own keystroke instead of being a
>> special-case "1" prefix for a command that's quite different
>> conceptually.
>
> Indeed. And the implementation reflects this dissonance as well.
>
> But the resulting buffer looks fine, I agree.

I've done the following in my .emacs:

(global-set-key [(super c)] (lambda ()
			      (interactive)
			      (let ((current-prefix-arg 1))
				(call-interactively
				 'vc-print-root-log))))

But those contortions I have to do to even bind this to a different
keystrokes further illustrates the problems with having this
functionality be in the same command...

What were the reasons for squishing these two things into the same
function, anyway?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 19.07.2020 22:51, Lars Ingebrigtsen wrote:
> What were the reasons for squishing these two things into the same
> function, anyway?

The dangers of "bending VC to Git's twisted mindset":

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=38044#83

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Lars Ingebrigtsen]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 19.07.2020 22:51, Lars Ingebrigtsen wrote:
>> What were the reasons for squishing these two things into the same
>> function, anyway?
>
> The dangers of "bending VC to Git's twisted mindset":
>
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=38044#83

Uhm...  I don't quite follow the logic.  "Looking at a specific vc
commit" is surely VC agnostic?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 22.07.2020 00:44, Lars Ingebrigtsen wrote:
> Dmitry Gutov<dgutov@yandex.ru>  writes:
> 
>> On 19.07.2020 22:51, Lars Ingebrigtsen wrote:
>>> What were the reasons for squishing these two things into the same
>>> function, anyway?
>> The dangers of "bending VC to Git's twisted mindset":
>>
>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=38044#83
> Uhm...  I don't quite follow the logic.  "Looking at a specific vc
> commit" is surely VC agnostic?

SVN doesn't have a separate command to "show commit", so VC shouldn't 
either. Right, Eli?

Or did I miss some of your arguments here?

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

Forgot to add in Cc.

On 22.07.2020 18:36, Dmitry Gutov wrote:
> On 22.07.2020 00:44, Lars Ingebrigtsen wrote:
>> Dmitry Gutov<dgutov@yandex.ru>  writes:
>>
>>> On 19.07.2020 22:51, Lars Ingebrigtsen wrote:
>>>> What were the reasons for squishing these two things into the same
>>>> function, anyway?
>>> The dangers of "bending VC to Git's twisted mindset":
>>>
>>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=38044#83
>> Uhm...  I don't quite follow the logic.  "Looking at a specific vc
>> commit" is surely VC agnostic?
> 
> SVN doesn't have a separate command to "show commit", so VC shouldn't 
> either. Right, Eli?
> 
> Or did I miss some of your arguments here?

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Eli Zaretskii]

> Cc: 38044@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org>
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 22 Jul 2020 18:36:28 +0300
> 
> >> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=38044#83
> > Uhm...  I don't quite follow the logic.  "Looking at a specific vc
> > commit" is surely VC agnostic?
> 
> SVN doesn't have a separate command to "show commit", so VC shouldn't 
> either. Right, Eli?

There are VCS commands to show one or more commits; they are generally
called "log" with some command-line arguments to control how many
revisions to show and what to show about each one.  Git also has a
separate command "show" (which can "show" many different objects, not
just a commit), but the concept is still the same.

bug#38044: 27.0.50; There should be an easier way to look at a specific vc commit

[Dmitry Gutov]

On 22.07.2020 19:25, Eli Zaretskii wrote:
> There are VCS commands to show one or more commits; they are generally
> called "log" with some command-line arguments to control how many
> revisions to show and what to show about each one.

Some do, some don't.

Surely none of them conflate the "show diff as well" argument with "show 
only one revision".