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

2017-07-28 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 108651. Typz added a comment. Rebase https://reviews.llvm.org/D33589 Files: lib/Format/ContinuationIndenter.cpp lib/Format/ContinuationIndenter.h lib/Format/UnwrappedLineFormatter.cpp unittests/Format/FormatTest.cpp Index:

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

2017-07-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 106442. Typz added a comment. Move code out of optimizer, directly into ContinuationIndenter::breakProtrudingToken(), to minimize impact on performance. Comment reflowing of breakable items which break the limit will be slightly slower, since we now consider

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

2017-07-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz marked 2 inline comments as done. Typz added inline comments. Comment at: lib/Format/UnwrappedLineFormatter.cpp:723 FormatDecision LastFormat = Node->State.NextToken->Decision; if (LastFormat == FD_Unformatted || LastFormat == FD_Continue) +

[PATCH] D35483: clang-format: fix block OpeningLineIndex around preprocessor

2017-07-17 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Herald added a subscriber: klimek. The current code would return an incorrect value when a preprocessor directive is present immediately after the opening brace: this causes the nanespace end comment fixer to break in some places, for exemple it would not add the

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

2017-07-12 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 106221. Typz marked 3 inline comments as done. Typz added a comment. Rename option to AlignAfterOperator https://reviews.llvm.org/D32478 Files: include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp lib/Format/ContinuationIndenter.h

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

2017-07-12 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. I renamed the option to `AlignAfterOperator`, is it acceptable now? (I also thought of `UnindentOperator`, but I think it is better to keep the notion of alignment) Note that this style is used in multiple open-source projects: Skia , parts of QtCreator

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

2017-07-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. ping? https://reviews.llvm.org/D33440 ___ 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-07-13 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added inline comments. Comment at: lib/Format/UnwrappedLineFormatter.cpp:723 FormatDecision LastFormat = Node->State.NextToken->Decision; if (LastFormat == FD_Unformatted || LastFormat == FD_Continue) +addNextStateToQueue(Penalty, Node,

[PATCH] D35483: clang-format: fix block OpeningLineIndex around preprocessor

2017-07-18 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 107030. Typz marked an inline comment as done. Typz added a comment. Add more tests https://reviews.llvm.org/D35483 Files: lib/Format/UnwrappedLineParser.cpp lib/Format/UnwrappedLineParser.h unittests/Format/NamespaceEndCommentsFixerTest.cpp Index:

[PATCH] D35557: clang-format: merge short case labels with trailing comments

2017-07-18 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Herald added a subscriber: klimek. Allow merging short case labels when they actually end with a comment (like a comment after the ``break``) and when followed by switch-level comments (e.g. aligned with next case): switch(a) { case 0: break; // comment at end of

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

2017-07-18 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. t>>! In https://reviews.llvm.org/D33440#812645, @djasper wrote: > So, there are two things in this patch: Statement macros and namespace > macros. Lets break this out and handle them individually. They really aren't > related that much. Indeed, the only "relation" is the

[PATCH] D35557: clang-format: merge short case labels with trailing comments

2017-07-23 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 107841. Typz marked 4 inline comments as done. Typz added a comment. Address review comments https://reviews.llvm.org/D35557 Files: lib/Format/UnwrappedLineFormatter.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp

[PATCH] D35557: clang-format: merge short case labels with trailing comments

2017-07-23 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added inline comments. Comment at: unittests/Format/FormatTest.cpp:912 + " break;\n" + "}", + Style); krasimir wrote: > I'd suggest adding more cases here, like: > ``` >"case 6: /* comment */ x = 1;

[PATCH] D35483: clang-format: fix block OpeningLineIndex around preprocessor

2017-07-23 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 107843. Typz marked 3 inline comments as done. Typz added a comment. Address review commentsx https://reviews.llvm.org/D35483 Files: lib/Format/UnwrappedLineParser.cpp lib/Format/UnwrappedLineParser.h unittests/Format/NamespaceEndCommentsFixerTest.cpp

[PATCH] D35483: clang-format: fix block OpeningLineIndex around preprocessor

