Felipe Contreras [off-list ref] writes:
quoted
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.
That would make the result just as expensive as a single run of "-C
-C" (or higher mode) [*1*] without having to run an extra "git
blame" without any -C (even though that mode is much cheaper).
Representing the result for a terminal with reasonable width might
become unwieldy, but for --porcelain output format that should not
be a problem.
[Footnote]
*1* It would make it slightly more expensive than the current code,
as it is very likely that you have to split a single blame_entry
into many separate pieces.
On Wed, May 22, 2013 at 7:08 PM, Junio C Hamano [off-list ref] wrote:
Felipe Contreras [off-list ref] writes:
quoted
quoted
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:
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1509,16 +1509,6 @@ static void found_guilty_entry(struct blame_entry *ent)
if (ent->guilty)
return;
ent->guilty = 1;
- if (incremental) {
- struct origin *suspect = ent->suspect;
-
- printf("%s %d %d %d\n",
- sha1_to_hex(suspect->commit->object.sha1),
- ent->s_lno + 1, ent->lno + 1, ent->num_lines);
- emit_one_suspect_detail(suspect, 0);
- write_filename_info(suspect->path);
- maybe_flush_or_die(stdout, "stdout");
- }
}
/*@@ -1536,12 +1526,24 @@ static void assign_blame(struct scoreboard *sb, int opt)
struct origin *suspect = NULL;
/* find one suspect to break down */
- for (ent = sb->ent; !suspect && ent; ent = ent->next)
- if (!ent->guilty)
+ for (ent = sb->ent; ent; ent = ent->next)
+ if (!ent->guilty) {
suspect = ent->suspect;
+ break;
+ }
+
if (!suspect)
return; /* all done */
+ if (incremental &&!is_null_sha1(suspect->commit->object.sha1)) {
+ printf("%s %d %d %d\n",
+
sha1_to_hex(suspect->commit->object.sha1),
+ ent->s_lno + 1, ent->lno + 1,
ent->num_lines);
+ emit_one_suspect_detail(suspect, 0);
+ write_filename_info(suspect->path);
+ maybe_flush_or_die(stdout, "stdout");
+ }
+
/*
* We will use this suspect later in the loop,
* so hold onto it in the meantime.
--
Felipe Contreras
On Wed, May 22, 2013 at 11:07 PM, Felipe Contreras
[off-list ref] wrote:
On Wed, May 22, 2013 at 7:08 PM, Junio C Hamano [off-list ref] wrote:
quoted
Felipe Contreras [off-list ref] writes:
quoted
quoted
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:
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1500,25 +1500,16 @@ static int emit_one_suspect_detail(struct
origin *suspect, int repeat)
return 1;
}
-/*
- * The blame_entry is found to be guilty for the range. Mark it
- * as such, and show it in incremental output.
- */
-static void found_guilty_entry(struct blame_entry *ent)
+static void print_guilty_entry(struct blame_entry *ent)
{
- if (ent->guilty)
- return;
- ent->guilty = 1;
- if (incremental) {
- struct origin *suspect = ent->suspect;
-
- printf("%s %d %d %d\n",
- sha1_to_hex(suspect->commit->object.sha1),
- ent->s_lno + 1, ent->lno + 1, ent->num_lines);
- emit_one_suspect_detail(suspect, 0);
- write_filename_info(suspect->path);
- maybe_flush_or_die(stdout, "stdout");
- }
+ struct origin *suspect = ent->suspect;
+
+ printf("%s %d %d %d\n",
+ sha1_to_hex(suspect->commit->object.sha1),
+ ent->s_lno + 1, ent->lno + 1, ent->num_lines);
+ emit_one_suspect_detail(suspect, 0);
+ write_filename_info(suspect->path);
+ maybe_flush_or_die(stdout, "stdout");
}
/*@@ -1531,14 +1522,19 @@ static void assign_blame(struct scoreboard *sb, int opt)
struct rev_info *revs = sb->revs;
while (1) {
- struct blame_entry *ent;
+ struct blame_entry *ent, tmp_ent;
struct commit *commit;
struct origin *suspect = NULL;
+ int found_guilty = 0;
/* find one suspect to break down */
- for (ent = sb->ent; !suspect && ent; ent = ent->next)
- if (!ent->guilty)
+ for (ent = sb->ent; ent; ent = ent->next)
+ if (!ent->guilty) {
+ tmp_ent = *ent;
suspect = ent->suspect;
+ break;
+ }
+
if (!suspect)
return; /* all done */
@@ -1564,9 +1560,20 @@ static void assign_blame(struct scoreboard *sb, int opt)
commit->object.flags |= UNINTERESTING;
/* Take responsibility for the remaining entries */
- for (ent = sb->ent; ent; ent = ent->next)
- if (same_suspect(ent->suspect, suspect))
- found_guilty_entry(ent);
+ for (ent = sb->ent; ent; ent = ent->next) {
+ if (same_suspect(ent->suspect, suspect)) {
+ if (ent->guilty)
+ continue;
+ found_guilty = ent->guilty = 1;
+ if (incremental)
+ print_guilty_entry(ent);
+ }
+ }
+
+ if (incremental && !found_guilty &&
+ !is_null_sha1(suspect->commit->object.sha1))
+ print_guilty_entry(&tmp_ent);
+
origin_decref(suspect);
if (DEBUG) /* sanity */
--
Felipe Contreras