Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
> The number of '-' and '+' is still linear. The idea is that
> scaled-length := floor(a * length + b) with the following constraints: if
> length == 1, scaled-length == 1, and the combined length of plusses
> and minusses should not be larger than the width by a small margin. Thus,
>
> a + b == 1
>
> and
> a * max_plusses + b + a * max_minusses + b = width + 1
>
> The solution is
>
> a * x + b = ((width - 1) * (x - 1) + max_change - 1)
> / (max_change - 1)
>
> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@xxxxxx>
Nice looking math in the log message aside,...
> diff --git a/diff.c b/diff.c
> index 98c29bf..53c30bd 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -640,9 +640,12 @@ const char mime_boundary_leader[] = "---
> static int scale_linear(int it, int width, int max_change)
> {
> /*
> - * round(width * it / max_change);
> + * make sure that at least one '-' is printed if there were deletions,
> + * and likewise for '+'.
> */
> - return (it * width * 2 + max_change) / (max_change * 2);
> + if (max_change < 2)
> + return it;
> + return ((it - 1) * (width - 1) + max_change - 1) / (max_change - 1);
> }
This does not feel right. Maybe the first conditional is to see
if it is less than 2, not max_change?
> static void show_name(const char *prefix, const char *name, int len,
> @@ -774,9 +777,9 @@ static void show_stats(struct diffstat_t
> dels += del;
>
> if (width <= max_change) {
> - total = scale_linear(total, width, max_change);
> add = scale_linear(add, width, max_change);
> - del = total - add;
> + del = scale_linear(del, width, max_change);
> + total = add + del;
> }
> show_name(prefix, name, len, reset, set);
> printf("%5d ", added + deleted);
The original was written a bit differently because it used
round() not floor(), and avoided the case of both ends end up
rounding up.
This version of scale_linear() rounds down so you can get away
with this without busting the total width.
But think about the case where width=7, max_change=10, 5
lines are added and 5 lines are removed.
The original makes total 7 to use full width, and gives 4 pluses
and 3 minuses, which is not quite right when your eyes notice
that they are not of equal length, but it makes it to use the
full width. I think this version gives 3 pluses and minuses,
and does not use the full width. However, if you have a file
that adds 10 without removing, it will be drawn as 7 pluses. In
other words, the line drawn for a new file that is 10 lines
long, and the line for a modified file that added 5 lines and
removed 5 lines, are drawn differently.
I _think_ the graph length is reasonably long enough that the
actual differences coming from rounding (3 vs 4 in the above
description for the current implementation) is less annoying
than the total number of pluses and minuses not lining up for
two files that had both 10 changes, one (add=10,del=0) and the
other (add=5,del=5). Illustration.
Compute total and add, make del=total-add:
foo | 10 ++++---
bar | 10 +++++++
Compute add and del independently:
foo | 10 +++---
bar | 10 +++++++
So I'd suggest either force the width to even number (if odd
drop one), of keep the current del=total-add.
How about doing something like this instead?
- first scale the total but make sure there is one column for
each of non-zero add and delete;
- scale add but make sure it is at least one if non-zero;
- del is the remainder of total after add is taken, but if
there are too many adds than dels, that can become zero. In
that case adjust by giving one column from add to del.
diff --git a/diff.c b/diff.c
index 3fd7a52..9b9a6d8 100644
--- a/diff.c
+++ b/diff.c
@@ -684,9 +684,15 @@ static void show_stats(struct diffstat_t
dels += del;
if (width <= max_change) {
- total = scale_linear(total, width, max_change);
- add = scale_linear(add, width, max_change);
+ int fl = !!add + !!del;
+ total = scale_linear(total, width-fl, max_change) + fl;
+ if (add)
+ add = scale_linear(add, width-fl, max_change) + 1;
del = total - add;
+ if (!del && deleted) {
+ add--;
+ del++;
+ }
}
show_name(prefix, name, len, reset, set);
printf("%5d ", added + deleted);
-
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