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

[Jim Gettys]

On Sun, 2004-10-03 at 15:25 -0700, Keith Packard wrote:
> Around 21 o'clock on Oct 3, "Ronny V. Vindenes" wrote:
> 
> > However one big obstacle remains; there is little or no consistency in
> > code throughout the project.
> 
> Given that the project started 17 years ago, it's not a huge surprise that 
> various style variations have been tried in different parts of the code.  
> Other projects have had the benefit of consistent leadership, which X has 
> not.

Actually, X11 was 17 years; X itself is over 20, and that was based on
W, yet again older.

> 
> The following is purely personal opinion and not intended to serve as a 
> set of requirements for coding styles.  But, I will continue to use the 
> described styles in code that I author, so you might as well get used to 
> seeing it...
> 
> In the historical sections of the code base (Xserver, lib/X11), there are 
> two main styles which differ only in how braces are applied.  Both use 4 
> space indentation with a mixture of tab and spaces used to line up the 
> text.  Howver, tab stops are *always* on 8 character boundaries.
> 
> lib/X11:
> 
>     if (foo) {
> 	some code;
>     } else {
> 	some more code;
>     }
> 

This is my personal favorite, though I'm much less rabid about it than
when I established its use so long ago.  The intent was to save some
space on 24 line terminals; this is less of an issue than it once was.

> Xserver:
> 
>     if (foo)
>     {
> 	some code;
>     }
>     else
>     {
> 	some more code;
>     }
> 
> I'm OK with either varient, with a slight preference to the separate 
> braces (having worked mostly in Xserver).  It does encourage single 
> statement code blocks to avoid braces, which is plus/minus on readability.
> 

Yup.  Which is why I prefer my style ;-).

> Having said all of the above, I think the most important thing to do while
> editing code is to adopt the existing style and not patch your own style
> into a large body of existing code.  If the existing style on some long
> abandoned is so horrific or inconsistenly applied as to make working on the
> code painful, I'd much rather see a pure whitespace patch applied before
> any functional changes are made.
> 

I agree with Keith entirely on this one: please respect the style of the
module you are working in, even when you don't agree with it.

If things are really massacred (as they probably are) in some modules,
we should probably reindent them (and when we do, to match the
predominant style elsewhere in X).  I think we should first identify
which modules/files these are, and then choose good times to do so.

Please *don't* reindent without checking first with other people as to
whether it is seen as a problem.

And I suspect we should figure out a default set of arguments for
"indent" for people to use on new modules.
				- Jim