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

Add .clang_format #1542

Merged
merged 1 commit into from Jun 25, 2019
Merged

Add .clang_format #1542

merged 1 commit into from Jun 25, 2019

Conversation

Wunkolo
Copy link
Contributor

@Wunkolo Wunkolo commented Mar 19, 2019

This is to automate the styling of C code to aid contributors in making pull requests that conform to the existing style and so any future C style decisions can be made to a single file and automatically applied to the whole project.
I configured the .clang_format to suite most of the code found through-out the already-existing C code and based on the code review of my previous PRs.
More options can be found here
With this any edits to the c code can be automatically formatted by calling clang-format or find . -regex '.*\.\(c\|h\)' -exec clang-format -style=file -i {} \;

Copy link
Member

@CounterPillow CounterPillow left a comment

Choose a reason for hiding this comment

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

Overall these changes look like a great step forward in terms of Overviewer's overall code quality by making it easier to keep the code readable no matter the number of hands that touch it. I've left two comments requesting some clarifications on some things that stood out to me.

Feel free to tune in as well, @agrif.

overviewer_core/src/composite.c Show resolved Hide resolved
Priority: 1
IndentCaseLabels: false
MaxEmptyLinesToKeep: '1'
PointerAlignment: Left
Copy link
Member

Choose a reason for hiding this comment

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

A lot of code in Overviewer appears to use right-aligned pointers. My C knowledge isn't the best, but I think

int *foo, bar;

makes foo an int pointer and bar an int, whereas

int* foo, bar;

makes both of them int pointers, correct?

In that case, wouldn't right-aligned pointers be the lesser trap?

Feel free to change my opinion on this, I'm mostly just noticing a lot of foo *bar to foo* bar transformations in this PR which have me a little worried over whether this was 100% intentional.

Copy link
Member

Choose a reason for hiding this comment

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

Nope, both of those declare foo as a pointer and bar as an int. Yeah, it's pretty bad.

I used to use right-pointers because morally, I think pointerness should be part of the type. But C's syntax is such that right-pointers will get you in to trouble, like the confusion above, so nowadays I try to stick to left-pointers. I think that's a good choice for overviewer.

Copy link
Member

Choose a reason for hiding this comment

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

So: I think I swapped left and right here... 😄

I meant, the int *foo style is probably more appropriate. I don't know enough about .clang-format to know if that's what's selected here.

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'm gonna be a vote for left-pointers as defining a mix of ints and int-pointers on the same line is kind of a weird basis for having it all be on the right through-out the codebase. Would rather keep all type and type modifiers next to the type, with the name on its own(static volatile void* pointy).

@CounterPillow
Copy link
Member

Instead of backmerging master, can you rebase on top of it?

Also applies clang-format to the current code base, using command:
`find . -regex '.*\.\(c\|h\)' -exec clang-format -style=file -i {} \;`
@Wunkolo
Copy link
Contributor Author

Wunkolo commented Jun 24, 2019

Cleaned the commit history up so it's just one Add .clang_format commit, with the current .clang_format revision applied.

@CounterPillow CounterPillow merged commit 8162f3f into overviewer:master Jun 25, 2019
CounterPillow added a commit that referenced this pull request Jun 25, 2019
@Wunkolo Wunkolo deleted the clang-format branch June 25, 2019 16:56
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.

None yet

3 participants