-
Notifications
You must be signed in to change notification settings - Fork 67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Checked-in code inconsistent with .clang-format #268
Comments
I think it's bad to apply the clang-format automatically, it should be done manually and any changes reviewed for how appropriate they are. Build systems that change source code automatically are dangerous and would strongly recommend against doing so. FYI, this is clang-format version I'm using:
My intention with the built in make clang-format targets in the VSG projects if for them to be run occasionally, mostly something I will do to keep the codebase tidy. If later clang-format versions move the goal posts on how it interprets existing .clang-format files then we'll either have to be aware of this, as to what versions might cause problems we'll just need to test them and flag up the problem. I don't know how formal we need to go with such build utilities. |
The way the project's set up at the moment, you've already got it applying automatically when a Visual Studio CMake generator is used. The root cause is the same as vsg-dev/VulkanSceneGraph#989, but I hadn't found that problem yet when I reported this. |
I wasn't aware that Visual Studio was automatically running targets that users hadn't explicitly run. Is this a CMake error or just the way VisualStudio attempts to do things? |
As discussed in the linked MR, kind of a mixture of both. |
The source code as it exists in the repo has formatting inconsistent with what
.clang-format
specifies. This means that when the build system automatically applies it, it makes loads of changes to formatting. I suspect this is in part due to different versions of the tool behaving differently when options aren't explicitly set. For context, I have 15.0.1.One example is
EmptyLineAfterAccessModifier
introduced in version 13. It's unspecified in.clang-format
, so falls back to what's specified in theBasedOnStyle
, which is left unset, and defaults toLLVM
. TheLLVM
style setsEmptyLineAfterAccessModifier
toNever
, so all empty lines after access modifiers get removed from the many files that have them.I've just had a bit of a look at how you're supposed to make a version independent
.clang-format
file, and it seems that in general, it can't be done - if you don't specify every setting, new versions change the defaults, so your effective settings change, and if you do specify everything, older versions choke when they hit new settings. I guess that means the only real solution to this is specifying in the readme which version of the tool needs to be used.The text was updated successfully, but these errors were encountered: