From 5502039d87337e076e69f7de949cb0e0360de16f Mon Sep 17 00:00:00 2001 From: Johan Herland Date: Fri, 29 Apr 2011 11:36:16 +0200 Subject: Add several testcases for --dirstat and friends Currently, t4013 is the only selftest that exercises the --dirstat machinery, but it only does a superficial verification of --dirstat's output. This patch adds a new selftest - t4047-diff-dirstat.sh - which prepares a commit containing: - unchanged files, changed files and files with rearranged lines - copied files, moved files, and unmoved files It then verifies the correct dirstat output for that commit in the following dirstat modes: - --dirstat - -X - --dirstat=0 - -X0 - --cumulative - --dirstat-by-file - (plus combinations of the above) Each of the above tests are also run with: - no rename detection - rename detection (-M) - expensive copy detection (-C -C) Improved-by: Junio C Hamano Signed-off-by: Johan Herland Signed-off-by: Junio C Hamano --- t/t4047-diff-dirstat.sh | 585 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 585 insertions(+) create mode 100755 t/t4047-diff-dirstat.sh diff --git a/t/t4047-diff-dirstat.sh b/t/t4047-diff-dirstat.sh new file mode 100755 index 0000000000..ce7c4033f8 --- /dev/null +++ b/t/t4047-diff-dirstat.sh @@ -0,0 +1,585 @@ +#!/bin/sh + +test_description='diff --dirstat tests' +. ./test-lib.sh + +# set up two commits where the second commit has these files +# (10 lines in each file): +# +# unchanged/text (unchanged from 1st commit) +# changed/text (changed 1st line) +# rearranged/text (swapped 1st and 2nd line) +# dst/copy/unchanged/text (copied from src/copy/unchanged/text, unchanged) +# dst/copy/changed/text (copied from src/copy/changed/text, changed) +# dst/copy/rearranged/text (copied from src/copy/rearranged/text, rearranged) +# dst/move/unchanged/text (moved from src/move/unchanged/text, unchanged) +# dst/move/changed/text (moved from src/move/changed/text, changed) +# dst/move/rearranged/text (moved from src/move/rearranged/text, rearranged) + +test_expect_success 'setup' ' + mkdir unchanged && + mkdir changed && + mkdir rearranged && + mkdir src && + mkdir src/copy && + mkdir src/copy/unchanged && + mkdir src/copy/changed && + mkdir src/copy/rearranged && + mkdir src/move && + mkdir src/move/unchanged && + mkdir src/move/changed && + mkdir src/move/rearranged && + cat <unchanged/text && +unchanged line #0 +unchanged line #1 +unchanged line #2 +unchanged line #3 +unchanged line #4 +unchanged line #5 +unchanged line #6 +unchanged line #7 +unchanged line #8 +unchanged line #9 +EOF + cat <changed/text && +changed line #0 +changed line #1 +changed line #2 +changed line #3 +changed line #4 +changed line #5 +changed line #6 +changed line #7 +changed line #8 +changed line #9 +EOF + cat <rearranged/text && +rearranged line #0 +rearranged line #1 +rearranged line #2 +rearranged line #3 +rearranged line #4 +rearranged line #5 +rearranged line #6 +rearranged line #7 +rearranged line #8 +rearranged line #9 +EOF + cat <src/copy/unchanged/text && +copy unchanged line #0 +copy unchanged line #1 +copy unchanged line #2 +copy unchanged line #3 +copy unchanged line #4 +copy unchanged line #5 +copy unchanged line #6 +copy unchanged line #7 +copy unchanged line #8 +copy unchanged line #9 +EOF + cat <src/copy/changed/text && +copy changed line #0 +copy changed line #1 +copy changed line #2 +copy changed line #3 +copy changed line #4 +copy changed line #5 +copy changed line #6 +copy changed line #7 +copy changed line #8 +copy changed line #9 +EOF + cat <src/copy/rearranged/text && +copy rearranged line #0 +copy rearranged line #1 +copy rearranged line #2 +copy rearranged line #3 +copy rearranged line #4 +copy rearranged line #5 +copy rearranged line #6 +copy rearranged line #7 +copy rearranged line #8 +copy rearranged line #9 +EOF + cat <src/move/unchanged/text && +move unchanged line #0 +move unchanged line #1 +move unchanged line #2 +move unchanged line #3 +move unchanged line #4 +move unchanged line #5 +move unchanged line #6 +move unchanged line #7 +move unchanged line #8 +move unchanged line #9 +EOF + cat <src/move/changed/text && +move changed line #0 +move changed line #1 +move changed line #2 +move changed line #3 +move changed line #4 +move changed line #5 +move changed line #6 +move changed line #7 +move changed line #8 +move changed line #9 +EOF + cat <src/move/rearranged/text && +move rearranged line #0 +move rearranged line #1 +move rearranged line #2 +move rearranged line #3 +move rearranged line #4 +move rearranged line #5 +move rearranged line #6 +move rearranged line #7 +move rearranged line #8 +move rearranged line #9 +EOF + git add . && + git commit -m "initial" && + mkdir dst && + mkdir dst/copy && + mkdir dst/copy/unchanged && + mkdir dst/copy/changed && + mkdir dst/copy/rearranged && + mkdir dst/move && + mkdir dst/move/unchanged && + mkdir dst/move/changed && + mkdir dst/move/rearranged && + cat <changed/text && +CHANGED XXXXXXX line #0 +changed line #1 +changed line #2 +changed line #3 +changed line #4 +changed line #5 +changed line #6 +changed line #7 +changed line #8 +changed line #9 +EOF + cat <rearranged/text && +rearranged line #1 +rearranged line #0 +rearranged line #2 +rearranged line #3 +rearranged line #4 +rearranged line #5 +rearranged line #6 +rearranged line #7 +rearranged line #8 +rearranged line #9 +EOF + cat <dst/copy/unchanged/text && +copy unchanged line #0 +copy unchanged line #1 +copy unchanged line #2 +copy unchanged line #3 +copy unchanged line #4 +copy unchanged line #5 +copy unchanged line #6 +copy unchanged line #7 +copy unchanged line #8 +copy unchanged line #9 +EOF + cat <dst/copy/changed/text && +copy XXXCHANGED line #0 +copy changed line #1 +copy changed line #2 +copy changed line #3 +copy changed line #4 +copy changed line #5 +copy changed line #6 +copy changed line #7 +copy changed line #8 +copy changed line #9 +EOF + cat <dst/copy/rearranged/text && +copy rearranged line #1 +copy rearranged line #0 +copy rearranged line #2 +copy rearranged line #3 +copy rearranged line #4 +copy rearranged line #5 +copy rearranged line #6 +copy rearranged line #7 +copy rearranged line #8 +copy rearranged line #9 +EOF + cat <dst/move/unchanged/text && +move unchanged line #0 +move unchanged line #1 +move unchanged line #2 +move unchanged line #3 +move unchanged line #4 +move unchanged line #5 +move unchanged line #6 +move unchanged line #7 +move unchanged line #8 +move unchanged line #9 +EOF + cat <dst/move/changed/text && +move XXXCHANGED line #0 +move changed line #1 +move changed line #2 +move changed line #3 +move changed line #4 +move changed line #5 +move changed line #6 +move changed line #7 +move changed line #8 +move changed line #9 +EOF + cat <dst/move/rearranged/text && +move rearranged line #1 +move rearranged line #0 +move rearranged line #2 +move rearranged line #3 +move rearranged line #4 +move rearranged line #5 +move rearranged line #6 +move rearranged line #7 +move rearranged line #8 +move rearranged line #9 +EOF + git add . && + git rm -r src/move/unchanged && + git rm -r src/move/changed && + git rm -r src/move/rearranged && + git commit -m "changes" +' + +cat <expect_diff_stat + changed/text | 2 +- + dst/copy/changed/text | 10 ++++++++++ + dst/copy/rearranged/text | 10 ++++++++++ + dst/copy/unchanged/text | 10 ++++++++++ + dst/move/changed/text | 10 ++++++++++ + dst/move/rearranged/text | 10 ++++++++++ + dst/move/unchanged/text | 10 ++++++++++ + rearranged/text | 2 +- + src/move/changed/text | 10 ---------- + src/move/rearranged/text | 10 ---------- + src/move/unchanged/text | 10 ---------- + 11 files changed, 62 insertions(+), 32 deletions(-) +EOF + +cat <expect_diff_stat_M + changed/text | 2 +- + dst/copy/changed/text | 10 ++++++++++ + dst/copy/rearranged/text | 10 ++++++++++ + dst/copy/unchanged/text | 10 ++++++++++ + {src => dst}/move/changed/text | 2 +- + {src => dst}/move/rearranged/text | 2 +- + {src => dst}/move/unchanged/text | 0 + rearranged/text | 2 +- + 8 files changed, 34 insertions(+), 4 deletions(-) +EOF + +cat <expect_diff_stat_CC + changed/text | 2 +- + {src => dst}/copy/changed/text | 2 +- + {src => dst}/copy/rearranged/text | 2 +- + {src => dst}/copy/unchanged/text | 0 + {src => dst}/move/changed/text | 2 +- + {src => dst}/move/rearranged/text | 2 +- + {src => dst}/move/unchanged/text | 0 + rearranged/text | 2 +- + 8 files changed, 6 insertions(+), 6 deletions(-) +EOF + +test_expect_success 'sanity check setup (--stat)' ' + git diff --stat HEAD^..HEAD >actual_diff_stat && + test_cmp expect_diff_stat actual_diff_stat && + git diff --stat -M HEAD^..HEAD >actual_diff_stat_M && + test_cmp expect_diff_stat_M actual_diff_stat_M && + git diff --stat -C -C HEAD^..HEAD >actual_diff_stat_CC && + test_cmp expect_diff_stat_CC actual_diff_stat_CC +' + +# changed/text and rearranged/text falls below default 3% threshold +cat <expect_diff_dirstat + 10.8% dst/copy/changed/ + 10.8% dst/copy/rearranged/ + 10.8% dst/copy/unchanged/ + 10.8% dst/move/changed/ + 10.8% dst/move/rearranged/ + 10.8% dst/move/unchanged/ + 10.8% src/move/changed/ + 10.8% src/move/rearranged/ + 10.8% src/move/unchanged/ +EOF + +# rearranged/text falls below default 3% threshold +cat <expect_diff_dirstat_M + 5.8% changed/ + 29.3% dst/copy/changed/ + 29.3% dst/copy/rearranged/ + 29.3% dst/copy/unchanged/ + 5.8% dst/move/changed/ +EOF + +# rearranged/text falls below default 3% threshold +cat <expect_diff_dirstat_CC + 32.6% changed/ + 32.6% dst/copy/changed/ + 32.6% dst/move/changed/ +EOF + +test_expect_success 'various ways to misspell --dirstat' ' + test_must_fail git show --dirstat10 && + test_must_fail git show -X=20 +' + +test_expect_success 'vanilla --dirstat' ' + git diff --dirstat HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git diff --dirstat -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git diff --dirstat -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + +test_expect_success 'vanilla -X' ' + git diff -X HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git diff -X -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git diff -X -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + +# rearranged/text falls below 0% threshold (1 / (240 * 9 + 48 + 1) ~= 0.045 %) +cat <expect_diff_dirstat + 2.1% changed/ + 10.8% dst/copy/changed/ + 10.8% dst/copy/rearranged/ + 10.8% dst/copy/unchanged/ + 10.8% dst/move/changed/ + 10.8% dst/move/rearranged/ + 10.8% dst/move/unchanged/ + 10.8% src/move/changed/ + 10.8% src/move/rearranged/ + 10.8% src/move/unchanged/ +EOF + +cat <expect_diff_dirstat_M + 5.8% changed/ + 29.3% dst/copy/changed/ + 29.3% dst/copy/rearranged/ + 29.3% dst/copy/unchanged/ + 5.8% dst/move/changed/ + 0.1% dst/move/rearranged/ + 0.1% rearranged/ +EOF + +cat <expect_diff_dirstat_CC + 32.6% changed/ + 32.6% dst/copy/changed/ + 0.6% dst/copy/rearranged/ + 32.6% dst/move/changed/ + 0.6% dst/move/rearranged/ + 0.6% rearranged/ +EOF + +test_expect_success '--dirstat=0' ' + git diff --dirstat=0 HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git diff --dirstat=0 -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git diff --dirstat=0 -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + +test_expect_success '-X0' ' + git diff -X0 HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git diff -X0 -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git diff -X0 -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + +# rearranged/text falls below 0% threshold (1 / (240 * 9 + 48 + 1) ~= 0.045 %) +cat <expect_diff_dirstat + 2.1% changed/ + 10.8% dst/copy/changed/ + 10.8% dst/copy/rearranged/ + 10.8% dst/copy/unchanged/ + 32.5% dst/copy/ + 10.8% dst/move/changed/ + 10.8% dst/move/rearranged/ + 10.8% dst/move/unchanged/ + 32.5% dst/move/ + 65.1% dst/ + 10.8% src/move/changed/ + 10.8% src/move/rearranged/ + 10.8% src/move/unchanged/ + 32.5% src/move/ +EOF + +cat <expect_diff_dirstat_M + 5.8% changed/ + 29.3% dst/copy/changed/ + 29.3% dst/copy/rearranged/ + 29.3% dst/copy/unchanged/ + 88.0% dst/copy/ + 5.8% dst/move/changed/ + 0.1% dst/move/rearranged/ + 5.9% dst/move/ + 94.0% dst/ + 0.1% rearranged/ +EOF + +cat <expect_diff_dirstat_CC + 32.6% changed/ + 32.6% dst/copy/changed/ + 0.6% dst/copy/rearranged/ + 33.3% dst/copy/ + 32.6% dst/move/changed/ + 0.6% dst/move/rearranged/ + 33.3% dst/move/ + 66.6% dst/ + 0.6% rearranged/ +EOF + +test_expect_success '--dirstat=0 --cumulative' ' + git diff --dirstat=0 --cumulative HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git diff --dirstat=0 --cumulative -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git diff --dirstat=0 --cumulative -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + +cat <expect_diff_dirstat + 9.0% changed/ + 9.0% dst/copy/changed/ + 9.0% dst/copy/rearranged/ + 9.0% dst/copy/unchanged/ + 9.0% dst/move/changed/ + 9.0% dst/move/rearranged/ + 9.0% dst/move/unchanged/ + 9.0% rearranged/ + 9.0% src/move/changed/ + 9.0% src/move/rearranged/ + 9.0% src/move/unchanged/ +EOF + +cat <expect_diff_dirstat_M + 14.2% changed/ + 14.2% dst/copy/changed/ + 14.2% dst/copy/rearranged/ + 14.2% dst/copy/unchanged/ + 14.2% dst/move/changed/ + 14.2% dst/move/rearranged/ + 14.2% rearranged/ +EOF + +cat <expect_diff_dirstat_CC + 16.6% changed/ + 16.6% dst/copy/changed/ + 16.6% dst/copy/rearranged/ + 16.6% dst/move/changed/ + 16.6% dst/move/rearranged/ + 16.6% rearranged/ +EOF + +test_expect_success '--dirstat-by-file' ' + git diff --dirstat-by-file HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git diff --dirstat-by-file -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git diff --dirstat-by-file -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + +cat <expect_diff_dirstat + 27.2% dst/copy/ + 27.2% dst/move/ + 27.2% src/move/ +EOF + +cat <expect_diff_dirstat_M + 14.2% changed/ + 14.2% dst/copy/changed/ + 14.2% dst/copy/rearranged/ + 14.2% dst/copy/unchanged/ + 14.2% dst/move/changed/ + 14.2% dst/move/rearranged/ + 14.2% rearranged/ +EOF + +cat <expect_diff_dirstat_CC + 16.6% changed/ + 16.6% dst/copy/changed/ + 16.6% dst/copy/rearranged/ + 16.6% dst/move/changed/ + 16.6% dst/move/rearranged/ + 16.6% rearranged/ +EOF + +test_expect_success '--dirstat-by-file=10' ' + git diff --dirstat-by-file=10 HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git diff --dirstat-by-file=10 -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git diff --dirstat-by-file=10 -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + +cat <expect_diff_dirstat + 9.0% changed/ + 9.0% dst/copy/changed/ + 9.0% dst/copy/rearranged/ + 9.0% dst/copy/unchanged/ + 27.2% dst/copy/ + 9.0% dst/move/changed/ + 9.0% dst/move/rearranged/ + 9.0% dst/move/unchanged/ + 27.2% dst/move/ + 54.5% dst/ + 9.0% rearranged/ + 9.0% src/move/changed/ + 9.0% src/move/rearranged/ + 9.0% src/move/unchanged/ + 27.2% src/move/ +EOF + +cat <expect_diff_dirstat_M + 14.2% changed/ + 14.2% dst/copy/changed/ + 14.2% dst/copy/rearranged/ + 14.2% dst/copy/unchanged/ + 42.8% dst/copy/ + 14.2% dst/move/changed/ + 14.2% dst/move/rearranged/ + 28.5% dst/move/ + 71.4% dst/ + 14.2% rearranged/ +EOF + +cat <expect_diff_dirstat_CC + 16.6% changed/ + 16.6% dst/copy/changed/ + 16.6% dst/copy/rearranged/ + 33.3% dst/copy/ + 16.6% dst/move/changed/ + 16.6% dst/move/rearranged/ + 33.3% dst/move/ + 66.6% dst/ + 16.6% rearranged/ +EOF + +test_expect_success '--dirstat-by-file --cumulative' ' + git diff --dirstat-by-file --cumulative HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git diff --dirstat-by-file --cumulative -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git diff --dirstat-by-file --cumulative -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + +test_done -- cgit v1.2.3 From 58a8756a98fbd08add1c25ac61ffa4f4f8a6e776 Mon Sep 17 00:00:00 2001 From: Johan Herland Date: Fri, 29 Apr 2011 11:36:17 +0200 Subject: Make --dirstat=0 output directories that contribute < 0.1% of changes The expected output from --dirstat=0, is to include any directory with changes, even if those changes contribute a minuscule portion of the total changes. However, currently, directories that contribute less than 0.1% are not included, since their 'permille' value is 0, and there is an 'if (permille)' check in gather_dirstat() that causes them to be ignored. This test is obviously intended to exclude directories that contribute no changes whatsoever, but in this case, it hits too broadly. The correct check is against 'this_dir' from which the permille is calculated. Only if this value is 0 does the directory truly contribute no changes, and should be skipped from the output. This patches fixes this issue, and updates corresponding testcases to expect the new behvaior. Signed-off-by: Johan Herland Signed-off-by: Junio C Hamano --- diff.c | 4 ++-- t/t4047-diff-dirstat.sh | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/diff.c b/diff.c index 3b40e597d5..004a46b529 100644 --- a/diff.c +++ b/diff.c @@ -1502,8 +1502,8 @@ static long gather_dirstat(struct diff_options *opt, struct dirstat_dir *dir, * under this directory (sources == 1). */ if (baselen && sources != 1) { - int permille = this_dir * 1000 / changed; - if (permille) { + if (this_dir) { + int permille = this_dir * 1000 / changed; int percent = permille / 10; if (percent >= dir->percent) { fprintf(opt->file, "%s%4d.%01d%% %.*s\n", line_prefix, diff --git a/t/t4047-diff-dirstat.sh b/t/t4047-diff-dirstat.sh index ce7c4033f8..1c5adadf32 100755 --- a/t/t4047-diff-dirstat.sh +++ b/t/t4047-diff-dirstat.sh @@ -351,7 +351,6 @@ test_expect_success 'vanilla -X' ' test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC ' -# rearranged/text falls below 0% threshold (1 / (240 * 9 + 48 + 1) ~= 0.045 %) cat <expect_diff_dirstat 2.1% changed/ 10.8% dst/copy/changed/ @@ -360,6 +359,7 @@ cat <expect_diff_dirstat 10.8% dst/move/changed/ 10.8% dst/move/rearranged/ 10.8% dst/move/unchanged/ + 0.0% rearranged/ 10.8% src/move/changed/ 10.8% src/move/rearranged/ 10.8% src/move/unchanged/ @@ -402,7 +402,6 @@ test_expect_success '-X0' ' test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC ' -# rearranged/text falls below 0% threshold (1 / (240 * 9 + 48 + 1) ~= 0.045 %) cat <expect_diff_dirstat 2.1% changed/ 10.8% dst/copy/changed/ @@ -414,6 +413,7 @@ cat <expect_diff_dirstat 10.8% dst/move/unchanged/ 32.5% dst/move/ 65.1% dst/ + 0.0% rearranged/ 10.8% src/move/changed/ 10.8% src/move/rearranged/ 10.8% src/move/unchanged/ -- cgit v1.2.3 From 333f3fb0c5307fed170cae336ab3c056f4d6ca1c Mon Sep 17 00:00:00 2001 From: Johan Herland Date: Fri, 29 Apr 2011 11:36:18 +0200 Subject: Refactor --dirstat parsing; deprecate --cumulative and --dirstat-by-file Instead of having multiple interconnected dirstat-related options, teach the --dirstat option itself to accept all behavior modifiers as parameters. - Preserve the current --dirstat= (where is an integer specifying a cut-off percentage) - Add --dirstat=cumulative, replacing --cumulative - Add --dirstat=files, replacing --dirstat-by-file - Also add --dirstat=changes and --dirstat=noncumulative for specifying the current default behavior. These allow the user to reset other --dirstat parameters (e.g. 'cumulative' and 'files') occuring earlier on the command line. The deprecated options (--cumulative and --dirstat-by-file) are still functional, although they have been removed from the documentation. Allow multiple parameters to be separated by commas, e.g.: --dirstat=files,10,cumulative Update the documentation accordingly, and add testcases verifying the behavior of the new syntax. Improved-by: Junio C Hamano Signed-off-by: Johan Herland Signed-off-by: Junio C Hamano --- Documentation/diff-options.txt | 44 +++++++++++---- diff.c | 69 ++++++++++++++++++++--- t/t4047-diff-dirstat.sh | 123 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 214 insertions(+), 22 deletions(-) diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index 34f01458c2..0962035faa 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -66,19 +66,39 @@ endif::git-format-patch[] number of modified files, as well as number of added and deleted lines. ---dirstat[=]:: - Output the distribution of relative amount of changes (number of lines added or - removed) for each sub-directory. Directories with changes below - a cut-off percent (3% by default) are not shown. The cut-off percent - can be set with `--dirstat=`. Changes in a child directory are not - counted for the parent directory, unless `--cumulative` is used. +--dirstat[=]:: + Output the distribution of relative amount of changes for each + sub-directory. The behavior of `--dirstat` can be customized by + passing it a comma separated list of parameters. + The following parameters are available: + -Note that the `--dirstat` option computes the changes while ignoring -the amount of pure code movements within a file. In other words, -rearranging lines in a file is not counted as much as other changes. - ---dirstat-by-file[=]:: - Same as `--dirstat`, but counts changed files instead of lines. +-- +`changes`;; + Compute the dirstat numbers by counting the lines that have been + removed from the source, or added to the destination. This ignores + the amount of pure code movements within a file. In other words, + rearranging lines in a file is not counted as much as other changes. + This is the default behavior when no parameter is given. +`files`;; + Compute the dirstat numbers by counting the number of files changed. + Each changed file counts equally in the dirstat analysis. This is + the computationally cheapest `--dirstat` behavior, since it does + not have to look at the file contents at all. +`cumulative`;; + Count changes in a child directory for the parent directory as well. + Note that when using `cumulative`, the sum of the percentages + reported may exceed 100%. The default (non-cumulative) behavior can + be specified with the `noncumulative` parameter. +;; + An integer parameter specifies a cut-off percent (3% by default). + Directories contributing less than this percentage of the changes + are not shown in the output. +-- ++ +Example: The following will count changed files, while ignoring +directories with less than 10% of the total amount of changed files, +and accumulating child directory counts in the parent directories: +`--dirstat=files,10,cumulative`. --summary:: Output a condensed summary of extended header information diff --git a/diff.c b/diff.c index 004a46b529..3b0cda43f1 100644 --- a/diff.c +++ b/diff.c @@ -66,6 +66,41 @@ static int parse_diff_color_slot(const char *var, int ofs) return -1; } +static int parse_dirstat_params(struct diff_options *options, const char *params) +{ + const char *p = params; + while (*p) { + if (!prefixcmp(p, "changes")) { + p += 7; + DIFF_OPT_CLR(options, DIRSTAT_BY_FILE); + } else if (!prefixcmp(p, "files")) { + p += 5; + DIFF_OPT_SET(options, DIRSTAT_BY_FILE); + } else if (!prefixcmp(p, "noncumulative")) { + p += 13; + DIFF_OPT_CLR(options, DIRSTAT_CUMULATIVE); + } else if (!prefixcmp(p, "cumulative")) { + p += 10; + DIFF_OPT_SET(options, DIRSTAT_CUMULATIVE); + } else if (isdigit(*p)) { + char *end; + options->dirstat_percent = strtoul(p, &end, 10); + p = end; + } else + return error("Unknown --dirstat parameter '%s'", p); + + if (*p) { + /* more parameters, swallow separator */ + if (*p != ',') + return error("Missing comma separator at char " + "%"PRIuMAX" of '%s'", + (uintmax_t) (p - params), params); + p++; + } + } + return 0; +} + static int git_config_rename(const char *var, const char *value) { if (!value) @@ -3149,6 +3184,18 @@ static int stat_opt(struct diff_options *options, const char **av) return argcount; } +static int parse_dirstat_opt(struct diff_options *options, const char *params) +{ + if (parse_dirstat_params(options, params)) + die("Failed to parse --dirstat/-X option parameter"); + /* + * The caller knows a dirstat-related option is given from the command + * line; allow it to say "return this_function();" + */ + options->output_format |= DIFF_FORMAT_DIRSTAT; + return 1; +} + int diff_opt_parse(struct diff_options *options, const char **av, int ac) { const char *arg = av[0]; @@ -3168,15 +3215,19 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac) options->output_format |= DIFF_FORMAT_NUMSTAT; else if (!strcmp(arg, "--shortstat")) options->output_format |= DIFF_FORMAT_SHORTSTAT; - else if (opt_arg(arg, 'X', "dirstat", &options->dirstat_percent)) - options->output_format |= DIFF_FORMAT_DIRSTAT; - else if (!strcmp(arg, "--cumulative")) { - options->output_format |= DIFF_FORMAT_DIRSTAT; - DIFF_OPT_SET(options, DIRSTAT_CUMULATIVE); - } else if (opt_arg(arg, 0, "dirstat-by-file", - &options->dirstat_percent)) { - options->output_format |= DIFF_FORMAT_DIRSTAT; - DIFF_OPT_SET(options, DIRSTAT_BY_FILE); + else if (!strcmp(arg, "-X") || !strcmp(arg, "--dirstat")) + return parse_dirstat_opt(options, ""); + else if (!prefixcmp(arg, "-X")) + return parse_dirstat_opt(options, arg + 2); + else if (!prefixcmp(arg, "--dirstat=")) + return parse_dirstat_opt(options, arg + 10); + else if (!strcmp(arg, "--cumulative")) + return parse_dirstat_opt(options, "cumulative"); + else if (!strcmp(arg, "--dirstat-by-file")) + return parse_dirstat_opt(options, "files"); + else if (!prefixcmp(arg, "--dirstat-by-file=")) { + parse_dirstat_opt(options, "files"); + return parse_dirstat_opt(options, arg + 18); } else if (!strcmp(arg, "--check")) options->output_format |= DIFF_FORMAT_CHECKDIFF; diff --git a/t/t4047-diff-dirstat.sh b/t/t4047-diff-dirstat.sh index 1c5adadf32..d0ed62c0dd 100755 --- a/t/t4047-diff-dirstat.sh +++ b/t/t4047-diff-dirstat.sh @@ -330,7 +330,9 @@ EOF test_expect_success 'various ways to misspell --dirstat' ' test_must_fail git show --dirstat10 && - test_must_fail git show -X=20 + test_must_fail git show --dirstat10,files && + test_must_fail git show -X=20 && + test_must_fail git show -X=20,cumulative ' test_expect_success 'vanilla --dirstat' ' @@ -351,6 +353,39 @@ test_expect_success 'vanilla -X' ' test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC ' +test_expect_success 'explicit defaults: --dirstat=changes,noncumulative,3' ' + git diff --dirstat=changes,noncumulative,3 HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git diff --dirstat=changes,noncumulative,3 -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git diff --dirstat=changes,noncumulative,3 -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + +test_expect_success 'explicit defaults: -Xchanges,noncumulative,3' ' + git diff -Xchanges,noncumulative,3 HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git diff -Xchanges,noncumulative,3 -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git diff -Xchanges,noncumulative,3 -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + +test_expect_success 'later options override earlier options:' ' + git diff --dirstat=files,10,cumulative,changes,noncumulative,3 HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git diff --dirstat=files,10,cumulative,changes,noncumulative,3 -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git diff --dirstat=files,10,cumulative,changes,noncumulative,3 -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC + git diff --dirstat=files --dirstat=10 --dirstat=cumulative --dirstat=changes --dirstat=noncumulative -X3 HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git diff --dirstat=files --dirstat=10 --dirstat=cumulative --dirstat=changes --dirstat=noncumulative -X3 -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git diff --dirstat=files --dirstat=10 --dirstat=cumulative --dirstat=changes --dirstat=noncumulative -X3 -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + cat <expect_diff_dirstat 2.1% changed/ 10.8% dst/copy/changed/ @@ -454,6 +489,24 @@ test_expect_success '--dirstat=0 --cumulative' ' test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC ' +test_expect_success '--dirstat=0,cumulative' ' + git diff --dirstat=0,cumulative HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git diff --dirstat=0,cumulative -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git diff --dirstat=0,cumulative -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + +test_expect_success '-X0,cumulative' ' + git diff -X0,cumulative HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git diff -X0,cumulative -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git diff -X0,cumulative -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + cat <expect_diff_dirstat 9.0% changed/ 9.0% dst/copy/changed/ @@ -496,6 +549,15 @@ test_expect_success '--dirstat-by-file' ' test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC ' +test_expect_success '--dirstat=files' ' + git diff --dirstat=files HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git diff --dirstat=files -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git diff --dirstat=files -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + cat <expect_diff_dirstat 27.2% dst/copy/ 27.2% dst/move/ @@ -530,6 +592,15 @@ test_expect_success '--dirstat-by-file=10' ' test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC ' +test_expect_success '--dirstat=files,10' ' + git diff --dirstat=files,10 HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git diff --dirstat=files,10 -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git diff --dirstat=files,10 -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + cat <expect_diff_dirstat 9.0% changed/ 9.0% dst/copy/changed/ @@ -582,4 +653,54 @@ test_expect_success '--dirstat-by-file --cumulative' ' test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC ' +test_expect_success '--dirstat=files,cumulative' ' + git diff --dirstat=files,cumulative HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git diff --dirstat=files,cumulative -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git diff --dirstat=files,cumulative -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + +cat <expect_diff_dirstat + 27.2% dst/copy/ + 27.2% dst/move/ + 54.5% dst/ + 27.2% src/move/ +EOF + +cat <expect_diff_dirstat_M + 14.2% changed/ + 14.2% dst/copy/changed/ + 14.2% dst/copy/rearranged/ + 14.2% dst/copy/unchanged/ + 42.8% dst/copy/ + 14.2% dst/move/changed/ + 14.2% dst/move/rearranged/ + 28.5% dst/move/ + 71.4% dst/ + 14.2% rearranged/ +EOF + +cat <expect_diff_dirstat_CC + 16.6% changed/ + 16.6% dst/copy/changed/ + 16.6% dst/copy/rearranged/ + 33.3% dst/copy/ + 16.6% dst/move/changed/ + 16.6% dst/move/rearranged/ + 33.3% dst/move/ + 66.6% dst/ + 16.6% rearranged/ +EOF + +test_expect_success '--dirstat=files,cumulative,10' ' + git diff --dirstat=files,cumulative,10 HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git diff --dirstat=files,cumulative,10 -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git diff --dirstat=files,cumulative,10 -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + test_done -- cgit v1.2.3 From 2d17495196d0dceb0ff492aa97bfff33ef1baa18 Mon Sep 17 00:00:00 2001 From: Johan Herland Date: Fri, 29 Apr 2011 11:36:19 +0200 Subject: Add config variable for specifying default --dirstat behavior The new diff.dirstat config variable takes the same arguments as '--dirstat=', and specifies the default arguments for --dirstat. The config is obviously overridden by --dirstat arguments passed on the command line. When not specified, the --dirstat defaults are 'changes,noncumulative,3'. The patch also adds several tests verifying the interaction between the diff.dirstat config variable, and the --dirstat command line option. Improved-by: Junio C Hamano Signed-off-by: Johan Herland Signed-off-by: Junio C Hamano --- Documentation/diff-config.txt | 36 +++++++++++++++++++++ Documentation/diff-options.txt | 2 ++ diff.c | 10 +++++- t/t4047-diff-dirstat.sh | 72 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 119 insertions(+), 1 deletion(-) diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt index 2b1605f5c8..228329d4ad 100644 --- a/Documentation/diff-config.txt +++ b/Documentation/diff-config.txt @@ -8,6 +8,42 @@ diff.autorefreshindex:: affects only 'git diff' Porcelain, and not lower level 'diff' commands such as 'git diff-files'. +diff.dirstat:: + A comma separated list of `--dirstat` parameters specifying the + default behavior of the `--dirstat` option to linkgit:git-diff[1]` + and friends. The defaults can be overridden on the command line + (using `--dirstat=`). The fallback defaults + (when not changed by `diff.dirstat`) are `changes,noncumulative,3`. + The following parameters are available: ++ +-- +`changes`;; + Compute the dirstat numbers by counting the lines that have been + removed from the source, or added to the destination. This ignores + the amount of pure code movements within a file. In other words, + rearranging lines in a file is not counted as much as other changes. + This is the default behavior when no parameter is given. +`files`;; + Compute the dirstat numbers by counting the number of files changed. + Each changed file counts equally in the dirstat analysis. This is + the computationally cheapest `--dirstat` behavior, since it does + not have to look at the file contents at all. +`cumulative`;; + Count changes in a child directory for the parent directory as well. + Note that when using `cumulative`, the sum of the percentages + reported may exceed 100%. The default (non-cumulative) behavior can + be specified with the `noncumulative` parameter. +;; + An integer parameter specifies a cut-off percent (3% by default). + Directories contributing less than this percentage of the changes + are not shown in the output. +-- ++ +Example: The following will count changed files, while ignoring +directories with less than 10% of the total amount of changed files, +and accumulating child directory counts in the parent directories: +`files,10,cumulative`. + diff.external:: If this config variable is set, diff generation is not performed using the internal diff machinery, but using the diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index 0962035faa..dc023523ee 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -70,6 +70,8 @@ endif::git-format-patch[] Output the distribution of relative amount of changes for each sub-directory. The behavior of `--dirstat` can be customized by passing it a comma separated list of parameters. + The defaults are controlled by the `diff.dirstat` configuration + variable (see linkgit:git-config[1]). The following parameters are available: + -- diff --git a/diff.c b/diff.c index 3b0cda43f1..c78f7d5218 100644 --- a/diff.c +++ b/diff.c @@ -31,6 +31,7 @@ static const char *external_diff_cmd_cfg; int diff_auto_refresh_index = 1; static int diff_mnemonic_prefix; static int diff_no_prefix; +static int diff_dirstat_percent_default = 3; static struct diff_options default_diff_options; static char diff_colors[][COLOR_MAXLEN] = { @@ -180,6 +181,13 @@ int git_diff_basic_config(const char *var, const char *value, void *cb) return 0; } + if (!strcmp(var, "diff.dirstat")) { + default_diff_options.dirstat_percent = diff_dirstat_percent_default; + (void) parse_dirstat_params(&default_diff_options, value); + diff_dirstat_percent_default = default_diff_options.dirstat_percent; + return 0; + } + if (!prefixcmp(var, "submodule.")) return parse_submodule_config_option(var, value); @@ -2926,7 +2934,7 @@ void diff_setup(struct diff_options *options) options->line_termination = '\n'; options->break_opt = -1; options->rename_limit = -1; - options->dirstat_percent = 3; + options->dirstat_percent = diff_dirstat_percent_default; options->context = 3; options->change = diff_change; diff --git a/t/t4047-diff-dirstat.sh b/t/t4047-diff-dirstat.sh index d0ed62c0dd..c1b3697bf7 100755 --- a/t/t4047-diff-dirstat.sh +++ b/t/t4047-diff-dirstat.sh @@ -386,6 +386,15 @@ test_expect_success 'later options override earlier options:' ' test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC ' +test_expect_success 'non-defaults in config overridden by explicit defaults on command line' ' + git -c diff.dirstat=files,cumulative,50 diff --dirstat=changes,noncumulative,3 HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git -c diff.dirstat=files,cumulative,50 diff --dirstat=changes,noncumulative,3 -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git -c diff.dirstat=files,cumulative,50 diff --dirstat=changes,noncumulative,3 -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + cat <expect_diff_dirstat 2.1% changed/ 10.8% dst/copy/changed/ @@ -437,6 +446,15 @@ test_expect_success '-X0' ' test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC ' +test_expect_success 'diff.dirstat=0' ' + git -c diff.dirstat=0 diff --dirstat HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git -c diff.dirstat=0 diff --dirstat -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git -c diff.dirstat=0 diff --dirstat -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + cat <expect_diff_dirstat 2.1% changed/ 10.8% dst/copy/changed/ @@ -507,6 +525,24 @@ test_expect_success '-X0,cumulative' ' test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC ' +test_expect_success 'diff.dirstat=0,cumulative' ' + git -c diff.dirstat=0,cumulative diff --dirstat HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git -c diff.dirstat=0,cumulative diff --dirstat -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git -c diff.dirstat=0,cumulative diff --dirstat -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + +test_expect_success 'diff.dirstat=0 & --dirstat=cumulative' ' + git -c diff.dirstat=0 diff --dirstat=cumulative HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git -c diff.dirstat=0 diff --dirstat=cumulative -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git -c diff.dirstat=0 diff --dirstat=cumulative -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + cat <expect_diff_dirstat 9.0% changed/ 9.0% dst/copy/changed/ @@ -558,6 +594,15 @@ test_expect_success '--dirstat=files' ' test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC ' +test_expect_success 'diff.dirstat=files' ' + git -c diff.dirstat=files diff --dirstat HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git -c diff.dirstat=files diff --dirstat -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git -c diff.dirstat=files diff --dirstat -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + cat <expect_diff_dirstat 27.2% dst/copy/ 27.2% dst/move/ @@ -601,6 +646,15 @@ test_expect_success '--dirstat=files,10' ' test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC ' +test_expect_success 'diff.dirstat=10,files' ' + git -c diff.dirstat=10,files diff --dirstat HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git -c diff.dirstat=10,files diff --dirstat -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git -c diff.dirstat=10,files diff --dirstat -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + cat <expect_diff_dirstat 9.0% changed/ 9.0% dst/copy/changed/ @@ -662,6 +716,15 @@ test_expect_success '--dirstat=files,cumulative' ' test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC ' +test_expect_success 'diff.dirstat=cumulative,files' ' + git -c diff.dirstat=cumulative,files diff --dirstat HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git -c diff.dirstat=cumulative,files diff --dirstat -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git -c diff.dirstat=cumulative,files diff --dirstat -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + cat <expect_diff_dirstat 27.2% dst/copy/ 27.2% dst/move/ @@ -703,4 +766,13 @@ test_expect_success '--dirstat=files,cumulative,10' ' test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC ' +test_expect_success 'diff.dirstat=10,cumulative,files' ' + git -c diff.dirstat=10,cumulative,files diff --dirstat HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git -c diff.dirstat=10,cumulative,files diff --dirstat -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git -c diff.dirstat=10,cumulative,files diff --dirstat -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + test_done -- cgit v1.2.3 From 712d2c7dd893212756c21787fc12d6f71327e167 Mon Sep 17 00:00:00 2001 From: Johan Herland Date: Fri, 29 Apr 2011 11:36:20 +0200 Subject: Allow specifying --dirstat cut-off percentage as a floating point number Only the first digit after the decimal point is kept, as the dirstat calculations all happen in permille. Selftests verifying floating-point percentage input has been added. Improved-by: Junio C Hamano Improved-by: Linus Torvalds Signed-off-by: Johan Herland Signed-off-by: Junio C Hamano --- diff.c | 26 ++++++++++++-------- diff.h | 2 +- t/t4047-diff-dirstat.sh | 64 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 11 deletions(-) diff --git a/diff.c b/diff.c index c78f7d5218..70464d9d64 100644 --- a/diff.c +++ b/diff.c @@ -31,7 +31,7 @@ static const char *external_diff_cmd_cfg; int diff_auto_refresh_index = 1; static int diff_mnemonic_prefix; static int diff_no_prefix; -static int diff_dirstat_percent_default = 3; +static int diff_dirstat_permille_default = 30; static struct diff_options default_diff_options; static char diff_colors[][COLOR_MAXLEN] = { @@ -85,8 +85,15 @@ static int parse_dirstat_params(struct diff_options *options, const char *params DIFF_OPT_SET(options, DIRSTAT_CUMULATIVE); } else if (isdigit(*p)) { char *end; - options->dirstat_percent = strtoul(p, &end, 10); + options->dirstat_permille = strtoul(p, &end, 10) * 10; p = end; + if (*p == '.' && isdigit(*++p)) { + /* only use first digit */ + options->dirstat_permille += *p - '0'; + /* .. and ignore any further digits */ + while (isdigit(*++p)) + ; /* nothing */ + } } else return error("Unknown --dirstat parameter '%s'", p); @@ -182,9 +189,9 @@ int git_diff_basic_config(const char *var, const char *value, void *cb) } if (!strcmp(var, "diff.dirstat")) { - default_diff_options.dirstat_percent = diff_dirstat_percent_default; + default_diff_options.dirstat_permille = diff_dirstat_permille_default; (void) parse_dirstat_params(&default_diff_options, value); - diff_dirstat_percent_default = default_diff_options.dirstat_percent; + diff_dirstat_permille_default = default_diff_options.dirstat_permille; return 0; } @@ -1498,7 +1505,7 @@ struct dirstat_file { struct dirstat_dir { struct dirstat_file *files; - int alloc, nr, percent, cumulative; + int alloc, nr, permille, cumulative; }; static long gather_dirstat(struct diff_options *opt, struct dirstat_dir *dir, @@ -1547,10 +1554,9 @@ static long gather_dirstat(struct diff_options *opt, struct dirstat_dir *dir, if (baselen && sources != 1) { if (this_dir) { int permille = this_dir * 1000 / changed; - int percent = permille / 10; - if (percent >= dir->percent) { + if (permille >= dir->permille) { fprintf(opt->file, "%s%4d.%01d%% %.*s\n", line_prefix, - percent, permille % 10, baselen, base); + permille / 10, permille % 10, baselen, base); if (!dir->cumulative) return 0; } @@ -1576,7 +1582,7 @@ static void show_dirstat(struct diff_options *options) dir.files = NULL; dir.alloc = 0; dir.nr = 0; - dir.percent = options->dirstat_percent; + dir.permille = options->dirstat_permille; dir.cumulative = DIFF_OPT_TST(options, DIRSTAT_CUMULATIVE); changed = 0; @@ -2934,7 +2940,7 @@ void diff_setup(struct diff_options *options) options->line_termination = '\n'; options->break_opt = -1; options->rename_limit = -1; - options->dirstat_percent = diff_dirstat_percent_default; + options->dirstat_permille = diff_dirstat_permille_default; options->context = 3; options->change = diff_change; diff --git a/diff.h b/diff.h index 6fe1597785..90f491e9e7 100644 --- a/diff.h +++ b/diff.h @@ -114,7 +114,7 @@ struct diff_options { int needed_rename_limit; int degraded_cc_to_c; int show_rename_progress; - int dirstat_percent; + int dirstat_permille; int setup; int abbrev; const char *prefix; diff --git a/t/t4047-diff-dirstat.sh b/t/t4047-diff-dirstat.sh index c1b3697bf7..4b25e10030 100755 --- a/t/t4047-diff-dirstat.sh +++ b/t/t4047-diff-dirstat.sh @@ -775,4 +775,68 @@ test_expect_success 'diff.dirstat=10,cumulative,files' ' test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC ' +cat <expect_diff_dirstat + 27.2% dst/copy/ + 27.2% dst/move/ + 54.5% dst/ + 27.2% src/move/ +EOF + +cat <expect_diff_dirstat_M + 42.8% dst/copy/ + 28.5% dst/move/ + 71.4% dst/ +EOF + +cat <expect_diff_dirstat_CC + 33.3% dst/copy/ + 33.3% dst/move/ + 66.6% dst/ +EOF + +test_expect_success '--dirstat=files,cumulative,16.7' ' + git diff --dirstat=files,cumulative,16.7 HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git diff --dirstat=files,cumulative,16.7 -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git diff --dirstat=files,cumulative,16.7 -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + +test_expect_success 'diff.dirstat=16.7,cumulative,files' ' + git -c diff.dirstat=16.7,cumulative,files diff --dirstat HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git -c diff.dirstat=16.7,cumulative,files diff --dirstat -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git -c diff.dirstat=16.7,cumulative,files diff --dirstat -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + +test_expect_success 'diff.dirstat=16.70,cumulative,files' ' + git -c diff.dirstat=16.70,cumulative,files diff --dirstat HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git -c diff.dirstat=16.70,cumulative,files diff --dirstat -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git -c diff.dirstat=16.70,cumulative,files diff --dirstat -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + +test_expect_success '--dirstat=files,cumulative,27.2' ' + git diff --dirstat=files,cumulative,27.2 HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git diff --dirstat=files,cumulative,27.2 -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git diff --dirstat=files,cumulative,27.2 -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + +test_expect_success '--dirstat=files,cumulative,27.09' ' + git diff --dirstat=files,cumulative,27.09 HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git diff --dirstat=files,cumulative,27.09 -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git diff --dirstat=files,cumulative,27.09 -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + test_done -- cgit v1.2.3 From 1c57a627bf269f3c83c48ad724cd8b14292502ef Mon Sep 17 00:00:00 2001 From: Johan Herland Date: Fri, 29 Apr 2011 11:36:21 +0200 Subject: New --dirstat=lines mode, doing dirstat analysis based on diffstat This patch adds an alternative implementation of show_dirstat(), called show_dirstat_by_line(), which uses the more expensive diffstat analysis (as opposed to show_dirstat()'s own (relatively inexpensive) analysis) to derive the numbers from which the --dirstat output is computed. The alternative implementation is controlled by the new "lines" parameter to the --dirstat option (or the diff.dirstat config variable). For binary files, the diffstat analysis counts bytes instead of lines, so to prevent binary files from dominating the dirstat results, the byte counts for binary files are divided by 64 before being compared to their textual/line-based counterparts. This is a stupid and ugly - but very cheap - heuristic. In linux-2.6.git, running the three different --dirstat modes: time git diff v2.6.20..v2.6.30 --dirstat=changes > /dev/null vs. time git diff v2.6.20..v2.6.30 --dirstat=lines > /dev/null vs. time git diff v2.6.20..v2.6.30 --dirstat=files > /dev/null yields the following average runtimes on my machine: - "changes" (default): ~6.0 s - "lines": ~9.6 s - "files": ~0.1 s So, as expected, there's a considerable performance hit (~60%) by going through the full diffstat analysis as compared to the default "changes" analysis (obviously, "files" is much faster than both). As such, the "lines" mode is probably only useful if you really need the --dirstat numbers to be consistent with the numbers returned from the other --*stat options. The patch also includes documentation and tests for the new dirstat mode. Improved-by: Junio C Hamano Signed-off-by: Johan Herland Signed-off-by: Junio C Hamano --- Documentation/diff-config.txt | 8 ++++ Documentation/diff-options.txt | 8 ++++ diff.c | 61 ++++++++++++++++++++++++- diff.h | 1 + t/t4047-diff-dirstat.sh | 100 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 176 insertions(+), 2 deletions(-) diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt index 228329d4ad..1aed79e7dc 100644 --- a/Documentation/diff-config.txt +++ b/Documentation/diff-config.txt @@ -23,6 +23,14 @@ diff.dirstat:: the amount of pure code movements within a file. In other words, rearranging lines in a file is not counted as much as other changes. This is the default behavior when no parameter is given. +`lines`;; + Compute the dirstat numbers by doing the regular line-based diff + analysis, and summing the removed/added line counts. (For binary + files, count 64-byte chunks instead, since binary files have no + natural concept of lines). This is a more expensive `--dirstat` + behavior than the `changes` behavior, but it does count rearranged + lines within a file as much as other changes. The resulting output + is consistent with what you get from the other `--*stat` options. `files`;; Compute the dirstat numbers by counting the number of files changed. Each changed file counts equally in the dirstat analysis. This is diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index dc023523ee..bddceb06b7 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -81,6 +81,14 @@ endif::git-format-patch[] the amount of pure code movements within a file. In other words, rearranging lines in a file is not counted as much as other changes. This is the default behavior when no parameter is given. +`lines`;; + Compute the dirstat numbers by doing the regular line-based diff + analysis, and summing the removed/added line counts. (For binary + files, count 64-byte chunks instead, since binary files have no + natural concept of lines). This is a more expensive `--dirstat` + behavior than the `changes` behavior, but it does count rearranged + lines within a file as much as other changes. The resulting output + is consistent with what you get from the other `--*stat` options. `files`;; Compute the dirstat numbers by counting the number of files changed. Each changed file counts equally in the dirstat analysis. This is diff --git a/diff.c b/diff.c index 70464d9d64..919a16d89e 100644 --- a/diff.c +++ b/diff.c @@ -73,9 +73,15 @@ static int parse_dirstat_params(struct diff_options *options, const char *params while (*p) { if (!prefixcmp(p, "changes")) { p += 7; + DIFF_OPT_CLR(options, DIRSTAT_BY_LINE); + DIFF_OPT_CLR(options, DIRSTAT_BY_FILE); + } else if (!prefixcmp(p, "lines")) { + p += 5; + DIFF_OPT_SET(options, DIRSTAT_BY_LINE); DIFF_OPT_CLR(options, DIRSTAT_BY_FILE); } else if (!prefixcmp(p, "files")) { p += 5; + DIFF_OPT_CLR(options, DIRSTAT_BY_LINE); DIFF_OPT_SET(options, DIRSTAT_BY_FILE); } else if (!prefixcmp(p, "noncumulative")) { p += 13; @@ -1671,6 +1677,50 @@ found_damage: gather_dirstat(options, &dir, changed, "", 0); } +static void show_dirstat_by_line(struct diffstat_t *data, struct diff_options *options) +{ + int i; + unsigned long changed; + struct dirstat_dir dir; + + if (data->nr == 0) + return; + + dir.files = NULL; + dir.alloc = 0; + dir.nr = 0; + dir.permille = options->dirstat_permille; + dir.cumulative = DIFF_OPT_TST(options, DIRSTAT_CUMULATIVE); + + changed = 0; + for (i = 0; i < data->nr; i++) { + struct diffstat_file *file = data->files[i]; + unsigned long damage = file->added + file->deleted; + if (file->is_binary) + /* + * binary files counts bytes, not lines. Must find some + * way to normalize binary bytes vs. textual lines. + * The following heuristic assumes that there are 64 + * bytes per "line". + * This is stupid and ugly, but very cheap... + */ + damage = (damage + 63) / 64; + ALLOC_GROW(dir.files, dir.nr + 1, dir.alloc); + dir.files[dir.nr].name = file->name; + dir.files[dir.nr].changed = damage; + changed += damage; + dir.nr++; + } + + /* This can happen even with many files, if everything was renames */ + if (!changed) + return; + + /* Show all directories with more than x% of the changes */ + qsort(dir.files, dir.nr, sizeof(dir.files[0]), dirstat_compare); + gather_dirstat(options, &dir, changed, "", 0); +} + static void free_diffstat_info(struct diffstat_t *diffstat) { int i; @@ -4088,6 +4138,7 @@ void diff_flush(struct diff_options *options) struct diff_queue_struct *q = &diff_queued_diff; int i, output_format = options->output_format; int separator = 0; + int dirstat_by_line = 0; /* * Order: raw, stat, summary, patch @@ -4108,7 +4159,11 @@ void diff_flush(struct diff_options *options) separator++; } - if (output_format & (DIFF_FORMAT_DIFFSTAT|DIFF_FORMAT_SHORTSTAT|DIFF_FORMAT_NUMSTAT)) { + if (output_format & DIFF_FORMAT_DIRSTAT && DIFF_OPT_TST(options, DIRSTAT_BY_LINE)) + dirstat_by_line = 1; + + if (output_format & (DIFF_FORMAT_DIFFSTAT|DIFF_FORMAT_SHORTSTAT|DIFF_FORMAT_NUMSTAT) || + dirstat_by_line) { struct diffstat_t diffstat; memset(&diffstat, 0, sizeof(struct diffstat_t)); @@ -4123,10 +4178,12 @@ void diff_flush(struct diff_options *options) show_stats(&diffstat, options); if (output_format & DIFF_FORMAT_SHORTSTAT) show_shortstats(&diffstat, options); + if (output_format & DIFF_FORMAT_DIRSTAT) + show_dirstat_by_line(&diffstat, options); free_diffstat_info(&diffstat); separator++; } - if (output_format & DIFF_FORMAT_DIRSTAT) + if ((output_format & DIFF_FORMAT_DIRSTAT) && !dirstat_by_line) show_dirstat(options); if (output_format & DIFF_FORMAT_SUMMARY && !is_summary_empty(q)) { diff --git a/diff.h b/diff.h index 90f491e9e7..937a903dce 100644 --- a/diff.h +++ b/diff.h @@ -78,6 +78,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data) #define DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES (1 << 25) #define DIFF_OPT_IGNORE_DIRTY_SUBMODULES (1 << 26) #define DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG (1 << 27) +#define DIFF_OPT_DIRSTAT_BY_LINE (1 << 28) #define DIFF_OPT_TST(opts, flag) ((opts)->flags & DIFF_OPT_##flag) #define DIFF_OPT_SET(opts, flag) ((opts)->flags |= DIFF_OPT_##flag) diff --git a/t/t4047-diff-dirstat.sh b/t/t4047-diff-dirstat.sh index 4b25e10030..b8ad92a354 100755 --- a/t/t4047-diff-dirstat.sh +++ b/t/t4047-diff-dirstat.sh @@ -839,4 +839,104 @@ test_expect_success '--dirstat=files,cumulative,27.09' ' test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC ' +cat <expect_diff_dirstat + 10.6% dst/copy/changed/ + 10.6% dst/copy/rearranged/ + 10.6% dst/copy/unchanged/ + 10.6% dst/move/changed/ + 10.6% dst/move/rearranged/ + 10.6% dst/move/unchanged/ + 10.6% src/move/changed/ + 10.6% src/move/rearranged/ + 10.6% src/move/unchanged/ +EOF + +cat <expect_diff_dirstat_M + 5.2% changed/ + 26.3% dst/copy/changed/ + 26.3% dst/copy/rearranged/ + 26.3% dst/copy/unchanged/ + 5.2% dst/move/changed/ + 5.2% dst/move/rearranged/ + 5.2% rearranged/ +EOF + +cat <expect_diff_dirstat_CC + 16.6% changed/ + 16.6% dst/copy/changed/ + 16.6% dst/copy/rearranged/ + 16.6% dst/move/changed/ + 16.6% dst/move/rearranged/ + 16.6% rearranged/ +EOF + +test_expect_success '--dirstat=lines' ' + git diff --dirstat=lines HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git diff --dirstat=lines -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git diff --dirstat=lines -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + +test_expect_success 'diff.dirstat=lines' ' + git -c diff.dirstat=lines diff --dirstat HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git -c diff.dirstat=lines diff --dirstat -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git -c diff.dirstat=lines diff --dirstat -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + +cat <expect_diff_dirstat + 2.1% changed/ + 10.6% dst/copy/changed/ + 10.6% dst/copy/rearranged/ + 10.6% dst/copy/unchanged/ + 10.6% dst/move/changed/ + 10.6% dst/move/rearranged/ + 10.6% dst/move/unchanged/ + 2.1% rearranged/ + 10.6% src/move/changed/ + 10.6% src/move/rearranged/ + 10.6% src/move/unchanged/ +EOF + +cat <expect_diff_dirstat_M + 5.2% changed/ + 26.3% dst/copy/changed/ + 26.3% dst/copy/rearranged/ + 26.3% dst/copy/unchanged/ + 5.2% dst/move/changed/ + 5.2% dst/move/rearranged/ + 5.2% rearranged/ +EOF + +cat <expect_diff_dirstat_CC + 16.6% changed/ + 16.6% dst/copy/changed/ + 16.6% dst/copy/rearranged/ + 16.6% dst/move/changed/ + 16.6% dst/move/rearranged/ + 16.6% rearranged/ +EOF + +test_expect_success '--dirstat=lines,0' ' + git diff --dirstat=lines,0 HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git diff --dirstat=lines,0 -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git diff --dirstat=lines,0 -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + +test_expect_success 'diff.dirstat=0,lines' ' + git -c diff.dirstat=0,lines diff --dirstat HEAD^..HEAD >actual_diff_dirstat && + test_cmp expect_diff_dirstat actual_diff_dirstat && + git -c diff.dirstat=0,lines diff --dirstat -M HEAD^..HEAD >actual_diff_dirstat_M && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + git -c diff.dirstat=0,lines diff --dirstat -C -C HEAD^..HEAD >actual_diff_dirstat_CC && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC +' + test_done -- cgit v1.2.3 From 51670fc87e193a42022087d8d773cf2315736604 Mon Sep 17 00:00:00 2001 From: Johan Herland Date: Fri, 29 Apr 2011 11:36:22 +0200 Subject: Improve error handling when parsing dirstat parameters When encountering errors or unknown tokens while parsing parameters to the --dirstat option, it makes sense to die() with an error message informing the user of which parameter did not make sense. However, when parsing the diff.dirstat config variable, we cannot simply die(), but should instead (after warning the user) ignore the erroneous or unrecognized parameter. After all, future Git versions might add more dirstat parameters, and using two different Git versions on the same repo should not cripple the older Git version just because of a parameter that is only understood by a more recent Git version. This patch fixes the issue by refactoring the dirstat parameter parsing so that parse_dirstat_params() keeps on parsing parameters, even if an earlier parameter was not recognized. When parsing has finished, it returns zero if all parameters were successfully parsed, and non-zero if one or more parameters were not recognized (with appropriate error messages appended to the 'errmsg' argument). The parse_dirstat_params() callers then decide (based on the return value from parse_dirstat_params()) whether to warn and ignore (in case of diff.dirstat), or to warn and die (in case of --dirstat). The patch also adds a couple of tests verifying the correct behavior of --dirstat and diff.dirstat in the face of unknown (possibly future) dirstat parameters. Suggested-by: Junio C Hamano Improved-by: Junio C Hamano Signed-off-by: Johan Herland Signed-off-by: Junio C Hamano --- diff.c | 71 ++++++++++++++++++++++++++++--------------------- t/t4047-diff-dirstat.sh | 37 ++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 30 deletions(-) diff --git a/diff.c b/diff.c index 919a16d89e..5f497fac52 100644 --- a/diff.c +++ b/diff.c @@ -67,52 +67,56 @@ static int parse_diff_color_slot(const char *var, int ofs) return -1; } -static int parse_dirstat_params(struct diff_options *options, const char *params) +static int parse_dirstat_params(struct diff_options *options, const char *params, + struct strbuf *errmsg) { const char *p = params; + int p_len, ret = 0; + while (*p) { - if (!prefixcmp(p, "changes")) { - p += 7; + p_len = strchrnul(p, ',') - p; + if (!memcmp(p, "changes", p_len)) { DIFF_OPT_CLR(options, DIRSTAT_BY_LINE); DIFF_OPT_CLR(options, DIRSTAT_BY_FILE); - } else if (!prefixcmp(p, "lines")) { - p += 5; + } else if (!memcmp(p, "lines", p_len)) { DIFF_OPT_SET(options, DIRSTAT_BY_LINE); DIFF_OPT_CLR(options, DIRSTAT_BY_FILE); - } else if (!prefixcmp(p, "files")) { - p += 5; + } else if (!memcmp(p, "files", p_len)) { DIFF_OPT_CLR(options, DIRSTAT_BY_LINE); DIFF_OPT_SET(options, DIRSTAT_BY_FILE); - } else if (!prefixcmp(p, "noncumulative")) { - p += 13; + } else if (!memcmp(p, "noncumulative", p_len)) { DIFF_OPT_CLR(options, DIRSTAT_CUMULATIVE); - } else if (!prefixcmp(p, "cumulative")) { - p += 10; + } else if (!memcmp(p, "cumulative", p_len)) { DIFF_OPT_SET(options, DIRSTAT_CUMULATIVE); } else if (isdigit(*p)) { char *end; - options->dirstat_permille = strtoul(p, &end, 10) * 10; - p = end; - if (*p == '.' && isdigit(*++p)) { + int permille = strtoul(p, &end, 10) * 10; + if (*end == '.' && isdigit(*++end)) { /* only use first digit */ - options->dirstat_permille += *p - '0'; + permille += *end - '0'; /* .. and ignore any further digits */ - while (isdigit(*++p)) + while (isdigit(*++end)) ; /* nothing */ } - } else - return error("Unknown --dirstat parameter '%s'", p); - - if (*p) { - /* more parameters, swallow separator */ - if (*p != ',') - return error("Missing comma separator at char " - "%"PRIuMAX" of '%s'", - (uintmax_t) (p - params), params); - p++; + if (end - p == p_len) + options->dirstat_permille = permille; + else { + strbuf_addf(errmsg, " Failed to parse dirstat cut-off percentage '%.*s'\n", + p_len, p); + ret++; + } + } else { + strbuf_addf(errmsg, " Unknown dirstat parameter '%.*s'\n", + p_len, p); + ret++; } + + p += p_len; + + if (*p) + p++; /* more parameters, swallow separator */ } - return 0; + return ret; } static int git_config_rename(const char *var, const char *value) @@ -195,8 +199,12 @@ int git_diff_basic_config(const char *var, const char *value, void *cb) } if (!strcmp(var, "diff.dirstat")) { + struct strbuf errmsg = STRBUF_INIT; default_diff_options.dirstat_permille = diff_dirstat_permille_default; - (void) parse_dirstat_params(&default_diff_options, value); + if (parse_dirstat_params(&default_diff_options, value, &errmsg)) + warning("Found errors in 'diff.dirstat' config variable:\n%s", + errmsg.buf); + strbuf_release(&errmsg); diff_dirstat_permille_default = default_diff_options.dirstat_permille; return 0; } @@ -3250,8 +3258,11 @@ static int stat_opt(struct diff_options *options, const char **av) static int parse_dirstat_opt(struct diff_options *options, const char *params) { - if (parse_dirstat_params(options, params)) - die("Failed to parse --dirstat/-X option parameter"); + struct strbuf errmsg = STRBUF_INIT; + if (parse_dirstat_params(options, params, &errmsg)) + die("Failed to parse --dirstat/-X option parameter:\n%s", + errmsg.buf); + strbuf_release(&errmsg); /* * The caller knows a dirstat-related option is given from the command * line; allow it to say "return this_function();" diff --git a/t/t4047-diff-dirstat.sh b/t/t4047-diff-dirstat.sh index b8ad92a354..cc947fde91 100755 --- a/t/t4047-diff-dirstat.sh +++ b/t/t4047-diff-dirstat.sh @@ -939,4 +939,41 @@ test_expect_success 'diff.dirstat=0,lines' ' test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC ' +test_expect_success '--dirstat=future_param,lines,0 should fail loudly' ' + test_must_fail git diff --dirstat=future_param,lines,0 HEAD^..HEAD >actual_diff_dirstat 2>actual_error && + test_debug "cat actual_error" && + test_cmp /dev/null actual_diff_dirstat && + grep -q "future_param" actual_error && + grep -q "\--dirstat" actual_error +' + +test_expect_success '--dirstat=dummy1,cumulative,2dummy should report both unrecognized parameters' ' + test_must_fail git diff --dirstat=dummy1,cumulative,2dummy HEAD^..HEAD >actual_diff_dirstat 2>actual_error && + test_debug "cat actual_error" && + test_cmp /dev/null actual_diff_dirstat && + grep -q "dummy1" actual_error && + grep -q "2dummy" actual_error && + grep -q "\--dirstat" actual_error +' + +test_expect_success 'diff.dirstat=future_param,0,lines should warn, but still work' ' + git -c diff.dirstat=future_param,0,lines diff --dirstat HEAD^..HEAD >actual_diff_dirstat 2>actual_error && + test_debug "cat actual_error" && + test_cmp expect_diff_dirstat actual_diff_dirstat && + grep -q "future_param" actual_error && + grep -q "diff\\.dirstat" actual_error && + + git -c diff.dirstat=future_param,0,lines diff --dirstat -M HEAD^..HEAD >actual_diff_dirstat_M 2>actual_error && + test_debug "cat actual_error" && + test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && + grep -q "future_param" actual_error && + grep -q "diff\\.dirstat" actual_error && + + git -c diff.dirstat=future_param,0,lines diff --dirstat -C -C HEAD^..HEAD >actual_diff_dirstat_CC 2>actual_error && + test_debug "cat actual_error" && + test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC && + grep -q "future_param" actual_error && + grep -q "diff\\.dirstat" actual_error +' + test_done -- cgit v1.2.3 From 7478ac57c4b5cd589cb134d88e8cd78db1247b30 Mon Sep 17 00:00:00 2001 From: Johan Herland Date: Fri, 29 Apr 2011 11:36:23 +0200 Subject: Mark dirstat error messages for translation Suggested-by: Junio C Hamano Signed-off-by: Johan Herland Signed-off-by: Junio C Hamano --- diff.c | 8 ++++---- t/t4047-diff-dirstat.sh | 22 +++++++++++----------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/diff.c b/diff.c index 5f497fac52..097eb4c47a 100644 --- a/diff.c +++ b/diff.c @@ -101,12 +101,12 @@ static int parse_dirstat_params(struct diff_options *options, const char *params if (end - p == p_len) options->dirstat_permille = permille; else { - strbuf_addf(errmsg, " Failed to parse dirstat cut-off percentage '%.*s'\n", + strbuf_addf(errmsg, _(" Failed to parse dirstat cut-off percentage '%.*s'\n"), p_len, p); ret++; } } else { - strbuf_addf(errmsg, " Unknown dirstat parameter '%.*s'\n", + strbuf_addf(errmsg, _(" Unknown dirstat parameter '%.*s'\n"), p_len, p); ret++; } @@ -202,7 +202,7 @@ int git_diff_basic_config(const char *var, const char *value, void *cb) struct strbuf errmsg = STRBUF_INIT; default_diff_options.dirstat_permille = diff_dirstat_permille_default; if (parse_dirstat_params(&default_diff_options, value, &errmsg)) - warning("Found errors in 'diff.dirstat' config variable:\n%s", + warning(_("Found errors in 'diff.dirstat' config variable:\n%s"), errmsg.buf); strbuf_release(&errmsg); diff_dirstat_permille_default = default_diff_options.dirstat_permille; @@ -3260,7 +3260,7 @@ static int parse_dirstat_opt(struct diff_options *options, const char *params) { struct strbuf errmsg = STRBUF_INIT; if (parse_dirstat_params(options, params, &errmsg)) - die("Failed to parse --dirstat/-X option parameter:\n%s", + die(_("Failed to parse --dirstat/-X option parameter:\n%s"), errmsg.buf); strbuf_release(&errmsg); /* diff --git a/t/t4047-diff-dirstat.sh b/t/t4047-diff-dirstat.sh index cc947fde91..29e80a58cd 100755 --- a/t/t4047-diff-dirstat.sh +++ b/t/t4047-diff-dirstat.sh @@ -943,37 +943,37 @@ test_expect_success '--dirstat=future_param,lines,0 should fail loudly' ' test_must_fail git diff --dirstat=future_param,lines,0 HEAD^..HEAD >actual_diff_dirstat 2>actual_error && test_debug "cat actual_error" && test_cmp /dev/null actual_diff_dirstat && - grep -q "future_param" actual_error && - grep -q "\--dirstat" actual_error + test_i18ngrep -q "future_param" actual_error && + test_i18ngrep -q "\--dirstat" actual_error ' test_expect_success '--dirstat=dummy1,cumulative,2dummy should report both unrecognized parameters' ' test_must_fail git diff --dirstat=dummy1,cumulative,2dummy HEAD^..HEAD >actual_diff_dirstat 2>actual_error && test_debug "cat actual_error" && test_cmp /dev/null actual_diff_dirstat && - grep -q "dummy1" actual_error && - grep -q "2dummy" actual_error && - grep -q "\--dirstat" actual_error + test_i18ngrep -q "dummy1" actual_error && + test_i18ngrep -q "2dummy" actual_error && + test_i18ngrep -q "\--dirstat" actual_error ' test_expect_success 'diff.dirstat=future_param,0,lines should warn, but still work' ' git -c diff.dirstat=future_param,0,lines diff --dirstat HEAD^..HEAD >actual_diff_dirstat 2>actual_error && test_debug "cat actual_error" && test_cmp expect_diff_dirstat actual_diff_dirstat && - grep -q "future_param" actual_error && - grep -q "diff\\.dirstat" actual_error && + test_i18ngrep -q "future_param" actual_error && + test_i18ngrep -q "diff\\.dirstat" actual_error && git -c diff.dirstat=future_param,0,lines diff --dirstat -M HEAD^..HEAD >actual_diff_dirstat_M 2>actual_error && test_debug "cat actual_error" && test_cmp expect_diff_dirstat_M actual_diff_dirstat_M && - grep -q "future_param" actual_error && - grep -q "diff\\.dirstat" actual_error && + test_i18ngrep -q "future_param" actual_error && + test_i18ngrep -q "diff\\.dirstat" actual_error && git -c diff.dirstat=future_param,0,lines diff --dirstat -C -C HEAD^..HEAD >actual_diff_dirstat_CC 2>actual_error && test_debug "cat actual_error" && test_cmp expect_diff_dirstat_CC actual_diff_dirstat_CC && - grep -q "future_param" actual_error && - grep -q "diff\\.dirstat" actual_error + test_i18ngrep -q "future_param" actual_error && + test_i18ngrep -q "diff\\.dirstat" actual_error ' test_done -- cgit v1.2.3