Re: [Widelands-public] Reformatting Wideland's code and changing style

[Holger R. <Hol...@gm...>]

Hi,

> I personally also prefer tabs over spaces, but I don't feel very
> strongly on the topic.
so now we have 3 people prefering spaces and 2 prefering tabs. That pretty much shows that it is impossible to please everybody with this topic and it also proves that we will not find a consent that pleases everybody.

I feel like the distribution of clang-format is not that great these days. I am sure this will come with time, so we might wait a while till we introduce it. However it doesn't hurt to switch the style right away, so people who want use clang-format (like myself) can: the style checker is still there and nothing will change for other developers in the work flow at all. Plus, we could reconcile the mixed styles that we still have in the code (because the style checker is not very strict in many ways), so the code would be more uniform and therefor easier to understand. So that clang-format is not widely distributed is no argument of not merging right now, right?

Cheers,
Holger

> As for clang, I love clang, but I once looked how to build it on
> windows, and it seemed like a real pain. I don't know if this
> clang-format would be easier. And I wouldn't like to 1) always have to
> boot up my linux box to make sure the indentation's fine, 2) remember
> all the rules so it doesn't have to fix anything, 3) need someone else
> to clean up my formatting after me. So, as much as I like it, I would
> wait like to wait with it until it's a more mature project. I think
> that would probably also solve the "it's hard to format well with
> tabs" problem.
> 
> I understand it's great for Google, but inside a company, the build
> environment is more or less uniform, it's easy to push the use of a
> new tool. With an open source project, it's best if all the different
> environments can work on the source, compile, and push changes, with
> as little hassle as possible. (I think currently Widelands is actually
> doing rather well in this respect.)
> 
> Just my 2c as someone on Windows.
> 
> Andras
> 
> On 18 May 2013 07:59, Holger Rapp <Hol...@gm...> wrote:
>> Hello,
>> 
>> I just pushed up a branch for merge into trunk that contains significant
>> changes to how style in Widelands works:
>> https://code.launchpad.net/~widelands-dev/widelands/clang_format/+merge/164570
>> 
>> 
>> • It removes a very old style checker written in ada which did not run in
>> years
>> • it fixes some smaller stuff (const and passing by reference mostly)
>> • it removes and alters some of the style rules the current style checker
>> uses.
>> • it introduces a configuration file for clang-format
>> (http://clang.llvm.org/docs/ClangFormat.html).
>> 
>> My suggestion is now that code should be reformatted using clang-format -
>> i.e. if you are happy with the way your code works, you run clang-format -i
>> <file> on it and it will reformat it according to the style file. This will
>> be the new official Widelands style. Why is this cool? It removes
>> discussions about style and it makes it easy to fix wrong style checked in
>> by others. It is also adds more fun to programming: feel your line is to
>> long? do not worry, clang-format will fix this later for you. It basically
>> removes one bad manual step.
>> 
>> What are the downsides? For one, installing clang-format is not so
>> straightforward right now (I have no idea how this even works for Windows) -
>> but devs do not really have to use it. Just fix all style warnings and get
>> used to the new style and you are done. Secondly, the tool does not handle
>> tabs as indentation well - so we no longer use tabs. Thirdly: if you see odd
>> formatting in the code, and you feel like fixing it, don't. It makes no
>> sense now, because clang-format will just change it back again the next time
>> you run it. From my experience: it is easy to life with these shortcomings
>> and just never think about formatting again. And the code it produces is
>> very readable.
>> 
>> I really, really want to convince all devs that this change is "A good
>> thing"(™). Please raise your objections now - if you have any :).
>> 
>> If no-one complains, I will merge this this week.
>> 
>> Cheers,
>> Holger
>> 
>> ------------------------------------------------------------------------------
>> AlienVault Unified Security Management (USM) platform delivers complete
>> security visibility with the essential security capabilities. Easily and
>> efficiently configure, manage, and operate all of your security controls
>> from a single console and one unified framework. Download a free trial.
>> http://p.sf.net/sfu/alienvault_d2d
>> _______________________________________________
>> Widelands-public mailing list
>> Wid...@li...
>> https://lists.sourceforge.net/lists/listinfo/widelands-public
>> 
> 
> ------------------------------------------------------------------------------
> AlienVault Unified Security Management (USM) platform delivers complete
> security visibility with the essential security capabilities. Easily and
> efficiently configure, manage, and operate all of your security controls
> from a single console and one unified framework. Download a free trial.
> http://p.sf.net/sfu/alienvault_d2d
> _______________________________________________
> Widelands-public mailing list
> Wid...@li...
> https://lists.sourceforge.net/lists/listinfo/widelands-public