Skip to content
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

Open
AnyOldName3 opened this issue Oct 4, 2023 · 4 comments
Open

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

AnyOldName3 opened this issue Oct 4, 2023 · 4 comments

Comments

@AnyOldName3
Copy link
Contributor

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
Copy link
Collaborator

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
Copy link
Contributor Author

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.

@robertosfield
Copy link
Collaborator

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
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants