[PATCH] D29396: [clang-format] Don't reflow lines starting with TODO, FIXME or XXX.

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. https://reviews.llvm.org/D29396 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D29300: [clang-format] Refactor WhitespaceManager and friends

djasper updated this revision to Diff 86404. djasper added a comment. Added assert. Removed unused InToken. https://reviews.llvm.org/D29300 Files: lib/Format/BreakableToken.cpp lib/Format/BreakableToken.h lib/Format/ContinuationIndenter.cpp lib/Format/ContinuationIndenter.h

[PATCH] D29300: [clang-format] Refactor WhitespaceManager and friends

djasper closed this revision. djasper added a comment. Submitted as r293616. https://reviews.llvm.org/D29300 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

djasper added a comment. Sorry, it took a bit longer, but I have now submitted those changes in r293616. Repository: rL LLVM https://reviews.llvm.org/D28462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D29383: [clang-format] Fix regression about not aligning trailing comments in case they were previously aligned, but at different indent.

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good https://reviews.llvm.org/D29383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D29323: [clang-format] Don't reflow comment lines starting with '@'.

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. https://reviews.llvm.org/D29323 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

djasper added a comment. I apologize in advance if this causes merge conflicts with r293616. However, I do hope that that actually makes this patch easier. https://reviews.llvm.org/D21279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D29300: [clang-format] Refactor WhitespaceManager and friends

djasper marked an inline comment as done. djasper added inline comments. Comment at: lib/Format/WhitespaceManager.cpp:178 - LastBlockComment = -} else if (Change.Kind == tok::unknown) { - if ((Change.StartOfBlockComment = LastBlockComment))

[PATCH] D27651: [clang-format] Even with AlignConsecutiveDeclarations, PointerAlignment: Right should keep *s and to the right

djasper added a comment. I have given stuff in WhitespaceManager access to the actual FormatToken in r293616. Hopefully that simplifies this patch. https://reviews.llvm.org/D27651 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D29329: [clang-format] Fix regression about adding leading whitespace to the content of line comments

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. https://reviews.llvm.org/D29329 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D24703: [clang-format] BreakBeforeBinaryOperations and AlignAfterOpenBracket conflict, bug 30304

djasper closed this revision. djasper added a comment. Submitted as r294179. Sorry I missed this before. https://reviews.llvm.org/D24703 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

djasper accepted this revision. djasper added a comment. This looks very nice now :-D. Thanks for working on this!! Comment at: lib/Format/WhitespaceManager.cpp:196 + + // ScopeStack keeps track of the current scope depth. + // We only run the "Matches" function on tokens

[PATCH] D29626: [clang-format] Break before a sequence of line comments aligned with the next line.

djasper added inline comments. Comment at: lib/Format/UnwrappedLineParser.cpp:2206 +const SmallVectorImpl , +const FormatToken* NextTok) { bool CommentsInCurrentLine = true; krasimir wrote: > Any suggestions on how to improve the code quality a bit if

[PATCH] D29634: clang-format: [JS] exclaim preceding regex literals.

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. https://reviews.llvm.org/D29634 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D29635: clang-format: [JS] handle parenthesized class expressions.

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Thanks https://reviews.llvm.org/D29635 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D29635: clang-format: [JS] handle parenthesized class expressions.

djasper added a comment. Sorry.. Should have caught this in the initial review. Still looks good. https://reviews.llvm.org/D29635 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29656: clang-format: [JS] correcly format object literal methods.

djasper accepted this revision. djasper added inline comments. This revision is now accepted and ready to land. Comment at: lib/Format/UnwrappedLineParser.cpp:1306 +// Could be a method inside of a braced list `{a() { return 1; }}`. +if (tryToParseBracedList()) {

[PATCH] D29716: [clang-format] Move OriginalPrefix from base to subclass.

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. lg https://reviews.llvm.org/D29716 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D29444: [clang-format] Fix breaking of comment sections in unwrapped lines containing newlines.

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Nice :) https://reviews.llvm.org/D29444 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D29450: [clang-format] Don't reflow across comment pragmas.

