Xorg coding style (Was: Re: Radeon TV-in support in Xorg CVS.)

[Ronny V. Vindenes]

søn, 03,.10.2004 kl. 12.52 -0400, skrev Vladimir Dergachev:
> >
> > The following functions have no or partial indentation:
> >
> > fi1236.c:
> > MT2032_dump_parameters, MT2032_getid, MT2032_shutdown, MT2032_init,
> > MT2032_no_spur_in_band, MT2032_calculate_register_settings,
> > MT2032_wait_for_lock, MT2032_implement_settings, MT2032_optimize_VCO,
> > FI1236_get_afc_hint, MT2032_get_afc_hint, TUNER_get_afc_hint,
> > MT2032_dump_status, MT2032_tune, FI1236_set_tuner_type,
> > fi1236_dump_status
> >
> 
> Many of these functions do not have any identation levels to speak of.
> E.g. MT2032_dump_parameters, MT2032_shutdown.
> 
> >
> > I noticed it using vi and less. I suspect your editor is doing some
> > automagic to hide the ugliness from you. (I checked with hexdump - there
> > are no '\t's, it's strictly "one line\nthe next line")
> 
> I guess we have a different preference for identation styles, I hope you 
> won't think mine is as ugly as it seems once I explain why I use it.
> 
> It is based on linux kernel style, with the changes made to minimize 
> keypresses required.

I've read a lot of Linux kernel code (even written some) and it is my
preferred style too, but your code doesn't look much like any kernel
code I've ever read. It looks like code written by one person solely for
the use by that person.

> 
>     * No identation level for function bodies - there is nothing to
>       confuse a function body with.
> 

How did you arrive at this conclusion? I find it very confusing when you
have a bunch of simple (i.e. no further indention levels) functions all
packed together. Particularly with your minimal whitespace use and
mixing of function prototypes with regular code.

I'm sure it's perfectly usable for someone who knows the entire code and
has worked on it for years, but it's not very nice for us "tourists".
I've only programmed commercially and "for fun" for about two decades
but I've never seen anyone write C function bodies without indentation
before.

>     * The blocks { /* body */ } are interchangeable with single statements:
> 
>           if(condition)op1;
>  		else op2;
> 
>           if(condition){
>  	 	op1;
>  		} else {
>  		op2;
>  		}
> 

This example, like most of your code, is begging for more whitespace.

>     * tabs are 8 spaces - it is easier to see this way and it
>       works great with numbers.

I prefer 8 spaces too. And I would really like to see the "tradition" of
2, 4, 4 and 2 or 4 and 3, go away in some of the old drivers.

> 
> I do recognize that other people prefer other styles. And (as can be seen 
> from other code) I did try to comply with XFree86 style.
> 
> However, XFree86 style is a pain (literally - I don't think I have any 
> stress disorder, but if type for long my fingers hurt a little) for me to 
> use, so I try to format it like this only when a code is not likely to change.
> 

I agree the XFree86 style is less than optimal, but you can minimize the
pain by tuning your editor.

> If anyone knows any magic (based on indent or another program) that I can 
> simply run on the files when I am done, I'll be very thankful.
> 

See programs/Xserver/hw/xfree86/doc/CODING for the indent options for
the XFree86 style.

> My thoughts so far were to separate out MT2032 in different file and then 
> the formatting will be consistent again. I vaguely remember someone saying 
> that having a consistent style within one file is fine.
> 

Here is the core of the problem - I didn't mean to pick on your code
particularly - in the XFree86 days only a small group of people were
"allowed" to participate, after the X.Org fork things have gotten a lot
better; patches flow freely and things are open for discussion. Now
anyone can, at least try to, contribute.

However one big obstacle remains; there is little or no consistency in
code throughout the project. Contrast this with other big successful
projects like Linux or GNOME where you can (for the most part) work in
one part of the code then switch to (almost) any other part and the same
style and conventions apply.

I think more people would be willing to invest time and code if they
didn't feel they had to pay the "initial cost" again whenever they
looked at a different piece of the xorg puzzle.

Maybe I'm just getting my panties in a wad, so to speak, but I feel
X.Org needs to publicly appoint a dictator or board of dictators to set
some overall practical standards for the project or have the board of
directors or one of the task forces draft up something to address this
and then enforce it. 

IMHO any remotely reasonable standard is better than the current "one
file - one or two standards" approach. Since I'm nobody, I won't waste
any more bandwidth on this, but I would be a little surprised if I'm the
only one feeling this way.

-- 
Ronny V. Vindenes <s864 at ii.uib.no>