Re: [PATCH] D21658: clang-format: [JS] handle conditionals in fields, default params.
This revision was automatically updated to reflect the committed changes. Closed by commit rL273619: clang-format: [JS] handle conditionals in fields, default params. (authored by mprobst). Changed prior to commit: http://reviews.llvm.org/D21658?vs=61726=61729#toc Repository: rL LLVM http://reviews.llvm.org/D21658 Files: cfe/trunk/lib/Format/TokenAnnotator.cpp cfe/trunk/unittests/Format/FormatTestJS.cpp Index: cfe/trunk/lib/Format/TokenAnnotator.cpp === --- cfe/trunk/lib/Format/TokenAnnotator.cpp +++ cfe/trunk/lib/Format/TokenAnnotator.cpp @@ -639,7 +639,7 @@ } // Declarations cannot be conditional expressions, this can only be part // of a type declaration. - if (Line.MustBeDeclaration && + if (Line.MustBeDeclaration && !Contexts.back().IsExpression && Style.Language == FormatStyle::LK_JavaScript) break; parseConditional(); @@ -1009,7 +1009,7 @@ Current.Type = TT_UnaryOperator; } else if (Current.is(tok::question)) { if (Style.Language == FormatStyle::LK_JavaScript && - Line.MustBeDeclaration) { + Line.MustBeDeclaration && !Contexts.back().IsExpression) { // In JavaScript, `interface X { foo?(): bar; }` is an optional method // on the interface, not a ternary expression. Current.Type = TT_JsTypeOptionalQuestion; Index: cfe/trunk/unittests/Format/FormatTestJS.cpp === --- cfe/trunk/unittests/Format/FormatTestJS.cpp +++ cfe/trunk/unittests/Format/FormatTestJS.cpp @@ -1253,7 +1253,6 @@ verifyFormat("interface X {\n" " y?(): z;\n" "}"); - verifyFormat("x ? 1 : 2;"); verifyFormat("constructor({aa}: {\n" " aa?: string,\n" " ?: string,\n" @@ -1350,5 +1349,14 @@ verifyFormat("let x = {foo: 1}!;\n"); } +TEST_F(FormatTestJS, Conditional) { + verifyFormat("y = x ? 1 : 2;"); + verifyFormat("x ? 1 : 2;"); + verifyFormat("class Foo {\n" + " field = true ? 1 : 2;\n" + " method(a = true ? 1 : 2) {}\n" + "}"); +} + } // end namespace tooling } // end namespace clang Index: cfe/trunk/lib/Format/TokenAnnotator.cpp === --- cfe/trunk/lib/Format/TokenAnnotator.cpp +++ cfe/trunk/lib/Format/TokenAnnotator.cpp @@ -639,7 +639,7 @@ } // Declarations cannot be conditional expressions, this can only be part // of a type declaration. - if (Line.MustBeDeclaration && + if (Line.MustBeDeclaration && !Contexts.back().IsExpression && Style.Language == FormatStyle::LK_JavaScript) break; parseConditional(); @@ -1009,7 +1009,7 @@ Current.Type = TT_UnaryOperator; } else if (Current.is(tok::question)) { if (Style.Language == FormatStyle::LK_JavaScript && - Line.MustBeDeclaration) { + Line.MustBeDeclaration && !Contexts.back().IsExpression) { // In JavaScript, `interface X { foo?(): bar; }` is an optional method // on the interface, not a ternary expression. Current.Type = TT_JsTypeOptionalQuestion; Index: cfe/trunk/unittests/Format/FormatTestJS.cpp === --- cfe/trunk/unittests/Format/FormatTestJS.cpp +++ cfe/trunk/unittests/Format/FormatTestJS.cpp @@ -1253,7 +1253,6 @@ verifyFormat("interface X {\n" " y?(): z;\n" "}"); - verifyFormat("x ? 1 : 2;"); verifyFormat("constructor({aa}: {\n" " aa?: string,\n" " ?: string,\n" @@ -1350,5 +1349,14 @@ verifyFormat("let x = {foo: 1}!;\n"); } +TEST_F(FormatTestJS, Conditional) { + verifyFormat("y = x ? 1 : 2;"); + verifyFormat("x ? 1 : 2;"); + verifyFormat("class Foo {\n" + " field = true ? 1 : 2;\n" + " method(a = true ? 1 : 2) {}\n" + "}"); +} + } // end namespace tooling } // end namespace clang ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21658: clang-format: [JS] handle conditionals in fields, default params.
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.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21658: clang-format: [JS] handle conditionals in fields, default params.
mprobst updated this revision to Diff 61726. mprobst added a comment. ok http://reviews.llvm.org/D21658 Files: lib/Format/TokenAnnotator.cpp unittests/Format/FormatTestJS.cpp Index: unittests/Format/FormatTestJS.cpp === --- unittests/Format/FormatTestJS.cpp +++ unittests/Format/FormatTestJS.cpp @@ -1247,7 +1247,6 @@ verifyFormat("interface X {\n" " y?(): z;\n" "}"); - verifyFormat("x ? 1 : 2;"); verifyFormat("constructor({aa}: {\n" " aa?: string,\n" " ?: string,\n" @@ -1344,5 +1343,14 @@ verifyFormat("let x = {foo: 1}!;\n"); } +TEST_F(FormatTestJS, Conditional) { + verifyFormat("y = x ? 1 : 2;"); + verifyFormat("x ? 1 : 2;"); + verifyFormat("class Foo {\n" + " field = true ? 1 : 2;\n" + " method(a = true ? 1 : 2) {}\n" + "}"); +} + } // end namespace tooling } // end namespace clang Index: lib/Format/TokenAnnotator.cpp === --- lib/Format/TokenAnnotator.cpp +++ lib/Format/TokenAnnotator.cpp @@ -631,7 +631,7 @@ } // Declarations cannot be conditional expressions, this can only be part // of a type declaration. - if (Line.MustBeDeclaration && + if (Line.MustBeDeclaration && !Contexts.back().IsExpression && Style.Language == FormatStyle::LK_JavaScript) break; parseConditional(); @@ -998,7 +998,7 @@ Current.Type = TT_UnaryOperator; } else if (Current.is(tok::question)) { if (Style.Language == FormatStyle::LK_JavaScript && - Line.MustBeDeclaration) { + Line.MustBeDeclaration && !Contexts.back().IsExpression) { // In JavaScript, `interface X { foo?(): bar; }` is an optional method // on the interface, not a ternary expression. Current.Type = TT_JsTypeOptionalQuestion; Index: unittests/Format/FormatTestJS.cpp === --- unittests/Format/FormatTestJS.cpp +++ unittests/Format/FormatTestJS.cpp @@ -1247,7 +1247,6 @@ verifyFormat("interface X {\n" " y?(): z;\n" "}"); - verifyFormat("x ? 1 : 2;"); verifyFormat("constructor({aa}: {\n" " aa?: string,\n" " ?: string,\n" @@ -1344,5 +1343,14 @@ verifyFormat("let x = {foo: 1}!;\n"); } +TEST_F(FormatTestJS, Conditional) { + verifyFormat("y = x ? 1 : 2;"); + verifyFormat("x ? 1 : 2;"); + verifyFormat("class Foo {\n" + " field = true ? 1 : 2;\n" + " method(a = true ? 1 : 2) {}\n" + "}"); +} + } // end namespace tooling } // end namespace clang Index: lib/Format/TokenAnnotator.cpp === --- lib/Format/TokenAnnotator.cpp +++ lib/Format/TokenAnnotator.cpp @@ -631,7 +631,7 @@ } // Declarations cannot be conditional expressions, this can only be part // of a type declaration. - if (Line.MustBeDeclaration && + if (Line.MustBeDeclaration && !Contexts.back().IsExpression && Style.Language == FormatStyle::LK_JavaScript) break; parseConditional(); @@ -998,7 +998,7 @@ Current.Type = TT_UnaryOperator; } else if (Current.is(tok::question)) { if (Style.Language == FormatStyle::LK_JavaScript && - Line.MustBeDeclaration) { + Line.MustBeDeclaration && !Contexts.back().IsExpression) { // In JavaScript, `interface X { foo?(): bar; }` is an optional method // on the interface, not a ternary expression. Current.Type = TT_JsTypeOptionalQuestion; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21658: clang-format: [JS] handle conditionals in fields, default params.
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 { foo?(): bar; }` is an optional method Well, if you keep the Line.MustBeDeclaration here, then the test case does not regress. I believe that the test in itself probably doesn't make much sense, but at least then we aren't regressing it. At any rate, it seems bad to keep the Line.MustBeDeclaration above and remove it here. http://reviews.llvm.org/D21658 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21658: clang-format: [JS] handle conditionals in fields, default params.
mprobst created this revision. mprobst added a reviewer: djasper. mprobst added a subscriber: cfe-commits. Herald added a subscriber: klimek. This causes a regression for top level ternary expressions, but that's deemed acceptable, as those really should be very rare. http://reviews.llvm.org/D21658 Files: lib/Format/TokenAnnotator.cpp unittests/Format/FormatTestJS.cpp Index: unittests/Format/FormatTestJS.cpp === --- unittests/Format/FormatTestJS.cpp +++ unittests/Format/FormatTestJS.cpp @@ -1247,7 +1247,6 @@ verifyFormat("interface X {\n" " y?(): z;\n" "}"); - verifyFormat("x ? 1 : 2;"); verifyFormat("constructor({aa}: {\n" " aa?: string,\n" " ?: string,\n" @@ -1344,5 +1343,14 @@ verifyFormat("let x = {foo: 1}!;\n"); } +TEST_F(FormatTestJS, Conditional) { + verifyFormat("y = x ? 1 : 2;"); + verifyFormat("x? 1 : 2;"); // Known issue: top level expression not detected. + verifyFormat("class Foo {\n" + " field = true ? 1 : 2;\n" + " method(a = true ? 1 : 2) {}\n" + "}"); +} + } // end namespace tooling } // end namespace clang Index: lib/Format/TokenAnnotator.cpp === --- lib/Format/TokenAnnotator.cpp +++ lib/Format/TokenAnnotator.cpp @@ -631,7 +631,7 @@ } // Declarations cannot be conditional expressions, this can only be part // of a type declaration. - if (Line.MustBeDeclaration && + if (Line.MustBeDeclaration && !Contexts.back().IsExpression && Style.Language == FormatStyle::LK_JavaScript) break; parseConditional(); @@ -998,7 +998,7 @@ Current.Type = TT_UnaryOperator; } else if (Current.is(tok::question)) { if (Style.Language == FormatStyle::LK_JavaScript && - Line.MustBeDeclaration) { + !Contexts.back().IsExpression) { // In JavaScript, `interface X { foo?(): bar; }` is an optional method // on the interface, not a ternary expression. Current.Type = TT_JsTypeOptionalQuestion; Index: unittests/Format/FormatTestJS.cpp === --- unittests/Format/FormatTestJS.cpp +++ unittests/Format/FormatTestJS.cpp @@ -1247,7 +1247,6 @@ verifyFormat("interface X {\n" " y?(): z;\n" "}"); - verifyFormat("x ? 1 : 2;"); verifyFormat("constructor({aa}: {\n" " aa?: string,\n" " ?: string,\n" @@ -1344,5 +1343,14 @@ verifyFormat("let x = {foo: 1}!;\n"); } +TEST_F(FormatTestJS, Conditional) { + verifyFormat("y = x ? 1 : 2;"); + verifyFormat("x? 1 : 2;"); // Known issue: top level expression not detected. + verifyFormat("class Foo {\n" + " field = true ? 1 : 2;\n" + " method(a = true ? 1 : 2) {}\n" + "}"); +} + } // end namespace tooling } // end namespace clang Index: lib/Format/TokenAnnotator.cpp === --- lib/Format/TokenAnnotator.cpp +++ lib/Format/TokenAnnotator.cpp @@ -631,7 +631,7 @@ } // Declarations cannot be conditional expressions, this can only be part // of a type declaration. - if (Line.MustBeDeclaration && + if (Line.MustBeDeclaration && !Contexts.back().IsExpression && Style.Language == FormatStyle::LK_JavaScript) break; parseConditional(); @@ -998,7 +998,7 @@ Current.Type = TT_UnaryOperator; } else if (Current.is(tok::question)) { if (Style.Language == FormatStyle::LK_JavaScript && - Line.MustBeDeclaration) { + !Contexts.back().IsExpression) { // In JavaScript, `interface X { foo?(): bar; }` is an optional method // on the interface, not a ternary expression. Current.Type = TT_JsTypeOptionalQuestion; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits