[MPlayer-dev-eng] the great reformatting

Clément Bœsch ubitux at gmail.com
Tue Feb 22 12:18:21 CET 2011


On Tue, Feb 22, 2011 at 10:34:25AM +0100, Ingo Brückl wrote:
> Clément Boesch wrote on Tue, 22 Feb 2011 00:00:38 +0100:
> 
> > So there is another setting to set:
> 
> >     align_var_def_span = 1
> 
> I did this and the result is attached.
> 
> (BTW, I don't get whether aligning of var_def is favored, while var_struct
> isn't. I, as mentioned, would vote against all aligning :), but if that's
> convention, shouldn't it be consistent?)
> 

The var align set to 1 seems to force only 1 space, so in fact, no
alignment. Do you see a better option?

> > But, this is not enough too. In fact I was afraid uncrustify removed the
> > variable names from the prototypes here:
> 
> >> int fntFindID(char *);
> >> void fntFreeFont(void);
> >> int fntRead(char *, char *);
> >> txSample *fntRender(wItem *, int, char *);
> >> int fntTextWidth(int, char *);
> 
> > While in fact it was you :)
> 
> Oh, sorry for not mentioning. Yes, I'd like to remove the unnecessary
> variable names from header files (redundant and not needed by the compiler)
> unless of course this would be against the coding style.
> 
> (If uncrustify would have done this, it would have been an unauthorized code
> modification.)
> 

I can't see anything related in the mod_* option after a quick look.

> > But it also shows me the indent here is also too much tolerant. In the
> > real case, this:
> 
> >     int        fntRead( char * path, char * fname );
> 
> > Is replaced by:
> 
> >     int        fntRead(char *path, char *fname);
> 
> > Which is not fine.
> 
> This seems to be option sp_inside_fparen which also affects function calls,
> i.e. you'd get
> 
>   int fntRead( char *path, char *fname )
> 

The issue is not the inside parens, fntRead(char *path, char *fname) is
correct, the spaces between the type and the function name isn't. I didn't
find the correct setting. Also, this means a lot of extra spaces have to
be removed but aren't (typedef       foo       bar    ; for instance).

> but (if the programmer has used these spaces already)
> 
>   id = fntAddNewFont( fname );
> 
> as well.
> 
> IMHO the current setting (remove) is best.
> 

There is no "IMHO", it is of course :)

> [...]
> int fntRead(char *path, char *fname)
> {
>     FILE          *f;
>     unsigned char  tmp[512];
>     unsigned char *ptmp;
>     unsigned char  command[32];
>     unsigned char  param[256];
>     int            id, n;
> 

This alignement is wrong, we need to remove it; it's relative to what I
said previously.

>             if (!utf8 && (Fonts[id]->nonASCIIidx[i][0] == (*uchar >> 6 | 0xc0) && Fonts[id]->nonASCIIidx[i][1] == ((*uchar & 0x3f) | 0x80) && Fonts[id]->nonASCIIidx[i][2] == 0))

Huh? We need the 80 column setting too if it exists…

-- 
Clément B.


More information about the MPlayer-dev-eng mailing list