[MPlayer-dev-eng] Indentation changes in patches

[Attila Kinali]

On Sun, Aug 01, 2004 at 03:29:19PM +0200, Alexander Strasser wrote:

> This can be done by diff or any similar tool ( you know they can compare
> each line character by character :). Means a little more work for the
> reviewer as he has to extract the text he wants to compare or run diff
> with options to ignore those changes. But these are no pure cosmetical
> changes so he isn't misdirected by the patch to code that hasn't changed.
> The code changed as it is now executed only when it meets the condition
> for example. So you have to take that into account anyway. But I also
> understand your viewpoint here.

Yes and no. The problem is bughunting. If you are reading patches with
cvsweb or use cvs annotate it's hard to say whether a change in a line
was just pure cosmetical or not because as the line changed it will be
outputed.
Sure, if i do a diff -uw i can see that it is not the case, but that
would mean that i have to download the two revisions and manualy compare
them. Not exactly time saving.

> > Ie move the if() { } left instead of moving the code to the right.
> > I know this creates very low indenting levels after a while, but then
> > you can apply one big reindention at once instead of applying one for
> > every patch (as has happend with x11_common.c).
> This makes it not much better. Running indent over the code might be
> a solution, but i fear nice human indenting will be removed by
> automatic indentation.
> 
> Maybe it is best to submit two patches one that makes the functional
> change and the other that fixes the _broken_ indentation again.
> But it's suboptimal imho.

Yes, it is. But no real solution was fount until now.


		Attila Kinali