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

Formatted headers and added style check. #212

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

Kerilk
Copy link
Contributor

@Kerilk Kerilk commented Oct 5, 2022

This is a straw-man PR that formats and enable stye checking for the headers.

The reasoning behind this PR is thus:

  • Generating the headers from the xml specification is something the working group has been discussing for a long time;
  • Validating a newly generated set of headers would be difficult if sources cannot be easily compared;
  • Using an agreed upon style for the headers (eventually fixing inconsistencies) and enforcing it, would allow using the formatting tool on the generated headers;
  • This should allow easy validation of any change.

For now this PR uses clang-format using a custom style derived from Mozilla, which was the closest to ours. I assume it can still be refined. To apply the style to the headers run clang-format --style=file -i `git ls-files *.c *.h` from the headers directory.

@Kerilk Kerilk marked this pull request as draft October 5, 2022 17:22
@bashbaug
Copy link
Contributor

bashbaug commented Oct 5, 2022

Regarding the specific style we use, in my opinion there should not be a strong requirement to match the formatting used by the current headers, especially because the the current formatting isn't always consistent. As long as the style looks nice (yes, I know this is subjective) and is applied consistently it will be a readability win.

I do have a slight preference for styles that can be generated easily, although if we run clang-format as part of a generation script this is less important, also.

@Kerilk
Copy link
Contributor Author

Kerilk commented Oct 5, 2022

Regarding the specific style we use, in my opinion there should not be a strong requirement to match the formatting used by the current headers, especially because the the current formatting isn't always consistent. As long as the style looks nice (yes, I know this is subjective) and is applied consistently it will be a readability win.

I do have a slight preference for styles that can be generated easily, although if we run clang-format as part of a generation script this is less important, also.

Indeed, I started moving away from the style more and more. I would be very open to suggestions on the style to adopt. I am not opposed to radically changing the style, and adopt something used by other related communities.

Side note: based on my current experiments, there are a slight caveats with clang-format: it does not handle function pointers and function pointer typedefs extremely well.

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