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=<limit> (where <limit> 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 <gitster@pobox.com>
Signed-off-by: Johan Herland <johan@herland.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 34f01458c296ce68bcba4c08177aba127fc2333b..0962035faac1d8189bea4abf50ec6bd4bba2b058 100644 (file)
--- 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[=<limit>]::
-       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=<limit>`. Changes in a child directory are not
-       counted for the parent directory, unless `--cumulative` is used.
+--dirstat[=<param1,param2,...>]::
+       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[=<limit>]::
-       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.
+<limit>;;
+       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 004a46b529d162fcac070796f8d91d69ef1f5310..3b0cda43f16ccf844b711e10f487b1763f0f88ab 100644 (file)
--- 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 1c5adadf3266dab844bd7b9db97c76c9bb8b7804..d0ed62c0dd14a78c7c9ef97354583dc380502775 100755 (executable)
--- 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 <<EOF >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 <<EOF >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 <<EOF >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 <<EOF >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 <<EOF >expect_diff_dirstat
+  27.2% dst/copy/
+  27.2% dst/move/
+  54.5% dst/
+  27.2% src/move/
+EOF
+
+cat <<EOF >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 <<EOF >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