vc-git-show-log-entry

vc-git-show-log-entry

[Karl Chen]

Hi,

In Emacs 22.2.1, visiting a git-versioned file, when I do
`vc-print-log', point always moves to the end of the buffer
instead of the beginning.  This worked for me:
    
    (defun vc-git-rev-parse (commit)
      "Translate COMMIT string into full SHA1 object name.
    Returns nil if not possible."
      (and commit
           (with-temp-buffer
    	 (and
    	  (zerop
    	   (call-process "git" nil '(t nil) nil "rev-parse"
    			 "--verify"
    			 commit))
    	  (goto-char (point-min))
    	  (= (forward-line 2) 1)
    	  (bolp)
    	  (buffer-substring-no-properties (point-min) (1- (point-max)))))))
    
    (defun vc-git-show-log-entry (rev)
      (log-view-goto-rev (vc-git-rev-parse rev)))      

I looked at CVS HEAD and noticed a new vc-git-show-log-entry and
some recent modifications.  I believe the vc-git.el:1.72 version
would do the right thing for my use case, but I suspect the r1.73
change:
    * vc-git.el (vc-git-show-log-entry): Include the revision in
      the search string.
breaks it.  It looks like that function was supposed to parse
things like "HEAD~5", "HEAD^^" into a search repeat count, but the
1.73 change changed the intention of the search.

Personally, I like the vc-git-rev-parse solution better.  It
handles every method of specifying commits, the "git way".

vc-git-rev-parse can be refactored with vc-git-symbolic-commit, on
which it's based.

Karl

Re: vc-git-show-log-entry

[Eric Hanchrow]

>>>>> "Karl" == Karl Chen <quarl@cs.berkeley.edu> writes:

    Karl> I looked at CVS HEAD and noticed a new vc-git-show-log-entry
    Karl> and some recent modifications.  I believe the vc-git.el:1.72
    Karl> version would do the right thing for my use case, but I
    Karl> suspect the r1.73 change: * vc-git.el (vc-git-show-log-entry):
    Karl> Include the revision in the search string.  breaks it.  

I'm pretty sure that change was mine << glances around nervously >>.  I
didn't really understand what I was doing with that change; I just stuck
something in there that seemed to work for me.  It sounds like you
understand vc-git better than I did.  I'll see if yours works for me; if
so, let's get your change in instead.

    Karl> Personally, I like the vc-git-rev-parse solution better.  It
    Karl> handles every method of specifying commits, the "git way".

Sounds good to me!

-- 
As economics is known as "The Miserable Science", software
engineering should be known as "The Doomed Discipline"
        -- Edsger Dijkstra

Re: vc-git-show-log-entry

[Eric Hanchrow]

Your patch seems to work, but I notice something I don't like:

* I typed C-x C-f /usr/local/src/emacs-via-git/lisp/vc-git.el RET,
  visiting a file that is part of a git working tree (by an astounding
  coincidence, it's the very file we're hacking!)
* I typed C-x v g to annotate the file
* I moved point to a  more or less random line, and typed "l"

I did indeed see point move to that line's commit in the *vc-change-log*
buffer ... albeit slowly (presumably since it takes git-log rather a
long time to run).  What's really bad is that the next time I typed "l"
in the same annotated buffer, it _again_ took a long time; I think it
re-ran "git log" even though there was already a perfectly good
*vc-change-log* buffer.

I haven't figured out why this is happening.

My version seems to have this problem, too, for what it's worth.
-- 
Asking the Iraqi people to assume Saddam's debts
is rather like telling a man who has been shot in the head
that he has to pay for the bullet.
        -- James Surowiecki

Re: vc-git-show-log-entry

[Dan Nicolaescu]

Karl Chen <quarl@cs.berkeley.edu> writes:

  > Hi,
  > 
  > In Emacs 22.2.1, visiting a git-versioned file, when I do
  > `vc-print-log', point always moves to the end of the buffer
  > instead of the beginning.  This worked for me:
  >     
  >     (defun vc-git-rev-parse (commit)
  >       "Translate COMMIT string into full SHA1 object name.
  >     Returns nil if not possible."
  >       (and commit
  >            (with-temp-buffer
  >     	 (and
  >     	  (zerop
  >     	   (call-process "git" nil '(t nil) nil "rev-parse"
  >     			 "--verify"
  >     			 commit))
  >     	  (goto-char (point-min))
  >     	  (= (forward-line 2) 1)
  >     	  (bolp)
  >     	  (buffer-substring-no-properties (point-min) (1- (point-max)))))))
  >     
  >     (defun vc-git-show-log-entry (rev)
  >       (log-view-goto-rev (vc-git-rev-parse rev)))      
  > 
  > I looked at CVS HEAD and noticed a new vc-git-show-log-entry and
  > some recent modifications.  I believe the vc-git.el:1.72 version
  > would do the right thing for my use case, but I suspect the r1.73
  > change:
  >     * vc-git.el (vc-git-show-log-entry): Include the revision in
  >       the search string.
  > breaks it.  It looks like that function was supposed to parse
  > things like "HEAD~5", "HEAD^^" into a search repeat count, but the
  > 1.73 change changed the intention of the search.
  > 
  > Personally, I like the vc-git-rev-parse solution better.  It
  > handles every method of specifying commits, the "git way".
  > 
  > vc-git-rev-parse can be refactored with vc-git-symbolic-commit, on
  > which it's based.

It's hard to say what problem you are trying to report.

