added proposal for .clang-format

[georgkrause]

This is a starting point about a discussion about how to style the code.

I tried to get as near as the current style, but there are some differences, eg Constructor Initialisation lists which will look like this:

GenericMIDI::GenericMIDI(std::string file)
    : Controller()
    , MidiIO() {

I dont think this is optimal, but clang-format seems to be a little restricted at this point.

Anyway, if you want to style a file, you will need this command (Although you should make your editor doing this): clang-format -i filename

To test, i simply formatted files with this settings and viewed the git diffs.

There might be some stuff to discuss, eg line length, indention. So please test what this configuration does and tell me if you are happy.

[georgkrause]

applying this clang-format standard to the current master: 99 files changed, 30498 insertions(+), 26673 deletions(-)

I guess we can improve this a lot.

Edit: without the planning directory, just formatting files in src, its not that bad:

96 files changed, 13173 insertions(+), 13476 deletions(-)

[georgkrause]

Using Tabs for Indentation, its much better again. Thanks to @coderkun for this hint ;)

96 files changed, 7372 insertions(+), 7674 deletions(-)

[coderkun]

@georgkrause, with this clang file I get even better results: https://gist.github.com/coderkun/b55024de9ae019115d2501f3e5cd99dd

96 files changed, 6391 insertions(+), 5962 deletions(-)

[georgkrause]

Okay, i looked into this and yes, we get closer to the current code base. but there are some settings i would like to change.

AlignConsecutiveAssignments: false
AlignConsecutiveDeclarations: false

This is from my point of view much more readable.

BinPackArguments: false
BinPackParameters: true

Using different settings here does not make any sense for me. Same applys for the Brace Wrapping, maybe we should set all to true or false?

SortIncludes:    false

Would be a nice to have, but changes a lot, i know.

BreakConstructorInitializers: AfterColon

I already mentioned this, obviously clang-format 6 wont do the current style so we need to decide which one we choose. I know, this one has the smaller diff, but i think the previous settings with one initializer per line is better.

But this is all i question of taste.

[coderkun]

I suggest to try to get consistent formatting with as less changes as possible with this PR. And file new PRs for specific improvements/settings—which probably introduce vast changes.

[georgkrause]

@coderkun Sounds good, if you can push these changes to my branch, @harryhaaren can check this out and merge if he wants.

[coderkun]

Done. @harryhaaren, what do you think about this formatting?

[harryhaaren]

Doing this now will invalidate any PRs - not worth it at the moment. Perhaps if in future we have a "clean" state when nobody has significant features / things in flight. I do like the idea of using clang-format on things, but if it hinders developers its not worth it.

[georgkrause]

At first: This does not change any code right now, its just a set of rules. So merging this wont invalid any PRs, we dont have to apply this to all files. But this wont make any sense. So i agree with you, lets merge it when we have a clean state.

But maybe you can have a look at the settings and discuss what is good and what we might have to change, so we could merge this when its time.

[vale981]

but if it hinders developers its not worth it.

In fact, it helps quite a big deal (at least people like me ^^, I do remember the whitespace mess quite painfully). Maybe you could include it after the feature freeze. The modifications of Clang format are quite predictable, so it could be applied on minor PRs and/or via CI.

[georgkrause]

Same opinion, but still: if current active PRs are not able to be automatically merged because of new formatting, its useless more work. lets at first merge all features, do testing, fix everything and in the end we can merge this one and format everything.

[georgkrause]

Apply: find src -regex '.*\.\(cxx\|hxx\|h\|c\)' -exec clang-format -style=file -i {} \;

Result: 95 files changed, 8948 insertions(+), 6403 deletions(-)

[georgkrause]

@harryhaaren I would like to change AlignConsecutiveAssignments: false to true to have equal signs aligned.

[harryhaaren]

As per discussion on call today, would you post some developer tutorial here or on the wiki as to how we can format git diffs only (as opposed to full files) with clang-format? Cheers

[georgkrause]

Visual Studio Code has the ability to only format things on typing, so it will automatically keep the code styled correctly you are editing/writing without touching other code.

Besides from this you can use a script called clang-format-diff.py which gets shipped with clang format. Its documented here: https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting Basically you pipe the output of git diff into the script and it will reformat in place.

[georgkrause]

@harryhaaren Please merge this. I want a fixed set of rules how to format new code to avoid increasing the mess. Thanks!

[harryhaaren]

Sure - done - you're right this has been outstanding for long. Lets merge now, and improve/change the .clang-format if needs be over time.