Page MenuHomeWildfire Games

Add clang-format configuration for auto-formatting C++ code
Changes PlannedPublic

Authored by Itms on Dec 6 2019, 10:18 PM.



This is still WIP, I'm mostly publishing this because I want your opinion. To read more about clang-format, see
Is this something we want?
While testing this, I noticed that the source code is very inconsistent in style. This could help (in addition to potentially making reviews a little bit less about style issues). Perhaps we could even use clang-format as a linter for patches:

Test Plan


Event Timeline

echotangoecho created this revision.Dec 6 2019, 10:18 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build:

Successful build - Chance fights ever on the side of the prudent.

Link to build:

Stan added a subscriber: Stan.Dec 6 2019, 10:59 PM

Does it work on windows? Or is it Unix only?

This works on all platforms where you can use clang-format, which I assume is Windows as well (Visual Studio might even have integrated support).

I think you might add the exception to source/third_party as well.

Itms commandeered this revision.Aug 10 2020, 4:25 PM
Itms added a reviewer: echotangoecho.
Itms updated this revision to Diff 13161.Aug 10 2020, 4:50 PM
Itms retitled this revision from Add .clang-format configuration for auto-formatting C++ code to Add clang-format configuration for auto-formatting C++ code.
Itms edited reviewers, added: Krinkle; removed: echotangoecho.

Here is a working version using Coala (not enabled in .coafile here).

It is not yet ready to deploy. Indeed, whereas clang-format does a great job at proposing consistent style conventions, and is highly configurable, it has a limitation that I think is unacceptable.

clang-format has no option to accept spaces around binary operators. From Stack Overflow:

z = sqrt(x*x + y*y);

is reformatted as

z = sqrt(x * x + y * y);

which I believe is incorrect. Grouping some terms is very important to understand the code better, especially in mathematic formulae. Moreover, this would require us to change a huge part of our code, whereas we should follow our usual style and enforce its consistency.

There is no way to deactivate this enforcement. So I think we shouldn't use clang-format right now.

Maybe we could try to make upstream add the feature; or I could try to tweak the coala bear to discard this specific change.

In the meantime, I am proposing in D2490 a less strict linting using Artistic Style.

Itms planned changes to this revision.Aug 10 2020, 4:51 PM
Itms added a subscriber: wraitii.Aug 17 2020, 10:01 AM

@wraitii, @vladislavbelov, what do you think about the spaces-around-operators issue? If you agree with me, I will move forward with D2490 (see some discussion there) and try to find a way to fix this one issue with clang-format. If you both think spaces everywhere are better, I can activate this instead.

I don't have a very strong opinion overall, but starting with a more lazy approach seems fine to me.