[PATCH] D44831: [clang-format] Refine ObjC guesser to handle child lines of child lines

djasper added inline comments. Comment at: lib/Format/Format.cpp:1449 const AdditionalKeywords &Keywords) { +for (auto Line : AnnotatedLines) + if (LineContainsObjCCode(*Line, Keywords)) I would not create a second function her

[PATCH] D44816: [clang-format] Do not insert space before closing brace in ObjC dict literal

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Generally looks good, one minor simplification. Comment at: lib/Format/TokenAnnotator.cpp:2484 + if (Right.is(tok::r_brace) && Right.MatchingParen && + Right.Matching

[PATCH] D44831: [clang-format] Refine ObjC guesser to handle child lines of child lines

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

[PATCH] D44816: [clang-format] Do not insert space before closing brace in ObjC dict literal

djasper accepted this revision. djasper added a comment. Yeah, it's one of these things where neither way would be totally intuitive to everyone. Repository: rC Clang https://reviews.llvm.org/D44816 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D45004: [clang-format] New style option IndentWrappedObjCMethodNames

djasper added a comment. Do we have a public style guide that explicitly says to do this? That's a requirement for new style options as per https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options. Also, are we sure that somebody wants the other behavior? Does tha

[PATCH] D44994: [clang-format] Ensure wrapped ObjC selectors with 1 arg obey IndentWrappedFunctionNames

djasper added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:899 if (!State.Stack.back().ObjCSelectorNameFound) { + unsigned MinIndent = + (Style.IndentWrappedFunctionNames I think I'd now find this slightly easier to read as:

[PATCH] D45004: [clang-format] New style option IndentWrappedObjCMethodNames

djasper added a comment. Well, I disagree. It says: "If you break after the return type of a function declaration or definition, do not indent." Repository: rC Clang https://reviews.llvm.org/D45004 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D44994: [clang-format] Ensure wrapped ObjC selectors with 1 arg obey IndentWrappedFunctionNames

djasper added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:904 + : State.Stack.back().Indent); if (NextNonComment->LongestObjCSelectorName == 0) +return MinIndent; benhamilton wrote: > djasper wrote: > > Does this

[PATCH] D44996: [clang-format] Ensure ObjC selectors with 0 args are annotated correctly

djasper added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:1347 +} else if (Current.isOneOf(tok::identifier, tok::kw_new) && + Current.Previous && Current.Previous->is(TT_CastRParen) && + Current.Previous->MatchingParen && ---

[PATCH] D44994: [clang-format] Ensure wrapped ObjC selectors with 1 arg obey IndentWrappedFunctionNames

djasper accepted this revision. djasper added a comment. Looks good. Repository: rC Clang https://reviews.llvm.org/D44994 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45169: [clang-format/ObjC] Do not detect "[]" as ObjC method expression

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. I like option 2 :). Repository: rC Clang https://reviews.llvm.org/D45169 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45168: [clang-format/ObjC] Do not insert space after opening brace of ObjC dict literal

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

[PATCH] D45185: [clang-format] Support lightweight Objective-C generics

djasper added inline comments. Comment at: lib/Format/UnwrappedLineParser.cpp:2135 +nextToken(); +if (FormatTok->Tok.is(tok::less)) + NumOpenAngles++; The UnwrappedLineParser is very much about error recovery. Implemented like this, it will consume

[PATCH] D45004: [clang-format] New style option IndentWrappedObjCMethodNames

djasper added a comment. I still really believe that these config options do no make sense and are actively confusing. I see two options: - We leave this as is - We fix this right The right fix here is (IMO) that a style already is per language and thus already has a member specifying the lan

[PATCH] D44996: [clang-format] Ensure ObjC selectors with 0 args are annotated correctly

djasper accepted this revision. djasper added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:1347 +} else if (Current.isOneOf(tok::identifier, tok::kw_new) && + Current.Previous && Current.Previous->is(TT_CastRParen) && + Current.Previo

[PATCH] D45185: [clang-format] Support lightweight Objective-C generics

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good, thank you! Repository: rC Clang https://reviews.llvm.org/D45185 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://l

[PATCH] D45004: [clang-format] New style option IndentWrappedObjCMethodNames

djasper added a comment. I'd go to great lengths to avoid adding new config options and so I don't think this would be a bad trade-off to make. Also, note that you might not actually have to change much. A FormatStyle already contains a reference to the FormatStyleSet it was created from and yo

[PATCH] D44609: [Clang-Format] New option BreakBeforeLambdaBody to manage lambda line break inside function parameter call

djasper added a comment. Please read: https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options In this case in particular, I would be very interested in a style guide that defines how Allman brace style and lambdas work together. IMO, it has so many corner ca

[PATCH] D45004: [clang-format] New style option IndentWrappedObjCMethodNames

djasper added a comment. Ok, you know the ObjC community much better than I do. If you think that adding the indentation for ObjC irrespective of the option works for most people, I am happy to go with it. Repository: rC Clang https://reviews.llvm.org/D45004 _

[PATCH] D39478: [clang-format] Handle leading comments in using declaration

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Some minor remarks, but generally looks good. Thanks for fixing this! Comment at: lib/Format/UsingDeclarationsSorter.cpp:136 for (size_t I = 0, E = AnnotatedLines.size()

[PATCH] D39587: [clang-format] Handle unary operator overload with arguments and specifiers

djasper added a comment. Looks good, thank you. https://reviews.llvm.org/D39587 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39587: [clang-format] Handle unary operator overload with arguments and specifiers

djasper added a comment. Submitted as r317473. Thank you! https://reviews.llvm.org/D39587 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39478: [clang-format] Handle leading comments in using declaration

djasper accepted this revision. djasper added a comment. Looks good. Do you have submit access? https://reviews.llvm.org/D39478 ___ 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

djasper added a comment. Out of curiosity, will this be able to fix the two situations that you get for python extension? There, you usually have a PyObject_HEAD with out semicolon in a struct and than a PyObject_HEAD_INIT(..) in a braced init list. More info: http://starship.python.net/crew/mwh

[PATCH] D39806: [clang-format] Support python-style comments in text protos

djasper accepted this revision. djasper added inline comments. This revision is now accepted and ready to land. Comment at: lib/Format/FormatTokenLexer.cpp:344 + size_t To = Lex->getBuffer().find_first_of('\n', From); + if (To == StringRef::npos) To = Lex->getBuffer().size(); +

[PATCH] D39478: [clang-format] Handle leading comments in using declaration

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

[PATCH] D40178: clang-format: [JS] remove trailing lines in arrow functions.

djasper added a comment. Is this different for C++ lambdas? I would think that we never should add an empty line before the "}" of a child block. https://reviews.llvm.org/D40178 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

[PATCH] D40178: clang-format: [JS] remove trailing lines in arrow functions.

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. There is a chance that some people do not want this in their coding style. But if so, we can add an option later. https://reviews.llvm.org/D40178

[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). https://reviews.

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

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

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

[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] D27651: [clang-format] Even with AlignConsecutiveDeclarations, PointerAlignment: Right should keep *s and &s 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 == tok::ampam

[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|\ I'd

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

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

djasper added inline comments. Comment at: lib/Format/WhitespaceManager.cpp:287 SmallVector &Changes, +bool ConsiderScope, bool ConsiderCommas, unsigned StartAt) { I don't find

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

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

[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 http://lists.llvm.org/cgi-bin/mailman/listin

[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] 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 http://lists.l

[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 http://lists.llvm.org/cgi-bin/mailman/l

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

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

[PATCH] D32170: Add a FixItHint for -Wmissing-prototypes to insert 'static '.

djasper added inline comments. Comment at: lib/Sema/SemaDecl.cpp:11652 static bool ShouldWarnAboutMissingPrototype(const FunctionDecl *FD, - const FunctionDecl*& PossibleZeroParamPrototype) { + const FunctionDecl*& Possible

[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: > > enyquist wrote: > > > djasper wro

[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] D28462: clang-format: Add new style option AlignConsecutiveMacros

djasper added inline comments. Comment at: Format/FormatToken.h:148 + /// \brief Whether the token is the final token in the identifier of a PP + // macro. This will be either 1) the identifier token following the 'define' This adds a lot of code, runtime and

[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, Style.Languag

[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 http://lists.llvm.org/cgi-bin/mailm

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

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

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

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

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

djasper marked an inline comment as done. djasper added inline comments. Comment at: lib/Format/UnwrappedLineFormatter.cpp:904-907 +void UnwrappedLineFormatter::formatFirstToken(const AnnotatedLine &Line, const AnnotatedLine *Previo

[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 lib/Format

[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 http://lists.llvm.org/cgi-bin/mai

[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 http://lists.llvm.org/cgi-bin/mailman/listi

[PATCH] D27651: [clang-format] Even with AlignConsecutiveDeclarations, PointerAlignment: Right should keep *s and &s 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 http://lists

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

[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 = &Change; -} else if (Change.Kind == tok::unknown) { - if ((Change.StartOfBlockComment = LastBlockComment))

[PATCH] D29326: [clang-format] Fix reflow in block comment lines with leading whitespace.

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. I don't know. I could also make an argument that more than one space at the beginning of a line should just stop the reflow. E.g. I could see people writing paragraphs like this: /* *

[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 http://lists.llvm.org/cgi-bin/mailman/listi

[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 http://lists.llvm.org/cgi-bin/mailman/listin

[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 http://lists.llvm.org/cgi-bin/mailman/listi

[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 http://lists.llvm.org/cgi-bin/mailman/listinfo/

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

[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 http://lists.llvm.org/cgi-bin/mailman/list

[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 http://lists.llvm.

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

[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 http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi

[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 &Comments, +const FormatToken* NextTok) { bool CommentsInCurrentLine = true; krasimir wrote: > Any suggestions on how to improve the code quality

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

[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 http://lists.llvm.org/cgi-bin/mailman/listi

[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 http://lists.llvm.org/cgi-bin/mailman/listinfo/c

[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] 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 http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c

[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 http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c

[PATCH] D34623: [clang-format] Add a test for associative map proto buffer fields

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/D34623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/ma

[PATCH] D34621: [clang-format] Support <>-style proto message fields

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

[PATCH] D33932: [clang-format] Add support for case-insensitive header matching and use it to improve support for LLVM-style include sorting.

djasper added a comment. Just make clang-format always do this. I don't think anyone is relying on the current behavior. https://reviews.llvm.org/D33932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/l

[PATCH] D33932: [clang-format] Add support for case-insensitive header matching and use it to improve support for LLVM-style include sorting.

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/D33932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/ma

[PATCH] D34749: [clang-format] Fix parsing of msg{field}-style proto options

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

[PATCH] D34824: clang-format: add an option -verbose to list the files being processed

djasper added a comment. Can you add a test? Tests for this kind of behavior of the clang-format command line go in test/Format/. Comment at: tools/clang-format/ClangFormat.cpp:380 +if (Verbose.getNumOccurrences() != 0) +outs() << "Formatting " << FileNames[0] << '

[PATCH] D34441: [clang-format] Support text proto messages

djasper added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:71 + const FormatStyle &Style) { + assert(LessTok.is(tok::less)); + return Style.Language == FormatStyle::LK_TextProto || Maybe rename to opensP

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

djasper 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, /*NewLine=*/f

[PATCH] D35015: [clang-format] Add space between a message field key and the opening bracket in proto messages

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

[PATCH] D34225: [clang-format] Teach clang-format how to handle C++ coroutines

djasper added inline comments. Comment at: lib/Format/Format.cpp:1958 LangOpts.DeclSpecKeyword = 1; // To get __declspec. + LangOpts.CoroutinesTS = 1; return LangOpts; Can we change this only if Style.isCpp()? We should probably do that for other things,

[PATCH] D34225: [clang-format] Teach clang-format how to handle C++ coroutines

djasper added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:2267 + return true; +if (Right.is(tok::l_paren) && Left.is(tok::kw_co_await) && +Left.Previous && Left.Previous->is(tok::kw_operator)) EricWF wrote: > djasper wrote: > > I k

[PATCH] D34824: clang-format: add an option -verbose to list the files being processed

djasper added a comment. Generally upload diffs with full context to phabricator. That makes reviewing much easier. Comment at: test/Format/verbose.cpp:2 +// RUN: clang-format %s -verbose | FileCheck %s +// CHECK: Formatting + This seems like a pretty minimal

[PATCH] D35296: [clang-format] Keep level of comment before an empty line

djasper added a comment. I still think we should make this dependent on the current indentation. Why do you think this is better? https://reviews.llvm.org/D35296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/

<    1   2   3   4   5   >