Re: [PATCH] Avoid errors from git-rev-parse in gitweb blame

From: Jakub Narebski <jnareb@gmail.com>
Date: Jun 3, 2008, 17:50 UTC
Message-ID: <200806031950.39602.jnareb@gmail.com>
In-Reply-To: <b77c1dce0806030807t7654ac2cm96aa06690c7a5c02@mail.gmail.com>

On Tue, 3 Jun 2008, Rafael Garcia-Suarez wrote:

> 2008/6/3 Jakub Narebski <jnareb@gmail.com>:

By the way, could you please try to not remove all but last quote attributions? It should be, I think, as simple as replying then removing unnecessary parts, instead of selecting parts you want reply to and then hitting reply. TIA

Show 26 quoted lines

>>>> By the way, what is the difference between '<<' links and 'br' link
>>>> in the above mentioned annotate/blame interface?
>>>
>>> "br" navigates to another branch from which this file has been
>>> integrated (in p4 speak.)
>>
>> Does it mark merge commits then? Or perhaps branch points?  What
>> does "branch from which this file has been integrated" mean in git
>> speak (in the terms of DAG of commits)?
>>
>>
>> If the history of a file looks like this
>>
>>       ....*---*---A---M---C...
>>                      /
>>           ....*---B-/
>>
>> and the line comes from "evil merge" M git-blame would return M as
>> blamed commit.  If the line comes from one or the other branch, from
>> commit A or B, it makes I think no difference to git-blame; git tries
>> to be "branch agnostic" (no special meaning to first parent; well,
>> besides rev~n notation and --first-parent walk option).  I guess it
>> is not the case in Perforce?
> 
> No, in perforce the branch you integrate changes from is always
> explicit.

So, in git speak, it means that 'br' means that blamed commit (commit 
which brought current version of given line) is not in first-parent 
line, and '<<' means that commit is in --first-parent history of a file 
(taking into account code copying and movement... err, at least in git 
case...), doesn't it?

Show 16 quoted lines

>> [...]
>>>> [...].  Will you want to use git-diff-tree
>>>> to mark differences from the version we came from (marked by 'hp',
>>>> 'hpb' and 'fp' URI parameters), or would you rather extend
>>>> git-blame? 
>>>
>>> I don't know. I'll look at git-diff-tree.
>>
>> What I meant here, would you plan on extending git-blame, or would
>> you use patchset (textual) diff between revision we are at, and
>> revision we came from.  git-diff-tree just compares two trees (and
>> have to have patch output explicitely enabled).  Sorry for the
>> confusion. 
> 
> I'm under the impression that extending git-blame is a more flexible
> solution.

I don't think that it is correct solution in this case. I'm not sure if it can even be done.

What you have (what "annotated file view" in Perforce web interface has) is difference annotations (one sided side-bys side diff ;-)), something like Eclipse QuickDiff, or like word-diff (or "git diff --color-words") put _ON TOP_ of blame info.

Generating such single pane in-file diff is orthogonal to generating blame info. I think it would be best solved using patchset (textual) diff output; if git-diff would support "context" and not only "unified" patch output it could be used there.

What was I thinking when mentioning extending git-blame was "reblame", i.e. blaming only those lines which are different from some child version. But while this would be very useful for tools such like "git gui blame" or blameview, it just won't work well I guess for web application (unless caching everything, and generating blame diff from cached blame).

As to extending git-blame --porcelain to output "parents <hash>..." header, it is better solution than "git rev-list --no-walk" used as a kind of git-rev-parse sequencer not only because it is one fork less (and blame has has this parent info anyway), but mainly that it better fits with the streaming flow of gitweb's git_blame2(). (I'll write about it more in separate letter).

-- 
Jakub Narebski
Poland