Skip to content

Commit 366175e

Browse files
Junio C HamanoLinus Torvalds
Junio C Hamano
authored and
Linus Torvalds
committed
[PATCH] Rework -B output.
Patch for a completely rewritten file detected by the -B flag was shown as a pair of creation followed by deletion in earlier versions. This was an misguided attempt to make reviewing such a complete rewrite easier, and unnecessarily ended up confusing git-apply. Instead, show the entire contents of old version prefixed with '-', followed by the entire contents of new version prefixed with '+'. This gives the same easy-to-review for human consumer while keeping it a single, regular modification patch for machine consumption, something that even GNU patch can grok. Signed-off-by: Junio C Hamano <junkio@cox.net> Signed-off-by: Linus Torvalds <torvalds@osdl.org>
1 parent 232b75a commit 366175e

File tree

4 files changed

+186
-95
lines changed

4 files changed

+186
-95
lines changed

‎Documentation/diffcore.txt‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,15 @@ like these:
191191
-B/60 (the same as above, since diffcore-break defautls to
192192
50%).
193193

194+
Note that earlier implementation left a broken pair as a separate
195+
creation and deletion patches. This was unnecessary hack and
196+
the latest implementation always merges all the broken pairs
197+
back into modifications, but the resulting patch output is
198+
formatted differently to still let the reviewing easier for such
199+
a complete rewrite by showing the entire contents of old version
200+
prefixed with '-', followed by the entire contents of new
201+
version prefixed with '+'.
202+
194203

195204
diffcore-pickaxe
196205
----------------

‎diff.c‎

Lines changed: 159 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,88 @@ static struct diff_tempfile {
8383
char tmp_path[50];
8484
} diff_temp[2];
8585

