[PATCH] D43290: clang-format: tweak formatting of variable initialization blocks

2018-05-16 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 147002. Typz marked 6 inline comments as done. Typz added a comment. Address review comment & rebase Repository: rC Clang https://reviews.llvm.org/D43290 Files: lib/Format/TokenAnnotator.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/Form

[PATCH] D43290: clang-format: tweak formatting of variable initialization blocks

2018-05-16 Thread Francois Ferrand via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC332434: clang-format: tweak formatting of variable initialization blocks (authored by Typz, committed by ). Changed prior to commit: https://reviews.llvm.org/D43290?vs=147002&id=147003#toc Repository:

[PATCH] D42684: clang-format: Allow optimizer to break template declaration.

2018-05-16 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 147005. Typz marked 3 inline comments as done. Typz added a comment. Rebase on latest master & address review comments Repository: rC Clang https://reviews.llvm.org/D42684 Files: docs/ClangFormatStyleOptions.rst include/clang/Format/Format.h lib/Forma

[PATCH] D42684: clang-format: Allow optimizer to break template declaration.

2018-05-16 Thread Francois Ferrand via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL332436: clang-format: Allow optimizer to break template declaration. (authored by Typz, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D42684?

[PATCH] D43015: clang-format: Introduce BreakInheritanceList option

2018-05-16 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 147011. Typz added a comment. Rebase on latest master Repository: rC Clang https://reviews.llvm.org/D43015 Files: docs/ClangFormatStyleOptions.rst include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp lib/Format/Format.cpp lib/Format/T

[PATCH] D43015: clang-format: Introduce BreakInheritanceList option

2018-05-16 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. ping? Repository: rC Clang https://reviews.llvm.org/D43015 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33440: clang-format: better handle statement macros

2018-05-16 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 147024. Typz added a comment. Herald added a subscriber: mgrang. Rebase on latest master Repository: rC Clang https://reviews.llvm.org/D33440 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/FormatToken.h lib/Format/FormatTokenL

[PATCH] D37813: clang-format: better handle namespace macros

2018-05-16 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. In https://reviews.llvm.org/D37813#958222, @klimek wrote: > Some initial design work has been done, and Krasimir said that he's > interested. No timeline though :( @klimek , @krasimir : any progress on this new preprocessor-like way to configure macros ? Repository:

[PATCH] D37813: clang-format: better handle namespace macros

2018-05-16 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 147027. Typz added a comment. Rebase Repository: rC Clang https://reviews.llvm.org/D37813 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/FormatToken.h lib/Format/FormatTokenLexer.cpp lib/Format/NamespaceEndCommentsFixer.cpp

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

2018-05-16 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 147031. Typz added a comment. Rebase Repository: rC Clang https://reviews.llvm.org/D32478 Files: docs/ClangFormatStyleOptions.rst include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp lib/Format/ContinuationIndenter.h lib/Format/Format

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-05-16 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 147079. Typz added a comment. rebase Repository: rC Clang https://reviews.llvm.org/D43183 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/UnwrappedLineParser.cpp lib/Format/UnwrappedLineParser.h unittests/Format/FormatTest.cp

[PATCH] D33440: clang-format: better handle statement macros

2018-05-17 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 147293. Typz marked an inline comment as done. Typz added a comment. Address review comments Repository: rC Clang https://reviews.llvm.org/D33440 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/FormatToken.h lib/Format/FormatTo

[PATCH] D33440: clang-format: better handle statement macros

2018-05-17 Thread Francois Ferrand via Phabricator via cfe-commits
Typz marked an inline comment as done. Typz added inline comments. Comment at: lib/Format/Format.cpp:647-648 LLVMStyle.SortUsingDeclarations = true; + LLVMStyle.StatementMacros.push_back("Q_UNUSED"); + LLVMStyle.StatementMacros.push_back("QT_REQUIRE_VERSION");

[PATCH] D37813: clang-format: better handle namespace macros

2017-10-24 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. In https://reviews.llvm.org/D37813#876227, @klimek wrote: > I think instead of introducing more and more special cases of macros we might > want to handle, we should instead allow specifying macro productions globally. what do you mean? Do you mean to group all the macro

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

2017-10-24 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. ping? https://reviews.llvm.org/D32478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

2017-10-24 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. > My question is: if CanBreak is false, we currently don't call > breakProtrudingToken. So either we do something very wrong in that case > (which might be true, but I'd like to understand why) or we should be able > to just calculate the penalty by not breaking anything

