When editing Cinder code for bugfixes and such keeping the style
consistent with the rest of the project takes a good amount of brain
power that could be used for actually writing code.
To make this process a little easier, I started putting a
.clang-format file at the root of the repo. If you haven't used
ClangFormat, give it a try. Personally, it's almost completely
removed the time consuming aspect of my OCD need for code to look
visually consistent. Now I just press the key command and the code
magically formats itself as I drink my Piña Colada Slurpee™.
Anyhow.. Here's a Gist with the file I made for Cinder, and
some instructions. If you make any improvements, please fork the Gist
and leave a comment in the original.
It looks like you can select "File" from the Clang Format
menu. It should work the same as it does from the command line or in
Sublime Text, searching up the directory tree until it finds a
Cool! Do you know if ClangFormat can also do things like change between
names_like_this and namesLikeThis? I've been switching back and
forth a lot lately, and it would be super great the
Including a formatting file with Cinder sounds like a good idea to
me (golang has a formatter built in, which honestly makes me thank
them for not allowing me to worry about that, though I don't
really do anything in go).
AFAIK ClangFormat doesn't do any transformations on your actual
variable naming. It's a very localized thing, operating on single
files at a time, or lines of code within a file. You can have it sort
your includes, but I haven't enabled that.
There's also ClangTidy which is sort of a linter, but will also
transform your code if you pass the -fix flag. It even does some crazy
stuff like transforming raw loops into range-for when it knows
it's safe to do so.
I think it'd be great to include it with the library, if it could
completely match the cinder coding style. I know there are a couple
intricacies that we'd have to reconcile, but maybe they've added
support since. One in particular is that we indent class access
modifiers (private:, protected:, public:) with two spaces, like in this example
But if anything else, we could indicate that such a .clang-format
file would be a great place to start in reformatting some code before
issuing a pull request.
Cool. I changed the access modifier indent to 2 spaces. There was some
(I guess older) code that I was looking at where they were indented by
either 1 space or a whole tab. Looks like it's 2 in most of the
There are only 2 things in the style guide that I've found to
be unsupported by ClangFormat. There's no option AFAICT to insert
a space after the bang (!) operator. Nor can it indent the names of
multiple consecutive function declarations.
I tried adding the AlignConsecutiveDeclarations statement to my
clang-format file, but running the tool causes it to complain that the
key is unknown. Running version 3.7.1, which is the latest through homebrew.
Overall, this is a great tool, but it's a bit confusing not to
be able to use the formatting options listed on the clan site.
This fork includes a .clang-format config file based on
notlion's gist + Paul's suggested changes. ClangFormat was run
on all Cinder-owned files... the diff is pretty massive. (This is a
discrete commit so it's easy to roll back.)
It adds a python script to run formatting on the whole repo,
and bundles the ClangFormat 3.8 binary. (Mac only for now...)
Richard as you've said it would be great to come up with a
.clang-format config that would exactly match Cinder's existing
style rules — but I think the advantages of consistent and automated
formatting might outweigh finding a perfect match over the longer term.
Thanks notlion, yeah it's probably not worth the crazy diff.
I had missed some of the third-party code in the blocks
directory, which is why Box2d etc. were touched. These directories
are now ignored by ClangFormat in the fork.
Adding .clang-format to the repo's root might be
confusing if the files in the repo don't actually follow it?
Would we just end up with the same noisy diffs on a file-by-file
basis if contributors respected the repo's .clang-format when
editing files? Or would it be something that people would be
expected to only use on new files, necessitating very scrupulous
use of any ClangFormat IDE plugins? (Would it make more sense in a
subfolder so that it's not invoked accidentally by such plugins?)
I tried to structure the fork so that you can run clang-format
-style="file" on any file of the repo and get
"correct" behavior when clang-format searches up the
hierarchy for a config file — E.g. third-party files are untouched,
cinder-owned files are correctly formatted. This would also make it
trivial to run clang-format as part of a pre-commit hook or similar.
Anyway, wanted to put this out there to see if anyone felt
that an epochal diff might be worth it in exchange for eliminating
format-related noise (and cognitive overhead) going forward.
"Nope" would not be a surprise. : )
Problem remains that there are some things that are getting modified
that we wouldn't want, for example these tab aligned methods
or this space
between the '!' and variable. We could only run the
formatter on the entire repo's source if it got everything correct,
which I don't think will ever be possible with things like alignment
to improve readability (think lambdas).
But if a certain chunk of code is particularly ill formatted, I
could see running the script on that and issuing a PR for those
updates. It might need some hand tuning afterwards to fix things like
what I mentioned above.
Also is it required that the clang-format file live in the root of
the directory? Seems like it would be nice to keep that as clutter
free and put something like this in the tools directory.
I agree with Rich that Clang-Format in its current form does not allow
us to create the coding style we have more or less established over the
past few years. Using it for Cinder at this time does not seem to be the
right thing to do.
However, I've been using it in my own projects lately, where
the .clang-format sits in the root directory and I have a batch script
that applies the formatting to all source files automatically. Files
that I want to be excluded from formatting are stored in their own
directories with their own .clang-format file, which disables
formatting altogether. For me this works well, even though it still
sometimes leads to ugly formatting like this:
glsl = mBatch->getGlslProg();
scpGlsl( glsl );
, which can be solved by adding an empty line between the two statements:
auto &glsl = mBatch->getGlslProg();
scpGlsl( glsl );
I'd advise to not run the formatter on Cinder's code at
this time. Just use it for your own projects.
Unless you're willing to live with some sub-optimal readability here
and there, clang-format is really a 95% type of tool. Rather than
formatting entire files, I imagined people would use it on the specific
sections of code they modified, changing things as necessary to improve
Alignment with tab characters is an option, but it results in a
combination of tabs and spaces being used to get precise alignment..
Probably not something we want.
Anyhow, if we can't really reproduce the Cinder code style
accurately, and are unwilling to adjust said style including a
clang-format file is DOA. But like Paul I'll continue to use one
in my own projects, and for code contributions (with perhaps some hand editing).
Thanks Paul, the exclusion strategy you've described is very handy —
that's how I skippedd third-party code in the forked repo.
Richard, in a typical invocation, ClangFormat climbs directories
from the source file it's called on until it finds a .clang_format
file to use. So I think putting it in the root would only make
intuitive sense if we actually ran the entire code base through the
formatter. Since that's out, I think dropping it in /tools with
some explanatory documentation makes the most sense.
Also just for posterity, you can explicitly exclude code from
clang-formatting at the line level by wrapping it inside // clang-format off
and // clang-format
on comments. (Though obviously this is not a tractable fix for
the number of mistakes and exceptions we're currently seeing in
the formatted fork!)
Leave a comment on kitschpatrol's reply
Change topic type
Link this topic
Provide the permalink of a topic that is related to this topic