Re: [PATCH] diff: Fixes shortstat number of files

[Antoine Pelisse <apelisse@xxxxxxxxx>]

Indeed stat seems to be broken on master by commit 74faaa16 from Linus Torvalds

There are three separated issues here:
 - unmerged files are marked as "interesting" in stat and probably
shouldn't, with some patch like this:

        data->is_interesting = p->status != 0;

        if (!one || !two) {
                data->is_unmerged = 1;
+               data->is_interesting = 0;
                return;
        }

By the way, I don't get the point of this code then:

        else if (data->files[i]->is_unmerged) {
            fprintf(options->file, "%s", line_prefix);
            show_name(options->file, prefix, name, len);
            fprintf(options->file, " Unmerged\n");
            continue;
        }

and

        if (file->is_unmerged) {
            /* "Unmerged" is 8 characters */
            bin_width = bin_width < 8 ? 8 : bin_width;
            continue;
        }

Are we ever supposed to print that ? I feel like it could be removed.

 - Unmerged files are not filtered out in shortstat, thus counted
twice (addressed by the patch)
 - no file has ever been filtered out of numstat, and probably should
the way it's done in stat. That is with something like this:

        if (!data->files[i]->is_interesting &&
             (added + deleted == 0)) {
            continue;
        }


Cheers,
Antoine Pelisse


---------- Forwarded message ----------
From: Junio C Hamano <gitster@xxxxxxxxx>
Date: Mon, Nov 26, 2012 at 4:28 AM
Subject: Re: [PATCH] diff: Fixes shortstat number of files
To: Antoine Pelisse <apelisse@xxxxxxxxx>
Cc: git@xxxxxxxxxxxxxxx


Antoine Pelisse <apelisse@xxxxxxxxx> writes:

> Subject: Re: [PATCH] diff: Fixes shortstat number of files

Please replace "Fixes" with "Fix at least (because our log messages
are written as if a patch is giving an order to the codebase, iow,
in imperative mood), but we would prefer to see a concrete
description on what is fixed, when we can.  And in this case, I
think we can, perhaps:

    diff: do not count unmerged paths twice in --shortstat/--numstat

or something.

> There is a discrepancy between the last line of `git diff --stat`
> and `git diff --shortstat` in case of a merge.
> The unmerged files are actually counted twice, thus doubling the
> value of "file changed".

I think the current 'master' and upward is broken with respect to
this; I am consistently getting two entries for unmerged paths
across --stat, --shortstat and --numstat options (iow, not just
shortstat and numstat but the '--stat' seems to be broken as well).

Thanks.
--
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