[MPlayer-dev-eng] the great reformatting

[Clément Bœsch]

On Tue, Feb 15, 2011 at 09:00:48PM +0100, Reimar Döffinger wrote:
> On Tue, Feb 15, 2011 at 07:16:48PM +0100, Clément Bœsch wrote:
> > > 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.
> 
> Have you tested that it does not incorrectly remove {} around a macro
> that require it?
> That seems very difficult to get right, particularly when something
> might be a macro or not depending on some defines.
> (of course those macros should be fixed, but code breakage in obscure
> cases is not a good way to find them).
> 

I certainly didn't test enough. We can of course put this in "ignore"
state if it causes issues.

> > > 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.
> 
> I have seen absolutely no code that does this, so I very much
> disagree here.

Disagree with what? I personally defend the no-space opinion for the same
reason given above, but if so, it's an exception to K&R and should be
written in the coding style rules.

-- 
Clément B.