86+
static int count_lines(const char *filename)
87+
{
88+
FILE *in;
89+
int count, ch, completely_empty = 1, nl_just_seen = 0;
90+
in = fopen(filename, "r");
91+
count = 0;
92+
while ((ch = fgetc(in)) != EOF)
93+
if (ch == '\n') {
94+
count++;
95+
nl_just_seen = 1;
96+
completely_empty = 0;
97+
}
98+
else {
99+
nl_just_seen = 0;
100+
completely_empty = 0;
101+
}
102+
fclose(in);
103+
if (completely_empty)
104+
return 0;
105+
if (!nl_just_seen)
106+
count++; /* no trailing newline */
107+
return count;
108+
}
109+
110+
static void print_line_count(int count)
111+
{
112+
switch (count) {
113+
case 0:
114+
printf("0,0");
115+
break;
116+
case 1:
117+
printf("1");
118+
break;
119+
default:
120+
printf("1,%d", count);
121+
break;
122+
}
123+
}
124+
125+
static void copy_file(int prefix, const char *filename)
126+
{
127+
FILE *in;
128+
int ch, nl_just_seen = 1;
129+
in = fopen(filename, "r");
130+
while ((ch = fgetc(in)) != EOF) {
131+
if (nl_just_seen)
132+
putchar(prefix);
133+
putchar(ch);
134+
if (ch == '\n')
135+
nl_just_seen = 1;
136+
else
137+
nl_just_seen = 0;
138+
}
139+
fclose(in);
140+
if (!nl_just_seen)
141+
printf("\n\\ No newline at end of file\n");
142+
}
143+
144+
static void emit_rewrite_diff(const char *name_a,
145+
const char *name_b,
146+
struct diff_tempfile *temp)
147+
{
148+
/* Use temp[i].name as input, name_a and name_b as labels */
149+
int lc_a, lc_b;
150+
lc_a = count_lines(temp[0].name);
151+
lc_b = count_lines(temp[1].name);
152+
printf("--- %s\n+++ %s\n@@ -", name_a, name_b);
153+
print_line_count(lc_a);
154+
printf(" +");
155+
print_line_count(lc_b);
156+
printf(" @@\n");
157+
if (lc_a)
158+
copy_file('-', temp[0].name);
159+
if (lc_b)
160+
copy_file('+', temp[1].name);
161+
}
162+
86163
static void builtin_diff(const char *name_a,
87164
const char *name_b,
88165
struct diff_tempfile *temp,
89-
const char *xfrm_msg)
166+
const char *xfrm_msg,
167+
int complete_rewrite)
90168
{
91169
int i, next_at, cmd_size;
92170
const char *diff_cmd = "diff -L'%s%s' -L'%s%s'";
@@ -149,12 +227,16 @@ static void builtin_diff(const char *name_a,
149227
}
150228
if (xfrm_msg && xfrm_msg[0])
151229
puts(xfrm_msg);
152-
153230
if (strncmp(temp[0].mode, temp[1].mode, 3))
154231
/* we do not run diff between different kind
155232
* of objects.
156233
*/
157234
exit(0);
235+
if (complete_rewrite) {
236+
fflush(NULL);
237+
emit_rewrite_diff(name_a, name_b, temp);
238+
exit(0);
239+
}
158240
}
159241
fflush(NULL);
160242
execlp("/bin/sh","sh", "-c", cmd, NULL);
@@ -474,7 +556,8 @@ static void run_external_diff(const char *pgm,
474556
const char *other,
475557
struct diff_filespec *one,
476558
struct diff_filespec *two,
477-
const char *xfrm_msg)
559+
const char *xfrm_msg,
560+
int complete_rewrite)
478561
{
479562
struct diff_tempfile *temp = diff_temp;
480563
pid_t pid;
@@ -524,7 +607,8 @@ static void run_external_diff(const char *pgm,
524607
* otherwise we use the built-in one.
525608
*/
526609
if (one && two)
527-
builtin_diff(name, other ? : name, temp, xfrm_msg);
610+
builtin_diff(name, other ? : name, temp, xfrm_msg,
611+
complete_rewrite);
528612
else
529613
printf("* Unmerged path %s\n", name);
530614
exit(0);
@@ -547,29 +631,75 @@ static void run_external_diff(const char *pgm,
547631
remove_tempfile();
548632
}
549633

550-
static void run_diff(const char *name,
551-
const char *other,
552-
struct diff_filespec *one,
553-
struct diff_filespec *two,
554-
const char *xfrm_msg)
634+
static void run_diff(struct diff_filepair *p)
555635
{
556636
const char *pgm = external_diff();
637+
char msg_[PATH_MAX*2+200], *xfrm_msg;
638+
struct diff_filespec *one;
639+
struct diff_filespec *two;
640+
const char *name;
641+
const char *other;
642+
int complete_rewrite = 0;
643+
644+
if (DIFF_PAIR_UNMERGED(p)) {
645+
/* unmerged */
646+
run_external_diff(pgm, p->one->path, NULL, NULL, NULL, NULL,
647+
0);
648+
return;
649+
}
650+
651+
name = p->one->path;
652+
other = (strcmp(name, p->two->path) ? p->two->path : NULL);
653+
one = p->one; two = p->two;
654+
switch (p->status) {
655+
case 'C':
656+
sprintf(msg_,
657+
"similarity index %d%%\n"
658+
"copy from %s\n"
659+
"copy to %s",
660+
(int)(0.5 + p->score * 100.0/MAX_SCORE),
661+
name, other);
662+
xfrm_msg = msg_;
663+
break;
664+
case 'R':
665+
sprintf(msg_,
666+
"similarity index %d%%\n"
667+
"rename from %s\n"
668+
"rename to %s",
669+
(int)(0.5 + p->score * 100.0/MAX_SCORE),
670+
name, other);
671+
xfrm_msg = msg_;
672+
break;
673+
case 'M':
674+
if (p->score) {
675+
sprintf(msg_,
676+
"dissimilarity index %d%%",
677+
(int)(0.5 + p->score * 100.0/MAX_SCORE));
678+
xfrm_msg = msg_;
679+
complete_rewrite = 1;
680+
break;
681+
}
682+
/* fallthru */
683+
default:
684+
xfrm_msg = NULL;
685+
}
686+
557687
if (!pgm &&
558-
one && two &&
559688
DIFF_FILE_VALID(one) && DIFF_FILE_VALID(two) &&
560689
(S_IFMT & one->mode) != (S_IFMT & two->mode)) {
561690
/* a filepair that changes between file and symlink
562691
* needs to be split into deletion and creation.
563692
*/
564693
struct diff_filespec *null = alloc_filespec(two->path);
565-
run_external_diff(NULL, name, other, one, null, xfrm_msg);
694+
run_external_diff(NULL, name, other, one, null, xfrm_msg, 0);
566695
free(null);
567696
null = alloc_filespec(one->path);
568-
run_external_diff(NULL, name, other, null, two, xfrm_msg);
697+
run_external_diff(NULL, name, other, null, two, xfrm_msg, 0);
569698
free(null);
570699
}
571700
else
572-
run_external_diff(pgm, name, other, one, two, xfrm_msg);
701+
run_external_diff(pgm, name, other, one, two, xfrm_msg,
702+
complete_rewrite);
573703
}
574704

575705
void diff_setup(int flags)
@@ -693,26 +823,22 @@ static void diff_flush_raw(struct diff_filepair *p,
693823
die(err, p->two->path);
694824
}
695825

826+
if (p->score)
827+
sprintf(status, "%c%03d", p->status,
828+
(int)(0.5 + p->score * 100.0/MAX_SCORE));
829+
else {
830+
status[0] = p->status;
831+
status[1] = 0;
832+
}
696833
switch (p->status) {
697834
case 'C': case 'R':
698835
two_paths = 1;
699-
sprintf(status, "%c%03d", p->status,
700-
(int)(0.5 + p->score * 100.0/MAX_SCORE));
701836
break;
702837
case 'N': case 'D':
703838
two_paths = 0;
704-
if (p->score)
705-
sprintf(status, "%c%03d", p->status,
706-
(int)(0.5 + p->score * 100.0/MAX_SCORE));
707-
else {
708-
status[0] = p->status;
709-
status[1] = 0;
710-
}
711839
break;
712840
default:
713841
two_paths = 0;
714-
status[0] = p->status;
715-
status[1] = 0;
716842
break;
717843
}
718844
printf(":%06o %06o %s ",
@@ -763,55 +889,14 @@ int diff_unmodified_pair(struct diff_filepair *p)
763889

764890
static void diff_flush_patch(struct diff_filepair *p)
765891
{
766-
const char *name, *other;
767-
char msg_[PATH_MAX*2+200], *msg;
768-
769892
if (diff_unmodified_pair(p))
770893
return;
771894

772-
name = p->one->path;
773-
other = (strcmp(name, p->two->path) ? p->two->path : NULL);
774895
if ((DIFF_FILE_VALID(p->one) && S_ISDIR(p->one->mode)) ||
775896
(DIFF_FILE_VALID(p->two) && S_ISDIR(p->two->mode)))
776897
return; /* no tree diffs in patch format */
777898

778-
switch (p->status) {
779-
case 'C':
780-
sprintf(msg_,
781-
"similarity index %d%%\n"
782-
"copy from %s\n"
783-
"copy to %s",
784-
(int)(0.5 + p->score * 100.0/MAX_SCORE),
785-
p->one->path, p->two->path);
786-
msg = msg_;
787-
break;
788-
case 'R':
789-
sprintf(msg_,
790-
"similarity index %d%%\n"
791-
"rename from %s\n"
792-
"rename to %s",
793-
(int)(0.5 + p->score * 100.0/MAX_SCORE),
794-
p->one->path, p->two->path);
795-
msg = msg_;
796-
break;
797-
case 'D': case 'N':
798-
if (DIFF_PAIR_BROKEN(p)) {
799-
sprintf(msg_,
800-
"dissimilarity index %d%%",
801-
(int)(0.5 + p->score * 100.0/MAX_SCORE));
802-
msg = msg_;
803-
}
804-
else
805-
msg = NULL;
806-
break;
807-
default:
808-
msg = NULL;
809-
}
810-
811-
if (DIFF_PAIR_UNMERGED(p))
812-
run_diff(name, NULL, NULL, NULL, NULL);
813-
else
814-
run_diff(name, other, p->one, p->two, msg);
899+
run_diff(p);
815900
}
816901

817902
int diff_queue_is_empty(void)
@@ -972,8 +1057,10 @@ static void diffcore_apply_filter(const char *filter)
9721057
int found;
9731058
for (i = found = 0; !found && i < q->nr; i++) {
9741059
struct diff_filepair *p = q->queue[i];
975-
if ((p->broken_pair && strchr(filter, 'B')) ||
976-
(!p->broken_pair && strchr(filter, p->status)))
1060+
if (((p->status == 'M') &&
1061+
((p->score && strchr(filter, 'B')) ||
1062+
(!p->score && strchr(filter, 'M')))) ||
1063+
((p->status != 'M') && strchr(filter, p->status)))
9771064
found++;
9781065
}
9791066
if (found)
@@ -991,8 +1078,10 @@ static void diffcore_apply_filter(const char *filter)
9911078
/* Only the matching ones */
9921079
for (i = 0; i < q->nr; i++) {
9931080
struct diff_filepair *p = q->queue[i];
994-
if ((p->broken_pair && strchr(filter, 'B')) ||
995-
(!p->broken_pair && strchr(filter, p->status)))
1081+
if (((p->status == 'M') &&
1082+
((p->score && strchr(filter, 'B')) ||
1083+
(!p->score && strchr(filter, 'M')))) ||
1084+
((p->status != 'M') && strchr(filter, p->status)))
9961085
diff_q(&outq, p);
9971086
else
9981087
diff_free_filepair(p);

‎diffcore-break.c‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ static void merge_broken(struct diff_filepair *p,
214214
struct diff_queue_struct *outq)
215215
{
216216
/* p and pp are broken pairs we want to merge */
217-
struct diff_filepair *c = p, *d = pp;
217+
struct diff_filepair *c = p, *d = pp, *dp;
218218
if (DIFF_FILE_VALID(p->one)) {
219219
/* this must be a delete half */
220220
d = p; c = pp;
@@ -229,7 +229,8 @@ static void merge_broken(struct diff_filepair *p,
229229
if (!DIFF_FILE_VALID(c->two))
230230
die("internal error in merge #4");
231231

232-
diff_queue(outq, d->one, c->two);
232+
dp = diff_queue(outq, d->one, c->two);
233+
dp->score = p->score;
233234
diff_free_filespec_data(d->two);
234235
diff_free_filespec_data(c->one);
235236
free(d);
@@ -251,15 +252,13 @@ void diffcore_merge_broken(void)
251252
/* we already merged this with its peer */
252253
continue;
253254
else if (p->broken_pair &&
254-
p->score == 0 &&
255255
!strcmp(p->one->path, p->two->path)) {
256256
/* If the peer also survived rename/copy, then
257257
* we merge them back together.
258258
*/
259259
for (j = i + 1; j < q->nr; j++) {
260260
struct diff_filepair *pp = q->queue[j];
261261
if (pp->broken_pair &&
262-
p->score == 0 &&
263262
!strcmp(pp->one->path, pp->two->path) &&
264263
!strcmp(p->one->path, pp->two->path)) {
265264
/* Peer survived. Merge them */

0 commit comments

Comments
 (0)