From 3e81cd1a4a791034c843e5474af96e84b1beb1e9 Mon Sep 17 00:00:00 2001 From: Iwan Date: Fri, 21 Sep 2018 22:45:44 +0200 Subject: [PATCH] Improve printing of breaking layout with single fastpipe. (#2193) 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 898af1d24..b071a8006 100644 --- a/src/reason-parser/reason_pprint_ast.ml +++ b/src/reason-parser/reason_pprint_ast.ml @@ -3671,7 +3671,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 @@ -3698,7 +3698,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 @@ -3709,6 +3713,7 @@ let printer = object(self:'self) in makeList ( self#formatFunAppl + ?prefix ~jsxAttrs:[] ~args ~funExpr:exp @@ -3813,6 +3818,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 -> @@ -7420,7 +7450,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 @@ -7444,6 +7484,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: