Style consistency using clang-format

[boazsegev]

The styles in the different files weren't as consistent as one might prefer. This fixes this issue.

Also, this makes it easier for collaborators to author PRs, by making use of an automated styling approach.

The clang-format settings I authored took what I could understand as the prevailing style (4 spaces indentation, when to brace, etc') and I applied the nearest match.

Aside for macro and include statements that I don't know how to make clang-format indent, this is a good-enough collaborative setting.

Please consider this as a PR that would make it easier to review future PRs (specifically the one I hope to author for #241).

Kindly.

[tarcieri]

The test failures look unrelated, but should probably be addressed.

[ioquatix]

Sorry, but I don't agree with this styling at all. It looks ripe for edit amplification, and I don't see how it makes contribution easier.

[boazsegev]

Hi @ioquatix ,

First thing, it's your show and the whole of the Ruby community really appreciate your work on this. We will do as you decide and be thankful.

Please consider that clang-format is as important to workflow in C in the same way rubocop is for Ruby.

It's not just about readability and keeping my indents the same length (when currently they are sometimes 4 spaces wide and sometimes 2 spaces wide), it's also about avoiding styling related bugs (i.e., for / while spin-loops without a block and an extra or missing ;).

In addition, many development environments (mine included) use clang-format whenever you hit "save" for a C file - same as I would set of my dev environment for Ruby (so collaborators need to disable this whenever working on your project).

This PR isn't about "which style to choose". I tried to match it to the style you wrote in.

This PR is about making it easier for C programmers to submit PRs by telling their IDE which style to use for this project.

Kindly, Bo.

[ioquatix]

I am happy to revisit this when I have time or if you can come up with a configuration which minimises changes but brings consistency. I don't want PRs that mix subjective stylistic changes with bug fixes. Thanks for your efforts.

[boazsegev]

Thank you @ioquatix for your consideration.

I don't want PRs that mix subjective stylistic changes with bug fixes

This PR has no code changes. I simply authored the .clang-format file and allowed clang-format to update the files automatically by opening them in SublimeText and re-saving them (unedited).

It looks ripe for edit amplification

I understand you might be concerned about code injection attacks, and I propose to submit a PR only with the .clang-format file and you can perform the automatic formatting directly, securing your knowledge that no addition were slipped in unobserved.

if you can come up with a configuration which minimises changes but brings consistency

This was the minimal change that I could manage.

The code has number of inconsistent styles. There is no way I can consolidate them without changing all the code.

Some functions have a new line before braces, some don't. Some indentation is 4 spaces, other indentation is 2 spaces. MACRO alignment is performed sometimes, but not always. Assignment alignment is performed sometimes, but not always...

... whatever style you want to adopt (I recommend LLVM with minimal changes if any), the code base will shift.

Good luck with everything 🍀 I have my own projects to attend to. Cheers.

[ioquatix]

Thanks for your comments.

When I said PRs that mix style changes and bug fixes I was referring to #242 not this one.

Regarding this PR, I have no problem with clang-format generally.

I have a problem with enforcing changes which ensure write amplification, which is when you have something like:

int first  = 10;
int second = 20;

When you add one more variable, e.g.

int first          = 10;
int second         = 20;
int something_else = 30;

Write amplification is when you modify one line, but 3 lines must be updated due to style rules.

This is objectively bad as it obscures the actual changes being made.

[boazsegev]

Hi @ioquatix ,

I understand your concern.

I think we can reconsider styling choices and then I'll updated the .clan-format file before we merge.

To minimize edit amplifications, these two lines need to be changed (from true to false):

AlignConsecutiveMacros: true
AlignConsecutiveAssignments: true

Although I love macro alignment, I think this might be the best way to avoid these issues.

Another concern might be the braces (BreakBeforeBraces). I would consider using Attach instead, so braces always start on the same line that resulted in a block being used.

I believe this could make it easier to see certain bugs. i.e.:

if (1)
{
}
// vs.
if (1) ;
{
}

Let me know what you prefer, and I will follow.

Kindly, B.

[ioquatix]

I’m fine with both your suggestions.

[boazsegev]

I'll open a new PR, as there's some updates in the main branch.

So you want a PR with only the .clang-fromat file? or should I re-save existing files?

B.

[boazsegev]

Replaced by #246