[PATCH] D33440: clang-format: better handle statement macros

2017-10-24 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 120084. Typz added a comment. rebase https://reviews.llvm.org/D33440 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/FormatToken.h lib/Format/FormatTokenLexer.cpp lib/Format/FormatTokenLexer.h lib/Format/UnwrappedLineParser.cp

[PATCH] D37813: clang-format: better handle namespace macros

2017-10-24 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 120085. Typz added a comment. rebase https://reviews.llvm.org/D37813 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/FormatToken.h lib/Format/FormatTokenLexer.cpp lib/Format/NamespaceEndCommentsFixer.cpp lib/Format/TokenAnnota

[PATCH] D50147: clang-format: support external styles

2018-09-04 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. clang-format does indeed support .clang-format, which is great for *isolated* projects, and which is not affected by this patch. This patch addresses the issue of *centralizing* the definition of styles, e.g. allowing individual projects to reference externally defined sty

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

2018-07-31 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 158302. Typz added a comment. Herald added a subscriber: acoomans. rebase Repository: rC Clang https://reviews.llvm.org/D32478 Files: docs/ClangFormatStyleOptions.rst include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp lib/Format/Conti

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

2018-07-31 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Typz added reviewers: krasimir, djasper, klimek. Herald added a subscriber: acoomans. When multiple ternary operators are chained, e.g. like an if/else-if/ else-if/.../else sequence, clang-format will keep aligning the colon with the question mark, which increases the i

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

