Re: [PATCH] D19204: clang-format: [JS] generator and async functions.

2016-04-24 Thread Daniel Jasper via cfe-commits
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. http://reviews.llvm.org/D19204 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listin

Re: [PATCH] D18551: Added Fixer implementation and fix() interface in clang-format for removing redundant code.

2016-04-25 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: cfe/trunk/lib/Format/AffectedRangeManager.h:59 @@ +58,3 @@ + const AnnotatedLine *PreviousLine); + SourceManager &SourceMgr; + const SmallVector Ranges; And an empty line between functions and da

Re: [PATCH] D20632: clang-format: [JS] Support shebang lines on the very first line.

2016-05-25 Thread Daniel Jasper via cfe-commits
djasper added a comment. Thinking some more, I think this is actually very close to what we do for #include(-like) statements. That is done in TokenAnnotator::parseLine() and TokenAnnotator::parseIncludeDirective(). Could you move the logic there? http://reviews.llvm.org/D20632

Re: [PATCH] D20632: clang-format: [JS] Support shebang lines on the very first line.

2016-05-25 Thread Daniel Jasper via cfe-commits
djasper added a comment. That's the same for #include directives (with <>). Just turn the tokens into TT_ImplicitStringLiteral, same as is done for #includes. I am not saying it's better, but I don't think we should have to different approaches.. http://reviews.llvm.org/D20632 _

r270975 - clang-format: Allow splitting the line after /**/-comments.

2016-05-27 Thread Daniel Jasper via cfe-commits
Author: djasper Date: Fri May 27 03:59:34 2016 New Revision: 270975 URL: http://llvm.org/viewvc/llvm-project?rev=270975&view=rev Log: clang-format: Allow splitting the line after /**/-comments. While it might change the meaning of the comment in rare circumstances, it is better than violating the

Re: [PATCH] D20737: clang-format: [JS] fix async parsing.

2016-05-29 Thread Daniel Jasper via cfe-commits
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. http://reviews.llvm.org/D20737 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listin

Re: [PATCH] D20632: clang-format: [JS] Support shebang lines on the very first line.

2016-05-29 Thread Daniel Jasper via cfe-commits
djasper accepted this revision. djasper added a comment. Looks good. http://reviews.llvm.org/D20632 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20734: [clang-format] insert new #includes into correct blocks when cleaning up Replacement with cleanupAroundReplacements().

