[Mesa-dev] [PATCH] i965/msaa: Only do multisample rasterization if GL_MULTISAMPLE enabled.
Kenneth Graunke
kenneth at whitecape.org
Tue Jun 19 17:18:22 PDT 2012
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.
More information about the mesa-dev
mailing list