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 #143

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

add .clang-format #143

wants to merge 3 commits into from

Conversation

mr-martian
Copy link
Contributor

@mr-martian mr-martian commented May 26, 2022

In which I am mildly opinionated about C++ code formatting.

Not sure if this should go here and get copied to all the other repos or if there's somewhere else it should go.

Anyway, I'm sure someone will disagree with at least one of these settings, so let the bikeshedding begin. :)

Explanation of options: https://clang.llvm.org/docs/ClangFormatStyleOptions.html (I couldn't figure out how to get newer than 10.0 via apt, so I'm ignoring options from more recent versions.)

Version 10 manual: https://releases.llvm.org/10.0.0/tools/clang/docs/ClangFormatStyleOptions.html
Working interactive configurator: https://clang-format-configurator.site/

@mr-martian mr-martian requested a review from TinoDidriksen May 26, 2022 21:22
@mr-martian
Copy link
Contributor Author

This was previously suggested in apertium/apertium-lex-tools#84 and while there are still several open PRs, most of them are mine and have a fair amount of changes that need to be integrated anyway.

@TinoDidriksen TinoDidriksen force-pushed the master branch 3 times, most recently from 5bcdbfa to 3edf7c0 Compare July 14, 2022 20:55
@TinoDidriksen
Copy link
Member

Committed my opinion. In particular:

  • AlignConsecutiveAssignments: true is fundamentally a good idea, but clang-format makes so many mistakes. But with false then it mangles existing manual alignments. Not sure which is the best default - probably true but then other changes are needed.
  • clang-format mangles preprocessor directives regardless of what options or versions used. It has no way to simply leave them alone.
  • I very strongly want breaks before else and catch - Egyptian braces makes moving code around so much easier.
  • ColumnLimit: 0 because IDEs will show very long lines broken with proper indentation. There is no reason to bake in such breaks any longer. We're well past the days of teletype terminals.

I "solved" similar issues in CG-3 with https://github.com/GrammarSoft/cg3/blob/main/scripts/clang-format.pl

@flammie
Copy link
Member

flammie commented Jul 22, 2022

  • ColumnLimit: 0 because IDEs will show very long lines broken with proper indentation. There is no reason to bake in such breaks any longer. We're well past the days of teletype terminals.

I have no bikeshed hills to die on in code formatting issues, but I do have a strong opinion on this, long lines are really unreadable in 90 % of editors, ides (though suggestions for good such visualisations for vim and emacs are welcome) but also diff formats on console or web. Using more than 80 characters in a reasonable (non-java non-xml) language is almost always a sign of a really badly structured code.

@TinoDidriksen
Copy link
Member

Using more than 80 characters in a reasonable (non-java non-xml) language is almost always a sign of a really badly structured code.

My usual compromise is 120 characters. 80 is just too damn small, imo.

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.

3 participants