Improving clang-format
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
Be nice to others! Respect the FreeCAD code of conduct!
Improving clang-format
We now have a .clang-format file as part of the main FreeCAD codebase: https://github.com/FreeCAD/FreeCAD/blob ... ang-format
This means, among other things, that IDEs can (and do!) automatically format our code according to that file's specified format: which means we should probably make sure it actually formats code the way we want it to. To that end, I've applied it to Metadata.cpp and Metadata.h (basically a self-contained class used almost entirely by the Addon Manager, so unlikely to break anything or seriously bother anyone while we work on this project).
If you are someone with opinions about what C++ code "should" look like, please take a moment to have a look at these files, and suggest changes to the .clang-format file to improve things. As we refine the .clang-format file I can periodically apply it to that class so we can get things dialed in. For reference here is the Qt coding standard, which is the one we often recommend to people looking for a set of guidelines to follow for new code.
This means, among other things, that IDEs can (and do!) automatically format our code according to that file's specified format: which means we should probably make sure it actually formats code the way we want it to. To that end, I've applied it to Metadata.cpp and Metadata.h (basically a self-contained class used almost entirely by the Addon Manager, so unlikely to break anything or seriously bother anyone while we work on this project).
If you are someone with opinions about what C++ code "should" look like, please take a moment to have a look at these files, and suggest changes to the .clang-format file to improve things. As we refine the .clang-format file I can periodically apply it to that class so we can get things dialed in. For reference here is the Qt coding standard, which is the one we often recommend to people looking for a set of guidelines to follow for new code.
Re: Improving clang-format
'case' formatting is horrible : https://github.com/FreeCAD/FreeCAD/blob ... #L542-L547
Re: Improving clang-format
It depends on what you start with - clang-format appears to differentiate between single-line and multi-line case statements.openBrain wrote: ↑Wed Sep 28, 2022 6:53 am 'case' formatting is horrible : https://github.com/FreeCAD/FreeCAD/blob ... #L542-L547
If you start with the latter, it doesn't look too bad, IMO.
I personally don't have strong opinions on how formatting should look like as long as it is consistent.
What appears a little strange is that - after I configured the QtCreator code beautifier to use the settings file - it didn't accept the formatting of the example file but appeared to require a reformat?
Cheers,
Markus
Markus
Re: Improving clang-format
There is no need to have a "sample commit". Thats just BS and messes up master. It is not a proper capability test or format.
Anyone with an IDE (which is surely just about everyone) can see first hand what clang-tidy does as it is integrated into most all of them.
Now that clang-format file is in FreeCAD source, your IDE is likely picking it up automatically.
As you look at a piece of code, just do a reformat. In Jetbrains it is just (Mac) cmd-shift L.
Since I introduced clang-format to FreeCAD some months ago it is really pleasing to see its rapid adoption. But its is going to take a little time to nail down the finer details. There are loads of edge cases especially where it is desired to leave code alone (to a lessor or greater degree).
So report your thoughts and have the discussion. There are some areas that will be quite opinionated.
At some point down the track it will be possible to do a global reformat. After that anyone can change code and just hit reformat to get the accepted format on the whole section or file. Easy. I've been doing this for years and it is sweet.
Avoid having a PR rejected because you used four character indent amongst code that is two-character indented. Yes, this is real.
Anyone with an IDE (which is surely just about everyone) can see first hand what clang-tidy does as it is integrated into most all of them.
Now that clang-format file is in FreeCAD source, your IDE is likely picking it up automatically.
As you look at a piece of code, just do a reformat. In Jetbrains it is just (Mac) cmd-shift L.
Since I introduced clang-format to FreeCAD some months ago it is really pleasing to see its rapid adoption. But its is going to take a little time to nail down the finer details. There are loads of edge cases especially where it is desired to leave code alone (to a lessor or greater degree).
So report your thoughts and have the discussion. There are some areas that will be quite opinionated.
At some point down the track it will be possible to do a global reformat. After that anyone can change code and just hit reformat to get the accepted format on the whole section or file. Easy. I've been doing this for years and it is sweet.
Avoid having a PR rejected because you used four character indent amongst code that is two-character indented. Yes, this is real.
- wandererfan
- Veteran
- Posts: 6321
- Joined: Tue Nov 06, 2012 5:42 pm
- Contact:
Re: Improving clang-format
Yeah, that's an interesting one -- I was taught (and agree with) putting braces around everything, but the Qt coding guidelines have you omit them on the one-liners, so a TON of our code (including most of what I've written for this project, as far as I recall) omits them. I can't tell from your comment whether you are advocating for adding those requirements or removing them, though. Do you prefer the braces? (I mean, I'd guess so since I can't imagine how you'd be burned by their presence and not their absence...)wandererfan wrote: ↑Wed Sep 28, 2022 1:44 pm -checks="readability-braces-around-statements"
and
"InsertBraces"
Can not count the number of times this has burned me.
- wandererfan
- Veteran
- Posts: 6321
- Joined: Tue Nov 06, 2012 5:42 pm
- Contact:
Re: Improving clang-format
Sorry I was rushing out the door and didn't pay enough attention to what I had written.chennes wrote: ↑Wed Sep 28, 2022 5:31 pm [I was taught (and agree with) putting braces around everything, but the Qt coding guidelines have you omit them on the one-liners, so a TON of our code (including most of what I've written for this project, as far as I recall) omits them. I can't tell from your comment whether you are advocating for adding those requirements or removing them, though. Do you prefer the braces?
I would like braces around everything to be enforced, please.
- adrianinsaval
- Veteran
- Posts: 5551
- Joined: Thu Apr 05, 2018 5:15 pm
Re: Improving clang-format
I agree that having braces everywhere makes things clearer, this might be due to me being an amateur but many times I was very confused by missing braces in deeply nested conditionals.
Re: Improving clang-format
Anyone doubting all braces should search for Apple goto fail. Freaked a lot of people out.
Re: Improving clang-format
Just for the sake of completeness:
https://nakedsecurity.sophos.com/2014/0 ... ial-patch/
https://nakedsecurity.sophos.com/2014/0 ... ial-patch/