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

Enforce coding style with clang-format #22

Open
0x00002a opened this issue Jun 25, 2021 · 9 comments
Open

Enforce coding style with clang-format #22

0x00002a opened this issue Jun 25, 2021 · 9 comments
Labels
enhancement New feature or request

Comments

@0x00002a
Copy link
Contributor

I noticed a coding style document has been added. I use clang-format personally which I disabled when working on this project. Only (some of) my changes are clang-formatted (and therefore do wrap at 80 :p). I suggest having a .clang-format file that enforces the style checked in. I've heard you can setup github actions to also run it over pull requests automagically but I'm not sure how (I'll look into it).

If we do add a clang-format file I'd also like to request that we run it over the whole codebase once and check that in, that way I can turn format on save back on without messing up the entire file :p.

@Tectu
Copy link
Owner

Tectu commented Jun 25, 2021

I'm all pro adding a .clang-format file and adding it to the CI/CD pipeline.

This is also already on the project's ToDo list. I should probably add that ToDo list to this github project (eg. issues) given that I am no longer the only one using this library :)

@Tectu Tectu closed this as completed Jun 25, 2021
@Tectu Tectu reopened this Jun 25, 2021
@Tectu
Copy link
Owner

Tectu commented Jun 25, 2021

I will create a corresponding .clang-format soon.

Tectu added a commit that referenced this issue Jun 26, 2021
@Tectu
Copy link
Owner

Tectu commented Jun 26, 2021

I've added a preliminary .clang-format to the repository. I might still tweak it tho.
Seems like we need something like this: https://github.com/marketplace/actions/clang-format-check

@0x00002a
Copy link
Contributor Author

I found that but it only verifies the code, it doesn't run clang-format which was what I was thinking of. I think I can just have a script which runs on ubuntu-latest and uses run-clang-format.py to format it. I'm just not sure how to get that back to the repo

Tectu added a commit that referenced this issue Jun 29, 2021
@Tectu Tectu added the enhancement New feature or request label Jun 30, 2021
@0x00002a
Copy link
Contributor Author

0x00002a commented Jul 6, 2021

Is the .clang-format file ready for use @Tectu? If so I'll run it over the codebase and PR it

@Tectu
Copy link
Owner

Tectu commented Jul 6, 2021

Nope, I'm currently unhappy with it.

Notably I haven't yet found the settings for putting attributes such as [[nodiscard]] on a separate line.
Also minor things need tweaking. For example, if-else statement bodies should not use curly braces when there's just one statement enclosed.
I have to figure out what the proper way is to identify those settings for .clang-format.

@0x00002a
Copy link
Contributor Author

0x00002a commented Jul 6, 2021

Not sure all of those can be done with clang-format. clang-tidy however has a lot more options. Can even do stuff like the casing of classes (CamelCase for example) or adding a prefix to member fields (e.g. m_). The code has to compile with clang though, which I'm working on

@Tectu
Copy link
Owner

Tectu commented Jul 6, 2021

I'm currently trying to find a way for clang-format to not just provide a generic error message but instead be verbose about which rule is being violated as an attempt to fix the .clang-format file. However, no luck so far - any input/experience/advice?

@0x00002a
Copy link
Contributor Author

0x00002a commented Jul 6, 2021

I don't know of a way to make clang-format give that detailed reporting. I usually use https://zed0.co.uk/clang-format-configurator/ for new setups and/or major changes. It gives a live preview on how the code is formatted and you can upload an existing config too

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

No branches or pull requests

2 participants