git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Narebski <jnareb@gmail.com>
To: "Rafael Garcia-Suarez" <rgarciasuarez@gmail.com>
Cc: git@vger.kernel.org, "Luben Tuikov" <ltuikov@yahoo.com>,
	"Sam Vilain" <sam@vilain.net>
Subject: Re: [PATCH] Avoid errors from git-rev-parse in gitweb blame
Date: Tue, 3 Jun 2008 19:50:38 +0200	[thread overview]
Message-ID: <200806031950.39602.jnareb@gmail.com> (raw)
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

>>>> 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?
 
>> [...]
>>>> [...].  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

  reply	other threads:[~2008-06-03 17:51 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-03 10:46 [PATCH] Avoid errors from git-rev-parse in gitweb blame Rafael Garcia-Suarez
2008-06-03 11:42 ` Lea Wiemann
2008-06-03 11:43 ` Jakub Narebski
2008-06-03 12:03   ` Rafael Garcia-Suarez
2008-06-03 12:45     ` Jakub Narebski
2008-06-03 13:00       ` Rafael Garcia-Suarez
2008-06-03 13:12         ` Jakub Narebski
2008-06-03 13:36           ` Rafael Garcia-Suarez
2008-06-03 14:14             ` Jakub Narebski
2008-06-03 14:40               ` Rafael Garcia-Suarez
2008-06-03 14:56                 ` Jakub Narebski
2008-06-03 15:07                   ` Rafael Garcia-Suarez
2008-06-03 17:50                     ` Jakub Narebski [this message]
2008-06-03 21:09                   ` Luben Tuikov
2008-06-03 21:03               ` Luben Tuikov
2008-06-03 20:35         ` Luben Tuikov
2008-06-03 21:31           ` Jakub Narebski
2008-06-04  5:58             ` Junio C Hamano
2008-06-04 14:03               ` Jakub Narebski
2008-06-05  6:07                 ` Junio C Hamano
2008-06-05  6:09                   ` [PATCH 1/2] git-blame: refactor code to emit "porcelain format" output Junio C Hamano
2008-06-06  9:22                     ` Jakub Narebski
2008-06-05  6:09                   ` [PATCH 2/2] blame: show "previous" information in --porcelain/--incremental format Junio C Hamano
2008-06-06  9:27                     ` Jakub Narebski
2008-06-06 15:17                       ` Junio C Hamano
2008-06-06 15:44                         ` Jakub Narebski
2008-06-06  0:26                   ` [PATCH] Avoid errors from git-rev-parse in gitweb blame Jakub Narebski
2008-06-04 22:24               ` Luben Tuikov
2008-06-03 14:24       ` Lea Wiemann
2008-06-03 20:24         ` Jakub Narebski
2008-06-03 23:11           ` Lea Wiemann
2008-06-04  0:11             ` Jakub Narebski
2008-06-04  0:39               ` Lea Wiemann
2008-06-04 12:31                 ` Jakub Narebski
2008-06-08 18:19             ` Lea Wiemann
2008-06-08 20:28               ` Jakub Narebski
2008-06-03 20:18   ` Luben Tuikov
2008-06-03 20:29     ` Jakub Narebski
2008-06-03 21:27       ` Luben Tuikov
2008-06-03 21:34         ` Jakub Narebski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200806031950.39602.jnareb@gmail.com \
    --to=jnareb@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=ltuikov@yahoo.com \
    --cc=rgarciasuarez@gmail.com \
    --cc=sam@vilain.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).