This revision was automatically updated to reflect the committed changes.
Closed by commit rC326426: [clang-format] Add SpaceBeforeColon option (authored
by Typz, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D32525?vs=136305=136483#toc
Repository:
rC Clang
This revision was automatically updated to reflect the committed changes.
Closed by commit rL326426: [clang-format] Add SpaceBeforeColon option (authored
by Typz, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D32525
Files:
djasper accepted this revision.
djasper added a comment.
This revision is now accepted and ready to land.
Ah, I thought it was somehow possible to create:
Constructor(): aa()
, bb() {},
but I guess clang-format always inserts a break there. Sorry for chasing you in
Typz added inline comments.
Comment at: unittests/Format/FormatTest.cpp:8969
+ "barr(1) {}", CtorInitializerStyle);
+ CtorInitializerStyle.BreakConstructorInitializers =
FormatStyle::BCIS_BeforeComma;
+
Typz updated this revision to Diff 136305.
Typz marked 2 inline comments as done.
Typz added a comment.
Address review comments
Repository:
rC Clang
https://reviews.llvm.org/D32525
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
lib/Format/Format.cpp
djasper added inline comments.
Comment at: unittests/Format/FormatTest.cpp:8969
+ "barr(1) {}", CtorInitializerStyle);
+ CtorInitializerStyle.BreakConstructorInitializers =
FormatStyle::BCIS_BeforeComma;
+
Typz updated this revision to Diff 136299.
Typz marked 2 inline comments as done.
Typz added a comment.
Address review comments.
Repository:
rC Clang
https://reviews.llvm.org/D32525
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
lib/Format/Format.cpp
djasper added a comment.
I think this generally looks good, but needs a few more tests.
Comment at: include/clang/Format/Format.h:1204
+ /// \brief If ``false``, spaces will be removed before constructor
initializer
+ /// colon.
When this file is changed,
Typz added a comment.
ping?
https://reviews.llvm.org/D32525
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Typz added a comment.
Indeed, I have yet find more precisely documented coding rules which require
this format, but I thought I could at least address the non-precise aspect of
the patch itself in the mean-time.
https://reviews.llvm.org/D32525
djasper added a comment.
No. The reason for us generally asking for a style guide is because it
unambiguously clarifies the exact style that is to be preferred. Projects that
don't have a style guide written down also often do not agree on what the style
should be.
That said, I think the
karies added a comment.
Does this qualify?
https://github.com/root-project/root/blob/master/.clang-format#L84
# You want this : enable it if you have https://reviews.llvm.org/D32525
# SpaceBeforeColon: false
in ROOT's `.clang-format`.
https://reviews.llvm.org/D32525
djasper added a comment.
You still haven't addressed my comment about there not being a publicly
accessible style guide recommending these.
https://reviews.llvm.org/D32525
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
Typz updated this revision to Diff 132963.
Typz added a comment.
Split the option into 3 separate options: SpaceBeforeCtorInitializerColon,
SpaceBeforeInheritanceColon and SpaceBeforeRangeBasedForLoopColon.
This makes each option clearer and more consistent, with no ambiguities due to
djasper added a comment.
Hm, can't really remember what I meant by "my comment". Probably not important.
So, I still see two problems:
- I would not count the link you mentioned as a publicly accessible style guide.
- I don't think the naming and granularity of this option is right.
You
Typz added a comment.
I checked the code of POCO, and it indeed follows this convention (though there
does not seem to be any C++11 for loop indeed).
We also use this style at our company.
> Also see my comment.
I could not find your comment... can you please re-post?
> It's very hard to even
djasper added a comment.
I have looked through the PDF you linked to, but I couldn't really find much
about formatting. I have found one instance of a constructor initializer list,
but there is no accompanying text saying that this is even intentional. Haven't
found a range-based for loop. As
Typz added a comment.
ping?
https://reviews.llvm.org/D32525
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
18 matches
Mail list logo
