Re: [PATCHv2 1/2] diff: introduce --stat-lines to limit the stat lines
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 3, 2011, 18:47 UTC
- Message-ID
- <7vfwoviptm.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <102e117f7a9f7fd6e652f23d5630db973e8e52b8.1304419588.git.git@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
> In the case with <count>+1 items one may argue whether it makes more sense to > ignore the user wish and output all <count>+1 lines, or <count> lines (as > requested) plus the "..." line.
I think that is a must if we care about consistency. fmt-merge-msg should already do this (I remember being careful about this particular case when I wrote its first version, but I do not know it has regressed as I do not remember writing tests for this boundary case---my bad ;-).
> (I saw the suggestion about N-2...2 just now. Would work also, but I guess > we would do this in more cases then, as Junio indicated.)
It is not clear what you mean by N-2...2 but if you are referring to my "first N-1 entries, dots and the last one, to make the total N+1 lines that show N entries", then yes I think it would make sense to do that also in fmt-merge-msg.c::shortlog() as well as here. But that would be a separate topic.
Show 17 quoted lines
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt > index 34f0145..000eae0 100644 > --- a/Documentation/diff-options.txt > +++ b/Documentation/diff-options.txt > @@ -48,11 +48,14 @@ endif::git-format-patch[] > --patience:: > Generate a diff using the "patience diff" algorithm. > > ---stat[=<width>[,<name-width>]]:: > +--stat[=<width>[,<name-width>[,<count>]]]:: > Generate a diffstat. You can override the default > output width for 80-column terminal by `--stat=<width>`. > The width of the filename part can be controlled by > giving another width to it separated by a comma. > + By giving a third parameter `<count>`, you can limit the > + output to the first `<count>` lines, followed by > + `...` if there are more.
Does an empty-string <count> mean "use default" (currently "no limit")? This matters when we teach a new parameter to --stat and make the above:
--stat=[=<width>[,<name-width>[,<count>[,<nitfol>]]]]
Show 14 quoted lines
> + for (i = count; i < data->nr; i++) {
> + uintmax_t added = data->files[i]->added;
> + uintmax_t deleted = data->files[i]->deleted;
> + if (!data->files[i]->is_renamed &&
> + (added + deleted == 0)) {
> + total_files--;
> + continue;
> + }
> + adds += added;
> + dels += deleted;
> + }
> fprintf(options->file, "%s", line_prefix);
> fprintf(options->file,
> " %d files changed, %d insertions(+), %d deletions(-)\n",This is culling the output of what is in struct diffstat that we have already spent cycles to possibly fill thousands of entries. I first thought it may make sense to also tweak the loop in diff_flush() that runs diff_flush_stat() to all filepairs to run it only on the first <count> (and later the first <count-1> and the last) filepairs, but we have to show the short-stat summary at the end, so this cannot be avoided.
What happens when I say "diff --numstat --stat-count=4"?
Should it error out upon seeing a limit that is not infinite, or should it also elide the lines in its output?