2017-07-28 Thread Francois Ferrand via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL309369: clang-format: fix block OpeningLineIndex around preprocessor (authored by Typz). Changed prior to commit: https://reviews.llvm.org/D35483?vs=107843=108598#toc Repository: rL LLVM

[PATCH] D35557: clang-format: merge short case labels with trailing comments

2017-07-28 Thread Francois Ferrand via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL309370: clang-format: merge short case labels with trailing comments (authored by Typz). Repository: rL LLVM https://reviews.llvm.org/D35557 Files: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp

[PATCH] D35483: clang-format: fix block OpeningLineIndex around preprocessor

2017-07-24 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added inline comments. Comment at: lib/Format/UnwrappedLineParser.cpp:464 + for (const auto : PPStack) { +hash_combine(h, i.Kind); +hash_combine(h, i.Line); krasimir wrote: > When I patch this, I get an `UnwrappedLineParser.cpp:457:16 error:

[PATCH] D34395: clang-format: add options to merge empty record body

2017-06-30 Thread Francois Ferrand via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL306874: clang-format: add options to merge empty record body (authored by Typz). Repository: rL LLVM https://reviews.llvm.org/D34395 Files: cfe/trunk/include/clang/Format/Format.h

[PATCH] D34238: clang-format: Do not binpack initialization lists

2017-06-30 Thread Francois Ferrand via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL306868: clang-format: Do not binpack initialization lists (authored by Typz). Changed prior to commit: https://reviews.llvm.org/D34238?vs=103057=104914#toc Repository: rL LLVM

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

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

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

2017-06-29 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added inline comments. Comment at: include/clang/Format/Format.h:167 +/// \endcode +OAS_StrictAlign, + }; djasper wrote: > The name is not intuitive. I don't think this is any more or less strict than > the other version. It is a bit stricter in

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

2017-07-05 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] D33440: clang-format: better handle statement and namespace macros

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

[PATCH] D32524: [clang-format] Fix MatchingOpeningBlockLineIndex computation

2017-05-17 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. I don't have commit access, can someone please commit this patch? https://reviews.llvm.org/D32524 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-05-17 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added inline comments. Comment at: include/clang/Format/Format.h:153 + /// \endcode + bool AllowSemicolonAfterNamespace; + djasper wrote: > While I am not entirely opposed to this feature, I think it should be a > separate patch. I totally agree, which

[PATCH] D33282: clang-format: fix prefix for doxygen comments after member

2017-05-17 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Herald added a subscriber: klimek. Doxygen supports putting documentation blocks after member, by adding an additional < marker in the comment block. This patch makes sure this marker is used in lines which are introduced by breaking the comment. int foo; ///< Some

[PATCH] D33285: clang-format: do not reflow bullet lists

2017-05-17 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Herald added a subscriber: klimek. This patch prevents reflowing bullet lists in block comments. It handles all lists supported by doxygen and markdown, e.g. bullet lists starting with '-', '*', '+', as well as numbered lists starting with -# or a number followed by a

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

2017-05-17 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. we are using this style at our company, not sure if it is used elsewhere; I will check. however, it seems to me that this behavior does not match the name of the option : AlignOperands does not align the operands anymore when BreakBeforeBinaryOperators is set... so, for

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

2017-05-17 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added inline comments. Comment at: unittests/Format/FormatTest.cpp:2476 "bool value = a\n" - " + a\n" - " +

[PATCH] D32477: [clang-format] Allow customizing the penalty for breaking assignment

2017-05-18 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 99415. Typz added a comment. Add test to verify the option actually has some effect https://reviews.llvm.org/D32477 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/TokenAnnotator.cpp unittests/Format/FormatTest.cpp Index:

[PATCH] D33282: clang-format: fix prefix for doxygen comments after member

2017-05-18 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. I don't have commit access, can someone please commit this patch? https://reviews.llvm.org/D33282 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33314: clang-format: Add option to remove semicolon at end of namespace

2017-05-18 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Herald added a subscriber: klimek. This option allows cleaning up namespace declaration, by removing the extra semicolon after namespace closing brace. https://reviews.llvm.org/D33314 Files: include/clang/Format/Format.h lib/Format/Format.cpp

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

2017-05-18 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 99421. Typz added a comment. clang-format: Add CompactNamespaces option - Change option name to CompactNamespaces - Clarify & test behavior when wrapping is needed - Separate from the 'remove semicolon' patch https://reviews.llvm.org/D32480 Files:

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

2017-05-18 Thread Francois Ferrand via Phabricator via cfe-commits
Typz marked 2 inline comments as done. Typz added inline comments. Comment at: include/clang/Format/Format.h:153 + /// \endcode + bool AllowSemicolonAfterNamespace; + Typz wrote: > djasper wrote: > > While I am not entirely opposed to this feature, I think it

[PATCH] D32524: [clang-format] Fix MatchingOpeningBlockLineIndex computation

2017-05-16 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. I tried to add some test, but could not find a simple way: I could not find any 'parser' tests from which to start, and I don't see with current master how this can be an issue (though it becomes an issue with some of other patches). Any hint how to implement some test?

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

2017-05-16 Thread Francois Ferrand via Phabricator via cfe-commits
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

[PATCH] D32477: [clang-format] Allow customizing the penalty for breaking assignment

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

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

2017-05-16 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] D32479: [clang-format] Add BreakConstructorInitializersBeforeColon option

2017-05-16 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. Or would it be better to replace (i.e. deprecate) the BreakConstructorInitializersBeforeComma option with a multiple choice option, as suggested here:

[PATCH] D32524: [clang-format] Fix MatchingOpeningBlockLineIndex computation

2017-05-16 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 99136. Typz added a comment. Reformat and remove unneeded comment https://reviews.llvm.org/D32524 Files: lib/Format/UnwrappedLineParser.cpp Index: lib/Format/UnwrappedLineParser.cpp === ---

[PATCH] D32479: [clang-format] Add BreakConstructorInitializersBeforeColon option

2017-05-16 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. ping? https://reviews.llvm.org/D32479 ___ 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] 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-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] D34395: clang-format: add options to merge empty record body

2017-06-20 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Herald added a subscriber: klimek. This patch introduces a few extra BraceWrapping options, similar to `SplitEmptyFunction`, to allow merging empty 'record' bodies (e.g. class, struct, union and namespace): - SplitEmptyClass - SplitEmptyStruct - SplitEmptyUnion -

[PATCH] D33491: clang-format: Fix C99 designated initializers corner cases

2017-06-19 Thread Francois Ferrand via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL305696: clang-format: Fix C99 designated initializers corner cases (authored by Typz). Changed prior to commit: https://reviews.llvm.org/D33491?vs=103018=103041#toc Repository: rL LLVM

[PATCH] D34399: clang-format: introduce InlineOnly short function style

2017-06-21 Thread Francois Ferrand via Phabricator via cfe-commits
Typz marked 3 inline comments as done. Typz added inline comments. Comment at: include/clang/Format/Format.h:234 + bool allowEmptyFunctionsOnASingleLine() const { + return AllowShortFunctionsOnASingleLine >= ShortFunctionStyle::SFS_Empty; djasper wrote:

[PATCH] D34399: clang-format: introduce InlineOnly short function style

2017-06-20 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Herald added subscribers: rengolin, klimek. This is the same as Inline, except it does not imply all empty functions are merged: with this style, empty functions are merged only if they also match the 'inline' criteria (i.e. defined in a class). This is helpful to

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

2017-06-20 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 103195. Typz added a comment. Rebase & fix indentation when newline is inserted after return or equal. https://reviews.llvm.org/D32478 Files: include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp lib/Format/ContinuationIndenter.h

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

2017-06-20 Thread Francois Ferrand via Phabricator via cfe-commits
Typz marked 7 inline comments as done. Typz added a comment. This style is used in the Skia project. https://reviews.llvm.org/D32478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D34395: clang-format: add options to merge empty record body

2017-06-20 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 103202. Typz added a comment. Enable merging records for Mozilla style https://reviews.llvm.org/D34395 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/FormatToken.h lib/Format/UnwrappedLineFormatter.cpp

[PATCH] D34399: clang-format: introduce InlineOnly short function style

2017-06-21 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 103357. Typz marked an inline comment as done. Typz added a comment. Fix according to review comments https://reviews.llvm.org/D34399 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/TokenAnnotator.cpp

[PATCH] D34238: clang-format: Do not binpack initialization lists

2017-06-19 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 103057. Typz added a comment. Fix indentation https://reviews.llvm.org/D34238 Files: lib/Format/ContinuationIndenter.cpp unittests/Format/FormatTest.cpp unittests/Format/FormatTestJava.cpp Index: unittests/Format/FormatTestJava.cpp

[PATCH] D34238: clang-format: Do not binpack initialization lists

2017-06-19 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 103056. Typz added a comment. Fix case where the content fits on a line, by wrapping after each comma, like this: static int types[] = { 0, 1, 2, }; https://reviews.llvm.org/D34238 Files:

[PATCH] D34399: clang-format: introduce InlineOnly short function style

2017-06-21 Thread Francois Ferrand via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL305912: clang-format: introduce InlineOnly short function style (authored by Typz). Changed prior to commit: https://reviews.llvm.org/D34399?vs=103357=103370#toc Repository: rL LLVM

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

2017-06-23 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. In https://reviews.llvm.org/D33589#789002, @alexfh wrote: > why do we want to make an exception for comments and not for regular code? This is not an exception for comments: the `PenaltyExcessCharacter` is used whenever the code is longer than the `ColumnLimit`, and used

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

2017-06-26 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 103935. Typz added a comment. Complete refactor to make the processing much more generic https://reviews.llvm.org/D33440 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/FormatToken.h lib/Format/FormatTokenLexer.cpp

[PATCH] D34395: clang-format: add options to merge empty record body

2017-06-26 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 103939. Typz added a comment. Merge `SplitEmptyClass/Struct/Union` options into a single `SplitEmptyRecord` option. https://reviews.llvm.org/D34395 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/FormatToken.h

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

2017-06-26 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 103937. Typz added a comment. Fix typo 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

[PATCH] D34395: clang-format: add options to merge empty record body

2017-06-26 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. I don't know if some style would want different styles, and I agree with you on principle; but since the brace wrapping is already configured for each kind of record, I choose to keep things consistent and have flags for each kind of record. But I can merge the options,

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

2017-06-23 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] D34238: clang-format: Do not binpack initialization lists

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

[PATCH] D34395: clang-format: add options to merge empty record body

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

[PATCH] D33285: clang-format: do not reflow bullet lists

2017-05-18 Thread Francois Ferrand via Phabricator via cfe-commits
Typz marked an inline comment as done. Typz added inline comments. Comment at: lib/Format/BreakableToken.cpp:313 + // Numbered lists may also start with a number followed by '.' + static const char *kNumberedListPattern = "^[0-9]+\\. "; + hasSpecialMeaningPrefix =

[PATCH] D32524: [clang-format] Fix MatchingOpeningBlockLineIndex computation

2017-05-18 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. Indeed, I don't have commit access. But I was wondering if I should not get it, to simplify landing the patches after review. I read http://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access about this, but I am still wondering what is considered a "track record of

[PATCH] D33285: clang-format: do not reflow bullet lists

2017-05-18 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 99436. Typz marked an inline comment as done. Typz added a comment. Limit to 2 digits and not break before a matching numbered list sequence followed by a fullstop, to avoid interpreting numbers at the end of sentence as numbered bullets (and thus preventing

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

2017-05-18 Thread Francois Ferrand via Phabricator via cfe-commits
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

[PATCH] D33285: clang-format: do not reflow bullet lists

2017-05-18 Thread Francois Ferrand via Phabricator via cfe-commits
Typz marked 3 inline comments as done. Typz added inline comments. Comment at: lib/Format/BreakableToken.cpp:313 + // Numbered lists may also start with a number followed by '.' + static const char *kNumberedListPattern = "^[0-9]+\\. "; + hasSpecialMeaningPrefix =

[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

[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 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

[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] 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.

[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:

[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 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

[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:

[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.

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

2017-05-19 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. In https://reviews.llvm.org/D32478#758258, @djasper wrote: > When you say "this doesn't happen in tests", do you mean this never happens > when there are parentheses around the expression? By 'test' I meant 'conditional construct' : it happens when there are parentheses

[PATCH] D32477: clang-format: Allow customizing the penalty for breaking assignment

2017-05-22 Thread Francois Ferrand via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL303534: clang-format: Allow customizing the penalty for breaking assignment (authored by Typz). Changed prior to commit: https://reviews.llvm.org/D32477?vs=99415=99720#toc Repository: rL LLVM

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

2017-05-22 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. Merging the 2 options is definitely a "safe" option, as it prevents ensures only the most obvious behavior is accessible. However, it has significant (IMO) drawbacks: - "Compact" is a not an namespace indentation type, this will make the option quite confusing - If

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

2017-05-22 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added inline comments. Comment at: include/clang/Format/Format.h:358 + /// \endcode + bool BinPackNamespaces; + djasper wrote: > Typz wrote: > > djasper wrote: > > > Typz wrote: > > > > djasper wrote: > > > > > This is not bin packing at all. Maybe

[PATCH] D33285: clang-format: do not reflow bullet lists

2017-05-22 Thread Francois Ferrand via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL303556: clang-format: do not reflow bullet lists (authored by Typz). Changed prior to commit: https://reviews.llvm.org/D33285?vs=99436=99764#toc Repository: rL LLVM https://reviews.llvm.org/D33285

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

2017-05-22 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 99763. Typz added a comment. Remove dependency on https://reviews.llvm.org/D33314 https://reviews.llvm.org/D32480 Files: include/clang/Format/Format.h lib/Format/Format.cpp lib/Format/NamespaceEndCommentsFixer.cpp lib/Format/UnwrappedLineFormatter.cpp

[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

[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

[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

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

2017-05-19 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added inline comments. Comment at: include/clang/Format/Format.h:358 + /// \endcode + bool BinPackNamespaces; + djasper wrote: > Typz wrote: > > djasper wrote: > > > This is not bin packing at all. Maybe CompactNamespaces? Or > > > SingleLineNamespaces?

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

2017-05-19 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 99575. Typz marked 11 inline comments as done. Typz edited the summary of this revision. Typz added a comment. update for review comments https://reviews.llvm.org/D32480 Files: include/clang/Format/Format.h lib/Format/Format.cpp

[PATCH] D32479: [clang-format] Add BreakConstructorInitializersBeforeColon option

2017-05-19 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 99553. Typz added a comment. Deprecate BreakConstructorInitializersBeforeComma and replace it with a more generic BreakConstructorInitializers option. https://reviews.llvm.org/D32479 Files: include/clang/Format/Format.h

[PATCH] D33314: clang-format: Add option to remove semicolon at end of namespace

2017-05-19 Thread Francois Ferrand via Phabricator via cfe-commits
Typz abandoned this revision. Typz added a comment. ATM, in the presence of semicolons, the closing braces will simply not be merged : i will check if I can handle this case easily (in the CompactNamespaces patch), and I'll abandon this one for now. https://reviews.llvm.org/D33314

[PATCH] D33285: clang-format: do not reflow bullet lists

2017-05-18 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 99427. Typz added a comment. - Use static regex to avoid recreating it each time - Add more tests https://reviews.llvm.org/D33285 Files: lib/Format/BreakableToken.cpp unittests/Format/FormatTestComments.cpp Index: unittests/Format/FormatTestComments.cpp

[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

[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

[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.

[PATCH] D34238: clang-format: Do not binpack initialization lists

2017-06-15 Thread Francois Ferrand via Phabricator via cfe-commits
Typz created this revision. Herald added a subscriber: klimek. This patch tries to avoid binpacking when initializing lists/arrays, to allow things like: static int types[] = { registerType1(), registerType2(), registerType3(), }; std::map x = { {

[PATCH] D34238: clang-format: Do not binpack initialization lists

2017-06-15 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. This patch is probably not complete, though it works fine in all situations I could think of: nested initializers, "short" statement (properly merged), column layout is still performed when needed... static int types[] = { SourcePrivate::registerTypes(),

  1   2   3   4   >