Open
Conversation
Reformatting this huge chunk of essentially raw data dwarfs and obscures all the other formatting changes clang-tidy will make and makes for a very messy commit introducing the initial formatting, so for now we'll tell clang-format to ignore most of the file. We can still re-format this file later if we want to, as its own single commit (and I personally believe we do want that, but that's for later).
For some reason that I have not found the reason for yet, clang-format changes cause this file to fail to compile.
This file started life as a copy of the .clang-format file from SFML
with additional clang-format defaults added by doing
clang-format --style=file --dump-config
The resulting file was then tweaked over multiple iterations to
minimize the diff generated when applying it.
The C++ standard to format as is set to c++14 since that's what the
library guarantees as the minimal usable standard.
One possibly controversial setting is the ColumnLimit. I have set it
to 130 based on that being the length of the line of slashes /////
used to separate functions and starting/ending comment blocks and the
fact that the the average line length in the repository being just
64.54 characters and the median being 50.00 characters, so anything
over 130 seems like it is unintended and should be broken up.
The intention here is to start off with a .clang-format rules file
that matches the existing style as closely as possible. That's not to
say that we don't want to tweak this further in the future (I
absolutely believe we do want to do that) but we want to start off
with minimal changes and just get to a point where we can start using
clang format in IDE's and editors (and maybe even (later) git
pre-commit hooks or CI checks to enforce style) to enforce a
consistent style, with minimal friction to adoption.
Style changes beyond this starting point can be added gradually one by
one.
I was unable to construct rules for reordering header files according
to the prefered style, so in the end I opted to not try. This initial
rules file is intended to leave all header includes alone, but I have
left my best attempt at include sorting/grouping in the .clang-format
file in a commented out version as a starting point for future
contributors.
This applies the current .clang-format file to the entire source tree.
There are compromises here, it's impossible to create a rules file
that applies globally that doesn't involve some compromises, but this
is my best effort at matching the existing style and applying sensible
rules while not completely neutering clang-format.
This was done by
find . -type f \( -name '*.cpp' -o -name '*.hpp' \) ! -path './build/*' ! -path './include/TGUI/extlibs/*' -print0 | xargs -0 clang-format --style=file -i
and then dropping the changes to tests/catch.hpp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi Texus
Here's my current best bet at introducing clang-format to the TGUI code base.
Please read the commit comments as I've tried to explain there why I've done what I did (and some details on compromises etc).
I tried my hardest to match the current style as best I could. But since the style is inconsistent and not enforced currently there was no way I could create a rules file with zero changes.
But, what I ended up with is pretty good IMHO. I managed to not make huge changes to all files and for the files that are changed I think the changes mostly make sense. There are of course going to be some changes in some files that locally may not seem the best, but they are then at least globally consistent. There's always going to have to be compromises when you do something like this.
If there are style changes in this PR that you just absolutely cannot accept or just hate for various reasons, then let me know - perhaps I can tweak the rules to match what you want or just tell clang-format to leave those lines alone. I'll do my best to match what you want.
In the future you may see PR's from me suggesting style changes - we can discuss those one at a time when they arrive. The important thing right now is to establish a baseline that can be enforced going forward.
Any comments and feedback welcome.
I don't expect you to merge this PR as-is. Its purpose is to be the start of a discussion (but if you love it as-is, feel free to merge of course).
Kind regards,
Jesper Juhl