Jeff King <peff@xxxxxxxx> writes:
> On Tue, Feb 14, 2012 at 12:07:31PM -0800, Junio C Hamano wrote:
>
>> Of course, an easy way out without worrying about the correct math is to
>> scale the total and the smaller one and then declare that the scaled
>> larger one is the difference between the two. That way, both of these two
>> files have 109 in total so the length of the entire graph would be the
>> same ;-).
>
> It looks like we actually did that, pre-3ed74e6. I think it's a valid
> strategy. It is just pushing the error around,...
Yes, that is exactly why I suggested that approach. We have to deal with
rounding error somewhere no matter what we do, and the balance between
add/del is much less noticeable than the change with the same total not
lining up.
Anyway, here is an obvious patch to fix this. We did not have any test
that failed with this change, as all our test vectors fit comfortably on
the default 80-column output.
-- >8 --
Subject: diff --stat: resurrect "same for same" heuristic
When commit 3ed74e6 (diff --stat: ensure at least one '-' for deletions,
and one '+' for additions, 2006-09-28) improved the output for files with
tiny modifications, we accidentally broke rounding logic to ensure that
two equal sized changes are shown with the bars of the same length.
This updates the logic to compute the length of the graph bars, using the
same "non-zero changes is shown with at least one column" scaling logic,
but by scaling the sum of additions and deletions to come up with the
total length of the bar (this ensures that two equal sized changes result
in bars of the same length), and then scaling the smaller of the additions
or deletions. The other side is computed as the difference between the
two. This makes the apportioning between additions and deletions less
accurate due to rounding errors, but it is much less noticeable than two
files with the same amount of change showing bars of different length.
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
diff.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/diff.c b/diff.c
index 3550c18..76d4724 100644
--- a/diff.c
+++ b/diff.c
@@ -1273,13 +1273,17 @@ const char mime_boundary_leader[] = "------------";
static int scale_linear(int it, int width, int max_change)
{
+ if (!it)
+ return 0;
/*
- * make sure that at least one '-' is printed if there were deletions,
- * and likewise for '+'.
+ * make sure that at least one '-' or '+' is printed if
+ * there is any change to this path. The easiest way is to
+ * scale linearly as if all the quantities were one smaller
+ * than they actually are, and then add one to the result.
*/
if (max_change < 2)
- return it;
- return ((it - 1) * (width - 1) + max_change - 1) / (max_change - 1);
+ return 1;
+ return 1 + ((it - 1) * (width - 1) / (max_change - 1));
}
static void show_name(FILE *file,
@@ -1495,8 +1499,19 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
dels += del;
if (width <= max_change) {
- add = scale_linear(add, width, max_change);
- del = scale_linear(del, width, max_change);
+ int total = add + del;
+
+ total = scale_linear(add + del, width, max_change);
+ if (total < 2 && add && del)
+ /* width >= 2 due to the sanity check */
+ total = 2;
+ if (add < del) {
+ add = scale_linear(add, width, max_change);
+ del = total - add;
+ } else {
+ del = scale_linear(del, width, max_change);
+ add = total - del;
+ }
}
fprintf(options->file, "%s", line_prefix);
show_name(options->file, prefix, name, len);
--
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