Fixing up the code style

[Nikolay Sivov]

On 2/15/2014 10:32, Erich E. Hoover wrote:
> On Thu, Feb 13, 2014 at 7:24 AM, Stefan Dösinger
> <stefandoesinger at gmail.com> wrote:
>> ...
>> Maybe, or maybe not. The basic issue still stands. New contributors
>> will look at the existing code, and if its inconsistent how are they
>> supposed to write code that's consistent in itself. That's not a
>> question of pickiness but approach to the problem.
>> ...
> I'm not sure it's always clear what to do even the case of more
> experienced contributors, I've been taking a look at fixing a double
> unlock bug in RtlAcquireResourceExclusive (
> http://source.winehq.org/source/dlls/ntdll/rtl.c#L157 ).  And the
> style there indent-wise is so screwy that it's not clear what I should
> do in my patch.  This one function currently has (level wise):
> 1) Four space indents
> 2) A tab indent + a space indent (in some cases just the tab)
> 3) A tab indent + a five space indent (alternatively, 13 spaces)
> 4) Two tabs + a space indent
>
> So, what do I do?
Please send a fix that changes only what you need to change, why not?
>    Do I just change line 190 to fix the bug and keep
> the two tab + a space indent?  Do I fix just this line to conform to
> the standard since it's the only one I need to touch?  Do I fix this
> one function because a lot of it doesn't conform to the standard?
> Personally, I would like to have a separate formatting patch to fix
> the function so that it's clear that I'm just fixing the one thing -
> but that's expressly prohibited under the current guidelines.
>
>> ...
>> My impression was that the answers to that were no and no, with the
>> idea that the problem will be fixed by unifying the style together
>> with other, functional changes. That way we end up with picky reviews
>> and IMO that approach is bad.
>> ...
> I agree, Sebastian and I have been debating what to do with this
> function (and RtlAcquireResourceShared) for some time and style is the
> only thing keeping us from submitting the one-line fix for these.
I don't see a problem here, if it's a one-line fix just submit it while 
keeping existing formatting of a changed line, what's a big deal? Yes, 
sometimes style is inconsistent, sometimes whitespace formatting differs 
from line to line but that kind of existing code "features" should never 
stop you from sending a fix. In a situation like that I usually change 
several lines before and after to use 4 spaces indentation.
> Honestly, it would be really great if we could run something like
> astyle on the whole codebase so that we were always working off of a
> "known good" style.  If we could get that implemented then we could
> potentially think about verifying the patch style on submission and
> auto-rejecting patches that have style problems.  Personally, I think
> that would be an amazing improvement to our submission process,
I don't think it's ever a case that potential contributor decides not to 
do any work solely after looking at some code style issues and saying - 
oh, well, what a mess, I'm not going to touch it. What actually happens 
when people send patches (or attach them to a bug and then disappear) is 
that patch is wrong in a first place, and code style issues are a second 
problem that's easy to fix comparing to functional part. Of course if 
all code uses same style we could simply avoid step 2 but that's not a 
blocking issue for new contributors.

Another thing is that some parts of wine have long time contributors, 
like wined3d, mshtml, msvcrt or audio bits. Why should we force their 
maintainers to some particular code style?

I personally can live with any decision made regarding reformatting 
everything, as long as it doesn't use some ugly mixed case names, 
variable names with type prefixes, LPLPLP stuff and such.

Anyway formatting issues never surprised me that much to stop fixing a 
real problem, but that's probably because I'm constantly jumping from 
one part to another, and from project to project on my daily job.
>   but I
> have to use K&R style for work and I hate feeling like an idiot when I
> slip into the wrong style for a project.
>
> Best,
> Erich
>
>
>