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

fix formatting of files in place #11

Merged
merged 1 commit into from
Jun 28, 2024

Conversation

oliverlee
Copy link
Owner

@oliverlee oliverlee commented Jun 28, 2024

This commit renames and redefines the clang_format_aspect to take
clang-format options and execution requirements. It also defines a fix
aspect which modifies files in place with clang-format, removing the
need for an update script and subsequent need to determine the bazel
process.

resolves #5

Change-Id: I90d6c95e921d6ab9565bfe5318635901d3de8ad1

@oliverlee oliverlee force-pushed the I90d6c95e921d6ab9565bfe5318635901d3de8ad1 branch 3 times, most recently from f36e347 to d6adaa9 Compare June 28, 2024 09:47
Copy link
Contributor

@miklelappo miklelappo left a comment

Choose a reason for hiding this comment

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

Works like a charm. I didn't review too much in details, but LGTM

This commit renames and redefines the `clang_format_aspect` to take
`clang-format` options and execution requirements. It also defines a fix
aspect which modifies files in place with `clang-format`, removing the
need for an update script and subsequent need to determine the `bazel`
process.

resolves #5

Change-Id: I90d6c95e921d6ab9565bfe5318635901d3de8ad1
@oliverlee oliverlee force-pushed the I90d6c95e921d6ab9565bfe5318635901d3de8ad1 branch from d6adaa9 to 2655fa3 Compare June 28, 2024 10:26
@oliverlee oliverlee merged commit 7e319a3 into main Jun 28, 2024
4 checks passed
command = """
set -euo pipefail

test -e .clang-format || ln -s -f {config} .clang-format
Copy link
Contributor

Choose a reason for hiding this comment

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

3rd party should really avoid doing any symlinks especially using -f flag. Clang-tidy supports clang-format -style=file:<path/to/config/file> ... notation since 14.0.0.

For example one can do:
clang-format -style=file:/home/username/projects/someproject/.clang-format -i main.cpp

Would you fix that it might worth to mention minimal clang-format version on README site

Copy link
Owner Author

Choose a reason for hiding this comment

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

I forgot that sandboxing is disabled with the new fix aspect. Sorry!

Copy link
Contributor

Choose a reason for hiding this comment

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

No problem :-) Should be very straightforward to fix that

@oliverlee oliverlee deleted the I90d6c95e921d6ab9565bfe5318635901d3de8ad1 branch June 28, 2024 21:43
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.

Run @bazel_clang_format//:update fails
2 participants