vsg-dev / vsgExamples

Example programs that test and illustrate how to use the VSG and optional add-on libraries
MIT License
148 stars 67 forks source link

Checked-in code inconsistent with .clang-format #268

Open AnyOldName3 opened 1 year ago

AnyOldName3 commented 1 year ago

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 the BasedOnStyle, which is left unset, and defaults to LLVM. The LLVM style sets EmptyLineAfterAccessModifier to Never, 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.

robertosfield commented 1 year ago

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:

clang-format --version
Ubuntu clang-format version 14.0.6-1

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.

AnyOldName3 commented 1 year ago

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 https://github.com/vsg-dev/VulkanSceneGraph/pull/989, but I hadn't found that problem yet when I reported this.

robertosfield commented 1 year ago

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?

AnyOldName3 commented 1 year ago

As discussed in the linked MR, kind of a mixture of both.