From 7f814632f5d4d7af9f4225ece6039dbc44e03079 Mon Sep 17 00:00:00 2001 From: Nguyễn Thái Ngọc Duy Date: Wed, 1 Feb 2012 19:55:07 +0700 Subject: Use correct grammar in diffstat summary line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "git diff --stat" and "git apply --stat" now learn to print the line "%d files changed, %d insertions(+), %d deletions(-)" in singular form whenever applicable. "0 insertions" and "0 deletions" are also omitted unless they are both zero. This matches how versions of "diffstat" that are not prehistoric produced their output, and also makes this line translatable. [jc: with help from Thomas Dickey in archaeology of "diffstat"] [jc: squashed Jonathan's updates to illustrations in tutorials and a test] Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- diff.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 51 insertions(+), 5 deletions(-) (limited to 'diff.c') diff --git a/diff.c b/diff.c index 7e154265f7..84780fd5e6 100644 --- a/diff.c +++ b/diff.c @@ -1322,6 +1322,55 @@ static void fill_print_name(struct diffstat_file *file) file->print_name = pname; } +int print_stat_summary(FILE *fp, int files, int insertions, int deletions) +{ + struct strbuf sb = STRBUF_INIT; + int ret; + + if (!files) { + assert(insertions == 0 && deletions == 0); + return fputs(_(" 0 files changed\n"), fp); + } + + strbuf_addf(&sb, + Q_(" %d file changed", " %d files changed", files), + files); + + /* + * For binary diff, the caller may want to print "x files + * changed" with insertions == 0 && deletions == 0. + * + * Not omitting "0 insertions(+), 0 deletions(-)" in this case + * is probably less confusing (i.e skip over "2 files changed + * but nothing about added/removed lines? Is this a bug in Git?"). + */ + if (insertions || deletions == 0) { + /* + * TRANSLATORS: "+" in (+) is a line addition marker; + * do not translate it. + */ + strbuf_addf(&sb, + Q_(", %d insertion(+)", ", %d insertions(+)", + insertions), + insertions); + } + + if (deletions || insertions == 0) { + /* + * TRANSLATORS: "-" in (-) is a line removal marker; + * do not translate it. + */ + strbuf_addf(&sb, + Q_(", %d deletion(-)", ", %d deletions(-)", + deletions), + deletions); + } + strbuf_addch(&sb, '\n'); + ret = fputs(sb.buf, fp); + strbuf_release(&sb); + return ret; +} + static void show_stats(struct diffstat_t *data, struct diff_options *options) { int i, len, add, del, adds = 0, dels = 0; @@ -1475,9 +1524,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options) extra_shown = 1; } fprintf(options->file, "%s", line_prefix); - fprintf(options->file, - " %d files changed, %d insertions(+), %d deletions(-)\n", - total_files, adds, dels); + print_stat_summary(options->file, total_files, adds, dels); } static void show_shortstats(struct diffstat_t *data, struct diff_options *options) @@ -1507,8 +1554,7 @@ static void show_shortstats(struct diffstat_t *data, struct diff_options *option options->output_prefix_data); fprintf(options->file, "%s", msg->buf); } - fprintf(options->file, " %d files changed, %d insertions(+), %d deletions(-)\n", - total_files, adds, dels); + print_stat_summary(options->file, total_files, adds, dels); } static void show_numstat(struct diffstat_t *data, struct diff_options *options) -- cgit v1.2.3 From 6680a0874f86600cc8b4b5327384934d83d752a8 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 7 Feb 2012 13:23:02 -0500 Subject: drop odd return value semantics from userdiff_config When the userdiff_config function was introduced in be58e70 (diff: unify external diff and funcname parsing code, 2008-10-05), it used a return value convention unlike any other config callback. Like other callbacks, it used "-1" to signal error. But it returned "1" to indicate that it found something, and "0" otherwise; other callbacks simply returned "0" to indicate that no error occurred. This distinction was necessary at the time, because the userdiff namespace overlapped slightly with the color configuration namespace. So "diff.color.foo" could mean "the 'foo' slot of diff coloring" or "the 'foo' component of the "color" userdiff driver". Because the color-parsing code would die on an unknown color slot, we needed the userdiff code to indicate that it had matched the variable, letting us bypass the color-parsing code entirely. Later, in 8b8e862 (ignore unknown color configuration, 2009-12-12), the color-parsing code learned to silently ignore unknown slots. This means we no longer need to protect userdiff-matched variables from reaching the color-parsing code. We can therefore change the userdiff_config calling convention to a more normal one. This drops some code from each caller, which is nice. But more importantly, it reduces the cognitive load for readers who may wonder why userdiff_config is unlike every other config callback. There's no need to add a new test confirming that this works; t4020 already contains a test that sets diff.color.external. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/blame.c | 8 +------- builtin/cat-file.c | 8 +------- builtin/grep.c | 7 ++----- diff.c | 7 ++----- userdiff.c | 19 ++++++------------- 5 files changed, 12 insertions(+), 37 deletions(-) (limited to 'diff.c') diff --git a/builtin/blame.c b/builtin/blame.c index 5a67c202f0..01956c8081 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -2050,14 +2050,8 @@ static int git_blame_config(const char *var, const char *value, void *cb) return 0; } - switch (userdiff_config(var, value)) { - case 0: - break; - case -1: + if (userdiff_config(var, value) < 0) return -1; - default: - return 0; - } return git_default_config(var, value, cb); } diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 07bd984084..8ed501f220 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -226,14 +226,8 @@ static const char * const cat_file_usage[] = { static int git_cat_file_config(const char *var, const char *value, void *cb) { - switch (userdiff_config(var, value)) { - case 0: - break; - case -1: + if (userdiff_config(var, value) < 0) return -1; - default: - return 0; - } return git_default_config(var, value, cb); } diff --git a/builtin/grep.c b/builtin/grep.c index 5c2ae94e55..dc6de83ab7 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -325,11 +325,8 @@ static int grep_config(const char *var, const char *value, void *cb) struct grep_opt *opt = cb; char *color = NULL; - switch (userdiff_config(var, value)) { - case 0: break; - case -1: return -1; - default: return 0; - } + if (userdiff_config(var, value) < 0) + return -1; if (!strcmp(var, "grep.extendedregexp")) { if (git_config_bool(var, value)) diff --git a/diff.c b/diff.c index 7e154265f7..a656f5e83e 100644 --- a/diff.c +++ b/diff.c @@ -177,11 +177,8 @@ int git_diff_basic_config(const char *var, const char *value, void *cb) return 0; } - switch (userdiff_config(var, value)) { - case 0: break; - case -1: return -1; - default: return 0; - } + if (userdiff_config(var, value) < 0) + return -1; if (!prefixcmp(var, "diff.color.") || !prefixcmp(var, "color.diff.")) { int slot = parse_diff_color_slot(var, 11); diff --git a/userdiff.c b/userdiff.c index 76109da4bc..1e7184f7f0 100644 --- a/userdiff.c +++ b/userdiff.c @@ -210,14 +210,7 @@ static int parse_funcname(struct userdiff_funcname *f, const char *k, if (git_config_string(&f->pattern, k, v) < 0) return -1; f->cflags = cflags; - return 1; -} - -static int parse_string(const char **d, const char *k, const char *v) -{ - if (git_config_string(d, k, v) < 0) - return -1; - return 1; + return 0; } static int parse_tristate(int *b, const char *k, const char *v) @@ -226,13 +219,13 @@ static int parse_tristate(int *b, const char *k, const char *v) *b = -1; else *b = git_config_bool(k, v); - return 1; + return 0; } static int parse_bool(int *b, const char *k, const char *v) { *b = git_config_bool(k, v); - return 1; + return 0; } int userdiff_config(const char *k, const char *v) @@ -246,13 +239,13 @@ int userdiff_config(const char *k, const char *v) if ((drv = parse_driver(k, v, "binary"))) return parse_tristate(&drv->binary, k, v); if ((drv = parse_driver(k, v, "command"))) - return parse_string(&drv->external, k, v); + return git_config_string(&drv->external, k, v); if ((drv = parse_driver(k, v, "textconv"))) - return parse_string(&drv->textconv, k, v); + return git_config_string(&drv->textconv, k, v); if ((drv = parse_driver(k, v, "cachetextconv"))) return parse_bool(&drv->textconv_want_cache, k, v); if ((drv = parse_driver(k, v, "wordregex"))) - return parse_string(&drv->word_regex, k, v); + return git_config_string(&drv->word_regex, k, v); return 0; } -- cgit v1.2.3 From 2eeeef24ff5a6538f41f5fe44f3a4e7f6e36e7e5 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 14 Feb 2012 13:49:11 -0800 Subject: diff --stat: show bars of same length for paths with same amount of changes When commit 3ed74e6 (diff --stat: ensure at least one '-' for deletions, and one '+' for additions, 2006-09-28) improved the output for files with tiny modifications, we accidentally broke the logic to ensure that two equal sized changes are shown with the bars of the same length, even when rounding errors exist. Compute the length of the graph bars, using the same "non-zero changes is shown with at least one column" scaling logic, but by scaling the sum of additions and deletions to come up with the total length of the bar (this ensures that two equal sized changes result in bars of the same length), and then scaling the smaller of the additions or deletions. The other side is computed as the difference between the two. This makes the apportioning between additions and deletions less accurate due to rounding errors, but it is much less noticeable than two files with the same amount of change showing bars of different length. Signed-off-by: Junio C Hamano --- diff.c | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) (limited to 'diff.c') diff --git a/diff.c b/diff.c index 374ecf3b48..74db18846f 100644 --- a/diff.c +++ b/diff.c @@ -1267,13 +1267,15 @@ const char mime_boundary_leader[] = "------------"; static int scale_linear(int it, int width, int max_change) { + if (!it) + return 0; /* - * make sure that at least one '-' is printed if there were deletions, - * and likewise for '+'. + * make sure that at least one '-' or '+' is printed if + * there is any change to this path. The easiest way is to + * scale linearly as if the alloted width is one column shorter + * than it is, and then add 1 to the result. */ - if (max_change < 2) - return it; - return ((it - 1) * (width - 1) + max_change - 1) / (max_change - 1); + return 1 + (it * (width - 1) / max_change); } static void show_name(FILE *file, @@ -1440,8 +1442,19 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options) dels += del; if (width <= max_change) { - add = scale_linear(add, width, max_change); - del = scale_linear(del, width, max_change); + int total = add + del; + + total = scale_linear(add + del, width, max_change); + if (total < 2 && add && del) + /* width >= 2 due to the sanity check */ + total = 2; + if (add < del) { + add = scale_linear(add, width, max_change); + del = total - add; + } else { + del = scale_linear(del, width, max_change); + add = total - del; + } } fprintf(options->file, "%s", line_prefix); show_name(options->file, prefix, name, len); -- cgit v1.2.3