On sexta-feira, 15 de março de 2013 04.16.46, Axel Waggershauser wrote: > With the trailing whitespace issue pretty much fixed, the next step is > eliminating leading tabs. A few questions about that: > > 1) If in doubt, is it generally fair to assume that whatever > QtCreator's CTRL+i in the default built-in Qt coding style does, is > the way to go?
Right. Emacs in indent-tabs=nil mode is also quite acceptable.
> 2) Especially continuation lines are not really talked about in the
> style guide wiki but are obviously a main and reoccurring issue when
> replacing leading tabs. e.g. the QtCreator style suggest the following
> (replaced ' ' with '.' for visibility):
>
> return GenFramebuffersEXT
> ........&& GenRenderbuffersEXT
> ........&& BindRenderbufferEXT;
>
> So 8 space indenting, a bit much for my taste but I guess, someone
> properly thought about that and this how new code looks like, too?
Line up under the parenthesis nesting level if there is one, simple
indentation (4 or 8 spaces) if there isn't one like above.
These corner-cases are usually solved by "best visual elegance".
> 2 a) I found a place with 8 spaces indent but QtCreator would indent
> it further to 12 so that the following 'Q' aligns with the 's'
>
> viewport()->setFlag(
> ............QGraphicsItem::ItemClipsChildrenToShape, true);
I'd have indented to inside the (...
> 2 b) indenting a continuation of an assigment is only 8 spaces again:
>
> int hello > ........100 + test;
Right.
>
> 2 c) if vs. while lines seem obviously inconsistent
>
> if (test == true
> ........&& 1 == 0) // 8 spaces == 4 relative to the '('
This is a special case because of a {
If you write:
if (test == true
&& 1 = 0) {
hello();
}
Then "hello" and && line up, making it visually ambiguous. The extra indent is
to prevent that.
> while (test == true
> ......&& 1 == 0) // 7 spaces == 0 relative to the '('
Looks like Creator's special-case for ifs doesn't apply to whiles.
> Bottom line: might it be the current auto-indent styling is actually
> not completely trustworthy? (I hope the answer is 'no', otherwise this
> would have to be 'fixed' first, I'd say. Comments?)
>
> That is just stuff I ran across, I did not thoroughly checkout the
> indentation behavior.
As I said, do what makes it look best. It's subjective.
> 3) I came across a macro that had a semicolon at the end. I think this
> is sub-optimal. Without special support in the IDE (like for the
> Q_OBJECT macro) this leads to the following auto indented code:
>
> ....RESOLVE_GL_FUNC(GenFramebuffersEXT)
> ............RESOLVE_GL_FUNC(GenRenderbuffersEXT)
>
> instead of
>
> ....RESOLVE_GL_FUNC(GenFramebuffersEXT);
> ....RESOLVE_GL_FUNC(GenRenderbuffersEXT);
>
> Formally a non-whitespace change but in practice, it is.
Put the semi-colon if the semi-colon is necessary. Otherwise, don't. This is
not optional.
The IDE should expand the macro and realise that a semi-colon is not
necessary, thus not concluding it's a continuation.
Suggestion: don't touch the semi-colons. Just fix the leading tabs.
> 4) What about consistent placement of the ',' in class member
> initialization lists? I've seen
>
> : GvbWidget(parent),
>
> m_background()
>
> and
>
> : GvbWidget(parent)
>
> , m_background()
I prefer the former, though I've seen people do the latter because it allows
for adding new members without touching a previous line. I find lines starting
with a comma horrid.
Both are acceptable.
> 5) About switch/case with blocks: I find the following indentation not
> particularly easy to read
>
> switch (type) {
> case 1: {
> int a;
> return a;
> }
> default:
> return 0;
> }
>
> probably not having a majority appeal but I think the following
> (QtCreator auto-indent-compatible) version would be better:
>
> switch (type) {
> case 1: {
> int a;
> return a; }
> default:
> return 0;
> }
Why are you asking this?
Don't touch the styling. Just fix the whitespaces and tabs. I will reject any
patch that tries to fix minor styling deviations in QtDBus and QtCore. Do it
only if the code is completely out-of-style or if it's about whitespace.
The Qt coding style guide says that you're allowed to deviate if it makes the
code better.
> >> I see. Regarding false report rate, I think that checking for trailing
> >> ws as well as checking for leading tabs should be secure enough to
> >> actually strictly enforce them. See also 'git diff --check'.
> >
> > yes. there are a few legitimate exceptions, but most of the files are so
> > seldomly touched that it's ok to deal with them when it happens.
>
> Please note that the above comment ('secure enough') was made with
> only c++ source files in mind. I can't say anything about extending
> that enforcement to other text based files (txt, rc, pro, conf, xml,
> pl, qdoc, etc...).
qmake .pro files, perl scripts, and our XML files are tab-free in Qt. Obviously,
you should not touch the test data or 3rd-party code.
> Where are those existing enforced checks implemented (like the check,
> whether you have a valid Change-Id in your commit message)?
The sanity bot runs on all pushes to Gerrit. That has nothing to do with the
Change-Id check, which is implemented by Gerrit itself.
> So you are _the_ Guy to talk to about whitespace in Qt ;-). Regarding
> the above mentioned problem with missing semicolons, I might suggest
> to accept some patches that are indeed non-white-space only. The same
> would be the case with stripping braces from one-line if|for|while
> blocks.
Do not add or remove semi-colons. If the semi-colon is present and shouldn't
be there, it should be fixed as a code fix, not as a style change. If there is
no semi-colon, do not add one.
I also recommend not touching the unnecessary braces. Those are usually the
result of code changes inside the block. The non-whitespace churn is entirely
unnecessary.
--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
