[Development] White space / coding style patches welcome?
Axel Waggershauser
awagger at gmail.com
Wed Mar 13 01:38:31 CET 2013
On Tue, Mar 12, 2013 at 5:32 PM, Oswald Buddenhagen
<oswald.buddenhagen at digia.com> wrote:
> it went to the trash because the idea was not liked back then. the winds
> appear to have changed meanwhile.
That is a pitty. Given, that I already spent a bunch of hours looking
into this, I hope you are right (about the winds of change ;))
>> 1) 'simple' line-by-line checks running directly on the diff:
>>
> qt/qtrepotools/git-hooks/sanitize-commit
Very interesting. So what I had in mind is basically done already. I
found the following checks in sanitize-commit:
* Trailing whitespace
* Leading or mixed tabs
* CRLF line endings
* Flow control keywords must be followed by a single space
I'd say in terms of Lars's worries about having automatic checks in
place before attacking whitespace issues, there should not be any
objection against fixing at least those 4 issues immediately. One
might ask, though, if it would be a good idea to actually enforce the
checks instead of merely letting the Sanity Bot warn about them? Just
to handle the 3rdparty and autogenerated code? Or maybe to account for
the very last line of the coding style wiki: "Feel free to break a
rule if it makes your code look bad."?
I had a look at the statistics (ignoring both 3rdparty and auto
generated files): about 2k lines in qtbase suffer from trailing
whitespace and about 3k from leading tabs (of which more than 1k are
from two files in tests/auto alone). Interestingly, the overlap of
those two issues is about 10 lines only.
Given that the trailing ws issue is very easily fixed by a one-liner
as Corentin has used and that the overlap with the tab issue is so
small, I suggest to fix all those 2k lines of trailing whitespace
adding the couple tab-fixes manually, so the sanity bot is happy, and
commit that in one go.
Then in a second step deal with tabs and maybe other ws issues, that
require manual intervention in smaller chunks.
I'll also look into meaningful additional checks to be added to sanitize-commit.
>> 2) proper c++ parser based checks that require the patch to be applied
>> to the code to even run:
> to reliably parse c++, you need contextual information. that means that
> you need a complete build system. iow, you need to compile the whole
> thing, and have the style checking be done as a side effect.
>
>> (I hereby presume there is some fancy tool in the context of libclang
>> available...).
>>
> if not: have fun. ^^
There is project called clang-format, which seems to be going to be
part of clang 3.3: http://clang.llvm.org/docs/ClangFormat.html. There
is a design doc available with a nice 'competition' line-up at the
end, too: https://docs.google.com/document/d/1gpckL2U_6QuU9YW2L1ABsc4Fcogn5UngKk7fE5dDOoA/edit
But I don't intend to dig a lot deeper on that front, given that the
easy approach (sanitize-commit) turned out to be even easier than
anticipated.
- Axel
More information about the Development
mailing list