2016-05-29 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: lib/Format/Format.cpp:1411 @@ +1410,3 @@ +int getIncludePriority(const FormatStyle &Style, + SmallVector &CategoryRegexs, + StringRef IncludeName) { But you should pass it as an

r271191 - clang-format: Fix segfault introduced by allowing wraps after comments.

2016-05-29 Thread Daniel Jasper via cfe-commits
Author: djasper Date: Sun May 29 17:07:22 2016 New Revision: 271191 URL: http://llvm.org/viewvc/llvm-project?rev=271191&view=rev Log: clang-format: Fix segfault introduced by allowing wraps after comments. Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp cfe/trunk/unittests/Format/

Re: [PATCH] D20734: [clang-format] insert new #includes into correct blocks when cleaning up Replacement with cleanupAroundReplacements().

2016-05-30 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: include/clang/Format/Format.h:778 @@ -776,1 +777,3 @@ +/// replacement corresponding to the header insertion has offset UINT_MAX (i.e. +/// -1U). tooling::Replacements Why don't we just stick with UINT_MAX everywhere? =

Re: [PATCH] D20734: [clang-format] insert new #includes into correct blocks when cleaning up Replacement with cleanupAroundReplacements().

2016-05-30 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: lib/Format/Format.cpp:1284 @@ +1283,3 @@ + // if \p CheckMainHeader is true and \p IncludeName is a main header, returns + // 0. Otherwise, returns INT_MAX if \p IncludeName does not match any category + // pattern. N

Re: [PATCH] D20734: [clang-format] insert new #includes into correct blocks when cleaning up Replacement with cleanupAroundReplacements().

2016-05-31 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: lib/Format/Format.cpp:1549 @@ -1408,3 +1548,3 @@ // We need to use lambda function here since there are two versions of // `cleanup`. auto Cleanup = [](const FormatStyle &Style, StringRef Code, So, add a copy con

Re: [PATCH] D20734: [clang-format] insert new #includes into correct blocks when cleaning up Replacement with cleanupAroundReplacements().

2016-05-31 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: lib/Format/Format.cpp:1444 @@ +1443,3 @@ + if (!llvm::Regex(IncludeRegexPattern).match(Replace.getReplacementText())) { +llvm::errs() << "Insertions other than header #include insertion are " +"not supported! " --

Re: [PATCH] D20734: [clang-format] insert new #includes into correct blocks when cleaning up Replacement with cleanupAroundReplacements().

2016-05-31 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: unittests/Format/CleanupTest.cpp:310 @@ +309,3 @@ + Context.createInMemoryFile("fix.cpp", Code); + tooling::Replacements Replaces = { + tooling::Replacement("fix.cpp", UINT_MAX, 0, "#include \"b.h\"")}; Well, the o

Re: [PATCH] D20734: [clang-format] insert new #includes into correct blocks when cleaning up Replacement with cleanupAroundReplacements().

2016-05-31 Thread Daniel Jasper via cfe-commits
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. A few nitpicks, but otherwise looks good. Comment at: lib/Format/Format.cpp:1287 @@ +1286,3 @@ +int Ret = INT_MAX; +for (unsigned I = 0, E = CategoryRegexs.size(); I

Re: [PATCH] D20798: clang-format: [JS] Sort imported symbols.

2016-05-31 Thread Daniel Jasper via cfe-commits
djasper added a comment. In general, this is lacking test cases for imports that are wrapped over multiple lines to start with. Should probably add those both for the old and for the new behavior. Comment at: lib/Format/Format.cpp:1229 @@ -1227,9 +1228,3 @@ // the entire bl

Re: [PATCH] D20814: [include-fixer] Rank symbols based on the number of occurrences we found while merging.

2016-05-31 Thread Daniel Jasper via cfe-commits
djasper accepted this revision. This revision is now accepted and ready to land. Comment at: include-fixer/SymbolIndexManager.cpp:25 @@ +24,3 @@ + // First collect occurrences per header file. + std::map HeaderPopularity; + for (const SymbolInfo &Symbol : Symbols) { ---

Re: [PATCH] D20798: clang-format: [JS] Sort imported symbols.

2016-05-31 Thread Daniel Jasper via cfe-commits
djasper accepted this revision. This revision is now accepted and ready to land. Comment at: lib/Format/SortJavaScriptImports.cpp:248 @@ +247,3 @@ +// ... then the references in order ... +for (JsImportedSymbol *I = Symbols.begin(), *E = Symbols.end(); I != E;) { + Bu

Re: [PATCH] D20817: clang-format: [JS] no ASI on `import {x as\n y}`.

2016-05-31 Thread Daniel Jasper via cfe-commits
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. http://reviews.llvm.org/D20817 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listin

Re: [PATCH] D20858: [clang-format] make header guard identification stricter in header insertion.

2016-06-01 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: lib/Format/Format.cpp:1472 @@ -1471,2 +1471,3 @@ llvm::Regex IncludeRegex(IncludeRegexPattern); - llvm::Regex DefineRegex(R"(^[\t\ ]*#[\t\ ]*define[\t\ ]*[^\\]*$)"); + llvm::Regex IfNDefRegex(R"(^[\t\ ]*#[\t\ ]*ifndef[\t\ ]*([^\\]*)$

Re: [PATCH] D20898: [clang-format] skip empty lines and comments in the top of the code when inserting new headers.

2016-06-02 Thread Daniel Jasper via cfe-commits
djasper added a comment. Could we very easily use the FormatTokenLexer for this? I.e. find the first non-comment token an start from there? http://reviews.llvm.org/D20898 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org

Re: [PATCH] D20898: [clang-format] skip empty lines and comments in the top of the code when inserting new headers.

2016-06-03 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: lib/Format/Format.cpp:1484 @@ +1483,3 @@ + Environment::CreateVirtualEnvironment(Code, FileName, /*Ranges=*/{}); + FormatTokenLexer Lexer(Env->getSourceManager(), Env->getFileID(), Style, + encoding::detectEn

Re: [PATCH] D20898: [clang-format] skip empty lines and comments in the top of the code when inserting new headers.

2016-06-03 Thread Daniel Jasper via cfe-commits
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. http://reviews.llvm.org/D20898 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listin

Re: [PATCH] D20959: [clang-format] make header guard identification stricter (with Lexer).

2016-06-03 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: lib/Format/Format.cpp:1441 @@ +1440,3 @@ +void skipComments(Lexer &Lex, Token &Tok) { + while (!Lex.LexFromRawLexer(Tok)) +if (Tok.isNot(tok::comment)) I'd modify this to: while (Tok.is(comment)) if (Lex.LexFr

Re: [PATCH] D20959: [clang-format] make header guard identification stricter (with Lexer).

2016-06-06 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: lib/Format/Format.cpp:1491 @@ -1471,3 +1490,3 @@ std::unique_ptr Env = Environment::CreateVirtualEnvironment(Code, FileName, /*Ranges=*/{}); I think you should pull this part out into a separate function, poss

Re: [PATCH] D20959: [clang-format] make header guard identification stricter (with Lexer).

2016-06-06 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: lib/Format/Format.cpp:1466 @@ +1465,3 @@ +getFormattingLangOpts(Style)); + unsigned Ret = Code.size(); + Token Tok; Don't store this. Just inline Code.size() below. Comment at: lib/Format/F

Re: [PATCH] D20959: [clang-format] make header guard identification stricter (with Lexer).

2016-06-06 Thread Daniel Jasper via cfe-commits
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Nice :-) http://reviews.llvm.org/D20959 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/

r272124 - clang-format: Fix bug in function ref qualifier identification.

2016-06-08 Thread Daniel Jasper via cfe-commits
Author: djasper Date: Wed Jun 8 03:23:46 2016 New Revision: 272124 URL: http://llvm.org/viewvc/llvm-project?rev=272124&view=rev Log: clang-format: Fix bug in function ref qualifier identification. .. and simplify it. Before: void A::f()&& {} void f() && {} After: void A::f() && {} void

r272125 - clang-format: Fix incorrect calculation of "length" of /**/ comments.

2016-06-08 Thread Daniel Jasper via cfe-commits
Author: djasper Date: Wed Jun 8 04:45:08 2016 New Revision: 272125 URL: http://llvm.org/viewvc/llvm-project?rev=272125&view=rev Log: clang-format: Fix incorrect calculation of "length" of /**/ comments. This could lead to column limit violations. Modified: cfe/trunk/lib/Format/ContinuationI

Re: [PATCH] D21206: clang-format: [JS] recognized named functions in AnnotatingParser.

2016-06-09 Thread Daniel Jasper via cfe-commits
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. The change looks good, but can you add a before/after example to change description? http://reviews.llvm.org/D21206 ___ cfe-commits mailing li

Re: [PATCH] D21204: clang-format: [JS] post-fix non-null assertion operator.

2016-06-09 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:2127 @@ +2126,3 @@ +// Postfix non-null assertion operator, as in `foo!.bar()`. +if (Right.is(tok::exclaim) && Right.Next && +Right.Next->isNot(tok::identifier) && !Right.Next->Tok.isLiteral()

Re: [PATCH] D21204: clang-format: [JS] post-fix non-null assertion operator.

2016-06-09 Thread Daniel Jasper via cfe-commits
djasper accepted this revision. This revision is now accepted and ready to land. Comment at: lib/Format/TokenAnnotator.cpp:2128 @@ +2127,3 @@ +if (Right.is(tok::exclaim) && +(Left.isOneOf(tok::identifier, tok::r_paren, tok::r_square) || + Left.Tok.isLiteral()))

Re: [PATCH] D21273: clang-format: [JS] Introduce WrapJavaScriptImports option.

2016-06-13 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: include/clang/Format/Format.h:631 @@ +630,3 @@ + /// \brief Whether to wrap JavaScript import/export statements. + bool WrapJavaScriptImports; + Wondering whether we should call this JavaScriptWrapImports so that the J

r272538 - clang-format: Fix incorrect function type detection.

2016-06-13 Thread Daniel Jasper via cfe-commits
Author: djasper Date: Mon Jun 13 02:49:35 2016 New Revision: 272538 URL: http://llvm.org/viewvc/llvm-project?rev=272538&view=rev Log: clang-format: Fix incorrect function type detection. Before: returnsFunction (¶m1, ¶m2)(param); After: returnsFunction(¶m1, ¶m2)(param); Modified: cfe/tr

r272535 - clang-format: Don't indent lambda body relative to its return type.

2016-06-13 Thread Daniel Jasper via cfe-commits
Author: djasper Date: Mon Jun 13 02:48:45 2016 New Revision: 272535 URL: http://llvm.org/viewvc/llvm-project?rev=272535&view=rev Log: clang-format: Don't indent lambda body relative to its return type. Before: []() // -> int { return 1; // }; After: []() // -> in

r272536 - clang-format: Fix incorrect cast detection.

2016-06-13 Thread Daniel Jasper via cfe-commits
Author: djasper Date: Mon Jun 13 02:49:09 2016 New Revision: 272536 URL: http://llvm.org/viewvc/llvm-project?rev=272536&view=rev Log: clang-format: Fix incorrect cast detection. Before: auto s = sizeof...(Ts)-1; After: auto s = sizeof...(Ts) - 1; Modified: cfe/trunk/lib/Format/TokenAnno

r272537 - clang-format: Don't merge const and &, e.g. in function ref qualifiers.

2016-06-13 Thread Daniel Jasper via cfe-commits
Author: djasper Date: Mon Jun 13 02:49:28 2016 New Revision: 272537 URL: http://llvm.org/viewvc/llvm-project?rev=272537&view=rev Log: clang-format: Don't merge const and &, e.g. in function ref qualifiers. Before (when aligning & to the right): SomeType MemberFunction(const Deleted &) const&;

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

2016-06-13 Thread Daniel Jasper via cfe-commits
djasper added a comment. Could you upload a diff with full (i.e. the entire file as) context? Repository: rL LLVM http://reviews.llvm.org/D21279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinf

Re: [PATCH] D21273: clang-format: [JS] Introduce WrapJavaScriptImports option.

2016-06-13 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:789 @@ -788,3 +788,3 @@ if (Style.Language == FormatStyle::LK_JavaScript && -CurrentToken->is(Keywords.kw_import)) +CurrentToken->is(Keywords.kw_import) && !Style.JavaScriptWrapImports)

r272548 - clang-format: Restrict r272537 to function ref qualifiers.

2016-06-13 Thread Daniel Jasper via cfe-commits
Author: djasper Date: Mon Jun 13 09:45:12 2016 New Revision: 272548 URL: http://llvm.org/viewvc/llvm-project?rev=272548&view=rev Log: clang-format: Restrict r272537 to function ref qualifiers. Seems this isn't generally desirable. Before: int const * a; After: int const* a; Modified: c

Re: [PATCH] D21275: clang-format: [JS] Indent namespaces in JavaScript/TS by default.

2016-06-13 Thread Daniel Jasper via cfe-commits
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. http://reviews.llvm.org/D21275 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listin

Re: [PATCH] D21273: clang-format: [JS] Introduce WrapJavaScriptImports option.

2016-06-13 Thread Daniel Jasper via cfe-commits
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. http://reviews.llvm.org/D21273 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listin

r272654 - clang-format: [JS] Support annotated classes.

2016-06-14 Thread Daniel Jasper via cfe-commits
Author: djasper Date: Tue Jun 14 06:28:02 2016 New Revision: 272654 URL: http://llvm.org/viewvc/llvm-project?rev=272654&view=rev Log: clang-format: [JS] Support annotated classes. Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp cfe/trunk/unittests/Format/FormatTestJS.cpp Modified: cfe/

Re: [PATCH] D21323: [clang-format] do not add existing includes.

2016-06-14 Thread Daniel Jasper via cfe-commits
djasper added a comment. I am happy to get this in for now, but I actually think the right solution might be to let clang-format de-duplicate #includes in general. What do you think? http://reviews.llvm.org/D21323 ___ cfe-commits mailing list cfe-

Re: [PATCH] D21323: [clang-format] do not add existing includes.

2016-06-14 Thread Daniel Jasper via cfe-commits
djasper added a comment. Well, it is easy to not see duplicated #includes when they aren't properly sorted. When clang-format then goes in and sorts them, they end up next to each other. And I think no user really wants that. http://reviews.llvm.org/D21323 __

Re: [PATCH] D21323: [clang-format] do not add existing includes.

2016-06-14 Thread Daniel Jasper via cfe-commits
djasper added a comment. Lets get this in for now. We can always remove it if we think it is no longer useful. Comment at: unittests/Format/CleanupTest.cpp:692 @@ +691,3 @@ +TEST_F(CleanUpReplacementsTest, SkipExistingHeaders) { + std::string Code = "#include \"a.h\"\n#include

Re: [PATCH] D21323: [clang-format] do not add existing includes.

2016-06-14 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: unittests/Format/CleanupTest.cpp:693 @@ +692,3 @@ + std::string Code = "#include \"a.h\"\n#include \n"; + std::string Expected = "#include \"a.h\"\n#include \n"; + tooling::Replacements Replaces = {createInsertion("#include "), ---

r272668 - clang-format: [JS] Fix failing format with TypeScript casts.

2016-06-14 Thread Daniel Jasper via cfe-commits
Author: djasper Date: Tue Jun 14 08:54:38 2016 New Revision: 272668 URL: http://llvm.org/viewvc/llvm-project?rev=272668&view=rev Log: clang-format: [JS] Fix failing format with TypeScript casts. Before, this could be formatted at all (with BracketAlignmentStyle AlwaysBreak): foo = [ 1, /*

Re: [PATCH] D21323: [clang-format] do not add existing includes.

2016-06-14 Thread Daniel Jasper via cfe-commits
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good :-) http://reviews.llvm.org/D21323 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/lis

r273179 - clang-format: [Proto] Don't do bad things if imports are missing ; .

2016-06-20 Thread Daniel Jasper via cfe-commits
Author: djasper Date: Mon Jun 20 13:20:38 2016 New Revision: 273179 URL: http://llvm.org/viewvc/llvm-project?rev=273179&view=rev Log: clang-format: [Proto] Don't do bad things if imports are missing ;. Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp cfe/trunk/unittests/Format/Forma

r273196 - clang-format: [Proto] Fix "import public" after r273179.

2016-06-20 Thread Daniel Jasper via cfe-commits
Author: djasper Date: Mon Jun 20 15:39:53 2016 New Revision: 273196 URL: http://llvm.org/viewvc/llvm-project?rev=273196&view=rev Log: clang-format: [Proto] Fix "import public" after r273179. Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp cfe/trunk/unittests/Format/FormatTestProto.

Re: [PATCH] D21547: Added calculateRangesAfterReplaments() to calculate affacted ranges in the new code.

2016-06-21 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: include/clang/Tooling/Core/Replacement.h:68 @@ +67,3 @@ +/// \brief Less-than operator between two Ranges. +bool operator<(const Range &LHS, const Range &RHS); + Can we keep these private for now? Commen

Re: [PATCH] D21547: Added calculateRangesAfterReplaments() to calculate affacted ranges in the new code.

2016-06-21 Thread Daniel Jasper via cfe-commits
djasper added a comment. Nice :) Comment at: include/clang/Tooling/Core/Replacement.h:62 @@ +61,3 @@ + /// \brief Whether this range is less-than \p RHS or not. + bool operator<(const Range &RHS) const { +if (Offset != RHS.getOffset()) Can we just add a la

Re: [PATCH] D21547: Added calculateRangesAfterReplaments() to calculate affacted ranges in the new code.

2016-06-21 Thread Daniel Jasper via cfe-commits
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. A couple of small comments, otherwise looks good. Comment at: lib/Tooling/Core/Replacement.cpp:284 @@ +283,3 @@ + std::sort(Ranges.begin(), Ranges.end(), +[](co

r273285 - clang-format: [JS] Add a Closure Compiler JSDoc tags to the default

2016-06-21 Thread Daniel Jasper via cfe-commits
Author: djasper Date: Tue Jun 21 12:00:20 2016 New Revision: 273285 URL: http://llvm.org/viewvc/llvm-project?rev=273285&view=rev Log: clang-format: [JS] Add a Closure Compiler JSDoc tags to the default Google configuration so that it isn't line-wrapped. Modified: cfe/trunk/lib/Format/Format.c

Re: [PATCH] D21597: clang-format: [JS] recognize more type locations.

2016-06-22 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:480 @@ -471,1 +479,3 @@ + void parseJSTypeDefinition() { +// `type Name = Type Expression;` Why is this necessary? http://reviews.llvm.org/D21597 _

r273553 - clang-format: [Proto] Use more compact format for text-formatted options

2016-06-23 Thread Daniel Jasper via cfe-commits
Author: djasper Date: Thu Jun 23 04:40:19 2016 New Revision: 273553 URL: http://llvm.org/viewvc/llvm-project?rev=273553&view=rev Log: clang-format: [Proto] Use more compact format for text-formatted options Before: enum Type { UNKNOWN = 0 [(some_options) = { a: aa, b: bb }];

Re: [PATCH] D21597: clang-format: [JS] recognize more type locations.

2016-06-23 Thread Daniel Jasper via cfe-commits
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Basically looks good. Comment at: lib/Format/TokenAnnotator.cpp:155 @@ +154,3 @@ +} else if (Style.Language == FormatStyle::LK_JavaScript && Left->Previous && +

Re: [PATCH] D21658: clang-format: [JS] handle conditionals in fields, default params.

2016-06-23 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:1001 @@ -1000,3 +1000,3 @@ if (Style.Language == FormatStyle::LK_JavaScript && - Line.MustBeDeclaration) { + !Contexts.back().IsExpression) { // In JavaScript, `interface X {

Re: [PATCH] D21658: clang-format: [JS] handle conditionals in fields, default params.

2016-06-23 Thread Daniel Jasper via cfe-commits
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good except that the patch description can now be updated. http://reviews.llvm.org/D21658 ___ cfe-commits mailing list cfe-commits@lists.

Re: r273694 - clang-format: [JS] Fix build breakage.

2016-06-24 Thread Daniel Jasper via cfe-commits
The patch description seems wrong as this doesn't fix a build breakage AFAICT. Do you mean a test failure? If so, it would be helpful to #include what's actually changing (before/after or calling out the failing test case or something). On Fri, Jun 24, 2016 at 7:45 PM, Martin Probst via cfe-commi

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

2016-06-28 Thread Daniel Jasper via cfe-commits
djasper added a comment. Sorry.. Really busy at the moment, but will try to get this reviewed and submitted this week. If not, please ping again! Repository: rL LLVM http://reviews.llvm.org/D21279 ___ cfe-commits mailing list cfe-commits@lists.l

Re: [PATCH] D21603: [include-fixer] Fix namespace after inserting a missing header.

2016-06-28 Thread Daniel Jasper via cfe-commits
djasper added a comment. Sorry, I completely forgot about this. Will try to review today. Is this part about the patch description accurate? "This version only fixes the first discovered unidentified symbol. In the long run, include-fixer should fix all unidentified symbols with a same name at o

Re: [PATCH] D21603: [include-fixer] Fix namespace after inserting a missing header.

2016-06-30 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: include-fixer/IncludeFixer.cpp:234 @@ +233,3 @@ + std::string MinimizedFilePath = minimizeInclude( + ((FilePath[0] == '"' || FilePath[0] == '<') ? FilePath + : "\"" + File

Re: [PATCH] D21603: [include-fixer] Add missing namespace qualifiers after inserting a missing header.

2016-07-01 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: include-fixer/IncludeFixer.cpp:224 @@ +223,3 @@ + std::string MinimizedFilePath = minimizeInclude( + ((FilePath[0] == '"' || FilePath[0] == '<') ? FilePath + : "\"" + File

Re: [PATCH] D21603: [include-fixer] Add missing namespace qualifiers after inserting a missing header.

2016-07-06 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: include-fixer/IncludeFixer.cpp:166 @@ -159,1 +165,3 @@ QueryString = ExtendNestedNameSpecifier(Range); + SymbolRange = tooling::Range(SM.getDecomposedLoc(Range.getBegin()).second, + QueryStri

Re: [PATCH] D21603: [include-fixer] Add missing namespace qualifiers after inserting a missing header.

2016-07-07 Thread Daniel Jasper via cfe-commits
djasper accepted this revision. djasper added a comment. Looks good. http://reviews.llvm.org/D21603 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22147: clang-format: [JS] support trailing commas in imports.

2016-07-08 Thread Daniel Jasper via cfe-commits
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks Good. http://reviews.llvm.org/D22147 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listin

Re: [PATCH] D22146: clang-format: [JS] Sort imports case insensitive.

2016-07-08 Thread Daniel Jasper via cfe-commits
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. http://reviews.llvm.org/D22146 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listin

Re: r273694 - clang-format: [JS] Fix build breakage.

2016-07-12 Thread Daniel Jasper via cfe-commits
So, turns out top-level conditionals are actually used somewhat frequently. Can I undo this change? It doesn't break any other tests. Do you have other tests that I should run against? On Fri, Jun 24, 2016 at 11:48 PM, Martin Probst wrote: > Yes, test breakage. The problem was that with the chan

r275183 - clang-format: [JS] Allow top-level conditionals again.

2016-07-12 Thread Daniel Jasper via cfe-commits
Author: djasper Date: Tue Jul 12 10:45:53 2016 New Revision: 275183 URL: http://llvm.org/viewvc/llvm-project?rev=275183&view=rev Log: clang-format: [JS] Allow top-level conditionals again. I am not sure exactly which test breakage Martin was trying to fix in r273694. For now, fix the behavior for

Re: [PATCH] D22351: [include-fixer] Move cursor to #include line in vim after inserting a missing header.

2016-07-15 Thread Daniel Jasper via cfe-commits
djasper added a comment. FWIW, I think I'd like this behavior as it shows me what is actually being done. I can undo/redo to double-check or C-O to get back to where I was. Of course making it configurable seems like the right thing to do. Comment at: include-fixer/tool/clang-

Re: [PATCH] D22431: clang-format: [JS] nested and tagged template strings.

2016-07-18 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: lib/Format/FormatTokenLexer.cpp:231 @@ -230,3 +230,3 @@ void FormatTokenLexer::tryParseTemplateString() { FormatToken *BacktickToken = Tokens.back(); I think, this could now use an elaborate comment on what it is ac

r262216 - clang-format: Don't format unrelated nested blocks.

2016-02-29 Thread Daniel Jasper via cfe-commits
Author: djasper Date: Mon Feb 29 06:26:20 2016 New Revision: 262216 URL: http://llvm.org/viewvc/llvm-project?rev=262216&view=rev Log: clang-format: Don't format unrelated nested blocks. With this change: SomeFunction( [] { int i; return i; // Format this line. },

r262291 - clang-format: [JS] Support quoted object literal keys.

2016-02-29 Thread Daniel Jasper via cfe-commits
Author: djasper Date: Mon Feb 29 22:19:47 2016 New Revision: 262291 URL: http://llvm.org/viewvc/llvm-project?rev=262291&view=rev Log: clang-format: [JS] Support quoted object literal keys. Before: var x = { a: a, b: b, 'c': c, }; After: var x = { a: a, b: b, 'c': c, }

r262292 - clang-format: Increase the penalty for breaking between array subscripts.

2016-02-29 Thread Daniel Jasper via cfe-commits
Author: djasper Date: Mon Feb 29 22:19:55 2016 New Revision: 262292 URL: http://llvm.org/viewvc/llvm-project?rev=262292&view=rev Log: clang-format: Increase the penalty for breaking between array subscripts. Before: aa[aaa]

r262293 - clang-format: Correctly apply wrap before multi-line RHS rule to

2016-02-29 Thread Daniel Jasper via cfe-commits
Author: djasper Date: Mon Feb 29 22:19:59 2016 New Revision: 262293 URL: http://llvm.org/viewvc/llvm-project?rev=262293&view=rev Log: clang-format: Correctly apply wrap before multi-line RHS rule to ternary expressions. Before: return a ? a :

r262402 - [clang-format] Detect constructor initializers preceded by `noexcept`.

2016-03-01 Thread Daniel Jasper via cfe-commits
Author: djasper Date: Tue Mar 1 15:41:58 2016 New Revision: 262402 URL: http://llvm.org/viewvc/llvm-project?rev=262402&view=rev Log: [clang-format] Detect constructor initializers preceded by `noexcept`. Patch by Erik Kessler, thank you. Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp

Re: [PATCH] D17621: [clang-format] Detect constructor initializers preceded by `noexcept`.

2016-03-01 Thread Daniel Jasper via cfe-commits
djasper closed this revision. djasper added a comment. Submitted as r262402. Thank you. http://reviews.llvm.org/D17621 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17385: clang-format: [JS] re-quote single or double quoted strings.

2016-03-01 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: include/clang/Format/Format.h:606 @@ +605,3 @@ + /// \brief The QuoteStyle to use for JavaScript strings. + QuoteStyle Quotes; + As this only affects JavaScript and doesn't make any sense for Java or C++, I'd like to h

Re: [PATCH] D15643: [clang-format] Don't allow newline after uppercase Obj-C block return types

2016-03-01 Thread Daniel Jasper via cfe-commits
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Sorry for the delayed reply. I think this looks good. http://reviews.llvm.org/D15643 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

Re: [PATCH] D17385: clang-format: [JS] re-quote single or double quoted strings.

2016-03-01 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: lib/Format/Format.cpp:1131 @@ +1130,3 @@ +// and adjust ColumnWidth to take the added escapes into account. +FormatTok->ColumnWidth = ColumnWidth; + Maybe add a FIXME saying that we also need to modify the TokenTe

Re: r262232 - Implement new interfaces for code-formatting when applying replacements.

2016-03-01 Thread Daniel Jasper via cfe-commits
On Mon, Feb 29, 2016 at 8:49 AM, Manuel Klimek via cfe-commits < cfe-commits@lists.llvm.org> wrote: > On Mon, Feb 29, 2016 at 5:39 PM Chandler Carruth > wrote: > >> On Mon, Feb 29, 2016 at 11:32 AM Manuel Klimek via cfe-commits < >> cfe-commits@lists.llvm.org> wrote: >> >>> Author: klimek >>> Dat

Re: r262232 - Implement new interfaces for code-formatting when applying replacements.

2016-03-01 Thread Daniel Jasper via cfe-commits
On Tue, Mar 1, 2016 at 9:12 PM, Daniel Jasper wrote: > > > On Mon, Feb 29, 2016 at 8:49 AM, Manuel Klimek via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> On Mon, Feb 29, 2016 at 5:39 PM Chandler Carruth >> wrote: >> >>> On Mon, Feb 29, 2016 at 11:32 AM Manuel Klimek via cfe-commits <

Re: r262232 - Implement new interfaces for code-formatting when applying replacements.

2016-03-01 Thread Daniel Jasper via cfe-commits
On Tue, Mar 1, 2016 at 9:30 PM, Manuel Klimek wrote: > > > On Wed, Mar 2, 2016 at 6:20 AM Daniel Jasper wrote: > >> On Tue, Mar 1, 2016 at 9:12 PM, Daniel Jasper wrote: >> >>> >>> >>> On Mon, Feb 29, 2016 at 8:49 AM, Manuel Klimek via cfe-commits < >>> cfe-commits@lists.llvm.org> wrote: >>> >>>

Re: [PATCH] D17761: Added applyAllReplacementsAndFormat that works for multiple files.

2016-03-01 Thread Daniel Jasper via cfe-commits
djasper added a comment. I don't think this is the right abstraction: - Formatting so far is fundamentally per file. Grouping replacements per file is something that can be done outside of libFormat. - The logic to sort and deduplicate Replacements is important independent of whether formatting

r262487 - test/Driver/cl-pch-errorhandling.cpp: Copy input file to a temporary

2016-03-02 Thread Daniel Jasper via cfe-commits
Author: djasper Date: Wed Mar 2 08:26:59 2016 New Revision: 262487 URL: http://llvm.org/viewvc/llvm-project?rev=262487&view=rev Log: test/Driver/cl-pch-errorhandling.cpp: Copy input file to a temporary location as we cannot assume the location of the input file to be writable. Modified: cfe/

Re: r262487 - test/Driver/cl-pch-errorhandling.cpp: Copy input file to a temporary

2016-03-02 Thread Daniel Jasper via cfe-commits
Feel free to change. I have no idea what I am doing here. On Wed, Mar 2, 2016 at 12:55 PM, Nico Weber wrote: > (A different fix would've been to just add /Fp%.pch to set the output path > to be not next to the inputs.) > > On Wed, Mar 2, 2016 at 6:27 AM, Daniel Jasper via cf

Re: [PATCH] D17385: clang-format: [JS] re-quote single or double quoted strings.

2016-03-02 Thread Daniel Jasper via cfe-commits
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. Comment at: lib/Format/Format.cpp:1140 @@ +1139,3 @@ + + void insertReplacement(SourceLocation Start, unsigned Length, + StringRef Repla

r262534 - clang-format: [JS] Optionally re-quote string literals.

2016-03-02 Thread Daniel Jasper via cfe-commits
Author: djasper Date: Wed Mar 2 16:44:03 2016 New Revision: 262534 URL: http://llvm.org/viewvc/llvm-project?rev=262534&view=rev Log: clang-format: [JS] Optionally re-quote string literals. Turns "foo" into 'foo' (or vice versa, depending on configuration). This makes it more convenient to follow

Re: [PATCH] D17385: clang-format: [JS] re-quote single or double quoted strings.

2016-03-02 Thread Daniel Jasper via cfe-commits
djasper closed this revision. djasper added a comment. Submitted as r262534. http://reviews.llvm.org/D17385 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r262630 - clang-format: Use stable_sort when sorting #includes.

2016-03-03 Thread Daniel Jasper via cfe-commits
Author: djasper Date: Thu Mar 3 11:34:14 2016 New Revision: 262630 URL: http://llvm.org/viewvc/llvm-project?rev=262630&view=rev Log: clang-format: Use stable_sort when sorting #includes. Otherwise, clang-format can output useless replacements in the presence of identical #includes Modified:

Re: [PATCH] D17852: Added formatAndApplyAllReplacements that works on multiple files in libTooling.

2016-03-03 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: include/clang/Tooling/Core/Replacement.h:237 @@ +236,3 @@ +/// related to the same file entry are put into the same vector. +FileToReplacementsMap groupReplacementsByFile(const Replacements &Replaces, +

Re: [PATCH] D17852: Added formatAndApplyAllReplacements that works on multiple files in libTooling.

2016-03-04 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: include/clang/Tooling/Core/Replacement.h:228 @@ -226,3 +227,3 @@ /// \pre Replacements must be for the same file. -std::vector -calculateChangedRangesInFile(const tooling::Replacements &Replaces); +std::vector calculateChangedRangesInFil

r262776 - clang-format: [JS] Support destructuring assignments in for loops.

2016-03-05 Thread Daniel Jasper via cfe-commits
Author: djasper Date: Sat Mar 5 12:34:26 2016 New Revision: 262776 URL: http://llvm.org/viewvc/llvm-project?rev=262776&view=rev Log: clang-format: [JS] Support destructuring assignments in for loops. Before: for (let { a, b } of x) { } After: for (let {a, b} of x) { } Modified: cfe

Re: [PATCH] D17910: clang-format: [JS] Handle certain cases of ASI.

2016-03-06 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: lib/Format/UnwrappedLineParser.cpp:665 @@ +664,3 @@ +// well known cases. It *must not* return true in speculative cases. +bool UnwrappedLineParser::shouldInsertSemiBetween(FormatToken *Previous, +

Re: [PATCH] D17852: Added formatAndApplyAllReplacements that works on multiple files in libTooling.

2016-03-06 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: include/clang/Tooling/Core/Replacement.h:230 @@ +229,3 @@ + +typedef std::map +FileToReplacementsMap; ioeric wrote: > djasper wrote: > > I think the key type in a map is always const, so no need for "const". > I think

Re: [PATCH] D18015: Make functions in altivec.h be inline.

2016-03-11 Thread Daniel Jasper via cfe-commits
djasper added a comment. Want me to run clang-format on the file/diff? http://reviews.llvm.org/D18015 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18015: Make functions in altivec.h be inline.

2016-03-11 Thread Daniel Jasper via cfe-commits
djasper closed this revision. djasper added a comment. Formatted and submitted as r263302. http://reviews.llvm.org/D18015 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17910: clang-format: [JS] Handle certain cases of ASI.

2016-03-14 Thread Daniel Jasper via cfe-commits
djasper added a comment. Looks good. Comment at: lib/Format/UnwrappedLineParser.cpp:667 @@ +666,3 @@ +return true; + // FIXME(martinprobst): This returns true for C/C++ keywords like 'struct'. + return FormatTok->is(tok::identifier) && We don't usually put

<    1   2   3   4   5   6   7   8   >