The way that available columns are divided between the filename part and the graph part is modified to use as many columns as necessary for the filenames and up to 40 columns for the graph. If commits changing a lot of lines are displayed in a wide terminal window (200 or more columns), and the +- graph would use the full width, the output would look bad. Messages wrapped to about 80 columns would be interspersed with very long +- lines. It makes sense to limit the width of the graph part to a fixed value, even if more columns are available. This fixed value is subjectively hard-coded to be 40 columns, which seems to work well for git.git and linux-2.6.git and some other repositories. If there isn't enough columns to print both the filename and the graph, at least 5/8 of available space is devoted to filenames. On a standard 80 column terminal, or if not connected to a terminal and using the default of 80 columns, this gives the same partition as before. The effect of this change is visible in the patch to t4052 that fixes a few tests marked with test_expect_failure, and the change to shorten the maximum graph width to 40 columns. Since limiting the graph part to 40 columns makes COLUMNS=200 stop influencing diff --stat behaviour, because it isn't wide enough now, a new test with COLUMNS=40 is added to check that the environment variable influences diff, show, log, but not format-patch. The old test with COLUMNS=200 is added to check for regressions. Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@xxxxxxxxx> --- Documentation/diff-options.txt | 14 +++--- diff.c | 94 +++++++++++++++++++++++++++------------- t/t4052-stat-output.sh | 45 ++++++++++++------- 3 files changed, 102 insertions(+), 51 deletions(-) diff --git Documentation/diff-options.txt Documentation/diff-options.txt index 9ed78c9..e4d0e3e 100644 --- Documentation/diff-options.txt +++ Documentation/diff-options.txt @@ -53,13 +53,15 @@ endif::git-format-patch[] Generate a diff using the "patience diff" algorithm. --stat[=<width>[,<name-width>[,<count>]]]:: - Generate a diffstat. You can override the default - output width for 80-column terminal by `--stat=<width>`. - The width of the filename part can be controlled by - giving another width to it separated by a comma. + Generate a diffstat. By default, as much space as necessary + will be used for the filename part, and up to 40 columns for + the graph part. Maximum width defaults to terminal width, + or 80 columns if not connected to a terminal, and can be + overriden by `<width>`. The width of the filename part can be + limited by giving another width `<name-width>` after a comma. By giving a third parameter `<count>`, you can limit the - output to the first `<count>` lines, followed by - `...` if there are more. + output to the first `<count>` lines, followed by `...` if + there are more. + These parameters can also be set individually with `--stat-width=<width>`, `--stat-name-width=<name-width>` and `--stat-count=<count>`. diff --git diff.c diff.c index 1db46a4..8a9a387 100644 --- diff.c +++ diff.c @@ -1375,7 +1375,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options) int i, len, add, del, adds = 0, dels = 0; uintmax_t max_change = 0, max_len = 0; int total_files = data->nr; - int width, name_width, count; + int width, name_width, graph_width, number_width = 4, count; const char *reset, *add_c, *del_c; const char *line_prefix = ""; int extra_shown = 0; @@ -1389,28 +1389,15 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options) line_prefix = msg->buf; } - if (options->stat_width == -1) - width = term_columns(); - else - width = options->stat_width ? options->stat_width : 80; - name_width = options->stat_name_width ? options->stat_name_width : 50; count = options->stat_count ? options->stat_count : data->nr; - /* Sanity: give at least 5 columns to the graph, - * but leave at least 10 columns for the name. - */ - if (width < 25) - width = 25; - if (name_width < 10) - name_width = 10; - else if (width < name_width + 15) - name_width = width - 15; - - /* Find the longest filename and max number of changes */ reset = diff_get_color_opt(options, DIFF_RESET); add_c = diff_get_color_opt(options, DIFF_FILE_NEW); del_c = diff_get_color_opt(options, DIFF_FILE_OLD); + /* + * Find the longest filename and max number of changes + */ for (i = 0; (i < count) && (i < data->nr); i++) { struct diffstat_file *file = data->files[i]; uintmax_t change = file->added + file->deleted; @@ -1431,19 +1418,66 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options) } count = i; /* min(count, data->nr) */ - /* Compute the width of the graph part; - * 10 is for one blank at the beginning of the line plus - * " | count " between the name and the graph. + /* + * We have width = stat_width or term_columns() columns total. + * We want a maximum of min(max_len, stat_name_width) for the name part. + * We want a maximum of min(max_change, 40) for the +- part. + * We also need 1 for " " and 4 + decimal_width(max_change) + * for " | NNNN " and one the empty column at the end, altogether + * 6 + decimal_width(max_change). + * + * If there's not enough space, we will use the smaller of + * stat_name_width (if set) and 5/8*width for the filename, + * and the rest for constant elements + graph part, but no more + * than 40 for the graph part. + * (5/8 gives 50 for filename and 30 for the constant parts + graph + * for the standard terminal size). * - * From here on, name_width is the width of the name area, - * and width is the width of the graph area. + * In other words: stat_width limits the maximum width, and + * stat_name_width fixes the maximum width of the filename, + * and is also used to divide available columns if there + * aren't enough. */ - name_width = (name_width < max_len) ? name_width : max_len; - if (width < (name_width + 10) + max_change) - width = width - (name_width + 10); + + if (options->stat_width == -1) + width = term_columns(); else - width = max_change; + width = options->stat_width ? options->stat_width : 80; + /* + * Guarantee 3/8*16==6 for the graph part + * and 5/8*16==10 for the filename part + */ + if (width < 16 + 6 + number_width) + width = 16 + 6 + number_width; + + /* + * First assign sizes that are wanted, ignoring available width. + */ + graph_width = max_change < 40 ? max_change : 40; + name_width = (options->stat_name_width > 0 && + options->stat_name_width < max_len) ? + options->stat_name_width : max_len; + + /* + * Adjust adjustable widths not to exceed maximum width + */ + if (name_width + number_width + 6 + graph_width > width) { + if (graph_width > width * 3/8 - number_width - 6) + graph_width = width * 3/8 - number_width - 6; + if (graph_width > 40) + graph_width = 40; + if (name_width > width - number_width - 6 - graph_width) + name_width = width - number_width - 6 - graph_width; + else + graph_width = width - number_width - 6 - name_width; + } + + /* + * From here name_width is the width of the name area, + * and graph_width is the width of the graph area. + * max_change is used to scale graph properly. + */ for (i = 0; i < count; i++) { const char *prefix = ""; char *name = data->files[i]->print_name; @@ -1499,18 +1533,18 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options) adds += add; dels += del; - if (width <= max_change) { + if (graph_width <= max_change) { int total = add + del; - total = scale_linear(add + del, width, max_change); + total = scale_linear(add + del, graph_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); + add = scale_linear(add, graph_width, max_change); del = total - add; } else { - del = scale_linear(del, width, max_change); + del = scale_linear(del, graph_width, max_change); add = total - del; } } diff --git t/t4052-stat-output.sh t/t4052-stat-output.sh index 2b4510c..1b237b7 100755 --- t/t4052-stat-output.sh +++ t/t4052-stat-output.sh @@ -28,19 +28,19 @@ EOF while read cmd args do - test_expect_failure "$cmd graph width defaults to 80 columns" ' + test_expect_success "$cmd graph width defaults to 80 columns" ' git $cmd $args >output && grep " | " output >actual && test_cmp expect80 actual ' - test_expect_failure "$cmd --stat=width with long name" ' + test_expect_success "$cmd --stat=width with long name" ' git $cmd $args --stat=40 >output && grep " | " output >actual && test_cmp expect40 actual ' - test_expect_failure "$cmd --stat-width=width with long name" ' + test_expect_success "$cmd --stat-width=width with long name" ' git $cmd $args --stat-width=40 >output && grep " | " output >actual && test_cmp expect40 actual @@ -78,27 +78,42 @@ test_expect_success 'preparation for big change tests' ' ' cat >expect80 <<'EOF' - abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ + abcd | 1000 ++++++++++++++++++++++++++++++++++++++++ EOF -cat >expect200 <<'EOF' - abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ +while read verb expect cmd args +do + test_expect_success "$cmd $verb too many COLUMNS (big change)" ' + COLUMNS=200 git $cmd $args >output + grep " | " output >actual && + test_cmp "$expect" actual + ' +done <<\EOF +ignores expect80 format-patch -1 --stdout +respects expect80 diff HEAD^ HEAD --stat +respects expect80 show --stat +respects expect80 log -1 --stat +EOF + +cat >expect40 <<'EOF' + abcd | 1000 ++++++++++++++++++++++++++ EOF while read verb expect cmd args do - test_expect_success "$cmd $verb COLUMNS (big change)" ' - COLUMNS=200 git $cmd $args >output + test_expect_success "$cmd $verb not enough COLUMNS (big change)" ' + COLUMNS=40 git $cmd $args >output grep " | " output >actual && test_cmp "$expect" actual ' done <<\EOF ignores expect80 format-patch -1 --stdout -respects expect200 diff HEAD^ HEAD --stat -respects expect200 show --stat -respects expect200 log -1 --stat +respects expect40 diff HEAD^ HEAD --stat +respects expect40 show --stat +respects expect40 log -1 --stat EOF + cat >expect <<'EOF' abcd | 1000 ++++++++++++++++++++++++++ EOF @@ -130,7 +145,7 @@ test_expect_success 'preparation for long filename tests' ' ' cat >expect <<'EOF' - ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 +++++ + ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 ++++++++++++ EOF while read cmd args @@ -151,7 +166,7 @@ cat >expect80 <<'EOF' ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 ++++++++++++++++++++ EOF cat >expect200 <<'EOF' - ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 ++++++++++++++++++++++++++++++++++++++++ EOF while read verb expect cmd args do @@ -168,7 +183,7 @@ respects expect200 log -1 --stat EOF cat >expect <<'EOF' - abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ + abcd | 1000 ++++++++++++++++++++++++++++++++++++++++ EOF test_expect_success 'merge --stat respects COLUMNS (big change)' ' git checkout -b branch HEAD^^ && @@ -178,7 +193,7 @@ test_expect_success 'merge --stat respects COLUMNS (big change)' ' ' cat >expect <<'EOF' - ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 ++++++++++++++++++++++++++++++++++++++++ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 +++++++++++++++++++++++++++++++++++++++ EOF test_expect_success 'merge --stat respects COLUMNS (long filename)' ' COLUMNS=100 git merge --stat --no-ff master >output && -- 1.7.9.1.353.g684b4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html