osara
osara copied to clipboard
jcsteh
Add a .clang-format file
I invested an hour or two writing down a .clang-format that more or less follows the code style that is now common in OSARA. @jcsteh What would you like? Should I add a reformatted version of one file as well? Then you and probably others can comment on changed lines you dislike. Before merge we can revert these changes, then after merge I can provide a pr that should be with the whole code base formatted, including a .git-blame-ignore-revs. We should merge that using rebase and merge or a regular merge commit, not squashing.
Fixes #923
Build succeeded! Build osara pr940-1178,7c33d940 completed (commit https://github.com/jcsteh/osara/commit/7c33d940dd by @leonardder) Downloads:
Build succeeded! Build osara pr940-1180,ac9dc06b completed (commit https://github.com/jcsteh/osara/commit/ac9dc06b9e by @leonardder) Downloads:
Build succeeded! Build osara pr940-1181,dba8a65f completed (commit https://github.com/jcsteh/osara/commit/dba8a65fdd by @leonardder) Downloads:
I addressed your comments and added one commit with the whole code base formatted, so you can have a look on what it will do.
Build succeeded! Build osara pr940-1182,c89b942e completed (commit https://github.com/jcsteh/osara/commit/c89b942ec6 by @leonardder) Downloads:
We're going to need to tweak makePot so that it can operate across lines. Right now, I think it expects translate, translate_plural, etc. to be on a single line. When we re-format, translate_plural in particular might get split across lines. Unfortunately, that's going to be non-trivial because makePot currently walks line by line.
One way to do that might be to create separate regexps to match just the beginnings of these functions; e.g. RE_CPP_TRANSLATE_PLURAL_BEGIN. If we match RE_CPP_TRANSLATE_PLURAL_BEGIN but not RE_CPP_TRANSLATE_PLURAL, we repeatedly add the next line until RE_CPP_TRANSLATE_PLURAL matches the whole string. Though... that may cause problems if we have multiple translate calls on a single line. Urgh.
May be a strange question, but why are we actually using a special script to create pot files rather than xgettext?
Build failed! Build osara pr940-1183,a69381b2 failed (commit https://github.com/jcsteh/osara/commit/a69381b2dd by @leonardder)
Not a strange question at all. The reason is that xgettext doesn't support some of the things we need such as .rc files and string lists (translateFirstString, etc.).
Actually with my last change, I think we are now pretty safe. WhitespaceSensitiveMacros will just leave alone the translate and translate_plural calls.
I just took a different tack and took the Microsoft style as a basis. This seems to be going a lot better.
O wait, Microsoft doesn't have a maximum column count it seems. Things go wild again if we limit the amount of columns on 80. I'm a bit surprised that clang format makes things so difficult for us.
Build succeeded! Build osara pr940-1184,95c113b0 completed (commit https://github.com/jcsteh/osara/commit/95c113b0e1 by @leonardder) Downloads:
Build succeeded! Build osara pr940-1185,32beb394 completed (commit https://github.com/jcsteh/osara/commit/32beb39469 by @leonardder) Downloads:
This severely breaks translate firstString comment blocks. I guess the only way to fix that is to disable clang-format for those sections, which seems pretty bad. Alternatively, i guess we could switch to a no-op macro purely for makePot which signifies that a string is translatable without actually trying to translate it. That might also allow us to switch to xgettext, though we'd still need custom code for the rc file.
Build failed! Build osara pr940-1193,f4c89b9a failed (commit https://github.com/jcsteh/osara/commit/f4c89b9aa7 by @leonardder)
Build failed! Build osara pr940-1198,aa5d0f97 failed (commit https://github.com/jcsteh/osara/commit/aa5d0f9752 by @leonardder)
Build failed! Build osara pr940-1199,8d680658 failed (commit https://github.com/jcsteh/osara/commit/8d680658fe by @leonardder)
I wonder if we can get around the include problem with tightly scoped clang-format ignores. It'd be nice if we didn't have to worry about include ordering except in very specific cases.
Build succeeded! Build osara pr940-1201,0cd70440 completed (commit https://github.com/jcsteh/osara/commit/0cd7044014 by @leonardder) Downloads:
Build succeeded! Build osara pr940-1202,bcd1d6fd completed (commit https://github.com/jcsteh/osara/commit/bcd1d6fd03 by @leonardder) Downloads:
Oh. clang-format has this to say about AlignAfterOpenBracket: BlockIndent:
This currently only applies to braced initializer lists (when Cpp11BracedListStyle is true) and parentheses.
I mean... I would have thought if conditions count as parentheses, but apparently not. But it does explain why it doesn't impact braces.
I wonder if we can get around the include problem with tightly scoped clang-format ignores. It'd be nice if we didn't have to worry about include ordering except in very specific cases.
I will look into that later today/this week.
Ii now chose the Microsoft style as a basis, which has a 120 character column limit. At least that saves us some very ugly code breaking.
Build succeeded! Build osara pr940-1208,b8ca0d3a completed (commit https://github.com/jcsteh/osara/commit/b8ca0d3a77 by @leonardder) Downloads:
Which breaks were bothering you with the Google style? If they're a real problem, can we just increase the max line length there?
We're now back at Google, and indeed, it formats some stuff slightly nicer than Microsoft.
Build succeeded! Build osara pr940-1209,bb85eb17 completed (commit https://github.com/jcsteh/osara/commit/bb85eb1752 by @leonardder) Downloads:
Just coming here because i'm noticing how long it takes to fix things like indentation all the time. I noticed that you seem to struggle quite alot with getting clang-format to work. Is there any specific reason why you absolutely want to go with this over any other, newer solution like a prettier or something similar? AFAIK clang-format works, but is pretty old and most likely outdated compare to more modern tools. We already have Python toolchain with scons, we could go for a Python-based implementation of a similar tool instead, maybe that would work better? Just a thought though.
