Hi,
On Thu, 28 Sep 2006, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
>
> > 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?
No, it is very much a mathematical constraint: we are going to divide by
(max_change - 1), which is 0 or -1 in said cases. In _both_ cases, "it"
happens to be what we actually want. (Okay, in reality we should check for
width > 0, but I think width == 0 would not make _any_ sense.)
> 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.
That is right.
However, I would argue it's actually an improvement. If you have as many
additions as deletions, there should be an equal number of '-' and '+'.
After all, the integer number of symbols is just an approximation anyway.
Ciao,
Dscho
-
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