Improving clang-format

Here's the place for discussion related to coding in FreeCAD, C++ or Python. Design, interfaces and structures.
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
wmayer
Founder
Posts: 20243
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Improving clang-format

Post by wmayer »

Bering relatively new to C++ I have been amazed at the way FreeCAD devs have been jamming up code as tight as possible, avoiding any use of spaces wherever possible, no space lines between methods etc.
Over the years I have modified my code formatting several times and in the recent years I avoid tight code as much as possible.

When reading the code of PRs I see how tiring it is if it lacks of any useful spaces, new lines or braces because you waste a lot of energy in deciphering every single character to get it in the right context. Thus, the risk is much higher to overlook serious bugs.
Personally I find this painful to read, but I've come to realise its a bit of a pattern in C++ land, particularly older C++ code.
I agree with you for the actual implementation. But for classes or functions I don't think that the readability suffers if the opening curly brace is at the same or next line.
I'm not sure clang-format can distinguish between h and cpp files. I doun't think so, but heh ..
I guess Qt devs must use a formatting tool and this it seems can distinguish between header and source files.
We're already seeing in ealier posts that the Qt settings are less than ideal.
IMO, most of the rules are OK but there are a few like curly braces for cascaded if-else blocks that I don't like. So, instead of

Code: Select all

 if (codec) {
 } else {
 }
 
if prefer

Code: Select all

 if (codec) {
 }
 else {
 }
 
because it's easier to read -- especially for nested and cascaded if-else blocks.
LarryWoestman
Posts: 98
Joined: Fri Oct 09, 2020 4:56 pm
Location: Oregon, USA

Re: Improving clang-format

Post by LarryWoestman »

A whole lot more time is spent reading already written code compared to writing it. It makes sense to me to organize the code to be understood as easily as possible. There are different opinions on how to do that, however ;)
berniev
Posts: 247
Joined: Wed Apr 13, 2022 10:45 pm
Location: Oz

Re: Improving clang-format

Post by berniev »

wmayer wrote: Thu Sep 29, 2022 3:00 pm So, instead of

Code: Select all

 if (codec) {
 } else {
 }
 
if prefer

Code: Select all

 if (codec) {
 }
 else {
 }
 
because it's easier to read -- especially for nested and cascaded if-else blocks.
My vote would be the first version: a) it keeps the 'else' kinda 'inside' the larger 'if' block rather than looking like a new structure, b) it keeps short blocks a bit shorter. On the other hand the latter version is easier to refactor, esp else if.
berniev
Posts: 247
Joined: Wed Apr 13, 2022 10:45 pm
Location: Oz

Re: Improving clang-format

Post by berniev »

I hadn't noticed before ...

In CLion, hover over a line of clang-format file, wizardry displays the various options for that line. Fantastic!
berniev
Posts: 247
Joined: Wed Apr 13, 2022 10:45 pm
Location: Oz

Re: Improving clang-format

Post by berniev »

NB: clang-format v15+ required to insert braces.
InsertBraces (Boolean) clang-format 15

Insert braces after control statements (if, else, for, do, and while) in C++ unless the control statements are inside macro definitions or the braces would enclose preprocessor directives.

Warning:

Setting this option to true could lead to incorrect code formatting due to clang-format’s lack of complete semantic information. As such, extra care should be taken to review code changes made by this option.
berniev
Posts: 247
Joined: Wed Apr 13, 2022 10:45 pm
Location: Oz

Re: Improving clang-format

Post by berniev »

Here's a version that implements some of discussions plus more.

Two difficult areas are

a) long lines (how to handle them); and
b) ALLOW some manually formatted code alone.

I've taken a stab at those, but there may be better ways.

Code: Select all

BasedOnStyle: LLVM
AccessModifierOffset: -4
AlignAfterOpenBracket: Align
AlignConsecutiveAssignments: Consecutive
AlignConsecutiveBitFields: Consecutive
AlignOperands: DontAlign
AllowAllArgumentsOnNextLine: false
AllowAllConstructorInitializersOnNextLine: true
AllowAllParametersOfDeclarationOnNextLine: true
AllowShortBlocksOnASingleLine: Never
AllowShortCaseLabelsOnASingleLine: true
AllowShortFunctionsOnASingleLine: Empty
AllowShortIfStatementsOnASingleLine: Never
AllowShortLambdasOnASingleLine: All
AllowShortLoopsOnASingleLine: false
AlwaysBreakAfterReturnType: None
AlwaysBreakTemplateDeclarations: MultiLine
BinPackArguments: false
BinPackParameters: false
BreakBeforeBraces: Custom
BraceWrapping:
  AfterCaseLabel: false
  AfterClass: true
  AfterControlStatement: Never
  AfterEnum: true
  AfterFunction: true
  AfterNamespace: true
  AfterUnion: true
  BeforeCatch: true
  BeforeElse: true
  IndentBraces: false
  SplitEmptyFunction: false
  SplitEmptyRecord: true
