Change .clang-format to apply linebreaks before open-braces

[jonesmz]

This pull request changes the .clang-format file so that a newline is inserted before opening braces in the code.

This changes, for example

if (bool) {
} else {
}

to

if (bool)
{
}
else
{
}

This has multiple benefits.

1. To find the matching brace for a particular line, you merely need to find the next brace that is indented to the same indentation level.
2. Parameter lists that are currently broken into two lines can sometimes be kept to a single line.
3. Provides a significant visual indicator about a scope beginning or ending, which for people with less than steller eyesight is a significant readability enhancement
4. Provides consistency with scopes that are not attached to a conditional, which is used in several places related to goto labels.

[jonesmz]

This setting is to control whether the brace itself follows indentation, which i don't think anyone wants.

[jonesmz]

goto labels are important to be able to quickly identify. Having them left-aligned in the text helps provide that visual indication.

[jonesmz]

Many headers are wrapped with extern "C" blocks, don't indent the code found inside those.

[jonesmz]

An example of parameters previously on two lines being able to be on one line. Helps with readability of the function.

[eakraly]

I'm fine with any format as long as it is automated :)
Would love to hear an opinion from @ggarber

[ggarber]

Hi @jonesmz, thx for looking at this.

I personally like more "your" style than the existing coturn style but most of these things are arguable and a matter of personal taste. That's why we decided last year to use a "standard" clang-format (the llvm) that maintains the existing style used in coturn for years.

My suggestion would be to keep it and not use a new custom clang-format. Otherwise tomorrow other person can come with similar reasons to change it back.

Also we have 28 Pull Requests open right now so any change will force all those authors to reformat their PRs :(

[jonesmz]

You don't have a standard clang format. You have a very custom one.

A standard clang format would be a single line saying BasedOnStyle: LLVM

[jonesmz]

Changing .clang-format to

---
Language:     Cpp
BasedOnStyle: LLVM
...

and applying this patch

diff --git a/Makefile.in b/Makefile.in
index 279c3a6..8bf3dc5 100755
--- a/Makefile.in
+++ b/Makefile.in
@@ -46,10 +46,10 @@ check:      bin/turnutils_rfc5769check
        bin/turnutils_rfc5769check

 format:
-       find . -iname "*.c" -o -iname "*.h" | xargs clang-format -i
+       find . -iname "*.c" -o -iname "*.h" | xargs clang-format -i --style=LLVM

 lint:
-       find . -iname "*.c" -o -iname "*.h" | xargs clang-format --dry-run -Werror
+       find . -iname "*.c" -o -iname "*.h" | xargs clang-format --style=LLVM --dry-run -Werror

 include/turn/ns_turn_defs.h:   src/ns_turn_defs.h
        ${RMCMD} include

gives a diff of 31186 lines of changes.

[jonesmz]

I personally like more "your" style than the existing coturn style but most of these things are arguable and a matter of personal taste.

Ultimately what matters is that the code is readable. While I agree that it's a matter of personal taste, I've yet to find someone who thinks braces-on-same-line is easier to read. Both of the only two maintainers (as far as I can tell???) think this is an improvement to readability.

I also don't represent only myself, my team at work also prefers the braces-on-next-line style, and we're trying to get serious about driving improvements to coturn. We had a deadlock in our turnserver service in December 2023 that got CTO level attention and I don't want that happening again.

The easier it is for my team members to work with the coturn codebase, the more contributions we can submit. While ultimately we're always going to be bottlenecked by maintainer's ability to review and merge changes, it's a smoother experience with a formatting style that doesn't make my eyes bleed.

My suggestion would be to keep it and not use a new custom clang-format. Otherwise tomorrow other person can come with similar reasons to change it back.

I doubt they'll be successful, since you seem to like braces-on-next-line more. But an automated formatting system enables a change of that nature to be quick and easy to try out and make decisions on. The goal is readability and productivity.

Also we have 28 Pull Requests open right now so any change will force all those authors to reformat their PRs :(

This can be done by the maintainer who submits the PR, github has the ability to notate when a PR was modified by the author, or it can be done as a separate fixup commit.

Alternatively, a github action that triggers on every submit to master could be used to automatically apply formatting as an automated followup commit. This would alleviate the need for anyone to manually run clang-format or make format

Regardless, make format will address that for the PR author very quickly anyway.

And I doubt many of the currently open PRs can be merged without rebase today regardless, since my changes to address the github CodeQL code scanner results (So far we've closed 31 results, so that's an improvement for sure!) don't stay focused to a single area of the code.

---

I'd also like to point out that I opened this PR on Feb 1st, and didn't really get any feedback until last week.

Of the 29 currently open PRs, 3 of them are dated 2022, 10 of them are dated from 2023. and of the remaining (29-13)=16 PRs that were opened in 2024, 10 of these 16 from 2024 are from myself or my team ( @redraincatching and @WHYHD ). My team has no problem re-running clang-format to comply with the project style, but we do need to have an understanding of what level of feedback to expect so that we can match your pace.

I currently have a very large amount of time in the late night / early morning where I can't go anywhere because I'm caring for my newborn, so between bottles and diapers I have nothing else to really do and I'm bored. As a result, I've opened 17 PRs that were merged by @eakraly in the last two weeks. is that too many too quickly?