2018-07-31 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. Notes: - I choose not to add an option to enable this behavior, as I think of it as just another way clang-format can (better) format the code; but I can one if needed - Currently, it relies on another patch (https://reviews.llvm.org/D32478), which supports aligning the w

[PATCH] D33440: clang-format: better handle statement macros

2018-07-31 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 158308. Typz marked an inline comment as done. Typz added a comment. Herald added a subscriber: acoomans. Regenerate documentation Repository: rC Clang https://reviews.llvm.org/D33440 Files: docs/ClangFormatStyleOptions.rst include/clang/Format/Format.h

[PATCH] D37813: clang-format: better handle namespace macros

2018-07-31 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 158309. Typz added a comment. Herald added a subscriber: acoomans. Regeneration documentation Repository: rC Clang https://reviews.llvm.org/D37813 Files: docs/ClangFormatStyleOptions.rst include/clang/Format/Format.h lib/Format/Format.cpp lib/Format

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-07-31 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 158313. Typz added a comment. Herald added a subscriber: acoomans. Regenerate documentation Repository: rC Clang https://reviews.llvm.org/D43183 Files: docs/ClangFormatStyleOptions.rst include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/U

[PATCH] D37813: clang-format: better handle namespace macros

2018-08-01 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. Ping! This has been stale for a while now. Last comment indicate this is not the right approach, but some prototyping has been done a more generic approach, with no timeline. I understand this, but in the meantime the problem is still there, with no solution at the moment

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

2018-08-01 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. In https://reviews.llvm.org/D50078#1184015, @djasper wrote: > - I'd be ok with the suggestion for BreakBeforeTernaryOperators=true Just to be clear, which suggestion would you be ok with? int fooo = ? 0 : ? 1 : ; or:

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

2018-08-01 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. In https://reviews.llvm.org/D50078#1184159, @krasimir wrote: > Could you clarify how each piece is supposed to be aligned in these examples? > This is what makes me happy: > > // column limit V > int a = condition1 ? result1 > : conditio2 ? result2

[PATCH] D50147: clang-format: support external styles

2018-08-01 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Typz added reviewers: krasimir, djasper, klimek. Herald added a subscriber: acoomans. This patch allows defining external styles, which can be used exactly like the embedded styles (llvm, google, mozilla...). These styles are clang-format files installed either systemw

[PATCH] D32525: [clang-format] Add SpaceBeforeColon option

2018-02-06 Thread Francois Ferrand via Phabricator via cfe-commits
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 interracti

[PATCH] D32525: [clang-format] Add SpaceBeforeColon option

2018-02-06 Thread Francois Ferrand via Phabricator via 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 ___

[PATCH] D43015: clang-format: Introduce BreakInheritanceList option

2018-02-07 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Typz added reviewers: djasper, krasimir, klimek. This option replaces the BreakBeforeInheritanceComma option with an enum, thus introducing a mode where the colon stays on the same line as constructor declaration: // When it fits on line: class A : public B, public

[PATCH] D43015: clang-format: Introduce BreakInheritanceList option

2018-02-07 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 133204. Typz added a comment. Fix indentation of inheritance list, which is actually based on `ConstructorInitializerIndentWidth`. Maybe a new option (`InheritanceListIndentWidth`) should be used instead? Repository: rC Clang https://reviews.llvm.org/D4301

[PATCH] D43114: clang-format: fix formatting of ObjC @synchronized blocks

2018-02-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Typz added reviewers: krasimir, djasper, klimek. The blocks used to be formatted using the "default" behavior, and would thus be mistaken for function calls followed by blocks: this could lead to unexpected inlining of the block and extra line-break before the opening b

[PATCH] D43114: clang-format: fix formatting of ObjC @synchronized blocks

2018-02-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added inline comments. Comment at: lib/Format/UnwrappedLineParser.cpp:1130 +if (FormatTok->Tok.is(tok::l_brace)) { + if (Style.BraceWrapping.AfterObjCDeclaration) +addUnwrappedLine(); Wondering if formatting with this style is ap

[PATCH] D42684: clang-format: Allow optimizer to break template declaration.

2018-02-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. ping? Repository: rC Clang https://reviews.llvm.org/D42684 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42787: clang-format: do not add extra indent when wrapping last parameter

2018-02-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. In https://reviews.llvm.org/D42787#1000687, @krasimir wrote: > We could adapt the single-argument version instead, turning: > > foo(bb + > c); > > > into: > > foo(bb + > c); > We could inde

[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

2018-02-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Typz added reviewers: krasimir, djasper, klimek. When the target object expression is short and the first selector name is long, clang-format used to break the colon alignment: [I performSelectorOnMainThread:@selector(loadAccessories) withObjec

[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

2018-02-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 133589. Typz added a comment. fix type in commit message Repository: rC Clang https://reviews.llvm.org/D43121 Files: lib/Format/ContinuationIndenter.cpp lib/Format/TokenAnnotator.cpp unittests/Format/FormatTestObjC.cpp Index: unittests/Format/Format

[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

2018-02-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 133596. Typz marked 2 inline comments as done. Typz added a comment. Address review comments Repository: rC Clang https://reviews.llvm.org/D43121 Files: lib/Format/ContinuationIndenter.cpp lib/Format/TokenAnnotator.cpp unittests/Format/FormatTestObjC.

[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

2018-02-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:900 + std::max(NextNonComment->LongestObjCSelectorName, + unsigned(NextNonComment->TokenText.size())) - NextNonComment->ColumnWidth; djasper

[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

2018-02-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 133619. Typz added a comment. rebase on latest master. Repository: rC Clang https://reviews.llvm.org/D43121 Files: lib/Format/ContinuationIndenter.cpp lib/Format/TokenAnnotator.cpp unittests/Format/FormatTestObjC.cpp Index: unittests/Format/FormatTe

[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

2018-02-09 Thread Francois Ferrand via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC324741: clang-format: keep ObjC colon alignment with short object name (authored by Typz, committed by ). Changed prior to commit: https://reviews.llvm.org/D43121?vs=133619&id=133620#toc Repository:

[PATCH] D43114: clang-format: fix formatting of ObjC @synchronized blocks

2018-02-09 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added inline comments. Comment at: lib/Format/UnwrappedLineParser.cpp:1130 +if (FormatTok->Tok.is(tok::l_brace)) { + if (Style.BraceWrapping.AfterObjCDeclaration) +addUnwrappedLine(); benhamilton wrote: > Typz wrote: > > Wonderin

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-12 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Typz added reviewers: krasimir, djasper, klimek. When a block is started after a case label, clang-format does add extra indent to the content of this block: the block content is indented one level (with respect to the switch) while the closing brace is not indented, an

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. > Do you have a reference to style guides recommending any of this? Unfortunately not... I think this is a really advanced topic, and often not recommended way to format code at all (e.g. "if you need a block, you may as well use a function"). I can find the current implem

[PATCH] D43232: clang-format: use AfterControlStatement to format ObjC control blocks

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Typz added reviewers: krasimir, djasper, klimek, benhamilton. ObjC defines `@autoreleasepool` and `@synchronized` control blocks. These used to be formatted according to the `AfterObjCDeclaration` brace- wrapping flag, which is not very consistent. This patch changes t

[PATCH] D43232: clang-format: use AfterControlStatement to format ObjC control blocks

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. This is done as discussed in https://reviews.llvm.org/D43114. But I can instead add a new `AfterObjCSpecialBlock` brace wrapping flag. Repository: rC Clang https://reviews.llvm.org/D43232 ___ cfe-commits mailing list cfe-com

[PATCH] D43114: clang-format: fix formatting of ObjC @synchronized blocks

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz marked 4 inline comments as done. Typz added inline comments. Comment at: lib/Format/UnwrappedLineParser.cpp:1130 +if (FormatTok->Tok.is(tok::l_brace)) { + if (Style.BraceWrapping.AfterObjCDeclaration) +addUnwrappedLine(); benham

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. > We'll just format those cases in a somewhat weird way and users can either > accept that or change their code to not need it. I think we have a really diverging opinion on this. From my experience, people will mostly accept the output of the formatter without question :

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. It is explicitly documented in google style guide: https://google.github.io/styleguide/cppguide.html#Loops_and_Switch_Statements : > case blocks in switch statements can have curly braces or not, depending on > your preference. If you do include curly braces they should be

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. Hum, not sure I fully got your proposal. So you actually mean that clang-format you format like this (with 4-space indent for clarity): switch (x) { case 0: break; case 1: { foo(); break; } case 2: { foo(); } break; case 3: {

[PATCH] D43290: clang-format: tweak formatting of variable initialization blocks

2018-02-14 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Typz added reviewers: krasimir, djasper, klimek. This patch changes the behavior of PenaltyBreakBeforeFirstCallParameter so that is does not apply when the brace comes after an assignment. This way, variable initialization is wrapped more like an initializer than like

[PATCH] D43290: clang-format: tweak formatting of variable initialization blocks

2018-02-15 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. Tweaking the penalty handling would still be needed in any case, since the problem happens also when Cpp11BracedListStyle is true (though the effect is more subtle) Generally, I think it is better to just rely on penalties, since it gives a way to compare and weight each

[PATCH] D43290: clang-format: tweak formatting of variable initialization blocks

2018-02-15 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 134411. Typz marked an inline comment as done. Typz added a comment. Address review comments. Repository: rC Clang https://reviews.llvm.org/D43290 Files: lib/Format/TokenAnnotator.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-15 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. In https://reviews.llvm.org/D43183#1008632, @djasper wrote: > Yes, that's what I mean. What do you mean, the style is too error prone? When `IndentCaseLabels` is false, the code gets formatted in a way that "hides" the structure of the code, by indenting the end of the ca

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

2018-02-15 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. A user can create an error by reasoning like this, after the code has been formatted this way (a long time ago) : "oh, I need to make an extra function call, but in all cases ah, here is the end of the switch, let's put my function call here". I am not saying it happe

[PATCH] D43232: clang-format: use AfterControlStatement to format ObjC control blocks

2018-02-15 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. In https://reviews.llvm.org/D43232#1008004, @benhamilton wrote: > Thanks! Can you add a test for this, please? There are actually tests, though they are performed by activating Allman brace wrapping mode. I am trying to change them, but this highlights another issue: when

[PATCH] D43232: clang-format: use AfterControlStatement to format ObjC control blocks

2018-02-15 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. I'll try to fix this. Can you please review D43114 in the mean time? This patch depends on it, so it must be merged first. Repository: rC Clang https://reviews.llvm.org/D43232 ___ cfe-comm

[PATCH] D43114: clang-format: fix formatting of ObjC @synchronized blocks

2018-02-15 Thread Francois Ferrand via Phabricator via cfe-commits
Typz marked an inline comment as done. Typz added inline comments. Comment at: unittests/Format/FormatTestObjC.cpp:193-198 + verifyFormat("@synchronized(self) {\n" + " f();\n" + "}\n" + "@synchronized(self) {\n" + " f();\

[PATCH] D43232: clang-format: use AfterControlStatement to format ObjC control blocks

2018-02-16 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 134641. Typz added a comment. Fix bug which was lingering: prevent inlining of ObjC special control blocks. Repository: rC Clang https://reviews.llvm.org/D43232 Files: include/clang/Format/Format.h lib/Format/UnwrappedLineFormatter.cpp lib/Format/Unwr

[PATCH] D33447: clang-format: add option to merge empty function body

2017-05-24 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 100053. Typz added a comment. Fix SFS_Empty & SFS_Inline merging of empty function when BraceWrapping.AfterFunction, and add missing test https://reviews.llvm.org/D33447 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/UnwrappedLine

[PATCH] D33447: clang-format: add option to merge empty function body

2017-05-24 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. In https://reviews.llvm.org/D33447#763045, @djasper wrote: > Does anything speak against making this behavior happen with > AllowShortFunctionsOnASingleLine = SFS_Empty and > MergeEmptyOnly.BraceWrapping.AfterFunction = true? I mean without the extra > style option? Tha

[PATCH] D33447: clang-format: add option to merge empty function body

2017-05-24 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 100060. Typz added a comment. fix indent https://reviews.llvm.org/D33447 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/UnwrappedLineFormatter.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp ==

[PATCH] D33447: clang-format: add option to merge empty function body

2017-05-24 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. Facebook's HHVM seems to use this for empty constructors: https://github.com/facebook/hhvm/blob/master/hphp/doc/coding-conventions.md This is also done systematically in Qt and QtCreator code. Personally, I don't care if this is an option or not; but I fear it would break

[PATCH] D32479: clang-format: Introduce BreakConstructorInitializers option

2017-05-24 Thread Francois Ferrand via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL303739: clang-format: Introduce BreakConstructorInitializers option (authored by Typz). Changed prior to commit: https://reviews.llvm.org/D32479?vs=99887&id=100068#toc Repository: rL LLVM https://re

[PATCH] D33440: clang-format: properly handle Q_UNUSED and QT_REQUIRE_VERSION

2017-05-24 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. Without this patch, macros with no trailing semicolon _in the body of a function_ are not handled properly, so I get: int foo(int a, int b) { Q_UNUSED(a) return b; } class Foo { void bar(int a, int b) { Q_UNUSED(a) Q_UNUSED(b) } } https://reviews.llvm.o

[PATCH] D33447: clang-format: add option to merge empty function body

2017-05-24 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. I am fine with "removing" the options: I only fear it would incorrectly affect existing users/styles, which would possibly end up in the patch being reverted... I could not find the info in Mozilla coding style, but looking at the code it seems it should be done indeed, bu

[PATCH] D33440: clang-format: properly handle Q_UNUSED and QT_REQUIRE_VERSION

2017-05-24 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. Digging back, it seem the issue was that I had this code: void foo(..) { Q_UNUSED(a) Q_UNUSED(b) } which got wrapped because the line was too long: void foo(..) { Q_UNUSED(a) Q_UNUSED(b) } I definitely understand your concern about introduceing special h

[PATCH] D33447: clang-format: add option to merge empty function body

2017-05-24 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. Right, I was not clear enough: this is indeed done only if everything does not fit on one line. There is an exemple in facebook's coding style: MyClass::MyClass(const Class* cls, const Func* func, const Class* ctx) : m_cls(cls) , m_func(func) , m_ctx(ctx)

[PATCH] D33447: clang-format: add option to merge empty function body

2017-05-24 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. Merging empty blocks is not allowed in webkit style: https://webkit.org/code-style-guidelines/#punctuation-member-init So it seems changing the behavior would actually fix mozilla style and break webkit style (as integrated in clang-format). https://reviews.llvm.org/D334

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

2017-05-24 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 100095. Typz added a comment. add new value to AlignOperands to support this mode. fix some corner cases. https://reviews.llvm.org/D32478 Files: include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp lib/Format/ContinuationIndenter.h lib/For

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

2017-05-26 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. It indeed does not happens inside any parenthesis (it would actually make things completely unreadable if there are imbricated parenthesis), and may get indented less than the ContinuationIndentWidth. https://reviews.llvm.org/D32478

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

2017-05-26 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:949 + Previous->is(tok::kw_return))) + NewParenState.UnindentOperator = true; djasper wrote: > I am not yet convinced you need a new flag in ParenState here. I guess you >

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

2017-05-26 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Herald added a subscriber: klimek. This patch tries to improve the optimizer a bit, to avoid splitting tokens (e.g. comments/strings) if only there are only few characters beyond the ColumnLimit. Previously, comments/strings would be split whenever they went beyond the

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

2017-05-26 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 100379. Typz added a comment. fix indentation issues https://reviews.llvm.org/D33589 Files: lib/Format/ContinuationIndenter.cpp lib/Format/ContinuationIndenter.h lib/Format/FormatToken.cpp lib/Format/FormatToken.h lib/Format/UnwrappedLineFormatter.cp

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

2017-05-26 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. Still some issues with the patch, I would need some feedback first: - Is this approach desirable, as a relatively easy fix? - Or should this be fixed with a complete refactoring of the way the strings/comments are split, making multiple tokens out of them to let them be re

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

2017-05-26 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 100384. Typz added a comment. fix style https://reviews.llvm.org/D32478 Files: include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp lib/Format/ContinuationIndenter.h lib/Format/Format.cpp unittests/Format/FormatTest.cpp unittests/Forma

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

2017-05-26 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. In https://reviews.llvm.org/D32478#765537, @djasper wrote: > In all honesty, I think this style isn't thought out well enough. It really > is a special case for only "=" and "return" and even there, it has many cases > where it simply doesn't make sense. And then you have

[PATCH] D33447: clang-format: add option to merge empty function body

2017-05-26 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. Webkit inherits AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All from LLVM style, so merging the options would introduce these explicitely-forbidden empty blocks. But the empty blocks should actually be used in code following Mozilla or Qt style. https://reviews.l

[PATCH] D33447: clang-format: add option to merge empty function body

2017-05-26 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 100393. Typz added a comment. update mozilla style to use empty function blocks https://reviews.llvm.org/D33447 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/UnwrappedLineFormatter.cpp unittests/Format/FormatTest.cpp Index: uni

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

2017-05-26 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. I actually don't know how, but it still manages somehow : I rebuilt this exact patch to ensure I gave you the correct output. And the same behavior can be seen in the test cases, where the operator with highest precedence is aligned with the equal sign. https://reviews.ll

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

2017-05-26 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. Nop, it's formatted like this: bool a = aa // == // && c; bool a = aa // == // + c; https://reviews.llvm.org/D32478 ___ cfe-commits mailing list c

[PATCH] D33447: clang-format: add option to merge empty function body

2017-05-26 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. In https://reviews.llvm.org/D33447#765610, @djasper wrote: > I think it's just wrong that WebKit inherits this. The style guide explicitly > says that this is wrong: > > MyOtherClass::MyOtherClass() : MySuperClass() {} I think this exemple applies to constructors only.

[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

2017-05-26 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. > Ah damn. Didn't think about the precedences. What I wanted was for the > highest level to have a one char operator, e.g. > > bool a = aa // > << // > | c; > Almost, but not quite: bool a = aa // << //

[PATCH] D33447: clang-format: add option to merge empty function body

2017-05-26 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 100412. Typz added a comment. move option to BraceWrapping https://reviews.llvm.org/D33447 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/UnwrappedLineFormatter.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatT

[PATCH] D33447: clang-format: add option to merge empty function body

2017-05-26 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 100420. Typz marked 2 inline comments as done. Typz added a comment. fix indent & rename option to SplitEmptyFunctionBody https://reviews.llvm.org/D33447 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/UnwrappedLineFormatter.cpp u

[PATCH] D32480: clang-format: Add CompactNamespaces option

2017-05-30 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. ping? https://reviews.llvm.org/D32480 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

2017-05-30 Thread Francois Ferrand via Phabricator via cfe-commits
Typz marked an inline comment as done. Typz added inline comments. Comment at: unittests/Format/FormatTest.cpp:8571 +"*/", +format("int a; /* first line second line third line */", Style)); +} Typz wrote: > This is not working as e

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

2017-05-30 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 100698. Typz marked an inline comment as done. Typz added a comment. fix code & tests https://reviews.llvm.org/D33589 Files: lib/Format/ContinuationIndenter.cpp lib/Format/ContinuationIndenter.h lib/Format/FormatToken.cpp lib/Format/FormatToken.h lib

[PATCH] D32480: clang-format: Add CompactNamespaces option

2017-05-31 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. That will be slightly more complicated to check, esp. we will need to keep track of the matching-closing-brace (currently only the matching-opening-brace) is stored. But I can try to update the patch in that direction, if that is the consensus. https://reviews.llvm.org/D3

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

2017-06-06 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. In https://reviews.llvm.org/D33589#769893, @krasimir wrote: > I think that what you're trying to solve is not practically that important, > is unlikely to improve the handling of comments, and will add a lot of > complexity. Not sure the 'approach' I have in this patch i

[PATCH] D32480: clang-format: Add CompactNamespaces option

2017-06-06 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. So how do I proceed? 1. Keep the CompactNamespace option, and make "compacted" namespaces always add at most one level of indentation 2. Or assume that this can only ever usefully work with the behavior of NI_None and add an additional enum value NI_Compact. And should we

[PATCH] D33447: clang-format: add option to merge empty function body

2017-06-06 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. ping https://reviews.llvm.org/D33447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

2017-06-12 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. ping? https://reviews.llvm.org/D33589 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33447: clang-format: add option to merge empty function body

2017-06-12 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. ping? https://reviews.llvm.org/D33447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32480: clang-format: Add CompactNamespaces option

2017-06-12 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. ping? https://reviews.llvm.org/D32480 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33447: clang-format: add option to merge empty function body

2017-06-13 Thread Francois Ferrand via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL305272: clang-format: add option to merge empty function body (authored by Typz). Changed prior to commit: https://reviews.llvm.org/D33447?vs=100420&id=102294#toc Repository: rL LLVM https://reviews

[PATCH] D32480: clang-format: Add CompactNamespaces option

2017-06-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 102346. Typz added a comment. - make "compacted" namespaces always add at most one level of indentation - compact only namespaces which both start and end on consecutive lines https://reviews.llvm.org/D32480 Files: include/clang/Format/Format.h lib/Format/

[PATCH] D32480: clang-format: Add CompactNamespaces option

2017-06-14 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 102525. Typz added a comment. Make tests more readable https://reviews.llvm.org/D32480 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/NamespaceEndCommentsFixer.cpp lib/Format/UnwrappedLineFormatter.cpp lib/Format/UnwrappedLineP

  1   2   3   4   >