From 6ea57703f660afd00159b67bf8749d90881df3b4 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 22 Jun 2016 17:01:28 +0200 Subject: log: prepare log/log-tree to reuse the diffopt.close_file attribute We are about to teach the log-tree machinery to reuse the diffopt.file field to output to a file stream other than stdout, in line with the diff machinery already writing to diffopt.file. However, we might want to write something after the diff in log_tree_commit() (e.g. with the --show-linear-break option), therefore we must not let the diff machinery close the file (as per diffopt.close_file. This means that log_tree_commit() itself must override the diffopt.close_file flag and close the file, and if log_tree_commit() is called in a loop, the caller is responsible to do the same. Note: format-patch has an `--output-directory` option. Due to the fact that format-patch's options are parsed first, and that the parse-options machinery accepts uniquely abbreviated options, the diff options `--output` (and `-o`) are shadowed. Therefore close_file is not set to 1 so that cmd_format_patch() does *not* need to handle the close_file flag differently, even if it calls log_tree_commit() in a loop. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/builtin/log.c b/builtin/log.c index 099f4f7..27bc88d 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -243,9 +243,10 @@ static struct itimerval early_output_timer; static void log_show_early(struct rev_info *revs, struct commit_list *list) { - int i = revs->early_output; + int i = revs->early_output, close_file = revs->diffopt.close_file; int show_header = 1; + revs->diffopt.close_file = 0; sort_in_topological_order(&list, revs->sort_order); while (list && i) { struct commit *commit = list->item; @@ -262,14 +263,19 @@ static void log_show_early(struct rev_info *revs, struct commit_list *list) case commit_ignore: break; case commit_error: + if (close_file) + fclose(revs->diffopt.file); return; } list = list->next; } /* Did we already get enough commits for the early output? */ - if (!i) + if (!i) { + if (close_file) + fclose(revs->diffopt.file); return; + } /* * ..if no, then repeat it twice a second until we @@ -331,7 +337,7 @@ static int cmd_log_walk(struct rev_info *rev) { struct commit *commit; int saved_nrl = 0; - int saved_dcctc = 0; + int saved_dcctc = 0, close_file = rev->diffopt.close_file; if (rev->early_output) setup_early_output(rev); @@ -347,6 +353,7 @@ static int cmd_log_walk(struct rev_info *rev) * and HAS_CHANGES being accumulated in rev->diffopt, so be careful to * retain that state information if replacing rev->diffopt in this loop */ + rev->diffopt.close_file = 0; while ((commit = get_revision(rev)) != NULL) { if (!log_tree_commit(rev, commit) && rev->max_count >= 0) /* @@ -367,6 +374,8 @@ static int cmd_log_walk(struct rev_info *rev) } rev->diffopt.degraded_cc_to_c = saved_dcctc; rev->diffopt.needed_rename_limit = saved_nrl; + if (close_file) + fclose(rev->diffopt.file); if (rev->diffopt.output_format & DIFF_FORMAT_CHECKDIFF && DIFF_OPT_TST(&rev->diffopt, CHECK_FAILED)) { diff --git a/log-tree.c b/log-tree.c index 78a5381..456d7e3 100644 --- a/log-tree.c +++ b/log-tree.c @@ -862,11 +862,12 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log int log_tree_commit(struct rev_info *opt, struct commit *commit) { struct log_info log; - int shown; + int shown, close_file = opt->diffopt.close_file; log.commit = commit; log.parent = NULL; opt->loginfo = &log; + opt->diffopt.close_file = 0; if (opt->line_level_traverse) return line_log_print(opt, commit); @@ -883,5 +884,7 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit) printf("\n%s\n", opt->break_bar); opt->loginfo = NULL; maybe_flush_or_die(stdout, "stdout"); + if (close_file) + fclose(opt->diffopt.file); return shown; } -- cgit v0.10.2-6-g49f6 From 4d7b0efc5e2eea1923803dd9c5d5d1288c99cd00 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 22 Jun 2016 17:01:32 +0200 Subject: log-tree: respect diffopt's configured output file stream The diff options already know how to print the output anywhere else than stdout. The same is needed for log output in general, e.g. when writing patches to files in `git format-patch`. Let's allow users to use log_tree_commit() *without* changing global state via freopen(). Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/log-tree.c b/log-tree.c index 456d7e3..cf24027 100644 --- a/log-tree.c +++ b/log-tree.c @@ -159,12 +159,12 @@ void load_ref_decorations(int flags) } } -static void show_parents(struct commit *commit, int abbrev) +static void show_parents(struct commit *commit, int abbrev, FILE *file) { struct commit_list *p; for (p = commit->parents; p ; p = p->next) { struct commit *parent = p->item; - printf(" %s", find_unique_abbrev(parent->object.oid.hash, abbrev)); + fprintf(file, " %s", find_unique_abbrev(parent->object.oid.hash, abbrev)); } } @@ -172,7 +172,7 @@ static void show_children(struct rev_info *opt, struct commit *commit, int abbre { struct commit_list *p = lookup_decoration(&opt->children, &commit->object); for ( ; p; p = p->next) { - printf(" %s", find_unique_abbrev(p->item->object.oid.hash, abbrev)); + fprintf(opt->diffopt.file, " %s", find_unique_abbrev(p->item->object.oid.hash, abbrev)); } } @@ -286,11 +286,11 @@ void show_decorations(struct rev_info *opt, struct commit *commit) struct strbuf sb = STRBUF_INIT; if (opt->show_source && commit->util) - printf("\t%s", (char *) commit->util); + fprintf(opt->diffopt.file, "\t%s", (char *) commit->util); if (!opt->show_decorations) return; format_decorations(&sb, commit, opt->diffopt.use_color); - fputs(sb.buf, stdout); + fputs(sb.buf, opt->diffopt.file); strbuf_release(&sb); } @@ -364,18 +364,18 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit, subject = "Subject: "; } - printf("From %s Mon Sep 17 00:00:00 2001\n", name); + fprintf(opt->diffopt.file, "From %s Mon Sep 17 00:00:00 2001\n", name); graph_show_oneline(opt->graph); if (opt->message_id) { - printf("Message-Id: <%s>\n", opt->message_id); + fprintf(opt->diffopt.file, "Message-Id: <%s>\n", opt->message_id); graph_show_oneline(opt->graph); } if (opt->ref_message_ids && opt->ref_message_ids->nr > 0) { int i, n; n = opt->ref_message_ids->nr; - printf("In-Reply-To: <%s>\n", opt->ref_message_ids->items[n-1].string); + fprintf(opt->diffopt.file, "In-Reply-To: <%s>\n", opt->ref_message_ids->items[n-1].string); for (i = 0; i < n; i++) - printf("%s<%s>\n", (i > 0 ? "\t" : "References: "), + fprintf(opt->diffopt.file, "%s<%s>\n", (i > 0 ? "\t" : "References: "), opt->ref_message_ids->items[i].string); graph_show_oneline(opt->graph); } @@ -432,7 +432,7 @@ static void show_sig_lines(struct rev_info *opt, int status, const char *bol) reset = diff_get_color_opt(&opt->diffopt, DIFF_RESET); while (*bol) { eol = strchrnul(bol, '\n'); - printf("%s%.*s%s%s", color, (int)(eol - bol), bol, reset, + fprintf(opt->diffopt.file, "%s%.*s%s%s", color, (int)(eol - bol), bol, reset, *eol ? "\n" : ""); graph_show_oneline(opt->graph); bol = (*eol) ? (eol + 1) : eol; @@ -553,17 +553,17 @@ void show_log(struct rev_info *opt) if (!opt->graph) put_revision_mark(opt, commit); - fputs(find_unique_abbrev(commit->object.oid.hash, abbrev_commit), stdout); + fputs(find_unique_abbrev(commit->object.oid.hash, abbrev_commit), opt->diffopt.file); if (opt->print_parents) - show_parents(commit, abbrev_commit); + show_parents(commit, abbrev_commit, opt->diffopt.file); if (opt->children.name) show_children(opt, commit, abbrev_commit); show_decorations(opt, commit); if (opt->graph && !graph_is_commit_finished(opt->graph)) { - putchar('\n'); + putc('\n', opt->diffopt.file); graph_show_remainder(opt->graph); } - putchar(opt->diffopt.line_termination); + putc(opt->diffopt.line_termination, opt->diffopt.file); return; } @@ -589,7 +589,7 @@ void show_log(struct rev_info *opt) if (opt->diffopt.line_termination == '\n' && !opt->missing_newline) graph_show_padding(opt->graph); - putchar(opt->diffopt.line_termination); + putc(opt->diffopt.line_termination, opt->diffopt.file); } opt->shown_one = 1; @@ -607,28 +607,28 @@ void show_log(struct rev_info *opt) log_write_email_headers(opt, commit, &ctx.subject, &extra_headers, &ctx.need_8bit_cte); } else if (opt->commit_format != CMIT_FMT_USERFORMAT) { - fputs(diff_get_color_opt(&opt->diffopt, DIFF_COMMIT), stdout); + fputs(diff_get_color_opt(&opt->diffopt, DIFF_COMMIT), opt->diffopt.file); if (opt->commit_format != CMIT_FMT_ONELINE) - fputs("commit ", stdout); + fputs("commit ", opt->diffopt.file); if (!opt->graph) put_revision_mark(opt, commit); fputs(find_unique_abbrev(commit->object.oid.hash, abbrev_commit), - stdout); + opt->diffopt.file); if (opt->print_parents) - show_parents(commit, abbrev_commit); + show_parents(commit, abbrev_commit, opt->diffopt.file); if (opt->children.name) show_children(opt, commit, abbrev_commit); if (parent) - printf(" (from %s)", + fprintf(opt->diffopt.file, " (from %s)", find_unique_abbrev(parent->object.oid.hash, abbrev_commit)); - fputs(diff_get_color_opt(&opt->diffopt, DIFF_RESET), stdout); + fputs(diff_get_color_opt(&opt->diffopt, DIFF_RESET), opt->diffopt.file); show_decorations(opt, commit); if (opt->commit_format == CMIT_FMT_ONELINE) { - putchar(' '); + putc(' ', opt->diffopt.file); } else { - putchar('\n'); + putc('\n', opt->diffopt.file); graph_show_oneline(opt->graph); } if (opt->reflog_info) { @@ -702,7 +702,7 @@ void show_log(struct rev_info *opt) } if (opt->show_log_size) { - printf("log size %i\n", (int)msgbuf.len); + fprintf(opt->diffopt.file, "log size %i\n", (int)msgbuf.len); graph_show_oneline(opt->graph); } @@ -718,11 +718,11 @@ void show_log(struct rev_info *opt) if (opt->graph) graph_show_commit_msg(opt->graph, &msgbuf); else - fwrite(msgbuf.buf, sizeof(char), msgbuf.len, stdout); + fwrite(msgbuf.buf, sizeof(char), msgbuf.len, opt->diffopt.file); if (opt->use_terminator && !commit_format_is_empty(opt->commit_format)) { if (!opt->missing_newline) graph_show_padding(opt->graph); - putchar(opt->diffopt.line_termination); + putc(opt->diffopt.line_termination, opt->diffopt.file); } strbuf_release(&msgbuf); @@ -759,7 +759,7 @@ int log_tree_diff_flush(struct rev_info *opt) struct strbuf *msg = NULL; msg = opt->diffopt.output_prefix(&opt->diffopt, opt->diffopt.output_prefix_data); - fwrite(msg->buf, msg->len, 1, stdout); + fwrite(msg->buf, msg->len, 1, opt->diffopt.file); } /* @@ -774,8 +774,8 @@ int log_tree_diff_flush(struct rev_info *opt) */ if (!opt->shown_dashes && (pch & opt->diffopt.output_format) == pch) - printf("---"); - putchar('\n'); + fprintf(opt->diffopt.file, "---"); + putc('\n', opt->diffopt.file); } } diff_flush(&opt->diffopt); @@ -873,7 +873,7 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit) return line_log_print(opt, commit); if (opt->track_linear && !opt->linear && !opt->reverse_output_stage) - printf("\n%s\n", opt->break_bar); + fprintf(opt->diffopt.file, "\n%s\n", opt->break_bar); shown = log_tree_diff(opt, commit, &log); if (!shown && opt->loginfo && opt->always_show_header) { log.parent = NULL; @@ -881,9 +881,9 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit) shown = 1; } if (opt->track_linear && !opt->linear && opt->reverse_output_stage) - printf("\n%s\n", opt->break_bar); + fprintf(opt->diffopt.file, "\n%s\n", opt->break_bar); opt->loginfo = NULL; - maybe_flush_or_die(stdout, "stdout"); + maybe_flush_or_die(opt->diffopt.file, "stdout"); if (close_file) fclose(opt->diffopt.file); return shown; -- cgit v0.10.2-6-g49f6 From 179795e51171ebf10932b553d7df72819fa4a7b5 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 22 Jun 2016 17:01:39 +0200 Subject: line-log: respect diffopt's configured output file stream The diff machinery can optionally output to a file stream other than stdout, by overriding diffopt.file. In such a case, the rest of the log tree machinery should also write to that stream. Currently, there is no user of the line level log that wants to redirect output to a file. Therefore, one might argue that it is superfluous to support that now. However, it is better to be consistent now, rather than to face hard-to-debug problems later. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/line-log.c b/line-log.c index bbe31ed..e62a7f4 100644 --- a/line-log.c +++ b/line-log.c @@ -841,7 +841,7 @@ static char *get_nth_line(long line, unsigned long *ends, void *data) static void print_line(const char *prefix, char first, long line, unsigned long *ends, void *data, - const char *color, const char *reset) + const char *color, const char *reset, FILE *file) { char *begin = get_nth_line(line, ends, data); char *end = get_nth_line(line+1, ends, data); @@ -852,14 +852,14 @@ static void print_line(const char *prefix, char first, had_nl = 1; } - fputs(prefix, stdout); - fputs(color, stdout); - putchar(first); - fwrite(begin, 1, end-begin, stdout); - fputs(reset, stdout); - putchar('\n'); + fputs(prefix, file); + fputs(color, file); + putc(first, file); + fwrite(begin, 1, end-begin, file); + fputs(reset, file); + putc('\n', file); if (!had_nl) - fputs("\\ No newline at end of file\n", stdout); + fputs("\\ No newline at end of file\n", file); } static char *output_prefix(struct diff_options *opt) @@ -898,12 +898,12 @@ static void dump_diff_hacky_one(struct rev_info *rev, struct line_log_data *rang fill_line_ends(pair->one, &p_lines, &p_ends); fill_line_ends(pair->two, &t_lines, &t_ends); - printf("%s%sdiff --git a/%s b/%s%s\n", prefix, c_meta, pair->one->path, pair->two->path, c_reset); - printf("%s%s--- %s%s%s\n", prefix, c_meta, + fprintf(opt->file, "%s%sdiff --git a/%s b/%s%s\n", prefix, c_meta, pair->one->path, pair->two->path, c_reset); + fprintf(opt->file, "%s%s--- %s%s%s\n", prefix, c_meta, pair->one->sha1_valid ? "a/" : "", pair->one->sha1_valid ? pair->one->path : "/dev/null", c_reset); - printf("%s%s+++ b/%s%s\n", prefix, c_meta, pair->two->path, c_reset); + fprintf(opt->file, "%s%s+++ b/%s%s\n", prefix, c_meta, pair->two->path, c_reset); for (i = 0; i < range->ranges.nr; i++) { long p_start, p_end; long t_start = range->ranges.ranges[i].start; @@ -945,7 +945,7 @@ static void dump_diff_hacky_one(struct rev_info *rev, struct line_log_data *rang } /* Now output a diff hunk for this range */ - printf("%s%s@@ -%ld,%ld +%ld,%ld @@%s\n", + fprintf(opt->file, "%s%s@@ -%ld,%ld +%ld,%ld @@%s\n", prefix, c_frag, p_start+1, p_end-p_start, t_start+1, t_end-t_start, c_reset); @@ -953,18 +953,18 @@ static void dump_diff_hacky_one(struct rev_info *rev, struct line_log_data *rang int k; for (; t_cur < diff->target.ranges[j].start; t_cur++) print_line(prefix, ' ', t_cur, t_ends, pair->two->data, - c_context, c_reset); + c_context, c_reset, opt->file); for (k = diff->parent.ranges[j].start; k < diff->parent.ranges[j].end; k++) print_line(prefix, '-', k, p_ends, pair->one->data, - c_old, c_reset); + c_old, c_reset, opt->file); for (; t_cur < diff->target.ranges[j].end && t_cur < t_end; t_cur++) print_line(prefix, '+', t_cur, t_ends, pair->two->data, - c_new, c_reset); + c_new, c_reset, opt->file); j++; } for (; t_cur < t_end; t_cur++) print_line(prefix, ' ', t_cur, t_ends, pair->two->data, - c_context, c_reset); + c_context, c_reset, opt->file); } free(p_ends); @@ -977,7 +977,7 @@ static void dump_diff_hacky_one(struct rev_info *rev, struct line_log_data *rang */ static void dump_diff_hacky(struct rev_info *rev, struct line_log_data *range) { - puts(output_prefix(&rev->diffopt)); + fprintf(rev->diffopt.file, "%s\n", output_prefix(&rev->diffopt)); while (range) { dump_diff_hacky_one(rev, range); range = range->next; -- cgit v0.10.2-6-g49f6 From c61008fdfb59aff00ec546be6bc6cf3bd8869165 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 22 Jun 2016 17:01:44 +0200 Subject: graph: respect the diffopt.file setting When the caller overrides diffopt.file (which defaults to stdout), the diff machinery already redirects its output, and the graph display should also write to that file. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/graph.c b/graph.c index 1350bdd..8ad8ba3 100644 --- a/graph.c +++ b/graph.c @@ -17,8 +17,8 @@ static void graph_padding_line(struct git_graph *graph, struct strbuf *sb); /* - * Print a strbuf to stdout. If the graph is non-NULL, all lines but the - * first will be prefixed with the graph output. + * Print a strbuf. If the graph is non-NULL, all lines but the first will be + * prefixed with the graph output. * * If the strbuf ends with a newline, the output will end after this * newline. A new graph line will not be printed after the final newline. @@ -1193,9 +1193,10 @@ void graph_show_commit(struct git_graph *graph) while (!shown_commit_line && !graph_is_commit_finished(graph)) { shown_commit_line = graph_next_line(graph, &msgbuf); - fwrite(msgbuf.buf, sizeof(char), msgbuf.len, stdout); + fwrite(msgbuf.buf, sizeof(char), msgbuf.len, + graph->revs->diffopt.file); if (!shown_commit_line) - putchar('\n'); + putc('\n', graph->revs->diffopt.file); strbuf_setlen(&msgbuf, 0); } @@ -1210,7 +1211,7 @@ void graph_show_oneline(struct git_graph *graph) return; graph_next_line(graph, &msgbuf); - fwrite(msgbuf.buf, sizeof(char), msgbuf.len, stdout); + fwrite(msgbuf.buf, sizeof(char), msgbuf.len, graph->revs->diffopt.file); strbuf_release(&msgbuf); } @@ -1222,7 +1223,7 @@ void graph_show_padding(struct git_graph *graph) return; graph_padding_line(graph, &msgbuf); - fwrite(msgbuf.buf, sizeof(char), msgbuf.len, stdout); + fwrite(msgbuf.buf, sizeof(char), msgbuf.len, graph->revs->diffopt.file); strbuf_release(&msgbuf); } @@ -1239,12 +1240,13 @@ int graph_show_remainder(struct git_graph *graph) for (;;) { graph_next_line(graph, &msgbuf); - fwrite(msgbuf.buf, sizeof(char), msgbuf.len, stdout); + fwrite(msgbuf.buf, sizeof(char), msgbuf.len, + graph->revs->diffopt.file); strbuf_setlen(&msgbuf, 0); shown = 1; if (!graph_is_commit_finished(graph)) - putchar('\n'); + putc('\n', graph->revs->diffopt.file); else break; } @@ -1259,7 +1261,8 @@ static void graph_show_strbuf(struct git_graph *graph, struct strbuf const *sb) char *p; if (!graph) { - fwrite(sb->buf, sizeof(char), sb->len, stdout); + fwrite(sb->buf, sizeof(char), sb->len, + graph->revs->diffopt.file); return; } @@ -1277,7 +1280,7 @@ static void graph_show_strbuf(struct git_graph *graph, struct strbuf const *sb) } else { len = (sb->buf + sb->len) - p; } - fwrite(p, sizeof(char), len, stdout); + fwrite(p, sizeof(char), len, graph->revs->diffopt.file); if (next_p && *next_p != '\0') graph_show_oneline(graph); p = next_p; @@ -1297,7 +1300,8 @@ void graph_show_commit_msg(struct git_graph *graph, * CMIT_FMT_USERFORMAT are already missing a terminating * newline. All of the other formats should have it. */ - fwrite(sb->buf, sizeof(char), sb->len, stdout); + fwrite(sb->buf, sizeof(char), sb->len, + graph->revs->diffopt.file); return; } @@ -1318,7 +1322,7 @@ void graph_show_commit_msg(struct git_graph *graph, * new line. */ if (!newline_terminated) - putchar('\n'); + putc('\n', graph->revs->diffopt.file); graph_show_remainder(graph); @@ -1326,6 +1330,6 @@ void graph_show_commit_msg(struct git_graph *graph, * If sb ends with a newline, our output should too. */ if (newline_terminated) - putchar('\n'); + putc('\n', graph->revs->diffopt.file); } } -- cgit v0.10.2-6-g49f6 From 0a7b357737e3352ce1b0385313c1efc1b2564dbb Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 22 Jun 2016 17:01:49 +0200 Subject: shortlog: support outputting to streams other than stdout This will be needed to avoid freopen() in `git format-patch`. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/builtin/shortlog.c b/builtin/shortlog.c index bfc082e..39d74fe 100644 --- a/builtin/shortlog.c +++ b/builtin/shortlog.c @@ -229,6 +229,7 @@ void shortlog_init(struct shortlog *log) log->wrap = DEFAULT_WRAPLEN; log->in1 = DEFAULT_INDENT1; log->in2 = DEFAULT_INDENT2; + log->file = stdout; } int cmd_shortlog(int argc, const char **argv, const char *prefix) @@ -310,22 +311,24 @@ void shortlog_output(struct shortlog *log) for (i = 0; i < log->list.nr; i++) { const struct string_list_item *item = &log->list.items[i]; if (log->summary) { - printf("%6d\t%s\n", (int)UTIL_TO_INT(item), item->string); + fprintf(log->file, "%6d\t%s\n", + (int)UTIL_TO_INT(item), item->string); } else { struct string_list *onelines = item->util; - printf("%s (%d):\n", item->string, onelines->nr); + fprintf(log->file, "%s (%d):\n", + item->string, onelines->nr); for (j = onelines->nr - 1; j >= 0; j--) { const char *msg = onelines->items[j].string; if (log->wrap_lines) { strbuf_reset(&sb); add_wrapped_shortlog_msg(&sb, msg, log); - fwrite(sb.buf, sb.len, 1, stdout); + fwrite(sb.buf, sb.len, 1, log->file); } else - printf(" %s\n", msg); + fprintf(log->file, " %s\n", msg); } - putchar('\n'); + putc('\n', log->file); onelines->strdup_strings = 1; string_list_clear(onelines, 0); free(onelines); diff --git a/shortlog.h b/shortlog.h index de4f86f..5a326c6 100644 --- a/shortlog.h +++ b/shortlog.h @@ -17,6 +17,7 @@ struct shortlog { char *common_repo_prefix; int email; struct string_list mailmap; + FILE *file; }; void shortlog_init(struct shortlog *log); -- cgit v0.10.2-6-g49f6 From 11f4eb19843c28b1c50d3c1bef1183e6313b87b8 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 22 Jun 2016 17:01:54 +0200 Subject: format-patch: explicitly switch off color when writing to files The --color=auto handling is done by seeing if file descriptor 1 (the standard output) is connected to a terminal. format-patch used freopen() to reuse the standard output stream even when sending its output to an on-disk file, and this check is appropriate. In the next step, however, we will stop reusing "FILE *stdout", and instead start using arbitrary file descriptor obtained by doing an fopen(3) ourselves. The check --color=auto does will become useless, as we no longer are writing to the standard output stream. But then, we do not need to guess to begin with. As argued in the commit message of 7787570c (format-patch: ignore ui.color, 2011-09-13), we do not allow the ui.color setting to affect format-patch's output. The only time, therefore, that we allow color sequences to be written to the output files is when the user specified the --color=always command-line option explicitly. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/builtin/log.c b/builtin/log.c index 27bc88d..1985ed3 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1578,6 +1578,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) setup_pager(); if (output_directory) { + if (rev.diffopt.use_color != GIT_COLOR_ALWAYS) + rev.diffopt.use_color = GIT_COLOR_NEVER; if (use_stdout) die(_("standard output, or directory, which one?")); if (mkdir(output_directory, 0777) < 0 && errno != EEXIST) -- cgit v0.10.2-6-g49f6 From 95235f5ba1a9398c8503b809370ece18bba72ba2 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 22 Jun 2016 17:01:59 +0200 Subject: format-patch: avoid freopen() We just taught the relevant functions to respect the diffopt.file field, to allow writing somewhere else than stdout. Let's make use of it. Technically, we do not need to avoid that call in a builtin: we assume that builtins (as opposed to library functions) are stand-alone programs that may do with their (global) state. Yet, we want to be able to reuse that code in properly lib-ified code, e.g. when converting scripts into builtins. Further, while we did not *have* to touch the cmd_show() and cmd_cherry() code paths (because they do not want to write anywhere but stdout as of yet), it just makes sense to be consistent, making it easier and safer to move the code later. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/builtin/log.c b/builtin/log.c index 1985ed3..caa9fe8 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -236,7 +236,7 @@ static void show_early_header(struct rev_info *rev, const char *stage, int nr) if (rev->commit_format != CMIT_FMT_ONELINE) putchar(rev->diffopt.line_termination); } - printf(_("Final output: %d %s\n"), nr, stage); + fprintf(rev->diffopt.file, _("Final output: %d %s\n"), nr, stage); } static struct itimerval early_output_timer; @@ -454,7 +454,7 @@ static void show_tagger(char *buf, int len, struct rev_info *rev) pp.fmt = rev->commit_format; pp.date_mode = rev->date_mode; pp_user_info(&pp, "Tagger", &out, buf, get_log_output_encoding()); - printf("%s", out.buf); + fprintf(rev->diffopt.file, "%s", out.buf); strbuf_release(&out); } @@ -465,7 +465,7 @@ static int show_blob_object(const unsigned char *sha1, struct rev_info *rev, con char *buf; unsigned long size; - fflush(stdout); + fflush(rev->diffopt.file); if (!DIFF_OPT_TOUCHED(&rev->diffopt, ALLOW_TEXTCONV) || !DIFF_OPT_TST(&rev->diffopt, ALLOW_TEXTCONV)) return stream_blob_to_fd(1, sha1, NULL, 0); @@ -505,7 +505,7 @@ static int show_tag_object(const unsigned char *sha1, struct rev_info *rev) } if (offset < size) - fwrite(buf + offset, size - offset, 1, stdout); + fwrite(buf + offset, size - offset, 1, rev->diffopt.file); free(buf); return 0; } @@ -514,7 +514,8 @@ static int show_tree_object(const unsigned char *sha1, struct strbuf *base, const char *pathname, unsigned mode, int stage, void *context) { - printf("%s%s\n", pathname, S_ISDIR(mode) ? "/" : ""); + FILE *file = context; + fprintf(file, "%s%s\n", pathname, S_ISDIR(mode) ? "/" : ""); return 0; } @@ -574,7 +575,7 @@ int cmd_show(int argc, const char **argv, const char *prefix) if (rev.shown_one) putchar('\n'); - printf("%stag %s%s\n", + fprintf(rev.diffopt.file, "%stag %s%s\n", diff_get_color_opt(&rev.diffopt, DIFF_COMMIT), t->tag, diff_get_color_opt(&rev.diffopt, DIFF_RESET)); @@ -593,12 +594,12 @@ int cmd_show(int argc, const char **argv, const char *prefix) case OBJ_TREE: if (rev.shown_one) putchar('\n'); - printf("%stree %s%s\n\n", + fprintf(rev.diffopt.file, "%stree %s%s\n\n", diff_get_color_opt(&rev.diffopt, DIFF_COMMIT), name, diff_get_color_opt(&rev.diffopt, DIFF_RESET)); read_tree_recursive((struct tree *)o, "", 0, 0, &match_all, - show_tree_object, NULL); + show_tree_object, rev.diffopt.file); rev.shown_one = 1; break; case OBJ_COMMIT: @@ -808,7 +809,7 @@ static FILE *realstdout = NULL; static const char *output_directory = NULL; static int outdir_offset; -static int reopen_stdout(struct commit *commit, const char *subject, +static int open_next_file(struct commit *commit, const char *subject, struct rev_info *rev, int quiet) { struct strbuf filename = STRBUF_INIT; @@ -832,7 +833,7 @@ static int reopen_stdout(struct commit *commit, const char *subject, if (!quiet) fprintf(realstdout, "%s\n", filename.buf + outdir_offset); - if (freopen(filename.buf, "w", stdout) == NULL) + if ((rev->diffopt.file = fopen(filename.buf, "w")) == NULL) return error(_("Cannot open patch file %s"), filename.buf); strbuf_release(&filename); @@ -891,15 +892,15 @@ static void gen_message_id(struct rev_info *info, char *base) info->message_id = strbuf_detach(&buf, NULL); } -static void print_signature(void) +static void print_signature(FILE *file) { if (!signature || !*signature) return; - printf("-- \n%s", signature); + fprintf(file, "-- \n%s", signature); if (signature[strlen(signature)-1] != '\n') - putchar('\n'); - putchar('\n'); + putc('\n', file); + putc('\n', file); } static void add_branch_description(struct strbuf *buf, const char *branch_name) @@ -968,7 +969,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout, committer = git_committer_info(0); if (!use_stdout && - reopen_stdout(NULL, rev->numbered_files ? NULL : "cover-letter", rev, quiet)) + open_next_file(NULL, rev->numbered_files ? NULL : "cover-letter", rev, quiet)) return; log_write_email_headers(rev, head, &pp.subject, &pp.after_subject, @@ -991,7 +992,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout, pp_title_line(&pp, &msg, &sb, encoding, need_8bit_cte); pp_remainder(&pp, &msg, &sb, 0); add_branch_description(&sb, branch_name); - printf("%s\n", sb.buf); + fprintf(rev->diffopt.file, "%s\n", sb.buf); strbuf_release(&sb); @@ -1000,6 +1001,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout, log.wrap = 72; log.in1 = 2; log.in2 = 4; + log.file = rev->diffopt.file; for (i = 0; i < nr; i++) shortlog_add_commit(&log, list[i]); @@ -1022,8 +1024,8 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout, diffcore_std(&opts); diff_flush(&opts); - printf("\n"); - print_signature(); + fprintf(rev->diffopt.file, "\n"); + print_signature(rev->diffopt.file); } static const char *clean_message_id(const char *msg_id) @@ -1333,7 +1335,7 @@ static void prepare_bases(struct base_tree_info *bases, } } -static void print_bases(struct base_tree_info *bases) +static void print_bases(struct base_tree_info *bases, FILE *file) { int i; @@ -1342,11 +1344,11 @@ static void print_bases(struct base_tree_info *bases) return; /* Show the base commit */ - printf("base-commit: %s\n", oid_to_hex(&bases->base_commit)); + fprintf(file, "base-commit: %s\n", oid_to_hex(&bases->base_commit)); /* Show the prerequisite patches */ for (i = bases->nr_patch_id - 1; i >= 0; i--) - printf("prerequisite-patch-id: %s\n", oid_to_hex(&bases->patch_id[i])); + fprintf(file, "prerequisite-patch-id: %s\n", oid_to_hex(&bases->patch_id[i])); free(bases->patch_id); bases->nr_patch_id = 0; @@ -1704,7 +1706,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) gen_message_id(&rev, "cover"); make_cover_letter(&rev, use_stdout, origin, nr, list, branch_name, quiet); - print_bases(&bases); + print_bases(&bases, rev.diffopt.file); total++; start_number--; } @@ -1750,7 +1752,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) } if (!use_stdout && - reopen_stdout(rev.numbered_files ? NULL : commit, NULL, &rev, quiet)) + open_next_file(rev.numbered_files ? NULL : commit, NULL, &rev, quiet)) die(_("Failed to create output files")); shown = log_tree_commit(&rev, commit); free_commit_buffer(commit); @@ -1765,15 +1767,15 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) rev.shown_one = 0; if (shown) { if (rev.mime_boundary) - printf("\n--%s%s--\n\n\n", + fprintf(rev.diffopt.file, "\n--%s%s--\n\n\n", mime_boundary_leader, rev.mime_boundary); else - print_signature(); - print_bases(&bases); + print_signature(rev.diffopt.file); + print_bases(&bases, rev.diffopt.file); } if (!use_stdout) - fclose(stdout); + fclose(rev.diffopt.file); } free(list); free(branch_name); @@ -1805,15 +1807,15 @@ static const char * const cherry_usage[] = { }; static void print_commit(char sign, struct commit *commit, int verbose, - int abbrev) + int abbrev, FILE *file) { if (!verbose) { - printf("%c %s\n", sign, + fprintf(file, "%c %s\n", sign, find_unique_abbrev(commit->object.oid.hash, abbrev)); } else { struct strbuf buf = STRBUF_INIT; pp_commit_easy(CMIT_FMT_ONELINE, commit, &buf); - printf("%c %s %s\n", sign, + fprintf(file, "%c %s %s\n", sign, find_unique_abbrev(commit->object.oid.hash, abbrev), buf.buf); strbuf_release(&buf); @@ -1894,7 +1896,7 @@ int cmd_cherry(int argc, const char **argv, const char *prefix) commit = list->item; if (has_commit_patch_id(commit, &ids)) sign = '-'; - print_commit(sign, commit, verbose, abbrev); + print_commit(sign, commit, verbose, abbrev, revs.diffopt.file); list = list->next; } -- cgit v0.10.2-6-g49f6 From 36a4d905c36a598d17274765b57690ecfb4a23f8 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 22 Jun 2016 17:02:04 +0200 Subject: format-patch: use stdout directly Earlier, we freopen()ed stdout in order to write patches to files. That forced us to duplicate stdout (naming it "realstdout") because we *still* wanted to be able to report the file names. As we do not abuse stdout that way anymore, we no longer need to duplicate stdout, either. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/builtin/log.c b/builtin/log.c index caa9fe8..1a6903b 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -805,7 +805,6 @@ static int git_format_config(const char *var, const char *value, void *cb) return git_log_config(var, value, cb); } -static FILE *realstdout = NULL; static const char *output_directory = NULL; static int outdir_offset; @@ -831,7 +830,7 @@ static int open_next_file(struct commit *commit, const char *subject, fmt_output_subject(&filename, subject, rev); if (!quiet) - fprintf(realstdout, "%s\n", filename.buf + outdir_offset); + printf("%s\n", filename.buf + outdir_offset); if ((rev->diffopt.file = fopen(filename.buf, "w")) == NULL) return error(_("Cannot open patch file %s"), filename.buf); @@ -1639,9 +1638,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) get_patch_ids(&rev, &ids); } - if (!use_stdout) - realstdout = xfdopen(xdup(1), "w"); - if (prepare_revision_walk(&rev)) die(_("revision walk setup failed")); rev.boundary = 1; -- cgit v0.10.2-6-g49f6 From 7f7d712bcfde8afe0a007042d5cb4b809617fb96 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 22 Jun 2016 17:02:07 +0200 Subject: shortlog: respect the --output= setting Thanks to the diff option parsing, we already know about this option. We just have to make use of it. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/builtin/shortlog.c b/builtin/shortlog.c index 39d74fe..be80547 100644 --- a/builtin/shortlog.c +++ b/builtin/shortlog.c @@ -229,7 +229,6 @@ void shortlog_init(struct shortlog *log) log->wrap = DEFAULT_WRAPLEN; log->in1 = DEFAULT_INDENT1; log->in2 = DEFAULT_INDENT2; - log->file = stdout; } int cmd_shortlog(int argc, const char **argv, const char *prefix) @@ -277,6 +276,7 @@ parse_done: log.user_format = rev.commit_format == CMIT_FMT_USERFORMAT; log.abbrev = rev.abbrev; + log.file = rev.diffopt.file; /* assume HEAD if from a tty */ if (!nongit && !rev.pending.nr && isatty(0)) @@ -290,6 +290,8 @@ parse_done: get_from_rev(&rev, &log); shortlog_output(&log); + if (log.file != stdout) + fclose(log.file); return 0; } diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh index a977365..bd699e1 100755 --- a/t/t4201-shortlog.sh +++ b/t/t4201-shortlog.sh @@ -184,4 +184,10 @@ test_expect_success 'shortlog with revision pseudo options' ' git shortlog --exclude=refs/heads/m* --all ' +test_expect_success 'shortlog with --output=' ' + git shortlog --output=shortlog master >output && + test ! -s output && + test_line_count = 7 shortlog +' + test_done -- cgit v0.10.2-6-g49f6 From c1496934cfd6603ac3fbf74cc3a16fcd584d5484 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 22 Jun 2016 17:02:13 +0200 Subject: t4211: ensure that log respects --output= The test script t4202-log.sh is already pretty long, and it is a good idea to test --output with a more obscure option, anyway. So let's test it in conjunction with line-log. The most important part of this test, of course, is to ensure that the file is not closed after writing the diff, but only at the very end of the log output. That is the entire reason why the test tries to generate a log that covers more than one commit. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh index 4451127..9d87777 100755 --- a/t/t4211-line-log.sh +++ b/t/t4211-line-log.sh @@ -99,4 +99,11 @@ test_expect_success '-L with --first-parent and a merge' ' git log --first-parent -L 1,1:b.c ' +test_expect_success '-L with --output' ' + git checkout parallel-change && + git log --output=log -L :main:b.c >output && + test ! -s output && + test_line_count = 70 log +' + test_done -- cgit v0.10.2-6-g49f6 From afc676f2c9e20a5b38d61c7803468d33e2ff45f2 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 22 Jun 2016 16:41:07 +0200 Subject: diff: do not color output when --color=auto and --output= is given "git diff --output= --color=auto" used to show the ANSI color sequence in the resulting file when the standard output is connected to a terminal, because --color=auto check always checks the standard output, not the actual file that receives the output. We could correct this by using freopen(3) to redirect the standard output to the specified file, which is in like with how format-patch used to match the world order, but following the same reasoning as the earlier "format-patch: explicitly switch off color when writing to files", let's be more strict by bypassing the "auto" check when the --output= option is in use. Strictly speaking, this is a backwards-incompatible change, but it is highly unlikely that any user would want to see ANSI color sequences in a file. The reason this was not caught earlier is most likely that either --output= is not used, or only when stdout is redirected anyway. Users can still give --color=always if they want a colored diff in the resulting file. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/diff.c b/diff.c index fa78fc1..b66b9be 100644 --- a/diff.c +++ b/diff.c @@ -3977,6 +3977,8 @@ int diff_opt_parse(struct diff_options *options, if (!options->file) die_errno("Could not open '%s'", path); options->close_file = 1; + if (options->use_color != GIT_COLOR_ALWAYS) + options->use_color = GIT_COLOR_NEVER; return argcount; } else return 0; -- cgit v0.10.2-6-g49f6 From bac233f2c28428c3e2c5279910bc4c027cd0b540 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 11 Jul 2016 15:11:37 +0200 Subject: mingw: fix the shortlog --output= test Adjust t4201 to pass on Windows; a couple of test cases need to be skipped on Windows which leads to a different shortlog than on Linux. Let's just fix that by limiting the shortlog's commit range to traverse only one commit: that guarantees that it does not matter how many test cases were skipped. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh index bd699e1..ae08b57 100755 --- a/t/t4201-shortlog.sh +++ b/t/t4201-shortlog.sh @@ -185,9 +185,9 @@ test_expect_success 'shortlog with revision pseudo options' ' ' test_expect_success 'shortlog with --output=' ' - git shortlog --output=shortlog master >output && + git shortlog --output=shortlog -1 master >output && test ! -s output && - test_line_count = 7 shortlog + test_line_count = 3 shortlog ' test_done -- cgit v0.10.2-6-g49f6