{"thread":{"id":"29139","subject":"Git blame only current branch","startedAt":"2011-12-12T15:24:47Z","lastAt":"2011-12-13T17:25:28Z","messageCount":10,"participants":["Stephen Bash","Jeff King","andreas.t.auer_gtml_37453@ursus.ath.cx","Vijay Lakshminarayanan","Junio C Hamano","Frans Klaver"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"180931","messageId":"d615954f-bed8-482d-a2e3-e1e741d6dd23@mail","threadId":"29139","inReplyTo":"e9e35956-a091-4143-8fd4-3516b54263a6@mail","subject":"Git blame only current branch","fromName":"Stephen Bash","fromEmail":"bash@genarts.com","sentAt":"2011-12-12T15:24:47Z","receivedAt":"2011-12-12T15:24:47Z","isPatch":false,"sender":{"key":"bash@genarts.com","avatar":null},"body":"Hi all-\n\nI'm curious if there's a method to make git blame merge commits that introduce code to the given branch rather than commits on the original (topic) branch? For example:\n\n A--B--C---M--D master\n \\ /\n 1--2--3 topicA\n\nI'd like a mode where 'git blame master ...' shows commit M for lines changed by topicA so I can easily do 'git blame M^ ...' to see changes (on master) prior to the merge of topicA. Unfortunately 'git blame master ^topicA ...' blames all the changes of topicA to 3 (which reading the docs appears to be the \"correct\" behavior).\n\nThanks!\n\nStephen\n"},{"id":"180934","messageId":"20111212165542.GA4802@sigill.intra.peff.net","threadId":"29139","inReplyTo":"d615954f-bed8-482d-a2e3-e1e741d6dd23@mail","subject":"Re: Git blame only current branch","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-12T16:55:42Z","receivedAt":"2011-12-12T16:55:42Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 12, 2011 at 10:24:47AM -0500, Stephen Bash wrote:\n\n> I'm curious if there's a method to make git blame merge commits that\n> introduce code to the given branch rather than commits on the original\n> (topic) branch? For example:\n\nUsually when you are interested in seeing merges like this in git-log,\nyou would use one of \"--first-parent\" or \"--merges\". However, though\n\"git blame\" takes revision arguments, it does its own traversal of the\ngraph that does not respect those options.\n\nModifying it to do --first-parent is pretty easy:\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 80febbe..c19a8cd 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -1191,6 +1191,8 @@ static int num_scapegoats(struct rev_info *revs, struct commit *commit)\n {\n \tint cnt;\n \tstruct commit_list *l = first_scapegoat(revs, commit);\n+\tif (revs->first_parent_only)\n+\t\treturn l ? 1 : 0;\n \tfor (cnt = 0; l; l = l->next)\n \t\tcnt++;\n \treturn cnt;\n\nWith that, \"git blame --first-parent\" produces reasonable results for\nme. But of course I didn't do more than 30 seconds of testing, so it is\nentirely possible there are corner cases or unforeseen side effects.\n\nHandling --merges is probably a little trickier, as you need to consider\nonly some commits as scapegoats, but still traverse through everything\nto find the merges.\n\n-Peff\n"},{"id":"180935","messageId":"5e2440c1-8d11-4d92-b42f-14169a62ced1@mail","threadId":"29139","inReplyTo":"20111212165542.GA4802@sigill.intra.peff.net","subject":"Re: Git blame only current branch","fromName":"Stephen Bash","fromEmail":"bash@genarts.com","sentAt":"2011-12-12T17:05:25Z","receivedAt":"2011-12-12T17:05:25Z","isPatch":false,"sender":{"key":"bash@genarts.com","avatar":null},"body":"----- Original Message -----\n> From: \"Jeff King\" \n> Sent: Monday, December 12, 2011 11:55:42 AM\n> Subject: Re: Git blame only current branch\n> \n> On Mon, Dec 12, 2011 at 10:24:47AM -0500, Stephen Bash wrote:\n> \n> > I'm curious if there's a method to make git blame merge commits\n> > that introduce code to the given branch rather than commits on \n> > the original (topic) branch? For example:\n> \n> Usually when you are interested in seeing merges like this in\n> git-log, you would use one of \"--first-parent\" or \"--merges\". \n> However, though \"git blame\" takes revision arguments, it does\n> its own traversal of the graph that does not respect those \n> options.\n\nMy first thought was --first-parent, and was disappointed when I didn't find it in the blame documentation :) I think for my purposes --first-parent is better than --merges because there are non-merge commits on the branch(es) of interest (and thus I think the problem would become ill-posed in the --merges case).\n\n> Modifying it to do --first-parent is pretty easy:\n> ... snip ...\n\nThat's pretty simple... I'll try to do a little testing this afternoon.\n\nThanks!\nStephen\n"},{"id":"180938","messageId":"4EE63799.6020409@ursus.ath.cx","threadId":"29139","inReplyTo":"5e2440c1-8d11-4d92-b42f-14169a62ced1@mail","subject":"Re: Git blame only current branch","fromName":"","fromEmail":"andreas.t.auer_gtml_37453@ursus.ath.cx","sentAt":"2011-12-12T17:19:21Z","receivedAt":"2011-12-12T17:19:21Z","isPatch":false,"sender":{"key":"andreas.t.auer_gtml_37453@ursus.ath.cx","avatar":null},"body":"\n\nOn 12.12.2011 18:05 Stephen Bash wrote:\n> ----- Original Message -----\n> > From: \"Jeff King\" Sent: Monday, December 12, 2011\n> > 11:55:42 AM Subject: Re: Git blame only current branch\n> >\n> > On Mon, Dec 12, 2011 at 10:24:47AM -0500, Stephen Bash wrote:\n> >\n> > Usually when you are interested in seeing merges like this in\n> > git-log, you would use one of \"--first-parent\" or \"--merges\".\n> > However, though \"git blame\" takes revision arguments, it does its\n> > own traversal of the graph that does not respect those options.\n>\n> My first thought was --first-parent, and was disappointed when I\n> didn't find it in the blame documentation :) I think for my purposes\n> --first-parent is better than --merges because there are non-merge\n> commits on the branch(es) of interest (and thus I think the problem\n> would become ill-posed in the --merges case).\n>\n> > Modifying it to do --first-parent is pretty easy: ... snip ...\n>\n> That's pretty simple... I'll try to do a little testing this\n> afternoon.\n\nYou might need to consider that if the master branch was first merged \ninto topicA before topicA was merged back to the master that the master \nwould only be fast-forwarded and so the first parent of M would be 3 not \nC. So depending how the developers merged you might get different results.\n"},{"id":"181014","messageId":"8739cpteat.fsf@gmail.com","threadId":"29139","inReplyTo":"20111212165542.GA4802@sigill.intra.peff.net","subject":"Re: Git blame only current branch","fromName":"Vijay Lakshminarayanan","fromEmail":"laksvij@gmail.com","sentAt":"2011-12-13T02:07:22Z","receivedAt":"2011-12-13T02:07:22Z","isPatch":false,"sender":{"key":"laksvij@gmail.com","avatar":null},"body":"Jeff King writes:\n\n[snip]\n\n> diff --git a/builtin/blame.c b/builtin/blame.c\n> index 80febbe..c19a8cd 100644\n> --- a/builtin/blame.c\n> +++ b/builtin/blame.c\n> @@ -1191,6 +1191,8 @@ static int num_scapegoats(struct rev_info *revs, struct commit *commit)\n> {\n> \tint cnt;\n> \tstruct commit_list *l = first_scapegoat(revs, commit);\n> +\tif (revs->first_parent_only)\n> +\t\treturn l ? 1 : 0;\n> \tfor (cnt = 0; l; l = l->next)\n> \t\tcnt++;\n> \treturn cnt;\n\nI just spent 30s staring at this wondering why you needed to do \n\n return 1 ? 1 : 0;\n\nwhich always returns 1 anyway before I realized it was a lowercase L.\n\nThe code reads fine when there's no numeral 1 around but now it doesn't\nread well. I think refactoring\n\n struct commit_list *l\n\nto \n\n struct commit_list *lst\n\nis justified. Thoughts?\n\n> -Peff\n\n-- \nCheers\n~vijay\n\nGnus should be more complicated.\n"},{"id":"181015","messageId":"20111213021442.GA4244@sigill.intra.peff.net","threadId":"29139","inReplyTo":"8739cpteat.fsf@gmail.com","subject":"Re: Git blame only current branch","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-12-13T02:14:42Z","receivedAt":"2011-12-13T02:14:42Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Dec 13, 2011 at 07:37:22AM +0530, Vijay Lakshminarayanan wrote:\n\n> > diff --git a/builtin/blame.c b/builtin/blame.c\n> > index 80febbe..c19a8cd 100644\n> > --- a/builtin/blame.c\n> > +++ b/builtin/blame.c\n> > @@ -1191,6 +1191,8 @@ static int num_scapegoats(struct rev_info *revs, struct commit *commit)\n> > {\n> > \tint cnt;\n> > \tstruct commit_list *l = first_scapegoat(revs, commit);\n> > +\tif (revs->first_parent_only)\n> > +\t\treturn l ? 1 : 0;\n> > \tfor (cnt = 0; l; l = l->next)\n> > \t\tcnt++;\n> > \treturn cnt;\n> \n> I just spent 30s staring at this wondering why you needed to do \n> \n> return 1 ? 1 : 0;\n> \n> which always returns 1 anyway before I realized it was a lowercase L.\n> \n> The code reads fine when there's no numeral 1 around but now it doesn't\n> read well. I think refactoring\n> \n> struct commit_list *l\n> \n> to \n> \n> struct commit_list *lst\n> \n> is justified. Thoughts?\n\nSure, that would help. I wasn't planning to push this forward as a\n\"real\" patch, but if somebody wants to do some testing and, more\nimportantly read through the code to make sure I am not violating some\nassumptions, then it might be worth including upstream.\n\n-Peff\n"},{"id":"181022","messageId":"7vobvdvx9c.fsf@alter.siamese.dyndns.org","threadId":"29139","inReplyTo":"8739cpteat.fsf@gmail.com","subject":"Re: Git blame only current branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-13T05:47:11Z","receivedAt":"2011-12-13T05:47:11Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vijay Lakshminarayanan writes:\n\n> The code reads fine when there's no numeral 1 around but now it doesn't\n> read well. I think refactoring\n>\n> struct commit_list *l\n>\n> to \n>\n> struct commit_list *lst\n>\n> is justified. Thoughts?\n\nNot justified at all.\n\nWhat is \"lst\" and why is it not spelled \"list\"? It is a disease to drop\nvowels when you do not have to.\n\nIf I were to name a new variable that points at one element of a linked\nlist and is used to walk the list (surprise!) \"element\" or perhaps \"elem\"\nfor short, but in the context of that short function I honestly do not see\nmuch need for such a naming. The variable is extremely short-lived and\nthere is no room for confusion.\n"},{"id":"181038","messageId":"87y5ugsguj.fsf@gmail.com","threadId":"29139","inReplyTo":"7vobvdvx9c.fsf@alter.siamese.dyndns.org","subject":"Re: Git blame only current branch","fromName":"Vijay Lakshminarayanan","fromEmail":"laksvij@gmail.com","sentAt":"2011-12-13T14:09:56Z","receivedAt":"2011-12-13T14:09:56Z","isPatch":false,"sender":{"key":"laksvij@gmail.com","avatar":null},"body":"Junio C Hamano writes:\n\n> Vijay Lakshminarayanan writes:\n>\n>> The code reads fine when there's no numeral 1 around but now it doesn't\n>> read well. I think refactoring\n>>\n>> struct commit_list *l\n>>\n>> to \n>>\n>> struct commit_list *lst\n>>\n>> is justified. Thoughts?\n>\n> Not justified at all.\n>\n> What is \"lst\" and why is it not spelled \"list\"? It is a disease to drop\n> vowels when you do not have to.\n\nlst is better than l in this particular context. I think fried_chicken\nis better than l in this particular context ;-)\n\n> If I were to name a new variable that points at one element of a linked\n> list and is used to walk the list (surprise!) \"element\" or perhaps \"elem\"\n> for short, but in the context of that short function I honestly do not see\n> much need for such a naming. The variable is extremely short-lived and\n> there is no room for confusion.\n\nBefore the introduction of the numeral 1, I am in complete agreement\nwith you for the exact reasons you've mentioned above. Post\nintroduction of \"l ? 1 : 0\" it warrants a refactoring. It's possible\nyou're using a different font so you never encounter the issue, but this\ndefinitely isn't a problem I alone face. For instance, it is a\nsufficiently common problem that it's one of the \"Java Puzzlers\" in Josh\nBloch's book of the same name. (Yes, elem is better than lst.)\n\nMy $0.02.\n\n-- \nCheers\n~vijay\n\nGnus should be more complicated.\n"},{"id":"181040","messageId":"op.v6fl05u30aolir@keputer","threadId":"29139","inReplyTo":"87y5ugsguj.fsf@gmail.com","subject":"Re: Git blame only current branch","fromName":"Frans Klaver","fromEmail":"fransklaver@gmail.com","sentAt":"2011-12-13T14:18:31Z","receivedAt":"2011-12-13T14:18:31Z","isPatch":false,"sender":{"key":"fransklaver@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1876483?v=4"},"body":"On Tue, 13 Dec 2011 15:09:56 +0100, Vijay Lakshminarayanan \n wrote:\n\n> Junio C Hamano writes:\n>\n>> Vijay Lakshminarayanan writes:\n>>\n>>> The code reads fine when there's no numeral 1 around but now it doesn't\n>>> read well. I think refactoring\n>>>\n>>> struct commit_list *l\n>>>\n>>> to\n>>>\n>>> struct commit_list *lst\n>>>\n>>> is justified. Thoughts?\n>>\n>> Not justified at all.\n>>\n>> What is \"lst\" and why is it not spelled \"list\"? It is a disease to drop\n>> vowels when you do not have to.\n>\n> lst is better than l in this particular context. I think fried_chicken\n> is better than l in this particular context ;-)\n\nI tend to agree. If you casually look over the code it may look odd, and \nwith several monospace fonts there really isn't a very big difference \nbetween 1?1:0 and l?1:0. You shouldn't have to squint to properly see the \nintention of the code.\n\nIf there's going to be a rename, there is no reason to leave out the i \nthough.\n"},{"id":"181051","messageId":"7vd3bswfhz.fsf@alter.siamese.dyndns.org","threadId":"29139","inReplyTo":"87y5ugsguj.fsf@gmail.com","subject":"Re: Git blame only current branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-13T17:25:28Z","receivedAt":"2011-12-13T17:25:28Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vijay Lakshminarayanan writes:\n\n> Before the introduction of the numeral 1, I am in complete agreement\n> with you for the exact reasons you've mentioned above. Post\n> introduction of \"l ? 1 : 0\" it warrants a refactoring.\n\nIf your main point is that \"return l ? 1 : 0;\", then a better thing to do\nwould be to use a well-known idiom to turn anything into a boolean, i.e.\n\n\treturn !!l;\n\nand your problem is solved without any renaming (we are not talking about\nany \"refactoring\" that changes code structure).\n\nI've seen enough bikeshedding, so I'd stop after pointing you in the right\ndirection by mentioning \"git grep -e ''\" and \"git grep -e ''\".\n"}]}