# [PATCH 0/7] Make git log --graph looks better with -p and other diff options 12 messages from 2010-05-15 to 2010-05-17. Participants: Bo Yang, Thomas Rast. Thread: https://gitlist.dev/t/23811 ## Bo Yang, 2010-05-15 11:02 Subject: [PATCH 0/7] Make git log --graph looks better with -p and other diff options Message-ID: <1273921331-32702-1-git-send-email-struggleyb.nku@gmail.com> URL: https://gitlist.dev/e/1273921331-32702-1-git-send-email-struggleyb.nku%40gmail.com ``` When we run git log --graph with any other diff output function, such as '-p','--check','--numstat' or other diff optoins, the diff output area have no graph lines ahead of it. And this make the text graph looks strange. The following 7 patches try to deal with this, and put the text graph columns before all the diff output lines. Bo Yang (7): Add a prefix output callback to diff output. Change to call the new emit_line. Output the graph columns at the end of the commit message. diff.c: Output the text graph padding before each diff line. Emit a whole line once a time. Register a callback for graph output. Make --color-words work well with --graph. color.c | 21 +++- color.h | 3 +- diff.c | 315 ++++++++++++++++++++++++++++++++++++++++++++---------------- diff.h | 5 + graph.c | 22 ++++ log-tree.c | 4 + 6 files changed, 281 insertions(+), 89 deletions(-) ``` ## Bo Yang, 2010-05-15 11:02 Subject: [PATCH 1/7] Add a prefix output callback to diff output. Message-ID: <1273921331-32702-2-git-send-email-struggleyb.nku@gmail.com> URL: https://gitlist.dev/e/1273921331-32702-2-git-send-email-struggleyb.nku%40gmail.com In-Reply-To: <1273921331-32702-1-git-send-email-struggleyb.nku@gmail.com> ``` The callback allow to output some prefix string for each line of diff output. Signed-off-by: Bo Yang --- diff.c | 11 ++++++++--- diff.h | 5 +++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/diff.c b/diff.c index e49f14a..2a38bef 100644 --- a/diff.c +++ b/diff.c @@ -282,11 +282,16 @@ static void check_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2, ecbdata->blank_at_eof_in_postimage = (at - l2) + 1; } -static void emit_line_0(FILE *file, const char *set, const char *reset, +static void emit_line_0(struct diff_options *o, const char *set, const char *reset, int first, const char *line, int len) { int has_trailing_newline, has_trailing_carriage_return; int nofirst; + FILE *file = o->file; + + if (o->output_prefix) { + o->output_prefix(file, 1, o->output_prefix_data); + } if (len == 0) { has_trailing_newline = (first == '\n'); @@ -316,10 +321,10 @@ static void emit_line_0(FILE *file, const char *set, const char *reset, fputc('\n', file); } -static void emit_line(FILE *file, const char *set, const char *reset, +static void emit_line(struct diff_options *o, const char *set, const char *reset, const char *line, int len) { - emit_line_0(file, set, reset, line[0], line+1, len-1); + emit_line_0(o, set, reset, line[0], line+1, len-1); } static int new_blank_line_at_eof(struct emit_callback *ecbdata, const char *line, int len) diff --git a/diff.h b/diff.h index 6a71013..0031dd8 100644 --- a/diff.h +++ b/diff.h @@ -9,6 +9,7 @@ struct rev_info; struct diff_options; struct diff_queue_struct; +struct strbuf; typedef void (*change_fn_t)(struct diff_options *options, unsigned old_mode, unsigned new_mode, @@ -25,6 +26,8 @@ typedef void (*add_remove_fn_t)(struct diff_options *options, typedef void (*diff_format_fn_t)(struct diff_queue_struct *q, struct diff_options *options, void *data); +typedef struct strbuf *(*diff_prefix_fn_t)(FILE *file, int print, void *data); + #define DIFF_FORMAT_RAW 0x0001 #define DIFF_FORMAT_DIFFSTAT 0x0002 #define DIFF_FORMAT_NUMSTAT 0x0004 @@ -122,6 +125,8 @@ struct diff_options { add_remove_fn_t add_remove; diff_format_fn_t format_callback; void *format_callback_data; + diff_prefix_fn_t output_prefix; + void *output_prefix_data; }; enum color_diff { -- 1.7.1.94.gc3269 ``` ## Bo Yang, 2010-05-15 11:02 Subject: [PATCH 2/7] Change to call the new emit_line. Message-ID: <1273921331-32702-3-git-send-email-struggleyb.nku@gmail.com> URL: https://gitlist.dev/e/1273921331-32702-3-git-send-email-struggleyb.nku%40gmail.com In-Reply-To: <1273921331-32702-2-git-send-email-struggleyb.nku@gmail.com> ``` Call the new emit_line function. Signed-off-by: Bo Yang --- diff.c | 34 +++++++++++++++++----------------- 1 files changed, 17 insertions(+), 17 deletions(-) diff --git a/diff.c b/diff.c index 2a38bef..9bd9063 100644 --- a/diff.c +++ b/diff.c @@ -194,8 +194,8 @@ struct emit_callback { sane_truncate_fn truncate; const char **label_path; struct diff_words_data *diff_words; + struct diff_options *opt; int *found_changesp; - FILE *file; struct strbuf *header; }; @@ -346,15 +346,15 @@ static void emit_add_line(const char *reset, const char *set = diff_get_color(ecbdata->color_diff, DIFF_FILE_NEW); if (!*ws) - emit_line_0(ecbdata->file, set, reset, '+', line, len); + emit_line_0(ecbdata->opt, set, reset, '+', line, len); else if (new_blank_line_at_eof(ecbdata, line, len)) /* Blank line at EOF - paint '+' as well */ - emit_line_0(ecbdata->file, ws, reset, '+', line, len); + emit_line_0(ecbdata->opt, ws, reset, '+', line, len); else { /* Emit just the prefix, then the rest. */ - emit_line_0(ecbdata->file, set, reset, '+', "", 0); + emit_line_0(ecbdata->opt, set, reset, '+', "", 0); ws_check_emit(line, len, ecbdata->ws_rule, - ecbdata->file, set, reset, ws); + ecbdata->opt->file, set, reset, ws); } } @@ -375,23 +375,23 @@ static void emit_hunk_header(struct emit_callback *ecbdata, if (len < 10 || memcmp(line, atat, 2) || !(ep = memmem(line + 2, len - 2, atat, 2))) { - emit_line(ecbdata->file, plain, reset, line, len); + emit_line(ecbdata->opt, plain, reset, line, len); return; } ep += 2; /* skip over @@ */ /* The hunk header in fraginfo color */ - emit_line(ecbdata->file, frag, reset, line, ep - line); + emit_line(ecbdata->opt, frag, reset, line, ep - line); /* blank before the func header */ for (cp = ep; ep - line < len; ep++) if (*ep != ' ' && *ep != '\t') break; if (ep != cp) - emit_line(ecbdata->file, plain, reset, cp, ep - cp); + emit_line(ecbdata->opt, plain, reset, cp, ep - cp); if (ep < line + len) - emit_line(ecbdata->file, func, reset, ep, line + len - ep); + emit_line(ecbdata->opt, func, reset, ep, line + len - ep); } static struct diff_tempfile *claim_diff_tempfile(void) { @@ -451,7 +451,7 @@ static void emit_rewrite_lines(struct emit_callback *ecb, len = endp ? (endp - data + 1) : size; if (prefix != '+') { ecb->lno_in_preimage++; - emit_line_0(ecb->file, old, reset, '-', + emit_line_0(ecb->opt, old, reset, '-', data, len); } else { ecb->lno_in_postimage++; @@ -463,7 +463,7 @@ static void emit_rewrite_lines(struct emit_callback *ecb, if (!endp) { const char *plain = diff_get_color(ecb->color_diff, DIFF_PLAIN); - emit_line_0(ecb->file, plain, reset, '\\', + emit_line_0(ecb->opt, plain, reset, '\\', nneof, strlen(nneof)); } } @@ -513,7 +513,7 @@ static void emit_rewrite_diff(const char *name_a, ecbdata.color_diff = color_diff; ecbdata.found_changesp = &o->found_changes; ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a); - ecbdata.file = o->file; + ecbdata.opt = o; if (ecbdata.ws_rule & WS_BLANK_AT_EOF) { mmfile_t mf1, mf2; mf1.ptr = (char *)data_one; @@ -828,7 +828,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len) } if (len < 1) { - emit_line(ecbdata->file, reset, reset, line, len); + emit_line(ecbdata->opt, reset, reset, line, len); return; } @@ -845,7 +845,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len) diff_words_flush(ecbdata); line++; len--; - emit_line(ecbdata->file, plain, reset, line, len); + emit_line(ecbdata->opt, plain, reset, line, len); return; } @@ -856,7 +856,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len) ecbdata->lno_in_preimage++; if (line[0] == ' ') ecbdata->lno_in_postimage++; - emit_line(ecbdata->file, color, reset, line, len); + emit_line(ecbdata->opt, color, reset, line, len); } else { ecbdata->lno_in_postimage++; emit_add_line(reset, ecbdata, line + 1, len - 1); @@ -1421,7 +1421,7 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len) fprintf(data->o->file, "%s:%d: %s.\n", data->filename, data->lineno, err); free(err); - emit_line(data->o->file, set, reset, line, 1); + emit_line(data->o, set, reset, line, 1); ws_check_emit(line + 1, len - 1, data->ws_rule, data->o->file, set, reset, ws); } else if (line[0] == ' ') { @@ -1731,7 +1731,7 @@ static void builtin_diff(const char *name_a, ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a); if (ecbdata.ws_rule & WS_BLANK_AT_EOF) check_blank_at_eof(&mf1, &mf2, &ecbdata); - ecbdata.file = o->file; + ecbdata.opt = o; ecbdata.header = header.len ? &header : NULL; xpp.flags = XDF_NEED_MINIMAL | o->xdl_opts; xecfg.ctxlen = o->context; -- 1.7.1.94.gc3269 ``` ## Bo Yang, 2010-05-15 11:02 Subject: [PATCH 3/7] Output the graph columns at the end of the commit message. Message-ID: <1273921331-32702-4-git-send-email-struggleyb.nku@gmail.com> URL: https://gitlist.dev/e/1273921331-32702-4-git-send-email-struggleyb.nku%40gmail.com In-Reply-To: <1273921331-32702-3-git-send-email-struggleyb.nku@gmail.com> ``` There is an empty line between the commit message and the diff output. Add the graph columns as prefix of this line. Signed-off-by: Bo Yang --- log-tree.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/log-tree.c b/log-tree.c index d3ae969..1da4ea7 100644 --- a/log-tree.c +++ b/log-tree.c @@ -469,6 +469,10 @@ int log_tree_diff_flush(struct rev_info *opt) int pch = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_PATCH; if ((pch & opt->diffopt.output_format) == pch) printf("---"); + if (opt->diffopt.output_prefix) { + opt->diffopt.output_prefix(opt->diffopt.file, + 1, opt->diffopt.output_prefix_data); + } putchar('\n'); } } -- 1.7.1.94.gc3269 ``` ## Bo Yang, 2010-05-15 11:02 Subject: [PATCH 4/7] diff.c: Output the text graph padding before each diff line. Message-ID: <1273921331-32702-5-git-send-email-struggleyb.nku@gmail.com> URL: https://gitlist.dev/e/1273921331-32702-5-git-send-email-struggleyb.nku%40gmail.com In-Reply-To: <1273921331-32702-4-git-send-email-struggleyb.nku@gmail.com> ``` Change -p/--dirstat/--binary/--numstat/--stat/--shortstat to align with graph paddings. Signed-off-by: Bo Yang --- diff.c | 158 +++++++++++++++++++++++++++++++++++++++++++++++----------------- 1 files changed, 116 insertions(+), 42 deletions(-) diff --git a/diff.c b/diff.c index 9bd9063..4a10d16 100644 --- a/diff.c +++ b/diff.c @@ -487,6 +487,13 @@ static void emit_rewrite_diff(const char *name_a, char *data_one, *data_two; size_t size_one, size_two; struct emit_callback ecbdata; + char *line_prefix = ""; + struct strbuf *msgbuf; + + if (o && o->output_prefix) { + msgbuf = o->output_prefix(o->file, 0, o->output_prefix_data); + line_prefix = msgbuf->buf; + } if (diff_mnemonic_prefix && DIFF_OPT_TST(o, REVERSE_DIFF)) { a_prefix = o->b_prefix; @@ -528,9 +535,10 @@ static void emit_rewrite_diff(const char *name_a, lc_a = count_lines(data_one, size_one); lc_b = count_lines(data_two, size_two); fprintf(o->file, - "%s--- %s%s%s\n%s+++ %s%s%s\n%s@@ -", - metainfo, a_name.buf, name_a_tab, reset, - metainfo, b_name.buf, name_b_tab, reset, fraginfo); + "%s%s--- %s%s%s\n%s%s+++ %s%s%s\n%s%s@@ -", + line_prefix, metainfo, a_name.buf, name_a_tab, reset, + line_prefix, metainfo, b_name.buf, name_b_tab, reset, + line_prefix, fraginfo); print_line_count(o->file, lc_a); fprintf(o->file, " +"); print_line_count(o->file, lc_b); @@ -789,9 +797,17 @@ static void fn_out_consume(void *priv, char *line, unsigned long len) const char *meta = diff_get_color(ecbdata->color_diff, DIFF_METAINFO); const char *plain = diff_get_color(ecbdata->color_diff, DIFF_PLAIN); const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET); + struct diff_options *o = ecbdata->opt; + char *line_prefix = ""; + struct strbuf *msgbuf; + + if (o && o->output_prefix) { + msgbuf = o->output_prefix(o->file, 0, o->output_prefix_data); + line_prefix = msgbuf->buf; + } if (ecbdata->header) { - fprintf(ecbdata->file, "%s", ecbdata->header->buf); + fprintf(ecbdata->opt->file, "%s", ecbdata->header->buf); strbuf_reset(ecbdata->header); ecbdata->header = NULL; } @@ -803,10 +819,10 @@ static void fn_out_consume(void *priv, char *line, unsigned long len) name_a_tab = strchr(ecbdata->label_path[0], ' ') ? "\t" : ""; name_b_tab = strchr(ecbdata->label_path[1], ' ') ? "\t" : ""; - fprintf(ecbdata->file, "%s--- %s%s%s\n", - meta, ecbdata->label_path[0], reset, name_a_tab); - fprintf(ecbdata->file, "%s+++ %s%s%s\n", - meta, ecbdata->label_path[1], reset, name_b_tab); + fprintf(ecbdata->opt->file, "%s%s--- %s%s%s\n", + line_prefix, meta, ecbdata->label_path[0], reset, name_a_tab); + fprintf(ecbdata->opt->file, "%s%s+++ %s%s%s\n", + line_prefix, meta, ecbdata->label_path[1], reset, name_b_tab); ecbdata->label_path[0] = ecbdata->label_path[1] = NULL; } @@ -823,7 +839,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len) find_lno(line, ecbdata); emit_hunk_header(ecbdata, line, len); if (line[len-1] != '\n') - putc('\n', ecbdata->file); + putc('\n', ecbdata->opt->file); return; } @@ -1109,6 +1125,10 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options) } if (data->files[i]->is_binary) { + if (options->output_prefix) { + options->output_prefix(options->file, + 1, options->output_prefix_data); + } show_name(options->file, prefix, name, len); fprintf(options->file, " Bin "); fprintf(options->file, "%s%"PRIuMAX"%s", @@ -1121,6 +1141,10 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options) continue; } else if (data->files[i]->is_unmerged) { + if (options->output_prefix) { + options->output_prefix(options->file, + 1, options->output_prefix_data); + } show_name(options->file, prefix, name, len); fprintf(options->file, " Unmerged\n"); continue; @@ -1143,6 +1167,10 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options) add = scale_linear(add, width, max_change); del = scale_linear(del, width, max_change); } + if (options->output_prefix) { + options->output_prefix(options->file, + 1, options->output_prefix_data); + } show_name(options->file, prefix, name, len); fprintf(options->file, "%5"PRIuMAX"%s", added + deleted, added + deleted ? " " : ""); @@ -1150,6 +1178,10 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options) show_graph(options->file, '-', del, del_c, reset); fprintf(options->file, "\n"); } + if (options->output_prefix) { + options->output_prefix(options->file, + 1, options->output_prefix_data); + } fprintf(options->file, " %d files changed, %d insertions(+), %d deletions(-)\n", total_files, adds, dels); @@ -1176,6 +1208,10 @@ static void show_shortstats(struct diffstat_t *data, struct diff_options *option } } } + if (options->output_prefix) { + options->output_prefix(options->file, + 1, options->output_prefix_data); + } fprintf(options->file, " %d files changed, %d insertions(+), %d deletions(-)\n", total_files, adds, dels); } @@ -1190,6 +1226,11 @@ static void show_numstat(struct diffstat_t *data, struct diff_options *options) for (i = 0; i < data->nr; i++) { struct diffstat_file *file = data->files[i]; + if (options->output_prefix) { + options->output_prefix(options->file, + 1, options->output_prefix_data); + } + if (file->is_binary) fprintf(options->file, "-\t-\t"); else @@ -1225,10 +1266,12 @@ struct dirstat_dir { int alloc, nr, percent, cumulative; }; -static long gather_dirstat(FILE *file, struct dirstat_dir *dir, unsigned long changed, const char *base, int baselen) +static long gather_dirstat(struct diff_options *opt, struct dirstat_dir *dir, unsigned long changed, const char *base, int baselen) { unsigned long this_dir = 0; unsigned int sources = 0; + assert(opt); + FILE *file = opt->file; while (dir->nr) { struct dirstat_file *f = dir->files; @@ -1243,7 +1286,7 @@ static long gather_dirstat(FILE *file, struct dirstat_dir *dir, unsigned long ch slash = strchr(f->name + baselen, '/'); if (slash) { int newbaselen = slash + 1 - f->name; - this = gather_dirstat(file, dir, changed, f->name, newbaselen); + this = gather_dirstat(opt, dir, changed, f->name, newbaselen); sources++; } else { this = f->changed; @@ -1265,6 +1308,9 @@ static long gather_dirstat(FILE *file, struct dirstat_dir *dir, unsigned long ch if (permille) { int percent = permille / 10; if (percent >= dir->percent) { + if (opt->output_prefix) { + opt->output_prefix(file, 1, opt->output_prefix_data); + } fprintf(file, "%4d.%01d%% %.*s\n", percent, permille % 10, baselen, base); if (!dir->cumulative) return 0; @@ -1345,7 +1391,7 @@ static void show_dirstat(struct diff_options *options) /* Show all directories with more than x% of the changes */ qsort(dir.files, dir.nr, sizeof(dir.files[0]), dirstat_compare); - gather_dirstat(options->file, &dir, changed, "", 0); + gather_dirstat(options, &dir, changed, "", 0); } static void free_diffstat_info(struct diffstat_t *diffstat) @@ -1403,6 +1449,15 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len) const char *reset = diff_get_color(color_diff, DIFF_RESET); const char *set = diff_get_color(color_diff, DIFF_FILE_NEW); char *err; + char *line_prefix = ""; + struct strbuf *msgbuf; + + assert(data->o); + if (data->o->output_prefix) { + msgbuf = data->o->output_prefix(data->o->file, 0, + data->o->output_prefix_data); + line_prefix = msgbuf->buf; + } if (line[0] == '+') { unsigned bad; @@ -1410,16 +1465,16 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len) if (is_conflict_marker(line + 1, marker_size, len - 1)) { data->status |= 1; fprintf(data->o->file, - "%s:%d: leftover conflict marker\n", - data->filename, data->lineno); + "%s%s:%d: leftover conflict marker\n", + line_prefix, data->filename, data->lineno); } bad = ws_check(line + 1, len - 1, data->ws_rule); if (!bad) return; data->status |= bad; err = whitespace_error_string(bad); - fprintf(data->o->file, "%s:%d: %s.\n", - data->filename, data->lineno, err); + fprintf(data->o->file, "%s%s:%d: %s.\n", + line_prefix, data->filename, data->lineno, err); free(err); emit_line(data->o, set, reset, line, 1); ws_check_emit(line + 1, len - 1, data->ws_rule, @@ -1459,7 +1514,7 @@ static unsigned char *deflate_it(char *data, return deflated; } -static void emit_binary_diff_body(FILE *file, mmfile_t *one, mmfile_t *two) +static void emit_binary_diff_body(FILE *file, mmfile_t *one, mmfile_t *two, char *prefix) { void *cp; void *delta; @@ -1488,13 +1543,13 @@ static void emit_binary_diff_body(FILE *file, mmfile_t *one, mmfile_t *two) } if (delta && delta_size < deflate_size) { - fprintf(file, "delta %lu\n", orig_size); + fprintf(file, "%sdelta %lu\n", prefix, orig_size); free(deflated); data = delta; data_size = delta_size; } else { - fprintf(file, "literal %lu\n", two->size); + fprintf(file, "%sliteral %lu\n", prefix, two->size); free(delta); data = deflated; data_size = deflate_size; @@ -1512,18 +1567,19 @@ static void emit_binary_diff_body(FILE *file, mmfile_t *one, mmfile_t *two) line[0] = bytes - 26 + 'a' - 1; encode_85(line + 1, cp, bytes); cp = (char *) cp + bytes; + fprintf(file, "%s", prefix); fputs(line, file); fputc('\n', file); } - fprintf(file, "\n"); + fprintf(file, "%s\n", prefix); free(data); } -static void emit_binary_diff(FILE *file, mmfile_t *one, mmfile_t *two) +static void emit_binary_diff(FILE *file, mmfile_t *one, mmfile_t *two, char *prefix) { - fprintf(file, "GIT binary patch\n"); - emit_binary_diff_body(file, one, two); - emit_binary_diff_body(file, two, one); + fprintf(file, "%sGIT binary patch\n", prefix); + emit_binary_diff_body(file, one, two, prefix); + emit_binary_diff_body(file, two, one, prefix); } static void diff_filespec_load_driver(struct diff_filespec *one) @@ -1612,6 +1668,13 @@ static void builtin_diff(const char *name_a, struct userdiff_driver *textconv_one = NULL; struct userdiff_driver *textconv_two = NULL; struct strbuf header = STRBUF_INIT; + struct strbuf *msgbuf; + char *line_prefix = ""; + + if (o->output_prefix) { + msgbuf = o->output_prefix(o->file, 0, o->output_prefix_data); + line_prefix = msgbuf->buf; + } if (DIFF_OPT_TST(o, SUBMODULE_LOG) && (!one->mode || S_ISGITLINK(one->mode)) && @@ -1646,22 +1709,22 @@ static void builtin_diff(const char *name_a, b_two = quote_two(b_prefix, name_b + (*name_b == '/')); lbl[0] = DIFF_FILE_VALID(one) ? a_one : "/dev/null"; lbl[1] = DIFF_FILE_VALID(two) ? b_two : "/dev/null"; - strbuf_addf(&header, "%sdiff --git %s %s%s\n", set, a_one, b_two, reset); + strbuf_addf(&header, "%s%sdiff --git %s %s%s\n", line_prefix, set, a_one, b_two, reset); if (lbl[0][0] == '/') { /* /dev/null */ - strbuf_addf(&header, "%snew file mode %06o%s\n", set, two->mode, reset); + strbuf_addf(&header, "%s%snew file mode %06o%s\n", line_prefix, set, two->mode, reset); if (xfrm_msg && xfrm_msg[0]) strbuf_addf(&header, "%s%s%s\n", set, xfrm_msg, reset); } else if (lbl[1][0] == '/') { - strbuf_addf(&header, "%sdeleted file mode %06o%s\n", set, one->mode, reset); + strbuf_addf(&header, "%s%sdeleted file mode %06o%s\n", line_prefix, set, one->mode, reset); if (xfrm_msg && xfrm_msg[0]) strbuf_addf(&header, "%s%s%s\n", set, xfrm_msg, reset); } else { if (one->mode != two->mode) { - strbuf_addf(&header, "%sold mode %06o%s\n", set, one->mode, reset); - strbuf_addf(&header, "%snew mode %06o%s\n", set, two->mode, reset); + strbuf_addf(&header, "%s%sold mode %06o%s\n", line_prefix, set, one->mode, reset); + strbuf_addf(&header, "%s%snew mode %06o%s\n", line_prefix, set, two->mode, reset); } if (xfrm_msg && xfrm_msg[0]) strbuf_addf(&header, "%s%s%s\n", set, xfrm_msg, reset); @@ -1696,10 +1759,10 @@ static void builtin_diff(const char *name_a, fprintf(o->file, "%s", header.buf); strbuf_reset(&header); if (DIFF_OPT_TST(o, BINARY)) - emit_binary_diff(o->file, &mf1, &mf2); + emit_binary_diff(o->file, &mf1, &mf2, line_prefix); else - fprintf(o->file, "Binary files %s and %s differ\n", - lbl[0], lbl[1]); + fprintf(o->file, "%sBinary files %s and %s differ\n", + line_prefix, lbl[0], lbl[1]); o->found_changes = 1; } else { @@ -2309,28 +2372,36 @@ static void fill_metainfo(struct strbuf *msg, struct diff_options *o, struct diff_filepair *p) { + struct strbuf *msgbuf; + char *line_prefix = ""; + + if (o->output_prefix) { + msgbuf = o->output_prefix(o->file, 0, o->output_prefix_data); + line_prefix = msgbuf->buf; + } + strbuf_init(msg, PATH_MAX * 2 + 300); switch (p->status) { case DIFF_STATUS_COPIED: - strbuf_addf(msg, "similarity index %d%%", similarity_index(p)); - strbuf_addstr(msg, "\ncopy from "); + strbuf_addf(msg, "%ssimilarity index %d%%", line_prefix, similarity_index(p)); + strbuf_addf(msg, "\n%scopy from ", line_prefix); quote_c_style(name, msg, NULL, 0); - strbuf_addstr(msg, "\ncopy to "); + strbuf_addf(msg, "\n%scopy to ", line_prefix); quote_c_style(other, msg, NULL, 0); strbuf_addch(msg, '\n'); break; case DIFF_STATUS_RENAMED: - strbuf_addf(msg, "similarity index %d%%", similarity_index(p)); - strbuf_addstr(msg, "\nrename from "); + strbuf_addf(msg, "%ssimilarity index %d%%", line_prefix, similarity_index(p)); + strbuf_addf(msg, "\n%srename from ", line_prefix); quote_c_style(name, msg, NULL, 0); - strbuf_addstr(msg, "\nrename to "); + strbuf_addf(msg, "\n%srename to ", line_prefix); quote_c_style(other, msg, NULL, 0); strbuf_addch(msg, '\n'); break; case DIFF_STATUS_MODIFIED: if (p->score) { - strbuf_addf(msg, "dissimilarity index %d%%\n", - similarity_index(p)); + strbuf_addf(msg, "%sdissimilarity index %d%%\n", + line_prefix, similarity_index(p)); break; } /* fallthru */ @@ -2347,8 +2418,8 @@ static void fill_metainfo(struct strbuf *msg, (!fill_mmfile(&mf, two) && diff_filespec_is_binary(two))) abbrev = 40; } - strbuf_addf(msg, "index %.*s..%.*s", - abbrev, sha1_to_hex(one->sha1), + strbuf_addf(msg, "%sindex %.*s..%.*s", + line_prefix, abbrev, sha1_to_hex(one->sha1), abbrev, sha1_to_hex(two->sha1)); if (one->mode == two->mode) strbuf_addf(msg, " %06o", one->mode); @@ -3027,6 +3098,9 @@ static void diff_flush_raw(struct diff_filepair *p, struct diff_options *opt) { int line_termination = opt->line_termination; int inter_name_termination = line_termination ? '\t' : '\0'; + if (opt->output_prefix) { + opt->output_prefix(opt->file, 1, opt->output_prefix_data); + } if (!(opt->output_format & DIFF_FORMAT_NAME_STATUS)) { fprintf(opt->file, ":%06o %06o %s ", p->one->mode, p->two->mode, -- 1.7.1.94.gc3269 ``` ## Bo Yang, 2010-05-15 11:02 Subject: [PATCH 5/7] Emit a whole line once a time. Message-ID: <1273921331-32702-6-git-send-email-struggleyb.nku@gmail.com> URL: https://gitlist.dev/e/1273921331-32702-6-git-send-email-struggleyb.nku%40gmail.com In-Reply-To: <1273921331-32702-5-git-send-email-struggleyb.nku@gmail.com> ``` Use a strbuf to compose the whole line, and then call emit_line to output it once. This make output_prefix callback works well. Signed-off-by: Bo Yang --- diff.c | 34 +++++++++++++++++++++++++++++----- 1 files changed, 29 insertions(+), 5 deletions(-) diff --git a/diff.c b/diff.c index 4a10d16..ed97115 100644 --- a/diff.c +++ b/diff.c @@ -367,6 +367,18 @@ static void emit_hunk_header(struct emit_callback *ecbdata, const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET); static const char atat[2] = { '@', '@' }; const char *cp, *ep; + struct strbuf msgbuf = STRBUF_INIT; + int org_len = len; + + /* + * trailing \r\n + */ + int i = 1; + for (; i < 3; i++) { + if (line[len - i] == '\r' || line[len - i] == '\n') { + len --; + } + } /* * As a hunk header must begin with "@@ -, + @@", @@ -381,17 +393,29 @@ static void emit_hunk_header(struct emit_callback *ecbdata, ep += 2; /* skip over @@ */ /* The hunk header in fraginfo color */ - emit_line(ecbdata->opt, frag, reset, line, ep - line); + strbuf_add(&msgbuf, frag, strlen(frag)); + strbuf_add(&msgbuf, line, ep - line); + strbuf_add(&msgbuf, reset, strlen(reset)); /* blank before the func header */ for (cp = ep; ep - line < len; ep++) if (*ep != ' ' && *ep != '\t') break; - if (ep != cp) - emit_line(ecbdata->opt, plain, reset, cp, ep - cp); + if (ep != cp) { + strbuf_add(&msgbuf, plain, strlen(plain)); + strbuf_add(&msgbuf, cp, ep - cp); + strbuf_add(&msgbuf, reset, strlen(reset)); + } + + if (ep < line + len) { + strbuf_add(&msgbuf, func, strlen(func)); + strbuf_add(&msgbuf, ep, line + len - ep); + strbuf_add(&msgbuf, reset, strlen(reset)); + } - if (ep < line + len) - emit_line(ecbdata->opt, func, reset, ep, line + len - ep); + strbuf_add(&msgbuf, line + len, org_len - len); + emit_line(ecbdata->opt, "", "", msgbuf.buf, msgbuf.len); + strbuf_release(&msgbuf); } static struct diff_tempfile *claim_diff_tempfile(void) { -- 1.7.1.94.gc3269 ``` ## Bo Yang, 2010-05-15 11:02 Subject: [PATCH 6/7] Register a callback for graph output. Message-ID: <1273921331-32702-7-git-send-email-struggleyb.nku@gmail.com> URL: https://gitlist.dev/e/1273921331-32702-7-git-send-email-struggleyb.nku%40gmail.com In-Reply-To: <1273921331-32702-6-git-send-email-struggleyb.nku@gmail.com> ``` It will look better if the 'git log --graph' print the graph pading lines before the diff output just like what it does for commit message. And this patch leverage the new diff prefix callback function to achieve this. Signed-off-by: Bo Yang --- graph.c | 22 ++++++++++++++++++++++ 1 files changed, 22 insertions(+), 0 deletions(-) diff --git a/graph.c b/graph.c index e6bbcaa..d4d2719 100644 --- a/graph.c +++ b/graph.c @@ -211,6 +211,21 @@ struct git_graph { unsigned short default_column_color; }; +static struct strbuf *diff_output_prefix_callback(FILE *file, int print, void *data) +{ + struct git_graph *graph = data; + static struct strbuf msgbuf = STRBUF_INIT; + + assert(graph); + + strbuf_reset(&msgbuf); + graph_padding_line(graph, &msgbuf); + if (print) { + fwrite(msgbuf.buf, sizeof(char), msgbuf.len, stdout); + } + return &msgbuf; +} + struct git_graph *graph_init(struct rev_info *opt) { struct git_graph *graph = xmalloc(sizeof(struct git_graph)); @@ -244,6 +259,13 @@ struct git_graph *graph_init(struct rev_info *opt) graph->mapping = xmalloc(sizeof(int) * 2 * graph->column_capacity); graph->new_mapping = xmalloc(sizeof(int) * 2 * graph->column_capacity); + /* + * The diff output prefix callback, with this we can make + * all the diff output to align with the graph lines. + */ + opt->diffopt.output_prefix = diff_output_prefix_callback; + opt->diffopt.output_prefix_data = graph; + return graph; } -- 1.7.1.94.gc3269 ``` ## Bo Yang, 2010-05-15 11:02 Subject: [PATCH 7/7] Make --color-words work well with --graph. Message-ID: <1273921331-32702-8-git-send-email-struggleyb.nku@gmail.com> URL: https://gitlist.dev/e/1273921331-32702-8-git-send-email-struggleyb.nku%40gmail.com In-Reply-To: <1273921331-32702-7-git-send-email-struggleyb.nku@gmail.com> ``` Align color words output with text graph lines. Signed-off-by: Bo Yang --- color.c | 21 +++++++++++---- color.h | 3 +- diff.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++------------- 3 files changed, 83 insertions(+), 25 deletions(-) diff --git a/color.c b/color.c index bcf4e2c..e034e3b 100644 --- a/color.c +++ b/color.c @@ -218,22 +218,31 @@ int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...) * Returns 0 on success. */ int color_fwrite_lines(FILE *fp, const char *color, - size_t count, const char *buf) + size_t count, const char *buf, const char *prefix) { - if (!*color) - return fwrite(buf, count, 1, fp) != 1; + int printing = 0; + const char *reset = GIT_COLOR_RESET; + + if (strlen(color) == 0) + reset = ""; + while (count) { char *p = memchr(buf, '\n', count); - if (p != buf && (fputs(color, fp) < 0 || + if (printing) + fputs(prefix, fp); + if (p != buf) { + if (fputs(color, fp) < 0 || fwrite(buf, p ? p - buf : count, 1, fp) != 1 || - fputs(GIT_COLOR_RESET, fp) < 0)) - return -1; + fputs(reset, fp) < 0) + return -1; + } if (!p) return 0; if (fputc('\n', fp) < 0) return -1; count -= p + 1 - buf; buf = p + 1; + printing = 1; } return 0; } diff --git a/color.h b/color.h index 5c264b0..55fea4f 100644 --- a/color.h +++ b/color.h @@ -61,6 +61,7 @@ __attribute__((format (printf, 3, 4))) int color_fprintf(FILE *fp, const char *color, const char *fmt, ...); __attribute__((format (printf, 3, 4))) int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...); -int color_fwrite_lines(FILE *fp, const char *color, size_t count, const char *buf); +int color_fwrite_lines(FILE *fp, const char *color, size_t count, + const char *buf, const char *prefix); #endif /* COLOR_H */ diff --git a/diff.c b/diff.c index ed97115..216cd5b 100644 --- a/diff.c +++ b/diff.c @@ -600,7 +600,8 @@ static void diff_words_append(char *line, unsigned long len, struct diff_words_data { struct diff_words_buffer minus, plus; const char *current_plus; - FILE *file; + int last_minus; + struct diff_options *opt; regex_t *word_regex; }; @@ -609,11 +610,21 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len) struct diff_words_data *diff_words = priv; int minus_first, minus_len, plus_first, plus_len; const char *minus_begin, *minus_end, *plus_begin, *plus_end; + struct diff_options *opt = diff_words->opt; + struct strbuf *msgbuf; + char *line_prefix = ""; + int print = 0; if (line[0] != '@' || parse_hunk_header(line, len, &minus_first, &minus_len, &plus_first, &plus_len)) return; + assert(opt); + if (opt->output_prefix) { + msgbuf = opt->output_prefix(opt->file, 0, opt->output_prefix_data); + line_prefix = msgbuf->buf; + } + /* POSIX requires that first be decremented by one if len == 0... */ if (minus_len) { minus_begin = diff_words->minus.orig[minus_first].begin; @@ -629,20 +640,39 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len) } else plus_begin = plus_end = diff_words->plus.orig[plus_first].end; - if (diff_words->current_plus != plus_begin) - fwrite(diff_words->current_plus, - plus_begin - diff_words->current_plus, 1, - diff_words->file); - if (minus_begin != minus_end) - color_fwrite_lines(diff_words->file, + if (diff_words->current_plus != plus_begin) { + if ((diff_words->current_plus == diff_words->plus.text.ptr && + diff_words->last_minus == 0) || + (diff_words->current_plus > diff_words->plus.text.ptr && + *(diff_words->current_plus - 1) == '\n')) { + fputs(line_prefix, diff_words->opt->file); + print = 1; + } + color_fwrite_lines(diff_words->opt->file, "", + plus_begin - diff_words->current_plus, + diff_words->current_plus, line_prefix); + } + if (minus_begin != minus_end) { + if ((plus_begin > diff_words->plus.text.ptr && + *(plus_begin - 1) == '\n') || + (minus_first == 1 && print == 0)) + fputs(line_prefix, diff_words->opt->file); + color_fwrite_lines(diff_words->opt->file, diff_get_color(1, DIFF_FILE_OLD), - minus_end - minus_begin, minus_begin); - if (plus_begin != plus_end) - color_fwrite_lines(diff_words->file, + minus_end - minus_begin, minus_begin, line_prefix); + } + if (plus_begin != plus_end) { + if((minus_len == 0 && plus_first == 1 && print == 0) || + (plus_begin > diff_words->plus.text.ptr && + *(plus_begin - 1) == '\n' && minus_len == 0)) + fputs(line_prefix, diff_words->opt->file); + color_fwrite_lines(diff_words->opt->file, diff_get_color(1, DIFF_FILE_NEW), - plus_end - plus_begin, plus_begin); + plus_end - plus_begin, plus_begin, line_prefix); + } diff_words->current_plus = plus_end; + diff_words->last_minus = minus_first; } /* This function starts looking at *begin, and returns 0 iff a word was found. */ @@ -721,17 +751,29 @@ static void diff_words_show(struct diff_words_data *diff_words) xpparam_t xpp; xdemitconf_t xecfg; mmfile_t minus, plus; + struct diff_options *opt = diff_words->opt; + struct strbuf *msgbuf; + char *line_prefix = ""; + + assert(opt); + if (opt->output_prefix) { + msgbuf = opt->output_prefix(opt->file, 0, opt->output_prefix_data); + line_prefix = msgbuf->buf; + } /* special case: only removal */ if (!diff_words->plus.text.size) { - color_fwrite_lines(diff_words->file, + fputs(line_prefix, diff_words->opt->file); + color_fwrite_lines(diff_words->opt->file, diff_get_color(1, DIFF_FILE_OLD), - diff_words->minus.text.size, diff_words->minus.text.ptr); + diff_words->minus.text.size, + diff_words->minus.text.ptr, line_prefix); diff_words->minus.text.size = 0; return; } diff_words->current_plus = diff_words->plus.text.ptr; + diff_words->last_minus = 0; memset(&xpp, 0, sizeof(xpp)); memset(&xecfg, 0, sizeof(xecfg)); @@ -745,11 +787,17 @@ static void diff_words_show(struct diff_words_data *diff_words) free(minus.ptr); free(plus.ptr); if (diff_words->current_plus != diff_words->plus.text.ptr + - diff_words->plus.text.size) - fwrite(diff_words->current_plus, + diff_words->plus.text.size) { + if ((diff_words->current_plus == diff_words->plus.text.ptr && + diff_words->last_minus == 0) || + (diff_words->current_plus > diff_words->plus.text.ptr && + *(diff_words->current_plus - 1) == '\n')) + fputs(line_prefix, diff_words->opt->file); + color_fwrite_lines(diff_words->opt->file, "", diff_words->plus.text.ptr + diff_words->plus.text.size - - diff_words->current_plus, 1, - diff_words->file); + - diff_words->current_plus, diff_words->current_plus, + line_prefix); + } diff_words->minus.text.size = diff_words->plus.text.size = 0; } @@ -1835,7 +1883,7 @@ static void builtin_diff(const char *name_a, if (DIFF_OPT_TST(o, COLOR_DIFF_WORDS)) { ecbdata.diff_words = xcalloc(1, sizeof(struct diff_words_data)); - ecbdata.diff_words->file = o->file; + ecbdata.diff_words->opt = o; if (!o->word_regex) o->word_regex = userdiff_word_regex(one); if (!o->word_regex) -- 1.7.1.94.gc3269 ``` ## Thomas Rast, 2010-05-17 06:45 Subject: Re: [PATCH 2/7] Change to call the new emit_line. Message-ID: <201005170845.59018.trast@student.ethz.ch> URL: https://gitlist.dev/e/201005170845.59018.trast%40student.ethz.ch In-Reply-To: <1273921331-32702-3-git-send-email-struggleyb.nku@gmail.com> ``` Bo Yang wrote: > Call the new emit_line function. This should be squashed into the last patch so as to keep the resulting code working. -- Thomas Rast trast@{inf,student}.ethz.ch ``` ## Thomas Rast, 2010-05-17 06:52 Subject: Re: [PATCH 4/7] diff.c: Output the text graph padding before each diff line. Message-ID: <201005170852.31546.trast@student.ethz.ch> URL: https://gitlist.dev/e/201005170852.31546.trast%40student.ethz.ch In-Reply-To: <1273921331-32702-5-git-send-email-struggleyb.nku@gmail.com> ``` Bo Yang wrote: > diff --git a/diff.c b/diff.c > index 9bd9063..4a10d16 100644 > --- a/diff.c > +++ b/diff.c > @@ -487,6 +487,13 @@ static void emit_rewrite_diff(const char *name_a, > char *data_one, *data_two; > size_t size_one, size_two; > struct emit_callback ecbdata; > + char *line_prefix = ""; > + struct strbuf *msgbuf; > + > + if (o && o->output_prefix) { > + msgbuf = o->output_prefix(o->file, 0, o->output_prefix_data); > + line_prefix = msgbuf->buf; > + } Umm. This snippet of code means that the callback can't to much except return or print a string depending on the value of the second argument, doesn't it? So why not either make it explicit and just put a char* field there, or make a more generic callback that replaces write() (or so) and gets a chance to mangle the output in any way it likes? -- Thomas Rast trast@{inf,student}.ethz.ch ``` ## Thomas Rast, 2010-05-17 06:57 Subject: Re: [PATCH 6/7] Register a callback for graph output. Message-ID: <201005170857.47545.trast@student.ethz.ch> URL: https://gitlist.dev/e/201005170857.47545.trast%40student.ethz.ch In-Reply-To: <1273921331-32702-7-git-send-email-struggleyb.nku@gmail.com> ``` Bo Yang wrote: > +static struct strbuf *diff_output_prefix_callback(FILE *file, int print, void *data) > +{ > + struct git_graph *graph = data; > + static struct strbuf msgbuf = STRBUF_INIT; > + > + assert(graph); > + > + strbuf_reset(&msgbuf); > + graph_padding_line(graph, &msgbuf); > + if (print) { > + fwrite(msgbuf.buf, sizeof(char), msgbuf.len, stdout); > + } > + return &msgbuf; > +} > + Ok, this partially answers my last mail and means that a simple string would be too tedious, so a callback is needed. It leaks the 'msgbuf' in the 'printing' case though. -- Thomas Rast trast@{inf,student}.ethz.ch ``` ## Thomas Rast, 2010-05-17 06:59 Subject: Re: [PATCH 7/7] Make --color-words work well with --graph. Message-ID: <201005170859.28022.trast@student.ethz.ch> URL: https://gitlist.dev/e/201005170859.28022.trast%40student.ethz.ch In-Reply-To: <1273921331-32702-8-git-send-email-struggleyb.nku@gmail.com> ``` Bo Yang wrote: > @@ -629,20 +640,39 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len) > } else > plus_begin = plus_end = diff_words->plus.orig[plus_first].end; > > - if (diff_words->current_plus != plus_begin) > - fwrite(diff_words->current_plus, > - plus_begin - diff_words->current_plus, 1, > - diff_words->file); > - if (minus_begin != minus_end) > - color_fwrite_lines(diff_words->file, > + if (diff_words->current_plus != plus_begin) { > + if ((diff_words->current_plus == diff_words->plus.text.ptr && > + diff_words->last_minus == 0) || > + (diff_words->current_plus > diff_words->plus.text.ptr && > + *(diff_words->current_plus - 1) == '\n')) { > + fputs(line_prefix, diff_words->opt->file); > + print = 1; Are these border checks needed/the best solution? If so, please can you put an explanation in the commit message? -- Thomas Rast trast@{inf,student}.ethz.ch ```