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

formatting: run clang-format #3118

Closed
wants to merge 3 commits into from

Conversation

rsekman
Copy link
Contributor

@rsekman rsekman commented Sep 1, 2024

Many files in src/ and Tests/ were not formatted consistently according to the .clang-format in the repo. Perhaps this is because the .clang-format contained a duplicate entry that would cause clang-format to crash. This pull requests simply run

clang-format -i src/*.h src/*.c Tests/*.c{pp,} Tests/*.h

The files src/u8_lc_map.h src/u8_lc_map.c sj_to_unicode.c were excluded as the resulting diffs were extremely large.

I also tested running clang-format on all source files in plugins/ but the resulting diff is close to +500k/-500k.

@Oleksiy-Yakovenko
Copy link
Member

Oleksiy-Yakovenko commented Sep 3, 2024

It was not applied to the entire codebase intentionally, since it causes some unwanted changes.
I was trying to apply formatting over time to individual files, and improve the configuration file, but after some point I couldn't solve some problems and gave up.
For example, look how some tables are reformatted in junklib.c -- this causes a problem.
Unfortunately, I can't accept this PR -- if I could, I would have already reformatted the whole codebase myself.

@rsekman
Copy link
Contributor Author

rsekman commented Sep 4, 2024

I understand.

Just FYI -- it is possible to exclude particular files with a .clang-format-ignore file and even parts of files from being reformatted by

// clang-format off: this code will not be reformatted
...
// clang-format on: reformatting resumes here

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.

2 participants