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

Improve clang-format to check newlines at end of file #2127

Open
1 task done
NeroBurner opened this issue Sep 21, 2024 · 4 comments · May be fixed by #2139
Open
1 task done

Improve clang-format to check newlines at end of file #2127

NeroBurner opened this issue Sep 21, 2024 · 4 comments · May be fixed by #2139

Comments

@NeroBurner
Copy link
Contributor

Verification

  • I searched for similar issues (including closed issues) and found none was relevant.

Introduce the issue

I regularly criticize missing newline at end of file resulting in stop symbol of GitHub
stop symbol of GitHub at end of files

@mark9064 suggested to let clang-format do this check #2107 (comment)

@FintasticMan adds info on which clang-format version we could use #2107 (comment)

I've just checked, and the option to insert trailing newlines has been added in clang-format 16. Last time I updated it was 14, so it wasn't included in that. I'll do a upgrade to a newer version soon.

What version do you think is reasonable to update to? Unfortunately, I don't think that clang-format ignores unrecognised options, so it'll be a minimum version for anyone using it.

Preferred solution

clang-format checks the missing newline at end of file in CI

Version

main

@FintasticMan
Copy link
Member

The latest version of clang-format is 19, the latest version in the Ubuntu 24.04 repositories (which we use for CI) is 18, and the latest version in the Debian stable repos is 16 (just for a more conservative comparison). I'd be quite happy to go straight for 18, but I really don't know which versions other developers are using.

If anyone is using a version of clang-format below 18, could you let me know here, along with which OS and version? Thanks!

@NeroBurner
Copy link
Contributor Author

Ubuntu 22.04 has clang-format-14

https://packages.ubuntu.com/jammy/clang-format

Clang-format-16 is available since Ubuntu 23.10
https://packages.ubuntu.com/search?keywords=clang-format-16

Just a data point, although I'm on arch so I have newer packages available

@mark9064
Copy link
Contributor

mark9064 commented Oct 5, 2024

In other words, the option we want is supported by Debian stable, Ubuntu LTS and anything newer right? So in that case I think we're safe to enable it

Do we even specify a minimum clang format version anywhere (in code rather than docs)? If so I think it shouldn't be bumped past Debian stable (16) unless we want some newer feature

@FintasticMan
Copy link
Member

We do specify a minimum version in the clang-format commit hook, so I'll just bump to 16 for now.

@FintasticMan FintasticMan linked a pull request Oct 9, 2024 that will close this issue
@FintasticMan FintasticMan linked a pull request Oct 27, 2024 that will close this issue
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

Successfully merging a pull request may close this issue.

3 participants