djasper accepted this revision.
djasper added a comment.
This revision is now accepted and ready to land.
Looks good. Thank you for doing this!
https://reviews.llvm.org/D30532
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm
sylvestre.ledru marked 2 inline comments as done.
sylvestre.ledru added a comment.
Daniel, it is good for me now.
Once it is accepted, I will work on the rest in a separate commit if that is ok
with you;
https://reviews.llvm.org/D30532
___
cfe-com
sylvestre.ledru updated this revision to Diff 90486.
sylvestre.ledru added a comment.
updated => affected
+ align the vs
https://reviews.llvm.org/D30532
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
Index: include/clang/Format/Format.h
=
sylvestre.ledru updated this revision to Diff 90482.
sylvestre.ledru added a comment.
With the changes requested by Daniel. This is much better indeed!
https://reviews.llvm.org/D30532
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
Index: include/clang/Format/Format.h
krasimir added a comment.
I also think that examples for the flags are good. My use case is that while
developing/debugging its nice to see a short example for a random flag in the
documentation pop-up.
https://reviews.llvm.org/D30532
___
cfe-comm
djasper added a comment.
I agree, just generally we should aim for keeping these short.
The example you gave might actually be reasonable to compare in two columns,
i.e.:
true: false:
SomeClass::Constructor() vs. SomeClass::Constructor() : a(a),
sylvestre.ledru added a comment.
> For bool options it seems easiest to do something like:
> true: x = ( int32 )y;
> false: x = (int32)y;
That works for single declarations but for stuff like:
> SomeClass::Constructor()
> : a(a)
> , b(b)
> , c(c) {}
I won't be
kimgr added a comment.
For what it's worth, I also find this useful to be able to see at a glance what
the setting does. I know I've tried several different settings in the past
before finding the one I was looking for.
https://reviews.llvm.org/D30532
___
djasper added a comment.
Sure, then go ahead. If these examples would have helped you, that's one
datapoint :).
As for presenting the difference in options, that would be useful I think. So
if you are up to also doing that, that'd be appreciated.
For bool options it seems easiest to do somethin
sylvestre.ledru added a comment.
I see a lot of values in examples. If I started this review, it is because I
was lost in all the options and could not find what I was looking for.
If you want, I can update the example to provide results with and without the
option.
like
> With SpacesInCStyle
djasper added a comment.
Hm. I don't actually know whether these examples are useful as is. You only
present one setting of the value in most cases. What's interesting is actually
how the flag changes the behavior. I mean in most cases, this can be derived
from the example, but in those cases,
sylvestre.ledru updated this revision to Diff 90331.
sylvestre.ledru added a comment.
With the rst generation
https://reviews.llvm.org/D30532
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
Index: docs/ClangFormatStyleOptions.rst
==
sylvestre.ledru added inline comments.
Comment at: docs/ClangFormatStyleOptions.rst:218
+
+#define A \
+int ; \
klimek wrote:
> Do we really align these 3 spaces out?
nope, bad copy and paste it seems. Sorry
Comment at: docs/Cla
sylvestre.ledru updated this revision to Diff 90330.
sylvestre.ledru marked 3 inline comments as done.
https://reviews.llvm.org/D30532
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
Index: docs/ClangFormatStyleOptions.rst
===
klimek added inline comments.
Comment at: docs/ClangFormatStyleOptions.rst:218
+
+#define A \
+int ; \
Do we really align these 3 spaces out?
Comment at: docs/ClangFormatStyleOptions.rst:447
+ SomeClass::Constructor()
+
sylvestre.ledru added a comment.
If you think it makes sense, I will do that for the rest of the options
https://reviews.llvm.org/D30532
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi
sylvestre.ledru created this revision.
https://reviews.llvm.org/D30532
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
Index: docs/ClangFormatStyleOptions.rst
===
--- docs/ClangFormatStyleOptions.rst
+++ do
17 matches
Mail list logo
