[MPlayer-dev-eng] the great reformatting

[Clément Bœsch]

On Tue, Feb 15, 2011 at 06:32:43PM +0100, Dan Oscarsson wrote:
> Tested it a little, quite good, but:
> 
> I will have a lot of work getting my ten patches to work.

Apply uncrustify on your patched files and make a diff/use git?

> After reformatting I get a lot of failures by patch even with option -l.
> I assume this is mostly because it does not just adjust white space but
> also change the contents on many lines.

It adds/remove various \n, and IIRC the config also drops pointless {},
so yeah, it's not just whitespaces.

> Adding space on each side of an mathematical operator is not always
> better. For example:
>     for (i = 0; i <= num_output_surfaces+1; i++)
> is easier to understand (for me) than
>     for (i = 0; i <= num_output_surfaces + 1; i++)
> or
>   if (tdv != td2/10000 || tdv != td3/10000) {
> compared to
>   if (tdv != td2 / 10000 || tdv != td3 / 10000) {
> 

It's just K&R… The only issue we can have is a*b + c spacing behaviour for
instance, to emphasis priorities.

> 
> Why is a space added after a type conversion?
>    (const void *const*)&index_data,
> is changed to
>    (const void *const *) &index_data,
> or
>   vo_vsync_time = GetTimer()-(int)((now-show_time)/1000);
> is changed to
>   vo_vsync_time = GetTimer() - (int) ((now - show_time) / 1000);
> I think it is easier to read if type conversion is next to the data
> being converted to show what is being converted.
> 

I totally agree with this, but we should follow K&R.

> 
> It reformats
>             s3 = s2; s2 = s1; s1 = s3;
> into
>             s3     = s2;
>             s2     = s1;
>             s1     = s3;
> also here there may be cases where a single line is clearer.
> 

Not here imo.

> 
> If removes { and } around statements with only one line, like
> if (a) {
>    x = 1;
> } else {
>    y = 1;
> }
> 

Yes, we ask for it regularly.

> Which may destroy intended place for more lines and makes it easy to
> forget adding {} when adding lines. Personally I always use {} to avoid
> problems. I also thinks the code gets much easier to read then with
> clearer meaning.

As I said, it's the coding style we wanted to follow.

> [...]

-- 
Clément B.