The distinctive feature of any mature project is its established coding style and automatic verification by CI. I see that FreeCAD still lacks any linter of coding style.
One such a linter is clang-format. Its available in all major platforms - VS C++ on Windows, XCode on OS X and as a standalone CLI tool on Linux.
There are 2 points to discuss though - which style is being used and how to integrate it into CI/CMake/IDE.
For the style, it seems to be LLVM with modifications. As for the CI integration, I can hack something out.
The most difficult part, of course, it getting everyone onboard with this.
So, who's against, who's onboard?
Coding style for C++
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
Be nice to others! Respect the FreeCAD code of conduct!
-
DeepSOIC
- Veteran
- Posts: 7896
- Joined: Fri Aug 29, 2014 12:45 am
- Location: used to be Saint-Petersburg, Russia
Re: Coding style for C++
Harmonizing the coding style will cause a lot of merge conflicts for big-effort branches like realthunder's. My ConstraintSolver will be affected noticeably, but not critically, as most of the code is in a new module. So i'm a bit against.
If it has to be done, I think sooner is better than later, the situation is likely to get worse over time, not better.
If it has to be done, I think sooner is better than later, the situation is likely to get worse over time, not better.
I am somewhat strongly against. To every rule, there will always be exceptions. From code generated by generators, to special situations where rules should be violated for good. Please don't make it an unbreakable law. It should help, not harm.
Re: Coding style for C++
Another reason to merge PR sooner rather than later. If something takes a big effort, you can always introduce a runtime feature flag and merge incrementally.DeepSOIC wrote: ↑Sun Aug 02, 2020 9:46 pm Harmonizing the coding style will cause a lot of merge conflicts for big-effort branches like realthunder's. My ConstraintSolver will be affected noticeably, but not critically, as most of the code is in a new module. So i'm a bit against.
If it has to be done, I think sooner is better than later, the situation is likely to get worse over time, not better.
Do you prefer to argue in PR comments what deserves to be a special case? Or would you rather make the tooling take care of it? I doubt you ever make a comment about the generated code. Why even check-in the generated code? Every tool has plenty of options to ignore some rules in some files.
-
DeepSOIC
- Veteran
- Posts: 7896
- Joined: Fri Aug 29, 2014 12:45 am
- Location: used to be Saint-Petersburg, Russia
Re: Coding style for C++
I don't understand you. You seem to imply that having no tool will cause heated discussions on what should be an exception or not... while without the tool, there are no discussions, because the violations are not exposed...
The tooling will not by itself decide ("take care") of what should or what should not be an exception for us - it's our very manual job of setting up the tooling... Or am i missing something?
Indeed. All I see is unnecessary complication. All was easy before. Now, suddenly, if i want to add an exception, I have to fight yet another complicated system. One invented just for making code look good, that happens to prevent my code from looking good.donovaly commented 5 days ago
Oh no, I opened Pandora's box.
Though, it's hard to estimate the situation until the system is put into action, and I bump into a worthy exclusion.
Re: Coding style for C++
I imply that without the tooling it's getting annoying. One thing when the tool says you did a mistake, another thing when the person asks you to fix something trivial which was not worth waiting for another round of review. Also, in the first case, you fight with a tool, in the second case you fight with a person. I see it as a sign of immature project.
-
mikeprice99
- Posts: 100
- Joined: Sat Jun 27, 2020 9:06 am
- Location: Dorset, England
Re: Coding style for C++
This is exactly the problem on code formating. In FEM if I have done heavy code formating I was the one who has helped a lot in rebasing the branches which where not merged. But once done IMHO the code looks much better and most important for me it is much simpler to read and understand. Most big PR do relate to realthunder. Best is to convince him to use the codeformating too.DeepSOIC wrote: ↑Sun Aug 02, 2020 9:46 pm Harmonizing the coding style will cause a lot of merge conflicts for big-effort branches like realthunder's. My ConstraintSolver will be affected noticeably, but not critically, as most of the code is in a new module. So i'm a bit against.
If it has to be done, I think sooner is better than later, the situation is likely to get worse over time, not better.
+1 to DeepSOIC. For python and FEM I have been used code formating checking for years. No we should not do automatic verification
Re: Coding style for C++
About the code formatting I wonder whether it's possible to leave some flexibility and not to have define every little detail. If you look at https://clang.llvm.org/docs/ClangFormat ... tions.html you will see how many things can be handled differently and for many of them I personally don't care what's used (e.g. all the Align* options).
For me the most important point is readability. Nothing is more tedious and tiring to review unreadable code because of poor formatting. This does not only considerably reduce maintainability but also increases the likelihood to overlook serious bugs or later introduce them unintentionally.
At the moment a lot of FreeCAD code looks very squashed where adding some newlines would already help a lot to more easily understand what it is supposed to do. Therefore I suggest e.g. for lengthy (and nested) if-else cascades to put the else keyword to the next line after a brace.
Or often there are if-else constructs where in one else-branch curly braces are used because the body consists of several lines but in the next branch they are omitted because it consists of a single line. In this case most coding styles recommend to use curly braces for all bodies if at least one body needs it.
Or if a branch of an if-else block has only one statement but an additional comment curly braces should be used, too.
For me the most important point is readability. Nothing is more tedious and tiring to review unreadable code because of poor formatting. This does not only considerably reduce maintainability but also increases the likelihood to overlook serious bugs or later introduce them unintentionally.
At the moment a lot of FreeCAD code looks very squashed where adding some newlines would already help a lot to more easily understand what it is supposed to do. Therefore I suggest e.g. for lengthy (and nested) if-else cascades to put the else keyword to the next line after a brace.
Or often there are if-else constructs where in one else-branch curly braces are used because the body consists of several lines but in the next branch they are omitted because it consists of a single line. In this case most coding styles recommend to use curly braces for all bodies if at least one body needs it.
Or if a branch of an if-else block has only one statement but an additional comment curly braces should be used, too.
We need at least some coding styles where the readability is harmed at the moment.Indeed. All I see is unnecessary complication. All was easy before. Now, suddenly, if i want to add an exception, I have to fight yet another complicated system. One invented just for making code look good, that happens to prevent my code from looking good.