Re: [PATCH] --stat: ensure at least one '-' for deletions, and one '+' for additions
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Sep 28, 2006, 19:14 UTC
- Message-ID
- <Pine.LNX.4.63.0609281942360.14200@wbgn013.biozentrum.uni-wuerzburg.de>
- In-Reply-To
- <7v1wpvvrww.fsf@assigned-by-dhcp.cox.net>
Hi,
On Thu, 28 Sep 2006, Junio C Hamano wrote:
Show 22 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> 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.)
Show 8 quoted lines
> 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