Skip to content

Commit b9f2f7b

Browse files
authored
fix: do not print > on new line (rescript-lang#678)
1 parent 5868f41 commit b9f2f7b

File tree

6 files changed

+133
-16
lines changed

6 files changed

+133
-16
lines changed

src/res_printer.ml

Lines changed: 85 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,21 @@ let hasNestedJsxOrMoreThanOneChild expr =
113113
in
114114
loop false expr
115115

116+
let hasTailSingleLineComment tbl loc =
117+
let rec getLastElement elements =
118+
match elements with
119+
| [] -> None
120+
| [element] -> Some element
121+
| _ :: rest -> getLastElement rest
122+
in
123+
match Hashtbl.find_opt tbl.CommentTable.trailing loc with
124+
| None -> false
125+
| Some comments -> (
126+
let lastComment = getLastElement comments in
127+
match lastComment with
128+
| None -> false
129+
| Some comment -> Comment.isSingleLineComment comment)
130+
116131
let hasCommentsInside tbl loc =
117132
match Hashtbl.find_opt tbl.CommentTable.inside loc with
118133
| None -> false
@@ -4041,8 +4056,20 @@ and printJsxExpression ~customLayout lident args cmtTbl =
40414056
Pexp_construct ({txt = Longident.Lident "[]"}, None);
40424057
}
40434058
when isSelfClosing ->
4044-
Doc.concat [Doc.line; Doc.text "/>"]
4045-
| _ -> Doc.concat [Doc.softLine; Doc.greaterThan]);
4059+
Doc.text "/>"
4060+
| _ ->
4061+
(* if last trailing comment of tag is single line comment then put > on the next line
4062+
<A
4063+
// single line comment
4064+
>
4065+
</A>
4066+
*)
4067+
if hasTailSingleLineComment cmtTbl lident.Asttypes.loc then
4068+
Doc.concat [Doc.softLine; Doc.greaterThan]
4069+
else
4070+
Doc.ifBreaks
4071+
(Doc.lineSuffix Doc.greaterThan)
4072+
Doc.greaterThan);
40464073
]);
40474074
(if isSelfClosing then Doc.nil
40484075
else
@@ -4140,6 +4167,27 @@ and printJsxChildren ~customLayout (childrenExpr : Parsetree.expression) ~sep
41404167

41414168
and printJsxProps ~customLayout args cmtTbl :
41424169
Doc.t * Parsetree.expression option =
4170+
(* This function was introduced because we have different formatting behavior for self-closing tags and other tags
4171+
we always put /> on a new line for self-closing tag when it breaks
4172+
<A
4173+
a=""
4174+
/>
4175+
4176+
<A
4177+
a="">
4178+
<B />
4179+
</A>
4180+
we should remove this function once the format is unified
4181+
*)
4182+
let isSelfClosing children =
4183+
match children with
4184+
| {
4185+
Parsetree.pexp_desc = Pexp_construct ({txt = Longident.Lident "[]"}, None);
4186+
pexp_loc = loc;
4187+
} ->
4188+
not (hasCommentsInside cmtTbl loc)
4189+
| _ -> false
4190+
in
41434191
let rec loop props args =
41444192
match args with
41454193
| [] -> (Doc.nil, None)
@@ -4151,13 +4199,42 @@ and printJsxProps ~customLayout args cmtTbl :
41514199
Pexp_construct ({txt = Longident.Lident "()"}, None);
41524200
} );
41534201
] ->
4202+
let doc = if isSelfClosing children then Doc.line else Doc.nil in
4203+
(doc, Some children)
4204+
| ((_, expr) as lastProp)
4205+
:: [
4206+
(Asttypes.Labelled "children", children);
4207+
( Asttypes.Nolabel,
4208+
{
4209+
Parsetree.pexp_desc =
4210+
Pexp_construct ({txt = Longident.Lident "()"}, None);
4211+
} );
4212+
] ->
4213+
let loc =
4214+
match expr.Parsetree.pexp_attributes with
4215+
| ({Location.txt = "ns.namedArgLoc"; loc}, _) :: _attrs ->
4216+
{loc with loc_end = expr.pexp_loc.loc_end}
4217+
| _ -> expr.pexp_loc
4218+
in
4219+
let tailSingleLineCommentPresent = hasTailSingleLineComment cmtTbl loc in
4220+
let propDoc = printJsxProp ~customLayout lastProp cmtTbl in
41544221
let formattedProps =
4155-
Doc.indent
4156-
(match props with
4157-
| [] -> Doc.nil
4158-
| props ->
4159-
Doc.concat
4160-
[Doc.line; Doc.group (Doc.join ~sep:Doc.line (props |> List.rev))])
4222+
Doc.concat
4223+
[
4224+
Doc.indent
4225+
(Doc.concat
4226+
[
4227+
Doc.line;
4228+
Doc.group
4229+
(Doc.join ~sep:Doc.line (propDoc :: props |> List.rev));
4230+
]);
4231+
(* print > on new line if last comment is single line comment *)
4232+
(match (isSelfClosing children, tailSingleLineCommentPresent) with
4233+
(* we always put /> on a new line when a self-closing tag breaks *)
4234+
| true, _ -> Doc.line
4235+
| false, true -> Doc.softLine
4236+
| false, false -> Doc.nil);
4237+
]
41614238
in
41624239
(formattedProps, Some children)
41634240
| arg :: args ->

