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

Introduce tool for consistent formatting #905

Open
LorenzE opened this issue Dec 6, 2022 · 7 comments
Open

Introduce tool for consistent formatting #905

LorenzE opened this issue Dec 6, 2022 · 7 comments

Comments

@LorenzE
Copy link
Member

LorenzE commented Dec 6, 2022

I think it would be worth taking a look at a tool guaranteeing consistent formatting such as: https://clang.llvm.org/docs/ClangFormat.html. You can integrate it into QtCreator and VSStudio, see https://doc.qt.io/qtcreator/creator-beautifier.html.

@juangpc
Copy link
Collaborator

juangpc commented Dec 6, 2022

I agree.
Now the rules... my man. We need to agree on which rules make the code beautiful.

😄 just joking. I'm sure we'll agree pretty easily. 😮

@LorenzE
Copy link
Member Author

LorenzE commented Dec 6, 2022

I do not really mind about the rules, just that it is consistent and therefore easier to read :)

@LorenzE
Copy link
Member Author

LorenzE commented Dec 6, 2022

We could even think about forcing a format run every time we push to main? https://ivanludvig.github.io/blog/2020/07/05/clang-fromat-github-action.html

@juangpc
Copy link
Collaborator

juangpc commented Dec 6, 2022

I like that.
But instead of relying on the github actions. Could you find out the actual command it runs so that we can run it locally the same way it would run in github's runners?

@LorenzE
Copy link
Member Author

LorenzE commented Dec 9, 2022

@juangpc I gave clang-format a try. It was straight forward to use and install. Once installed you can do the following from your cmd line:

clang-format -style=LLVM -i connectivity.h // This will format the file in place
clang-format -style=LLVM connectivity.h > connectivity_formatted.h // This will create a new formatted file
clang-format -style=LLVM connectivity.h // This will output the formatted file to the cmd line

Integrating it to QtCreator and activate "on save formatting" was easy as well, see this guide https://doc.qt.io/qtcreator/creator-beautifier.html. Once the CMake integration is done, we will of course be able to use other IDEs as well. Most of them support clang-format.

I used the built in style LLVM, which I think would serve as a good basis for our project. Individualised styles are of course supported and can be put into a clang-format file, which could be added to the git repository.

@juangpc @gabrielbmotta Maybe you can give it a try on your side to get a feeling for some good style configs? What do you think?

@juangpc
Copy link
Collaborator

juangpc commented Dec 9, 2022

the way to go. 👍🏻

@LorenzE
Copy link
Member Author

LorenzE commented Dec 13, 2022

Formatting recursively can be achieved via (on Mac/Linux):

find . -iname '*.h' -o -iname '*.cpp' | xargs clang-format -style=LLVM -i

I started some clean up work here https://github.com/LorenzE/mne-cpp/tree/feature/formatting. Still WIP but please have a look and tell me what you think. I also started to remove some preamble stuff. Still have to make it build again and revert the changes to the eigen library.

Once there is a consensus on which formatting to use I can do a final pass over the cleaned up files.

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