djasper added a comment. I am bit unsure about the design here. Could we instead match against the CommentPragmas and then not even create a BreakableToken (or reflow) if that matches? I guess that would make us unable to reflow if only part of the comment is a pragma, but that seems ok (for

[PATCH] D29450: [clang-format] Don't reflow across comment pragmas.

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Makes sense. https://reviews.llvm.org/D29450 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D29713: [clang-format] Move comment tests to their own file.

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. lg https://reviews.llvm.org/D29713 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

djasper added inline comments. Comment at: lib/Format/WhitespaceManager.cpp:190 +struct TokenTypeAndLevel { + TokenType Type; I don't think you need this struct now. Just use the FormatToken itself, it should have all of this information.

[PATCH] D29486: [clang-format] Re-align broken comment lines where appropriate.

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Thanks and sorry if I caused this :( https://reviews.llvm.org/D29486 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D29451: Add a prototype for clangd v0.1

djasper accepted this revision. djasper added a comment. Just a few nits. I think this looks like a great starting point! Comment at: clangd/ClangDMain.cpp:33 + llvm::make_unique(Outs, Logs, Store)); + // FIXME: textDocument/didClose + Dispatcher.registerHandler(

[PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

djasper added inline comments. Comment at: lib/Format/WhitespaceManager.cpp:207 + +if (i != Start) { + if (Changes[i].NestingAndIndentLevel > Merge the two ifs into a single one? Comment at: lib/Format/WhitespaceManager.cpp:318 + for

[PATCH] D29033: [clang-format] Fix LanguageKind comments.

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. Thanks! https://reviews.llvm.org/D29033 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D29039: Proposal for clang-format -r option

djasper added a comment. No problem :) Repository: rL LLVM https://reviews.llvm.org/D29039 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29039: Proposal for clang-format -r option

djasper added a comment. I am happy to let other people in the community weigh in, but I would not move forward with this patch. Listing directories is not a task that clang-format should do. It does not seem useful to me to add this functionality to basically every single tool that you might

[PATCH] D29186: clang-format: [JS] do not format MPEG transport streams.

djasper accepted this revision. djasper added inline comments. This revision is now accepted and ready to land. Comment at: unittests/Format/FormatTestJS.cpp:1002 +TEST_F(FormatTestJS, IgnoresMpegTS) { + char mpegTS[200]; + mpegTS[0] = 0x47; nit: Should be

[PATCH] D29291: [clang-format] Separate line comment sections after a right brace from comment sections in the scope.

djasper accepted this revision. djasper added inline comments. This revision is now accepted and ready to land. Comment at: lib/Format/UnwrappedLineParser.cpp:2095 + const FormatToken *MinColumnToken = Line.Tokens.front().Tok; + { +// Scan for '{//'. If found, use the

[PATCH] D29300: [clang-format] Refactor WhitespaceManager and friends

djasper created this revision. The main motivation behind this is to cleanup the WhitespaceManager and make it more extensible for future alignment etc. features. Specifically, WhitespaceManager has started to copy more and more code that is already present in FormatToken. Instead, I think it

[PATCH] D28844: clang-format: fix fallback style set to "none" not formatting

djasper accepted this revision. djasper added inline comments. This revision is now accepted and ready to land. Comment at: lib/Format/Format.cpp:1906 - // FIXME: If FallbackStyle is explicitly "none", format is disabled. - if (!getPredefinedStyle(FallbackStyle,

[PATCH] D30399: clang-format: [JS] whitespace after async in arrow functions.

djasper added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:2229 +if (Left.is(Keywords.kw_async) && Right.is(tok::l_paren) && +Right.MatchingParen && Right.MatchingParen->getNextNonComment() && +

[PATCH] D30452: Blacklist @mods and several other JSDoc tags from wrapping.

djasper added inline comments. Comment at: lib/Format/Format.cpp:627 +"(@(" +"const|" +"define|" I'd vote for making this "@\w*\ *{". We have seen incorrectly spelled version and such of this in the past.

[PATCH] D30452: Blacklist @mods and several other JSDoc tags from wrapping.

djasper accepted this revision. djasper added inline comments. This revision is now accepted and ready to land. Comment at: lib/Format/Format.cpp:624 +// taze:, and @tag followed by { for a lot of JSDoc tags. +GoogleStyle.CommentPragmas = "(taze:|(@[A-Za-z_0-9-]*[

[PATCH] D30528: [clang-format] Use number of unwrapped lines for short namespace

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Please include the reasoning in the patch description, i.e. that otherwise clang-format might need to runs to add all the namespace comments. https://reviews.llvm.org/D30528

[PATCH] D30492: [clang-format] Allow all but the first string literal in a sequence to be put on a newline

djasper added a comment. As discussed offline, I think this solves the wrong problem. My guess is that breakProtrudingToken checks State.Stack.back().NoLinebreak, but I forget to make it also check NoLinebreakInOperand. https://reviews.llvm.org/D30492

[PATCH] D30405: [clang-format] Add a new flag FixNamespaceEndComments to FormatStyle

djasper added inline comments. Comment at: include/clang/Format/Format.h:354 + /// fixes invalid existing ones. + bool FixNamespaceEndComments; + To be consistent with the clang-tidy check, just call this "FixNamespaceComments". After a change like this, you

[PATCH] D30405: [clang-format] Add a new flag FixNamespaceComments to FormatStyle

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. LG https://reviews.llvm.org/D30405 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

djasper added a comment. Could you please upload a diff with the entire file as context? That makes reviewing this easier. Comment at: docs/ClangFormatStyleOptions.rst:428 +**BreakBeforeInhertianceDelimiter** (``boolt``) + If ``true``, in the class inheritance expression

[PATCH] D27615: [clang-format] calculate MaxInsertOffset in the original code correctly.

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. https://reviews.llvm.org/D27615 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D27377: clang-format: Support the Java 8 'default' method modifier

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. Thank you! https://reviews.llvm.org/D27377 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D27440: clang-format-vsix: fail when clang-format outputs to stderr

djasper added a comment. I agree that fallback-style should only be used when there is no .clang-format file. If we find one, and it doesn't parse correctly, we should neither use the fallback style nor scan in higher-level directories (not sure whether we currently do that).

[PATCH] D27440: clang-format-vsix: fail when clang-format outputs to stderr

djasper added a comment. Yes.. return non-zero seems right. This is an error condition. https://reviews.llvm.org/D27440 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27377: clang-format: Support the Java 8 'default' method modifier

djasper added inline comments. Comment at: lib/Format/UnwrappedLineParser.cpp:297 case tok::kw_default: + if (Style.Language != FormatStyle::LK_Java || !Line->MustBeDeclaration) { +if (!SwitchLabelEncountered && Same as below.

[PATCH] D27651: [clang-format] Even with AlignConsecutiveDeclarations, PointerAlignment: Right should keep *s and to the right

djasper accepted this revision. djasper added a comment. Just some nits. Thanks for working on this! Comment at: lib/Format/WhitespaceManager.cpp:158 +static bool IsPointerOrReference(tok::TokenKind Kind) { + return Kind == tok::star || Kind == tok::amp || Kind ==

[PATCH] D28218: Small optimizations for SourceManager::getFileID()

djasper added a comment. Ping? https://reviews.llvm.org/D28218 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27383: Add a new clang-format style option ObjCBlockResetsIndent.

djasper added a comment. I think generally, this makes sense. Can you add tests in unittests/Format/FormatTest.cpp (probably next to TEST_F(FormatTest, FormatsBlocks) {..})? What's the behavior when there is more than one block? Also note that we have some requirements for additional options

[PATCH] D27211: [clang-format] Implement comment reflowing

djasper added inline comments. Comment at: unittests/Format/FormatTest.cpp:1787 +TEST_F(FormatTest, ReflowsComments) { + // Break a long line and reflow with the full next line. I think it is time that me moved a lot of the comment handling logic into a

[PATCH] D26909: [ClangFormat] Only insert #include into the #include block in the beginning of the file.

djasper added inline comments. Comment at: lib/Format/Format.cpp:1521 +// tokens and returns an offset after the sequence. +unsigned getOffsetAfterTokenSequence( +StringRef FileName, StringRef Code, const FormatStyle , I am somewhat hesitant to put more and

[PATCH] D26909: [ClangFormat] Only insert #include into the #include block in the beginning of the file.

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. Comment at: unittests/Format/CleanupTest.cpp:898 + +TEST_F(CleanUpReplacementsTest, CanInsertAfterComment) { + std::string Code = "#include \"a.h\"\n"

[PATCH] D27054: Introducing clang::tooling::EditList for refactoring tools.

djasper added inline comments. Comment at: include/clang/Tooling/Refactoring/EditList.h:41 + /// \brief Creates an edit list for a key position. + EditList(const SourceManager , SourceLocation KeyPosition); + I wonder whether we should always use a

[PATCH] D27054: Introducing clang::tooling::EditList for refactoring tools.

djasper added inline comments. Comment at: include/clang/Tooling/Refactoring/EditList.h:82-94 + /// \brief Adds a replacement that inserts \p Text at \p Loc. If this + /// insertion conflicts with an existing insertion (at the same position), + /// this will be inserted

[PATCH] D27054: Introducing clang::tooling::EditList for refactoring tools.

djasper added inline comments. Comment at: include/clang/Tooling/Refactoring/EditList.h:41 + /// \brief Creates an edit list for a key position. + EditList(const SourceManager , SourceLocation KeyPosition); + ioeric wrote: > ioeric wrote: > > klimek wrote: > >

[PATCH] D28207: Add second fast path for DiagnosticsEngine::GetDiagStatePointForLoc

djasper added inline comments. Comment at: lib/Basic/Diagnostic.cpp:179 + + // 2nd most frequent case: L is before the first diag state change. + FullSourceLoc FirstStateChangePos = DiagStatePoints[1].Loc; rsmith wrote: > It's surprising to me that this would

[PATCH] D28207: Add second fast path for DiagnosticsEngine::GetDiagStatePointForLoc

djasper added inline comments. Comment at: lib/Basic/Diagnostic.cpp:179 + + // 2nd most frequent case: L is before the first diag state change. + FullSourceLoc FirstStateChangePos = DiagStatePoints[1].Loc; rsmith wrote: > djasper wrote: > > rsmith wrote: > > >

[PATCH] D28208: Remove isIgnored()-test that is more expensive than the analysis behind it

djasper closed this revision. djasper added a comment. Landed as r290842. https://reviews.llvm.org/D28208 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28207: Add second fast path for DiagnosticsEngine::GetDiagStatePointForLoc

djasper created this revision. djasper added a reviewer: rsmith. djasper added a subscriber: cfe-commits. Herald added a subscriber: sanjoy. In many translation units I have tested, many of the calls to DiagnosticsEngine::GetDiagStatePointForLoc are for source locations before the first diag

[PATCH] D28218: Small optimizations for SourceManager::getFileID()

djasper created this revision. djasper added a reviewer: rsmith. djasper added a subscriber: cfe-commits. Specifically: - Separate one-entry cache for loaded and local files - Use bound that can be deduced from that cache for LessIndex - Address FIXME to use a faster alternative to

[PATCH] D28208: Remove isIgnored()-test that is more expensive than the analysis behind it

djasper created this revision. djasper added a reviewer: rsmith. djasper added a subscriber: cfe-commits. In many translation units I have tried, the calls to isIgnored() removed in this patch are more expensive than doing the analysis that is behind it. The speed-up in translation units I have

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

djasper added inline comments. Comment at: lib/Format/WhitespaceManager.cpp:287 SmallVector , +bool ConsiderScope, bool ConsiderCommas, unsigned StartAt) {

[PATCH] D30777: Added `applyAtomicChanges` function.

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good.. Very nice :) Comment at: lib/Tooling/Refactoring/AtomicChange.cpp:102 + Code.substr(StartPos, EndPos - StartPos).split(Lines, '\n'); + for (llvm::StringRef

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

djasper added inline comments. Comment at: lib/Format/WhitespaceManager.cpp:413 + + while (Param && !Param->is(tok::l_paren)) { +if (!Param->is(tok::identifier) && !Param->is(tok::comma)) enyquist wrote: > djasper wrote: > > I think you should be able to

[PATCH] D31334: [clang-format] Add options for indenting preprocessor directives

djasper added a comment. Thank you for working on this. Unfortunately, this is done at the wrong level: - You are using a separate pass, which means that as soon as the preprocessor directives exceed the column limit, they won't be wrapped correctly. - You aren't using Clang's Lexer to separate

[PATCH] D31408: Add more examples to clang-format configuration

djasper added a comment. Generally please upload diffs with more contexts. For some here it's not even clear to which option they refer ;) Comment at: docs/ClangFormatStyleOptions.rst:941 + ContinuationIndentWidth: 2 + ColumnLimit: 15 + You could

[PATCH] D30990: Add more examples to clang-format configuration

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good, just minor remarks. Comment at: include/clang/Format/Format.h:974 + ///# With: + ///MacroBlockBegin: "^\ + ///NS_MAP_BEGIN|\

[PATCH] D31698: clang-format: [JS] fix whitespace around "of" operator.

djasper accepted this revision. djasper added inline comments. This revision is now accepted and ready to land. Comment at: lib/Format/TokenAnnotator.cpp:2275 + tok::kw_const) || + // "of" can only occur in a for loop, as a "const x of y". +

[PATCH] D31575: [clang-format] Use configured IndentWidth instead of 2

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. looks good https://reviews.llvm.org/D31575 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D31706: [clang-format] Handle NSString literals by merging tokens.

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. Thanks for cleaning this up. https://reviews.llvm.org/D31706 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D31408: Add more examples to clang-format configuration

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Yes, thank you! https://reviews.llvm.org/D31408 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D27651: [clang-format] Even with AlignConsecutiveDeclarations, PointerAlignment: Right should keep *s and to the right

djasper added inline comments. Comment at: lib/Format/WhitespaceManager.cpp:255 +// If PointerAlignment is PAS_Right, keep *s or next to the token +if (Style.PointerAlignment == FormatStyle::PAS_Right && +Changes[i].Spaces != 0) { This needs to

[PATCH] D32298: [clang-format] Turn IncompleteFormat into a string

djasper added inline comments. Comment at: include/clang/Format/Format.h:1527 +/// non-recoverable syntax error. tooling::Replacements reformat(const FormatStyle , StringRef Code, ArrayRef Ranges, This is a public interface

[PATCH] D32298: [clang-format] Replace IncompleteFormat by a struct with Line

djasper added inline comments. Comment at: include/clang/Format/Format.h:1519 + /// formatted due to a non-recoverable syntax error. + bool IncompleteFormat = false; + Maybe we should invert this and make this FormatComplete or something? Otherwise this is

[PATCH] D32298: [clang-format] Replace IncompleteFormat by a struct with Line

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Sounds good. https://reviews.llvm.org/D32298 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

djasper added inline comments. Comment at: lib/Format/WhitespaceManager.cpp:431 + + // Special case for AlignTokens: for all other alignment cases, + // the current sequence is ended when a comma or a scope change enyquist wrote: >

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. A few nits, otherwise looks good. Comment at: include/clang/Format/Format.h:426 + /// \brief If ``true``, in the class inheritance expression clang-format will + ///

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

djasper added a comment. I think the patch is fine, except for the name of the flag. It is not breaking inheritance ;). Maybe BreakBeforeInhertianceColonAndComma, but that's pretty long still. I think maybe we can shorten this to BreakBeforeInhertianceComma, as it never makes sense to break

[PATCH] D30777: Added `applyAtomicChanges` function.

djasper added inline comments. Comment at: include/clang/Tooling/Refactoring/AtomicChange.h:139 + // kNone: Don't format anything. + // kViolations: Format lines exceeding 80 columns. + enum FormatOption { kAll, kNone, kViolations }; Should probably be

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

djasper added inline comments. Comment at: include/clang/Format/Format.h:309 + /// inheritance. + bool BreakInhertianceBeforeColonAndComma; + Hm. I am still not sure about this flag and it's name. Fundamentally, this is currently controlling two different

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

djasper added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:2666 return true; + if (Left.is(TT_InheritanceComma) && + Style.BreakBeforeInheritanceComma) Do these now fit on one line? Repository: rL LLVM

[PATCH] D30860: [clang-format] Add more examples and fix a bug in the py generation script

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. Comment at: docs/tools/dump_format_style.py:67 def __str__(self): -return '* ``%s`` %s' % (self.name, doxygen2rst(self.comment)) +return '\n* ``%s``

[PATCH] D30874: clang-format: [JS] do not wrap after interface and type.

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. https://reviews.llvm.org/D30874 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D30883: clang-format: [JS] do not wrap @see tags.

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. https://reviews.llvm.org/D30883 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D30575: [clang-format] Look at NoLineBreak and NoLineBreakInOperand before breakProtrudingToken

djasper added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:852 + bool CanBreakProtrudingToken = + State.Stack.empty() || (!State.Stack.back().NoLineBreak && + !State.Stack.back().NoLineBreakInOperand); I

[PATCH] D30740: Remove a useless subsitution in doxygen2rst which was incorrectly replacing * by \*

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. https://reviews.llvm.org/D30740 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D30575: [clang-format] Look at NoLineBreak and NoLineBreakInOperand before breakProtrudingToken

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. https://reviews.llvm.org/D30575 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D30734: Add more examples to clang-format configuration

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Please upload patches with full context or use arc (https://secure.phabricator.com/book/phabricator/article/arcanist/). Generally looks good, just minor comments.

[PATCH] D30532: Add examples to clang-format configuration

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. Thank you for doing this! https://reviews.llvm.org/D30532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D30659: [clang-format] Make NamespaceEndCommentFixer add at most one comment

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Would probably be interesting to add these test cases: #if A namespace A { #else namespace B { #endif int i; int j; }//namespace A and: namespace A { int i; int j;

[PATCH] D30688: [clang-format] Support namespaces ending in semicolon

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Minor nit, otherwise looks good. Comment at: lib/Format/NamespaceEndCommentsFixer.cpp:152 +const FormatToken *EndCommentNextTok = EndCommentPrevTok->Next; +if

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

djasper added inline comments. Comment at: include/clang/Format/Format.h:309 + /// inheritance. + bool BreakInhertianceBeforeColonAndComma; + Abpostelnicu wrote: > djasper wrote: > > Hm. I am still not sure about this flag and it's name. Fundamentally, this

[PATCH] D30532: Add examples to clang-format configuration

djasper added a comment. Sure, then go ahead. If these examples would have helped you, that's one datapoint :). As for presenting the difference in options, that would be useful I think. So if you are up to also doing that, that'd be appreciated. For bool options it seems easiest to do

[PATCH] D30532: Add examples to clang-format configuration

djasper added a comment. I agree, just generally we should aim for keeping these short. The example you gave might actually be reasonable to compare in two columns, i.e.: true: false: SomeClass::Constructor() vs. SomeClass::Constructor() : a(a),

[PATCH] D30532: Add examples to clang-format configuration

djasper added a comment. Hm. I don't actually know whether these examples are useful as is. You only present one setting of the value in most cases. What's interesting is actually how the flag changes the behavior. I mean in most cases, this can be derived from the example, but in those cases,

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

djasper added a comment. Do you know whether that is intentional? The style guide isn't really conclusive. Repository: rL LLVM https://reviews.llvm.org/D30487 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

djasper added a comment. Before `'`? Can you give an example? Repository: rL LLVM https://reviews.llvm.org/D30487 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

djasper added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:2486 Style.BreakConstructorInitializersBeforeComma && !Style.ConstructorInitializerAllOnOneLineOrOnePerLine) At any rate, I think this is what makes single-item ctor init

[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

djasper added a comment. Hm. Unfortunately, this seems to have been implemented to support Webkit style and Webkit style is explicit about wanting this (https://webkit.org/code-style-guidelines/) :(. But maybe the solution to that is to add an extra flag like AlwaysWrapInitializerList. Really

[PATCH] D30111: [clang-format] Add a test to check at once all the Mozilla coding style

djasper requested changes to this revision. djasper added a comment. This revision now requires changes to proceed. So, while it might be convenient to view this all in one file, a test here is not convenient for me (or presumably other clang-format developers) to work with. You can make a

  1   2   3   4   5   >