search

Tectu / malloy

A cross-platform C++20 library providing embeddable server & client components for HTTP and WebSocket.
BSD 3-Clause "New" or "Revised" License
66 stars 8 forks source link

Enforce coding style with clang-format #22

Open 0x00002a opened 3 years ago

0x00002a commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

I will create a corresponding .clang-format soon.

Tectu commented 3 years ago

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 commented 3 years ago

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

0x00002a commented 3 years ago

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

Tectu commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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