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

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 27, 2012, 19:14 UTC
Message-ID: <7vhaoan9hy.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CALWbr2z+9fJGg34q7zp3knZVWgaTwtvzFBvFHRSmXfH+hc-jNw@mail.gmail.com>

Antoine Pelisse <apelisse@gmail.com> writes:

Show 32 quoted lines

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

Yes, we have been showing two entries in --stat output:

 file | Unmerged
 file | 4 ++++

and that is not going to change.

There are a few problems in diff.c around --stat area, partially caused by Linus's patch (like s/is_rename/is_interesting/ change started ignoring unmerged entries in the diff queue and made the existing loop not go into the codepath we see above), and largely caused by the earlier change that introduced when --stat-count was added (the second loop that decrements total_files does so only for the paths within the "count" horizon determined by the first loop; total_files must be adjusted for _all_ uninteresting and unchanged filepairs and exclude unmerged entries).

Also the initialization of data->is_interesting is wrong. These days, p->status is never zero.

I'll send out a fix later today.