[PATCH] D32825: [clang-format] Improve understanding of combined typedef+record declarations

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. One nit, otherwise looks good. Comment at: lib/Format/UnwrappedLineFormatter.cpp:368 // We don't merge short records. - if (Line.First->isOneOf(tok::kw_class, t

[PATCH] D33006: clang-format: refine calculating brace types.

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

[PATCH] D33029: [clang-format] add option for dangling parenthesis

djasper added a comment. Please read and address: https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options https://reviews.llvm.org/D33029 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org

[PATCH] D32997: clang-format: [JS] keep triple slash directives intact.

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

[PATCH] D32475: [clang-format] Don’t propagate AvoidBinPacking into argument subexpressions

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Submitted as r302427. https://reviews.llvm.org/D32475 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mai

[PATCH] D32733: [clang-format] Convert AlignEscapedNewlinesLeft to an enum, adding DontAlign

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

[PATCH] D32864: clang-format: [JS] exponentiation operator

djasper accepted this revision. djasper added a comment. Thanks https://reviews.llvm.org/D32864 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32864: clang-format: [JS] exponentiation operator

djasper accepted this revision. djasper added inline comments. This revision is now accepted and ready to land. Comment at: unittests/Format/FormatTestJS.cpp:1791 +TEST_F(FormatTestJS, Exponentiation) { + verifyFormat("squared = x ** 2;"); +} Also make this hand

[PATCH] D32733: [clang-format] Convert AlignEscapedNewlinesLeft to an enum, adding DontAlign

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. This is an edge case in actual C++. An escaped newline literally gets expanded to nothing. So what this reads is #define Onetwo \ ... https://reviews.llvm.org/D32733 ___

[PATCH] D32733: [clang-format] Convert AlignEscapedNewlinesLeft to an enum, adding DontAlign

djasper added inline comments. Comment at: lib/Format/WhitespaceManager.cpp:523 + if (C.NewlinesBefore > 0 && C.ContinuesPPDirective) +C.EscapedNewlineColumn = C.PreviousEndOfTokenColumn + 2; +return; I think we should not duplicate this loop. Tw

[PATCH] D32590: clang-format: [JS] parse async function declarations.

djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. Comment at: lib/Format/UnwrappedLineParser.cpp:1043 - // Parse function literal unless 'function' is the first token in a line - // in which case th

[PATCH] D32475: [clang-format] Don’t propagate AvoidBinPacking into argument subexpressions

djasper added a comment. My point is though that even with only one argument, the BinPackArguments setting might lead to this bug. If not, that's good :). https://reviews.llvm.org/D32475 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:

[PATCH] D32475: [clang-format] Don’t propagate AvoidBinPacking into argument subexpressions

djasper added a comment. What happens if the function call where this happens actually does not have multiple parameters but one parameter with many operands, e.g. changing your test case to: arg3 + is + quite + long + so + it + f(arguments << of << its << su

[PATCH] D32531: clang-format: [JS] prevent wraps before class members.

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

[PATCH] D32532: clang-format: [JS/Java] ignore Objective-C constructs in JS & Java.

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

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

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

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

[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 &Style, StringRef Code, ArrayRef Ranges, This is a public interface

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

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

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

[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 https://reviews.llvm.org/D30487

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

[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 + /// brea

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

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

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

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

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

[PATCH] D30705: clang-format: [JS] allow breaking after non-null assertions.

djasper accepted this revision. djasper added inline comments. This revision is now accepted and ready to land. Comment at: lib/Format/TokenAnnotator.cpp:1056 +} else if (Current.is(tok::exclaim)) { + if (Style.Language == FormatStyle::LK_JavaScript) { +if (Curre

[PATCH] D30705: clang-format: [JS] allow breaking after non-null assertions.

djasper added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:2292 return false; -// Postfix non-null assertion operator, as in `foo!.bar()`. -if (Right.is(tok::exclaim) && (Left.isOneOf(tok::identifier, tok::r_paren, -

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

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

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

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

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

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

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

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

djasper added a comment. Please don't add this as is. I don't usually run the file-based tests in my development workflow and suspect that I might be breaking this a lot. If you want something like this, please add it as unittest(s) in unittests/Format/... (either in a new file or in an existin

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

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

[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] 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-]*[ \\t]*{)

[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. https://reviews.llvm.org/D30452

[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() && +Right.MatchingParen->getNextNonComment()->is(TT_JsFa

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

[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

<    1   2   3   4   5   >