tests/conversion/reason/expected/bracedJsx.res.txt

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,7 @@ let make = () => {
111111
<div
112112
className=Styles.terminal
113113
onClick={event => (event->ReactEvent.Mouse.target)["querySelector"]("input")["focus"]()}
114-
ref={containerRef->ReactDOMRe.Ref.domRef}
115-
>
114+
ref={containerRef->ReactDOMRe.Ref.domRef}>
116115
{state.history
117116
->Array.mapWithIndex((index, item) =>
118117
<div key={j`$index`} className=Styles.line>

tests/printer/comments/expected/jsx.res.txt

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,27 @@ module Cite = {
3131
<B /* comment3 */ />
3232
</A>
3333

34+
<A
35+
value=""
36+
/* comment */>
37+
<B />
38+
</A>
39+
40+
<A
41+
// comment
42+
>
43+
<B />
44+
</A>
45+
46+
<A
47+
/* comment */>
48+
<B />
49+
</A>
50+
51+
<A /* comment */>
52+
<B />
53+
</A>
54+
3455
<div>
3556
// Must not jump inside braces
3657
{React.string("Hello, World!")}

tests/printer/comments/jsx.res

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,29 @@ value=""
3333
<B /* comment3 */ />
3434
</A>
3535

36+
<A
37+
value=""
38+
/* comment */
39+
>
40+
<B/>
41+
</A>
42+
43+
<A
44+
// comment
45+
>
46+
<B />
47+
</A>
48+
49+
<A
50+
/* comment */
51+
>
52+
<B />
53+
</A>
54+
55+
<A /* comment */>
56+
<B />
57+
</A>
58+
3659
<div>
3760
// Must not jump inside braces
3861
{React.string("Hello, World!")}

tests/printer/expr/expected/jsx.res.txt

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,7 @@ let avatarSection =
104104
onMouseLeave={_ => setHoveringAdmin(false)}
105105
onClick={_e => {
106106
stopImpersonating(csrfToken)
107-
}}
108-
>
107+
}}>
109108
<Avatar user={viewer} size={45} />
110109
</div>
111110
: React.nullElement}

tests/printer/other/expected/signaturePicker.res.txt

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,7 @@ let make = () => {
2828
<div className="pr-2 font-bold text-gray-400 text-lg"> {"Signature"->string} </div>
2929
<select
3030
id="country"
31-
className="transition duration-150 ease-in-out sm:text-sm sm:leading-5 border-none font-bold text-2xl text-gray-600 bg-transparent"
32-
>
31+
className="transition duration-150 ease-in-out sm:text-sm sm:leading-5 border-none font-bold text-2xl text-gray-600 bg-transparent">
3332
{options
3433
->Belt.List.map(option =>
3534
<option key={option->TimeSignature.toString}>
@@ -48,8 +47,7 @@ let make = () => {
4847
fill="none"
4948
strokeLinecap="round"
5049
strokeLinejoin="round"
51-
className="text-gray-400"
52-
>
50+
className="text-gray-400">
5351
<polyline points="6 9 12 15 18 9" />
5452
</svg>
5553
</label>

0 commit comments

Comments
 (0)