On Thu, May 23, 2013 at 11:54 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Felipe Contreras <felipe.contreras@xxxxxxxxx> writes: > >> On Wed, May 22, 2013 at 11:07 PM, Felipe Contreras >> <felipe.contreras@xxxxxxxxx> wrote: >>> On Wed, May 22, 2013 at 7:08 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: >>>> Felipe Contreras <felipe.contreras@xxxxxxxxx> writes: >>>> >>>>>> IIRC, git-gui runs two blames, one without any -C and one with (I do >>>>>> not offhand recall how many -C it uses) to show both. >>>>> >>>>> 'git blame' is a very expensive operation, perhaps we should add >>>>> another option so users don't need to run two blames to find this. >>>> >>>> Yes, you could add a "struct origin *suspect_in_the_same_file" next >>>> to the current "struct origin *suspect" to the blame_entry in order >>>> to represent the origin you would get if you were to stop at a "move >>>> across files" event, and keep digging, when you are operating under >>>> "-C -C" or higher mode. >>> >>> No, this is what I meant: >> >> But that would not print the right line numbers, so perhaps: > > Users of full-output may want to be able to see the same thing. > > I have a suspicion that you misread what assign_blame() does. The > first inner loop to find one "suspect" is really what it says. It > loops over blame entries in the scoreboard but it is not about > finding one "entry", but is to find one "suspect". The "ent" found > in the loop that you store in tmp_ent is no more special than any > other "ent" that shares the same "suspect" as its origin. It is stored because it's the only structure that has the line numbers. If the blame is passed to another 'ent', the original line numbers are gone. We could manually copy the line numbers to three variables, or we could copy the whole thing to one variable. The rest of the 'ent' doesn't matter. > Imagine that your scoreboard originally has three blocks of text > (i.e. blame_entry) A, B, and C, and the current suspect for A and C > are the same, while we already know what the final guilty party for > B is (which may be some descendant of the "suspect"). I don't see how that's possible, but whatever. > Once we find one "suspect" in the first inner loop, the call to > pass_blame() does everything it can do to exonerate that "suspect". > > It runs a single diff between the suspect and each of its parents to > find lines in both A and C that can be blamed to one of these > parents, and blame entries A and C are split into pieces, some of > which still have the same suspect as their origin, which are where > their lines originate from, while others are attributable to these > parents or their ancestors. > > If you keep the original entry for A to do something special, like > printing what the original range of A was before it was split by > pass_blame(), wouldn't you need to do the same for C? tmp_ent is only used when there's no entry taken the guilt away from A. If the blame entry of A is split, and the result of the split has a different filename, found_guilty() would not be called, and thus we won't show the blame entry, and we want to show it. And yes, we would need to do the same for C, and we would once the turn of C comes along. If it's possible that A descendant from A takes the blame for C, then sure, we would need to do ensure C is printed before it's gone, but pass_blame(A) would not destroy C. > We will not > be visiting C later in the outer while(1) loop, as a single call to > pass_blame() for "suspect" we did when we found it in A has already > taken care of it. It took care of A's suspect, but not C. Isn't it possible that C is split further and creates another blame entry? Either way, in --incremental we want to print C as well, whether it's the guilty one or not. > I am not sure what you are trying to achieve with that found-guilty > logic, even if the "save away in tmp_ent" logic is changed to keep > all the blame entries that have "suspect" we are looking at as their > origin. > When the current suspect is found to have passed all lines > intact from its parents, we will see found-guilty left to be false. What? When a 'blame entry' passes the whole blame to a parent, found_guilty is false, so we print the entry, which is exactly what we want to do. > How would it make the original blame_entry (perhaps halfway) guilty > in that situation? I thought 'guilty' meant that it was guilty of adding those lines to the 'final' text, so it ends up in the non-incremental blame. In that sense those blame entries are not guilty. But they are still blame entries, and we want to print all blame entries in incremental, guilty or not. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html