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

cmake: Apply cmake-format #615

Merged
merged 2 commits into from
Feb 20, 2024
Merged

cmake: Apply cmake-format #615

merged 2 commits into from
Feb 20, 2024

Conversation

jdemel
Copy link
Contributor

@jdemel jdemel commented Jan 15, 2023

This commit adds .cmake-format.py. It is mostly inspired by the GNU Radio version of it.

Steps to reproduce these results:

  • pip3 install cmakelang
  • cmake-format --dump-config python > .cmake-format.py
  • Modify .cmake-format.py to be close to the GNU Radio version.
  • cmake-format -c .cmake-format.py -i $(git ls-files | grep CMakeLists.txt) $(git ls-files | grep .cmake)

Fixes #600
Fixes #423

@jdemel jdemel force-pushed the add-clang-formatter branch from 701039a to 1337011 Compare January 15, 2023 12:05
@jdemel jdemel force-pushed the add-clang-formatter branch from 1337011 to 11ab110 Compare October 22, 2023 14:02
Comment on lines 12 to 15
if(
MINGW
OR CYGWIN
OR WIN32)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is extremely ugly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I re-added the corresponding file from gnuradio and the result is almost the same but without the new line after if(. I'd assume this is intended.

Comment on lines 19 to 22
set(
CMAKE_C_FLAGS
${CMAKE_CXX_FLAGS}
CACHE STRING "" FORCE) #same flags for C sources
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, but this looks pretty different than GR's formatting. You're closing an issue that says "format like GNU Radio" with this!

@marcusmueller
Copy link
Member

If you want to be close to the GNU Radio version, so go and copy the GNU Radio version, I'd say.

@jdemel
Copy link
Contributor Author

jdemel commented Nov 4, 2023

If you want to be close to the GNU Radio version, so go and copy the GNU Radio version, I'd say.

I'm pretty sure, I did. I saw you @marcusmueller made some further adjustments to the GR CMake formatting. I should probably get an updated version of this.

We want to use this to auto-format our cmake files.

Signed-off-by: Johannes Demel <[email protected]>
Run:
```
cmake-format -c .cmake-format.py -i $(git ls-files | grep CMakeLists.txt) $(git ls-files cmake/ | grep .cmake)
```
This applies the cmake formatting as defined in all files.

In order to install `cmake-format`, you might want to run
`pip install cmakelang`
Or use another package manager of your choice.

Signed-off-by: Johannes Demel <[email protected]>
@jdemel jdemel force-pushed the add-clang-formatter branch from 11ab110 to e165c92 Compare February 10, 2024 18:53
@jdemel
Copy link
Contributor Author

jdemel commented Feb 10, 2024

I copied the .clang-format.py file from GNU Radio again. I expect that the result looks like what GR does now. I diff'd the files and the VOLK version of .clang-format.py is the same + some comments that drop out of the default. I'd argue this is nice for future reference.

Copy link
Member

@marcusmueller marcusmueller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good; thanks!

@jdemel jdemel merged commit d5326d4 into gnuradio:main Feb 20, 2024
34 checks passed
@jdemel jdemel deleted the add-clang-formatter branch February 20, 2024 21:34
Alesha72003 pushed a commit to Alesha72003/volk that referenced this pull request May 15, 2024
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.

Use CMake format like GR auto format cmake files
2 participants