From bab661eeb707a6ee2046d330a6533ace00d17a7a Mon Sep 17 00:00:00 2001 From: Iwan Date: Fri, 21 Sep 2018 17:49:41 +0200 Subject: [PATCH] Improve printing of breaking layout with single fastpipe. Before: reactClass ->setNavigationOptions( NavigationOptions.t(~title="Title", ~gesturesEnabled=false, ()), ); After: reactClass->setNavigationOptions( NavigationOptions.t(~title="Title", ~gesturesEnabled=false, ()), ); --- .../unit_tests/expected_output/fastPipe.re | 53 +++++++++++++------ formatTest/unit_tests/input/fastPipe.re | 15 ++++++ src/reason-parser/reason_pprint_ast.ml | 47 ++++++++++++++-- 3 files changed, 97 insertions(+), 18 deletions(-) diff --git a/formatTest/unit_tests/expected_output/fastPipe.re b/formatTest/unit_tests/expected_output/fastPipe.re index 4a3a785d3..21ea6d7eb 100644 --- a/formatTest/unit_tests/expected_output/fastPipe.re +++ b/formatTest/unit_tests/expected_output/fastPipe.re @@ -156,22 +156,45 @@ ReasonReact.Router.watchUrl(url => Route.urlToRoute(url)->ChangeView->self.send ); -window -->Webapi.Dom.Window.open_( - ~url, - ~name="authWindow", - ~features=params, - ); +window->Webapi.Dom.Window.open_( + ~url, + ~name="authWindow", + ~features=params, +); + +window->Webapi.Dom.Window.open_( + ~url, + ~name="authWindow", + () => { + let x = 1; + let y = 2; + x + y; + }, +); + +reactClass->setNavigationOptions( + NavigationOptions.t( + ~title="Title", + ~gesturesEnabled=false, + (), + ), +); + +Foo.Bar.reactClass->setNavigationOptions( + NavigationOptions.t( + ~title="Title", + ~gesturesEnabled=false, + (), + ), +); -window -->Webapi.Dom.Window.open_( - ~url, - ~name="authWindow", - () => { - let x = 1; - let y = 2; - x + y; - }, +foo##bar +->setNavigationOptions( + NavigationOptions.t( + ~title="Title", + ~gesturesEnabled=false, + (), + ), );
diff --git a/formatTest/unit_tests/input/fastPipe.re b/formatTest/unit_tests/input/fastPipe.re index 138c3c8f1..cfa1c09a3 100644 --- a/formatTest/unit_tests/input/fastPipe.re +++ b/formatTest/unit_tests/input/fastPipe.re @@ -146,6 +146,21 @@ window->Webapi.Dom.Window.open_(~url, ~name="authWindow", ~features=params); window->Webapi.Dom.Window.open_(~url, ~name="authWindow", () => { let x = 1; let y = 2; x + y; }); +reactClass +->setNavigationOptions( + NavigationOptions.t(~title="Title", ~gesturesEnabled=false, ()), + ); + +Foo.Bar.reactClass +->setNavigationOptions( + NavigationOptions.t(~title="Title", ~gesturesEnabled=false, ()), + ); + +foo##bar +->setNavigationOptions( + NavigationOptions.t(~title="Title", ~gesturesEnabled=false, ()), + ); +
{items->Belt.Array.map(ReasonReact.string)->ReasonReact.array}
; a->(b->c); diff --git a/src/reason-parser/reason_pprint_ast.ml b/src/reason-parser/reason_pprint_ast.ml index 7a2309a62..be9f4429f 100644 --- a/src/reason-parser/reason_pprint_ast.ml +++ b/src/reason-parser/reason_pprint_ast.ml @@ -3673,7 +3673,7 @@ let printer = object(self:'self) } type t = node list - let formatNode ?(first=false) {exp; args; uncurried} = + let formatNode ?prefix ?(first=false) {exp; args; uncurried} = let formatLayout expr = let formatted = if first then self#ensureExpression ~reducesOnToken:(Token fastPipeToken) expr @@ -3700,7 +3700,11 @@ let printer = object(self:'self) | _ -> false in let layout = match args with - | [] -> formatLayout exp + | [] -> + let e = formatLayout exp in + (match prefix with + | Some l -> makeList [l; e] + | None -> e) | args -> let fakeApplExp = let loc_end = match List.rev args with @@ -3711,6 +3715,7 @@ let printer = object(self:'self) in makeList ( self#formatFunAppl + ?prefix ~jsxAttrs:[] ~args ~funExpr:exp @@ -3815,6 +3820,31 @@ let printer = object(self:'self) * [foo; ->f(a, b); ->g(c, d)] *) let pipeSegments = match pipetree with + (* Special case printing of + * foo->bar( + * aa, + * bb, + * ) + * + * We don't want + * foo + * ->bar( + * aa, + * bb + * ) + * + * Notice how `foo->bar` shouldn't break, it wastes space and is + * inconsistent with + * foo.bar( + * aa, + * bb, + * ) + *) + | [({exp = {pexp_desc = Pexp_ident _ }} as hd); last] -> + let prefix = Some ( + makeList [Fastpipetree.formatNode ~first:true hd; atom "->"] + ) in + [Fastpipetree.formatNode ?prefix last] | hd::tl -> let hd = Fastpipetree.formatNode ~first:true hd in let tl = List.map (fun node -> @@ -7422,7 +7452,17 @@ let printer = object(self:'self) | params -> makeTup ~uncurried (List.map self#label_x_expression_param params) - method formatFunAppl ~jsxAttrs ~args ~funExpr ~applicationExpr ?(uncurried=false) () = + (* + * Prefix represents an optional layout. When passed it will be "prefixed" to + * the funExpr. Example, given `bar(x, y)` with prefix `foo`, we get + * foobar(x,y). When the arguments break, the closing `)` is nicely aligned + * on the height of the prefix: + * foobar( + * x, + * y, + * ) --> notice how `)` sits on the height of `foo` instead of `bar` + *) + method formatFunAppl ?(prefix=(atom "")) ~jsxAttrs ~args ~funExpr ~applicationExpr ?(uncurried=false) () = let uncurriedApplication = uncurried in (* If there was a JSX attribute BUT JSX component wasn't detected, that JSX attribute needs to be pretty printed so it doesn't get @@ -7446,6 +7486,7 @@ let printer = object(self:'self) | Pexp_field _ -> self#unparseExpr funExpr | _ -> self#simplifyUnparseExpr funExpr in + let formattedFunExpr = makeList [prefix; formattedFunExpr] in begin match categorizeFunApplArgs args with | `LastArgIsCallback(callbackArg, args) -> (* This is the following case: