submodule.c: migrate diff output to use emit_diff_symbol

As the submodule process is no longer attached to the same file pointer 'o->file' as the superprojects process, there is a different result in color.c::check_auto_color. That is why we need to pass coloring explicitly, such that the submodule coloring decision will be made by the child process processing the submodule. Only DIFF_SYMBOL_SUBMODULE_PIPETHROUGH contains color, the other symbols are for embedding the submodule output into the superprojects output. Remove the colors from the function signatures, as all the coloring decisions will be made either inside the child process or the final emit_diff_symbol, but not in the functions driving the submodule diff. Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Stefan Beller committed Jun 29, 2017 at 17:07 UTC f3597138df4de7b695dec42153bdfb2b8164d963
4 files changed +121 -68
diff.c
+71 -12
@@ -561,6 +561,13 @@ static void emit_line(struct diff_options *o, const char *set, const char *reset
561 }
562
563 enum diff_symbol {
564 + DIFF_SYMBOL_SUBMODULE_ADD,
565 + DIFF_SYMBOL_SUBMODULE_DEL,
566 + DIFF_SYMBOL_SUBMODULE_UNTRACKED,
567 + DIFF_SYMBOL_SUBMODULE_MODIFIED,
568 + DIFF_SYMBOL_SUBMODULE_HEADER,
569 + DIFF_SYMBOL_SUBMODULE_ERROR,
570 + DIFF_SYMBOL_SUBMODULE_PIPETHROUGH,
571 DIFF_SYMBOL_REWRITE_DIFF,
572 DIFF_SYMBOL_BINARY_FILES,
573 DIFF_SYMBOL_HEADER,
@@ -625,6 +632,9 @@ static void emit_diff_symbol(struct diff_options *o, enum diff_symbol s,
632 emit_line_0(o, context, reset, '\\',
633 nneof, strlen(nneof));
634 break;
635 + case DIFF_SYMBOL_SUBMODULE_HEADER:
636 + case DIFF_SYMBOL_SUBMODULE_ERROR:
637 + case DIFF_SYMBOL_SUBMODULE_PIPETHROUGH:
638 case DIFF_SYMBOL_CONTEXT_FRAGINFO:
639 emit_line(o, "", "", line, len);
640 break;
@@ -701,11 +711,68 @@ static void emit_diff_symbol(struct diff_options *o, enum diff_symbol s,
711 reset = diff_get_color_opt(o, DIFF_RESET);
712 emit_line(o, fraginfo, reset, line, len);
713 break;
714 + case DIFF_SYMBOL_SUBMODULE_ADD:
715 + set = diff_get_color_opt(o, DIFF_FILE_NEW);
716 + reset = diff_get_color_opt(o, DIFF_RESET);
717 + emit_line(o, set, reset, line, len);
718 + break;
719 + case DIFF_SYMBOL_SUBMODULE_DEL:
720 + set = diff_get_color_opt(o, DIFF_FILE_OLD);
721 + reset = diff_get_color_opt(o, DIFF_RESET);
722 + emit_line(o, set, reset, line, len);
723 + break;
724 + case DIFF_SYMBOL_SUBMODULE_UNTRACKED:
725 + fprintf(o->file, "%sSubmodule %s contains untracked content\n",
726 + diff_line_prefix(o), line);
727 + break;
728 + case DIFF_SYMBOL_SUBMODULE_MODIFIED:
729 + fprintf(o->file, "%sSubmodule %s contains modified content\n",
730 + diff_line_prefix(o), line);
731 + break;
732 default:
733 die("BUG: unknown diff symbol");
734 }
735 }
736
737 +void diff_emit_submodule_del(struct diff_options *o, const char *line)
738 +{
739 + emit_diff_symbol(o, DIFF_SYMBOL_SUBMODULE_DEL, line, strlen(line), 0);
740 +}
741 +
742 +void diff_emit_submodule_add(struct diff_options *o, const char *line)
743 +{
744 + emit_diff_symbol(o, DIFF_SYMBOL_SUBMODULE_ADD, line, strlen(line), 0);
745 +}
746 +
747 +void diff_emit_submodule_untracked(struct diff_options *o, const char *path)
748 +{
749 + emit_diff_symbol(o, DIFF_SYMBOL_SUBMODULE_UNTRACKED,
750 + path, strlen(path), 0);
751 +}
752 +
753 +void diff_emit_submodule_modified(struct diff_options *o, const char *path)
754 +{
755 + emit_diff_symbol(o, DIFF_SYMBOL_SUBMODULE_MODIFIED,
756 + path, strlen(path), 0);
757 +}
758 +
759 +void diff_emit_submodule_header(struct diff_options *o, const char *header)
760 +{
761 + emit_diff_symbol(o, DIFF_SYMBOL_SUBMODULE_HEADER,
762 + header, strlen(header), 0);
763 +}
764 +
765 +void diff_emit_submodule_error(struct diff_options *o, const char *err)
766 +{
767 + emit_diff_symbol(o, DIFF_SYMBOL_SUBMODULE_ERROR, err, strlen(err), 0);
768 +}
769 +
770 +void diff_emit_submodule_pipethrough(struct diff_options *o,
771 + const char *line, int len)
772 +{
773 + emit_diff_symbol(o, DIFF_SYMBOL_SUBMODULE_PIPETHROUGH, line, len, 0);
774 +}
775 +
776 static int new_blank_line_at_eof(struct emit_callback *ecbdata, const char *line, int len)
777 {
778 if (!((ecbdata->ws_rule & WS_BLANK_AT_EOF) &&
@@ -2467,24 +2534,16 @@ static void builtin_diff(const char *name_a,
2534 if (o->submodule_format == DIFF_SUBMODULE_LOG &&
2535 (!one->mode || S_ISGITLINK(one->mode)) &&
2536 (!two->mode || S_ISGITLINK(two->mode))) {
2470 - const char *del = diff_get_color_opt(o, DIFF_FILE_OLD);
2471 - const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
2472 - show_submodule_summary(o->file, one->path ? one->path : two->path,
2473 - line_prefix,
2537 + show_submodule_summary(o, one->path ? one->path : two->path,
2538 &one->oid, &two->oid,
2475 - two->dirty_submodule,
2476 - meta, del, add, reset);
2539 + two->dirty_submodule);
2540 return;
2541 } else if (o->submodule_format == DIFF_SUBMODULE_INLINE_DIFF &&
2542 (!one->mode || S_ISGITLINK(one->mode)) &&
2543 (!two->mode || S_ISGITLINK(two->mode))) {
2481 - const char *del = diff_get_color_opt(o, DIFF_FILE_OLD);
2482 - const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
2483 - show_submodule_inline_diff(o->file, one->path ? one->path : two->path,
2484 - line_prefix,
2544 + show_submodule_inline_diff(o, one->path ? one->path : two->path,
2545 &one->oid, &two->oid,
2486 - two->dirty_submodule,
2487 - meta, del, add, reset, o);
2546 + two->dirty_submodule);
2547 return;
2548 }
2549
diff.h
+9
@@ -188,6 +188,15 @@ struct diff_options {
188 int diff_path_counter;
189 };
190
191 +void diff_emit_submodule_del(struct diff_options *o, const char *line);
192 +void diff_emit_submodule_add(struct diff_options *o, const char *line);
193 +void diff_emit_submodule_untracked(struct diff_options *o, const char *path);
194 +void diff_emit_submodule_modified(struct diff_options *o, const char *path);
195 +void diff_emit_submodule_header(struct diff_options *o, const char *header);
196 +void diff_emit_submodule_error(struct diff_options *o, const char *err);
197 +void diff_emit_submodule_pipethrough(struct diff_options *o,
198 + const char *line, int len);
199 +
200 enum color_diff {
201 DIFF_RESET = 0,
202 DIFF_CONTEXT = 1,
submodule.c
+37 -47
@@ -479,9 +479,7 @@ static int prepare_submodule_summary(struct rev_info *rev, const char *path,
479 return prepare_revision_walk(rev);
480 }
481
482 -static void print_submodule_summary(struct rev_info *rev, FILE *f,
483 - const char *line_prefix,
484 - const char *del, const char *add, const char *reset)
482 +static void print_submodule_summary(struct rev_info *rev, struct diff_options *o)
483 {
484 static const char format[] = " %m %s";
485 struct strbuf sb = STRBUF_INIT;
@@ -492,18 +490,12 @@ static void print_submodule_summary(struct rev_info *rev, FILE *f,
490 ctx.date_mode = rev->date_mode;
491 ctx.output_encoding = get_log_output_encoding();
492 strbuf_setlen(&sb, 0);
495 - strbuf_addstr(&sb, line_prefix);
496 - if (commit->object.flags & SYMMETRIC_LEFT) {
497 - if (del)
498 - strbuf_addstr(&sb, del);
499 - }
500 - else if (add)
501 - strbuf_addstr(&sb, add);
493 format_commit_message(commit, format, &sb, &ctx);
503 - if (reset)
504 - strbuf_addstr(&sb, reset);
494 strbuf_addch(&sb, '\n');
506 - fprintf(f, "%s", sb.buf);
495 + if (commit->object.flags & SYMMETRIC_LEFT)
496 + diff_emit_submodule_del(o, sb.buf);
497 + else
498 + diff_emit_submodule_add(o, sb.buf);
499 }
500 strbuf_release(&sb);
501 }
@@ -530,11 +522,9 @@ void prepare_submodule_repo_env(struct argv_array *out)
522 * attempt to lookup both the left and right commits and put them into the
523 * left and right pointers.
524 */
533 -static void show_submodule_header(FILE *f, const char *path,
534 - const char *line_prefix,
525 +static void show_submodule_header(struct diff_options *o, const char *path,
526 struct object_id *one, struct object_id *two,
536 - unsigned dirty_submodule, const char *meta,
537 - const char *reset,
527 + unsigned dirty_submodule,
528 struct commit **left, struct commit **right,
529 struct commit_list **merge_bases)
530 {
@@ -543,11 +533,10 @@ static void show_submodule_header(FILE *f, const char *path,
533 int fast_forward = 0, fast_backward = 0;
534
535 if (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
546 - fprintf(f, "%sSubmodule %s contains untracked content\n",
547 - line_prefix, path);
536 + diff_emit_submodule_untracked(o, path);
537 +
538 if (dirty_submodule & DIRTY_SUBMODULE_MODIFIED)
549 - fprintf(f, "%sSubmodule %s contains modified content\n",
550 - line_prefix, path);
539 + diff_emit_submodule_modified(o, path);
540
541 if (is_null_oid(one))
542 message = "(new submodule)";
@@ -589,31 +578,29 @@ static void show_submodule_header(FILE *f, const char *path,
578 }
579
580 output_header:
592 - strbuf_addf(&sb, "%s%sSubmodule %s ", line_prefix, meta, path);
581 + strbuf_addf(&sb, "Submodule %s ", path);
582 strbuf_add_unique_abbrev(&sb, one->hash, DEFAULT_ABBREV);
583 strbuf_addstr(&sb, (fast_backward || fast_forward) ? ".." : "...");
584 strbuf_add_unique_abbrev(&sb, two->hash, DEFAULT_ABBREV);
585 if (message)
597 - strbuf_addf(&sb, " %s%s\n", message, reset);
586 + strbuf_addf(&sb, " %s\n", message);
587 else
599 - strbuf_addf(&sb, "%s:%s\n", fast_backward ? " (rewind)" : "", reset);
600 - fwrite(sb.buf, sb.len, 1, f);
588 + strbuf_addf(&sb, "%s:\n", fast_backward ? " (rewind)" : "");
589 + diff_emit_submodule_header(o, sb.buf);
590
591 strbuf_release(&sb);
592 }
593
605 -void show_submodule_summary(FILE *f, const char *path,
606 - const char *line_prefix,
594 +void show_submodule_summary(struct diff_options *o, const char *path,
595 struct object_id *one, struct object_id *two,
608 - unsigned dirty_submodule, const char *meta,
609 - const char *del, const char *add, const char *reset)
596 + unsigned dirty_submodule)
597 {
598 struct rev_info rev;
599 struct commit *left = NULL, *right = NULL;
600 struct commit_list *merge_bases = NULL;
601
615 - show_submodule_header(f, path, line_prefix, one, two, dirty_submodule,
616 - meta, reset, &left, &right, &merge_bases);
602 + show_submodule_header(o, path, one, two, dirty_submodule,
603 + &left, &right, &merge_bases);
604
605 /*
606 * If we don't have both a left and a right pointer, there is no
@@ -625,11 +612,11 @@ void show_submodule_summary(FILE *f, const char *path,
612
613 /* Treat revision walker failure the same as missing commits */
614 if (prepare_submodule_summary(&rev, path, left, right, merge_bases)) {
628 - fprintf(f, "%s(revision walker failed)\n", line_prefix);
615 + diff_emit_submodule_error(o, "(revision walker failed)\n");
616 goto out;
617 }
618
632 - print_submodule_summary(&rev, f, line_prefix, del, add, reset);
619 + print_submodule_summary(&rev, o);
620
621 out:
622 if (merge_bases)
@@ -638,21 +625,18 @@ out:
625 clear_commit_marks(right, ~0);
626 }
627
641 -void show_submodule_inline_diff(FILE *f, const char *path,
642 - const char *line_prefix,
628 +void show_submodule_inline_diff(struct diff_options *o, const char *path,
629 struct object_id *one, struct object_id *two,
644 - unsigned dirty_submodule, const char *meta,
645 - const char *del, const char *add, const char *reset,
646 - const struct diff_options *o)
630 + unsigned dirty_submodule)
631 {
632 const struct object_id *old = &empty_tree_oid, *new = &empty_tree_oid;
633 struct commit *left = NULL, *right = NULL;
634 struct commit_list *merge_bases = NULL;
651 - struct strbuf submodule_dir = STRBUF_INIT;
635 struct child_process cp = CHILD_PROCESS_INIT;
636 + struct strbuf sb = STRBUF_INIT;
637
654 - show_submodule_header(f, path, line_prefix, one, two, dirty_submodule,
655 - meta, reset, &left, &right, &merge_bases);
638 + show_submodule_header(o, path, one, two, dirty_submodule,
639 + &left, &right, &merge_bases);
640
641 /* We need a valid left and right commit to display a difference */
642 if (!(left || is_null_oid(one)) ||
@@ -664,16 +648,16 @@ void show_submodule_inline_diff(FILE *f, const char *path,
648 if (right)
649 new = two;
650
667 - fflush(f);
651 cp.git_cmd = 1;
652 cp.dir = path;
670 - cp.out = dup(fileno(f));
653 + cp.out = -1;
654 cp.no_stdin = 1;
655
656 /* TODO: other options may need to be passed here. */
657 argv_array_pushl(&cp.args, "diff", "--submodule=diff", NULL);
658 + argv_array_pushf(&cp.args, "--color=%s", want_color(o->use_color) ?
659 + "always" : "never");
660
676 - argv_array_pushf(&cp.args, "--line-prefix=%s", line_prefix);
661 if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
662 argv_array_pushf(&cp.args, "--src-prefix=%s%s/",
663 o->b_prefix, path);
@@ -696,11 +680,17 @@ void show_submodule_inline_diff(FILE *f, const char *path,
680 argv_array_push(&cp.args, oid_to_hex(new));
681
682 prepare_submodule_repo_env(&cp.env_array);
699 - if (run_command(&cp))
700 - fprintf(f, "(diff failed)\n");
683 + if (start_command(&cp))
684 + diff_emit_submodule_error(o, "(diff failed)\n");
685 +
686 + while (strbuf_getwholeline_fd(&sb, cp.out, '\n') != EOF)
687 + diff_emit_submodule_pipethrough(o, sb.buf, sb.len);
688 +
689 + if (finish_command(&cp))
690 + diff_emit_submodule_error(o, "(diff failed)\n");
691
692 done:
703 - strbuf_release(&submodule_dir);
693 + strbuf_release(&sb);
694 if (merge_bases)
695 free_commit_list(merge_bases);
696 if (left)
submodule.h
+4 -9
@@ -63,17 +63,12 @@ extern int parse_submodule_update_strategy(const char *value,
63 struct submodule_update_strategy *dst);
64 extern const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);
65 extern void handle_ignore_submodules_arg(struct diff_options *, const char *);
66 -extern void show_submodule_summary(FILE *f, const char *path,
67 - const char *line_prefix,
66 +extern void show_submodule_summary(struct diff_options *o, const char *path,
67 struct object_id *one, struct object_id *two,
69 - unsigned dirty_submodule, const char *meta,
70 - const char *del, const char *add, const char *reset);
71 -extern void show_submodule_inline_diff(FILE *f, const char *path,
72 - const char *line_prefix,
68 + unsigned dirty_submodule);
69 +extern void show_submodule_inline_diff(struct diff_options *o, const char *path,
70 struct object_id *one, struct object_id *two,
74 - unsigned dirty_submodule, const char *meta,
75 - const char *del, const char *add, const char *reset,
76 - const struct diff_options *opt);
71 + unsigned dirty_submodule);
72 extern void set_config_fetch_recurse_submodules(int value);
73 /* Check if we want to update any submodule.*/
74 extern int should_update_submodules(void);