Skip to content

Add clang format file #31

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

Closed
wants to merge 18 commits into from
Closed

Add clang format file #31

wants to merge 18 commits into from

Conversation

alexander-novo
Copy link
Collaborator

@alexander-novo alexander-novo commented Dec 2, 2024

I added options in order of:

  1. How they appear in the style guide

  2. How they appear in the current code

  3. If there are conflicts in the current code, then I picked the option I prefer

There currently isn't a formatting option for forcing a newline between includes and the rest of the file, however there is a pull request to add this feature, so it may exist in a future version.

Please see #32 for the current up to date formatting results of the file.

- Fixed an incorrect option name causing the formatting to not work
- Increased column limit to 120
- Moved access modifiers back
this aligns more cloesly with the current codebase, and seems like a good idea since most of the code will be in the `GridKit` namespace
- Short functions will never be on a single line
- Don't binpack arguments/parameters
- Align pointers and references to the left
@alexander-novo alexander-novo requested a review from pelesh December 2, 2024 22:36
@alexander-novo alexander-novo linked an issue Dec 2, 2024 that may be closed by this pull request
@alexander-novo
Copy link
Collaborator Author

I've tested that this works with the current version of clang-format shipped with Ubuntu Lunar - version 15.0.7. I don't know what versions we'd want to support, maybe others can comment what version of clang-format they have, and whether the file works for them.

There is a new option added in (I believe) the latest version - version 20.0.0, which prevents function declarations from being aligned. I don't know exactly how much this will impact the codebase?

- Comments effectively have an extra 80 character column limit from the rest of code (bringing it up to 200 characters)
- Comments which are not meant to be written/read in-editor (such as in-documentation Latex or Matlab code, meant to be rendered by other software or copied/pasted directly from/to other software) can start with ** to indicate that they should not be formatted in any way, and clang-format will skip those lines.
@alexander-novo
Copy link
Collaborator Author

I believe I've fixed all outstanding concerns with column limits in comments. There are two new changes:

  • Comments effectively have a column limit of +80 additional columns, bringing it to 200.
  • Comments which begin with the characters ** will be left unformatted by clang-format. This can be used for comments which aren't meant to be read/written in the code itself, such as can be seen in Distributedgenerator.cpp

@alexander-novo
Copy link
Collaborator Author

@pelesh I looked into the issue of putting system header includes before local includes. It turns out that clang-format identifies the "main header" of a source file (e.g. Capacitor.hpp for Capacitor.cpp) and separates this include into its own group before applying the regular expressions and grouping to every other include. The main header is put in priority 0, which is why it was showing up before every other include. This seems like a useful idea to me, but let me know which way you would like it. I could:

  1. Leave the "main header" at the top, followed by system headers, followed by libraries and local includes
  2. Put the system headers at the top, but leave the "main header" in its own group
  3. Put the system headers at the top, and put the "main header" in the same group as local includes. From my testing it looks like this seems to put the "main header" at the top of this group.

@pelesh
Copy link
Collaborator

pelesh commented Mar 4, 2025

Closing in favor of #68.

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 this pull request may close these issues.

Create a .clang-format file
2 participants