BreakBeforeBinaryOperators: NonAssignment
BreakBeforeTernaryOperators: true
BreakConstructorInitializers: AfterColon
BreakInheritanceList: BeforeColon
ColumnLimit: 100
CompactNamespaces: false
ContinuationIndentWidth: 4
IndentCaseLabels: true
IndentPPDirectives: None
IndentWidth: 4
KeepEmptyLinesAtTheStartOfBlocks: true
MaxEmptyLinesToKeep: 2
NamespaceIndentation: None
ObjCSpaceAfterProperty: false
ObjCSpaceBeforeProtocolList: true
PackConstructorInitializers: Never
PointerAlignment: Left
QualifierAlignment: Leave
ReflowComments: false
SpaceAfterCStyleCast: false
SpaceAfterLogicalNot: false
SpaceAfterTemplateKeyword: false
SpaceBeforeAssignmentOperators: true
SpaceBeforeCpp11BracedList: true
SpaceBeforeCtorInitializerColon: true
SpaceBeforeInheritanceColon: false
SpaceAfterLogicalNot: true
SpaceBeforeParens: ControlStatements
SpaceBeforeRangeBasedForLoopColon: true
SpaceInEmptyParentheses: false
SpacesBeforeTrailingComments: 0
SpacesInAngles: false
SpacesInCStyleCastParentheses: false
SpacesInContainerLiterals: false
SpacesInParentheses: false
SpacesInSquareBrackets: false
TabWidth: 2
UseTab: Never
InsertBraces: true
User avatar
mfro
Posts: 663
Joined: Sat Sep 23, 2017 8:15 am

Re: Improving clang-format

Post by mfro »

berniev wrote: Sat Oct 01, 2022 6:21 am Here's a version that implements some of discussions plus more.
I'd like to see

Code: Select all

NameSpaceIndentation: All
(it looks strange to have all braced regions indented except namespaces) and

Code: Select all

AlwaysBreakAfterReturnType: false
Cheers,
Markus
berniev
Posts: 247
Joined: Wed Apr 13, 2022 10:45 pm
Location: Oz

Re: Improving clang-format

Post by berniev »

mfro wrote: Sat Oct 01, 2022 7:33 am

Code: Select all

NameSpaceIndentation: All
Your suggestion, if I understand it correctly, would force all namespaced code (which is likely most code) to be indented.

I understand your desire for uniformity. Others will likely chime in, but it appears to me that namespace contents are never indented in C++ land. The extra indent does seem a waste. The language gives great flexibility defining namespaces, but brackets are the price.
Last edited by wmayer on Sat Oct 01, 2022 10:28 am, edited 1 time in total.
Reason: Fixed quote tag
User avatar
uwestoehr
Veteran
Posts: 4961
Joined: Sun Jan 27, 2019 3:21 am
Location: Germany
Contact:

Re: Improving clang-format

Post by uwestoehr »

Concerning the topic to automatically braces:
Werner pointed out our current style:
https://github.com/FreeCAD/FreeCAD/pull ... 1405018752

Changing this leads to extra work for us and on some IDEs like VC 2022 also to problems:
https://github.com/FreeCAD/FreeCAD/pull ... 1405073520

So all in all I don't see a benefit. In any way it will cost us extra time.

This general issue applies for all changes to the clang file. When there is no benefit of making the code better readable, we should avoid changes and use the existing style to keep the diffs low. The clang file is taken as standard formatter in some IDEs like Visual studio and you cannot simply turn them off: https://github.com/FreeCAD/FreeCAD/pull ... 1404525501
User avatar
chennes
Veteran
Posts: 3878
Joined: Fri Dec 23, 2016 3:38 pm
Location: Norman, OK, USA
Contact:

Re: Improving clang-format

Post by chennes »

Two points here:

1) If we standardize on a clang-format file, we can submit one PR that auto formats all files. After that, using auto formatting will actually decrease changes in any PR, because formatting changes will never happen without some corresponding code structure change.

2) Format-aso-you-type is more frustrating than it is worth, IMO. I prefer to write code without worrying about it, and then let the auto formatter do its thing afterwards. Then I never have to think about formatting at all. This is always my approach in Python -- I run Black as a pre-commit hook.
Chris Hennes
Pioneer Library System
GitHub profile, LinkedIn profile, chrishennes.com
Post Reply