[Mesa-dev] [PATCH] i965/msaa: Only do multisample rasterization if GL_MULTISAMPLE enabled.

[Kenneth Graunke]

On 06/19/2012 11:27 AM, Paul Berry wrote:
> On 19 June 2012 10:29, Kenneth Graunke wrote:
>     Whitespace errors here.  I know Mesa isn't terribly consistent
>     w.r.t. tabs or spaces, but in general these files use 3 space indent
>     with 8 space tabs.  Please use tabs to match the surrounding lines.
>
>
> I'm reluctant to add fuel to coding convention debates, but I really
> don't think "follow whatever tab/space style the surrounding code
> follows" is a reasonable coding convention to enforce.  Most editors do
> automatic indentation of C/C++ code, and can be easily configured to
> either globally use tabs or globally not use tabs.  They can't be easily
> configured to follow the tab/space style of the surrounding code, which
> means that if this is our coding convention, it's going to be an extra
> developer burden.
>
> Also, our documentation says we prefer spaces to tabs.  From
> docs/devinfo.html:
>
>      Here's the GNU indent command which will best approximate my
>      preferred style: (Note that it won't format switch statements
>      in the preferred way)
>
>          indent -br -i3 -npcs --no-tabs infile.c -o outfile.c
>
> That "--no-tabs" option causes indent to use spaces, not tabs.
>
> And the toplevel .emacs-dirvars file (which emacs automatically consults
> if the dirvars package is in use) configures emacs to automatically
> indent using spaces, not tabs, using this line:
>
>      indent-tabs-mode: nil
>
> When I started working on Mesa code over a year ago I configured my
> editor to insert spaces rather than tabs (on Chad's recommendation), and
> the only time people have found fault with my whitespace has been
> situations like this, where I was editing code that didn't follow our
> documented convention.  So I really think there is a good precedent for
> spaces being our preferred indentation style, in spite of the fact that
> Mesa code is very inconsistent about following it.
>
> My preference would be to leave the patch as is.  If that's not
> acceptable, I'd propose changing tabs to spaces in the parts of the code
> that I'm modifying, so that at least this patch moves us toward
> consistent use of spaces instead of tabs, rather than away from it.

I'm sorry for being petty, but I simply don't understand why you need to 
make this into a large debate every time.  You already spent more time 
writing a lengthy email response about why you don't need to follow the 
existing style than it would take to incorporate my feedback.

As I understand it, the rule is: be consistent with the existing code, 
or (if it's totally wrong or internally inconsistent) make a wholesale 
change to fix it, independent of actual content changes.  I hold 
everyone to this standard, including new developers and people 
contributing in their free time.  Projects like the Linux kernel reject 
patches outright when they don't follow indendtation style.  Some people 
may think that's silly, but they deal with it anyway.

In this particular case, when you add the _NEW_MULTISAMPLE dirty bit, 
your patch adds inconsistency within a single assignment statement:

   .field = _NEW_FOO |
<tabs>  _NEW_BAR |
<spaces>_NEW_MULTISAMPLE

That's what I really find offensive.  If we can't even be consistent 
within a single line of code...sheesh.  Please be consistent and use tabs.

That said, I agree with you that it would save time in the long run to 
simply pick one and use it consistently.  Apparently, there was a 
misunderstanding at one point - Brian's coding style has always been 
three space indent, no tabs, but someone mistakenly began using them. 
Now, the whole GLSL compiler and most of our new code is mixed tabs and 
spaces.

At some point, I'd like to pick one, get everyone to agree, and 
mass-convert everything in one fell swoop.  Also, distribute the 
appropriate emacs and vim settings to indent things correctly.  Then we 
can move forward.  My preference is to go back to the documented 3 
space, no tabs style---it's much simpler to explain, and much harder to 
screw up than mixing the two.  It sounds like that's your preference as 
well.  We just need to agree on it and do it.

In the meantime, I'd like to keep things at least somewhat consistent 
within files.  But I did give you my Reviewed-by, so you're free to push 
it---I'll just be somewhat frustrated, that's all.