Can you please try again with a step by step description of what you are
doing, what are the results you are getting and what are the expected
results. Please start from
emacs -Q
Please use emacs from CVS HEAD.  It's unlikely that another release will
be made from the 22.x branch.

Re: vc-git-show-log-entry

[Karl Chen]

>>>>> On 2008-10-20 11:19 PDT, Dan Nicolaescu writes:

    Dan> Karl Chen <quarl@cs.berkeley.edu> writes:
    >> visiting a git-versioned file, when I do `vc-print-log',
    >> point always moves to the end of the buffer instead of the
    >> beginning.

    Dan> It's hard to say what problem you are trying to report.

    Dan> Can you please try again with a step by step description
    Dan> of what you are doing, what are the results you are
    Dan> getting and what are the expected results. Please start
    Dan> from emacs -Q Please use emacs from CVS HEAD.  It's
    Dan> unlikely that another release will be made from the 22.x
    Dan> branch.

git init
date > f1 ; git add *; git commit -m commit1
date >> f1 ; git add *; git commit -m commit2
emacs -Q f1
C-x C-v l

; point moves to the end of the *vc-change-log* buffer instead of
; the beginning.


As I said, if you undo the latest commit to vc-git.el, that fixes
it, but I suggest this replacement altogether.

    (defun vc-git-show-log-entry (rev)
      (log-view-goto-rev (vc-git-rev-parse rev)))      

Re: vc-git-show-log-entry

[Dan Nicolaescu]

Karl Chen <quarl@cs.berkeley.edu> writes:

  > >>>>> On 2008-10-20 11:19 PDT, Dan Nicolaescu writes:
  > 
  >     Dan> Karl Chen <quarl@cs.berkeley.edu> writes:
  >     >> visiting a git-versioned file, when I do `vc-print-log',
  >     >> point always moves to the end of the buffer instead of the
  >     >> beginning.
  > 
  >     Dan> It's hard to say what problem you are trying to report.
  > 
  >     Dan> Can you please try again with a step by step description
  >     Dan> of what you are doing, what are the results you are
  >     Dan> getting and what are the expected results. Please start
  >     Dan> from emacs -Q Please use emacs from CVS HEAD.  It's
  >     Dan> unlikely that another release will be made from the 22.x
  >     Dan> branch.
  > 
  > git init
  > date > f1 ; git add *; git commit -m commit1
  > date >> f1 ; git add *; git commit -m commit2
  > emacs -Q f1
  > C-x C-v l

I assume this is: C-x v l

  > ; point moves to the end of the *vc-change-log* buffer instead of
  > ; the beginning.

It moves to the beginning for me.  What version of emacs are you using?

  > As I said, if you undo the latest commit to vc-git.el, that fixes
  > it, but I suggest this replacement altogether.
  > 
  >     (defun vc-git-show-log-entry (rev)
  >       (log-view-goto-rev (vc-git-rev-parse rev)))      

Not sure if adding yet another git invocation here is a good idea.
Please run your change by Alexandre Julliard.

Re: vc-git-show-log-entry

[Karl Chen]

>>>>> On 2008-10-20 15:10 PDT, Dan Nicolaescu writes:

    Dan> It moves to the beginning for me.  What version of emacs
    Dan> are you using?

22.2, and I also mentioned what happens with CVS HEAD in my first
email (though I only tested the relevant functions, didn't build
the entire thing).  It only happens 90% of the time for me, I
think because of a race condition from running the subprocess
asynchronously.

    >> As I said, if you undo the latest commit to vc-git.el, that
    >> fixes it, but I suggest this replacement altogether.
    >> 
    >> (defun vc-git-show-log-entry (rev) (log-view-goto-rev
    >> (vc-git-rev-parse rev)))

    Dan> Not sure if adding yet another git invocation here is a
    Dan> good idea.  Please run your change by Alexandre Julliard.

Ah, I assumed you were the one to talk to since you made the
change and you replied to my email.  Is Alexandre not on this
mailing list?

Re: vc-git-show-log-entry

[Dan Nicolaescu]

Karl Chen <quarl@cs.berkeley.edu> writes:

  > >>>>> On 2008-10-20 15:10 PDT, Dan Nicolaescu writes:
  > 
  >     Dan> It moves to the beginning for me.  What version of emacs
  >     Dan> are you using?
  > 
  > 22.2, and I also mentioned what happens with CVS HEAD in my first
  > email (though I only tested the relevant functions, didn't build
  > the entire thing).  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   ^^^^^^^^^^^^^^^^^^
Please don't do that.
As I said, if you find a problem with CVS HEAD, please report it, with
detailed step by step instructions on how to reproduce it.

Re: vc-git-show-log-entry

[Alexandre Julliard]

Dan Nicolaescu <dann@ics.uci.edu> writes:

> Karl Chen <quarl@cs.berkeley.edu> writes:
>   > As I said, if you undo the latest commit to vc-git.el, that fixes
>   > it, but I suggest this replacement altogether.
>   > 
>   >     (defun vc-git-show-log-entry (rev)
>   >       (log-view-goto-rev (vc-git-rev-parse rev)))      
>
> Not sure if adding yet another git invocation here is a good idea.
> Please run your change by Alexandre Julliard.

It's definitely a good idea. There's just no way to reliably convert a
symbolic revision name to a number of commits to skip, the current
vc-git-show-log-entry implementation is clearly broken. The only sane
way is to use git rev-parse and then look for the sha1.

-- 
Alexandre Julliard
julliard@winehq.org