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

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] D33447: clang-format: add option to merge empty function body

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

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

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] D33589: clang-format: consider not splitting tokens in optimization

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] D33589: clang-format: consider not splitting tokens in optimization

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

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] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

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] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

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] D33285: clang-format: do not reflow bullet lists

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

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] D32480: clang-format: Add CompactNamespaces option

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

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] D32477: clang-format: Allow customizing the penalty for breaking assignment

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

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

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] D33314: clang-format: Add option to remove semicolon at end of namespace

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] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

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

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

Typz added a comment. I stumbled on the issue when working on the CompactNamespaces option, where the extra semicolon prevents merging the closing braces. There was an easy fix, which guaranteed that the closing braces would be properly merged, so I went for

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

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] D33285: clang-format: do not reflow bullet lists

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] D32524: [clang-format] Fix MatchingOpeningBlockLineIndex computation

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] D32525: [clang-format] Add SpaceBeforeColon option

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

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] D33285: clang-format: do not reflow bullet lists

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] D33314: clang-format: Add option to remove semicolon at end of namespace

Typz added a comment. Is this not the same reasoning as the whole NamespaceEndCommentsFixer? https://reviews.llvm.org/D33314 ___ 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

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] D32480: [clang-format] Add BinPackNamespaces option

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] D33314: clang-format: Add option to remove semicolon at end of namespace

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] D32477: [clang-format] Allow customizing the penalty for breaking assignment

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

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] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set

Typz added inline comments. Comment at: unittests/Format/FormatTest.cpp:2476 "bool value = a\n" - " + a\n" - " +

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

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] D32524: [clang-format] Fix MatchingOpeningBlockLineIndex computation

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] D33285: clang-format: do not reflow bullet lists

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] D33282: clang-format: fix prefix for doxygen comments after member

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] D32480: [clang-format] Add BinPackNamespaces option

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

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

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] D32477: [clang-format] Allow customizing the penalty for breaking assignment

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

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] D32525: [clang-format] Add SpaceBeforeColon option

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] D32524: [clang-format] Fix MatchingOpeningBlockLineIndex computation

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] D32524: [clang-format] Fix MatchingOpeningBlockLineIndex computation

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?

<    1   2   3   4