[MPlayer-dev-eng] the great reformatting

[Dan Oscarsson]

tis 2011-02-15 klockan 01:53 +0100 skrev Diego Biurrun:
> 
> I'm attaching the config file, it would be suitable to place under
> TOOLS or so.  Grab uncrustify from

> So, in short, the time for the great reformatting is finally arriving.
> Constructive comments are welcome, skip the flaming and bikeshedding
> please.

Tested it a little, quite good, but:

I will have a lot of work getting my ten patches to work. 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.


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) {


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.


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.


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

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.
Only case where I avoid {} is in: if (a) x = 1;
formatted on one line.
It gets worse if I do
if (a) {
   // comment
   x = 1;
} else {
   // comment
   y = 1;
}
It still removes the { } make code look like:
if (a)
   // comment
   x = 1;
else
   // comment
   y = 1;
which is even worse.

Apart from the above, most other things in my quick test look ok.

   Dan