Skip to content
This repository was archived by the owner on Jun 15, 2023. It is now read-only.

fix: do not print > on new line #678

Merged
merged 6 commits into from
Oct 15, 2022
Merged

fix: do not print > on new line #678

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e2752f1
dont print > on new line
ah-yu Oct 14, 2022
53eae21
clean and comment
ah-yu Oct 14, 2022
2094a63
avoid walk props twice
ah-yu Oct 15, 2022
1b849e1
consider tail comments of tag if there are no props
ah-yu Oct 15, 2022
0dbb050
add more test case
ah-yu Oct 15, 2022
cb2f04c
add more test case
ah-yu Oct 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 40 additions & 1 deletion src/res_printer.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3978,6 +3978,38 @@ and printPexpApply ~customLayout expr cmtTbl =

and printJsxExpression ~customLayout lident args cmtTbl =
let name = printJsxName lident in
let hasTailSingleLineComment =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We seem to traverse the props multiple times. Do we fuse this logic with printJsxProps? There we walk the props one by one. We eventually arrive at the children, so we have all information inside printJsxProps, don't we?

printJsxProps also seems like a good place to try and print the closing >. What do you think of this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make sense to me. Will take a look later

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We better not print > in the printJsxProps function, because we have additional logic to determine whether the tag is self-closing, and then choose to print (> and </A>) or />

let props =
args
|> List.filter (fun (label, _) ->
match label with
| Asttypes.Labelled "children" -> false
| Asttypes.Nolabel -> false
| _ -> true)
in
let getLast elements =
match List.rev elements with
| [] -> None
| last :: _ -> Some last
in
let tailComment =
match getLast props with
| None -> None
| Some (_, expr) -> (
let loc =
match expr.Parsetree.pexp_attributes with
| ({Location.txt = "ns.namedArgLoc"; loc}, _) :: _attrs ->
{loc with loc_end = expr.pexp_loc.loc_end}
| _ -> expr.pexp_loc
in
match Hashtbl.find_opt cmtTbl.CommentTable.trailing loc with
| None -> None
| Some comments -> getLast comments)
in
match tailComment with
| None -> false
| Some comment -> Comment.isSingleLineComment comment
in
let formattedProps, children = printJsxProps ~customLayout args cmtTbl in
(* <div className="test" /> *)
let hasChildren =
Expand Down Expand Up @@ -4042,7 +4074,14 @@ and printJsxExpression ~customLayout lident args cmtTbl =
}
when isSelfClosing ->
Doc.concat [Doc.line; Doc.text "/>"]
| _ -> Doc.concat [Doc.softLine; Doc.greaterThan]);
| _ ->
(* print_endline (string_of_bool hasTailSingleLineComment); *)
Doc.concat
[
(if hasTailSingleLineComment then Doc.softLine
else Doc.nil);
Doc.greaterThan;
]);
]);
(if isSelfClosing then Doc.nil
else
Expand Down
3 changes: 1 addition & 2 deletions tests/conversion/reason/expected/bracedJsx.res.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,7 @@ let make = () => {
<div
className=Styles.terminal
onClick={event => (event->ReactEvent.Mouse.target)["querySelector"]("input")["focus"]()}
ref={containerRef->ReactDOMRe.Ref.domRef}
>
ref={containerRef->ReactDOMRe.Ref.domRef}>
{state.history
->Array.mapWithIndex((index, item) =>
<div key={j`$index`} className=Styles.line>
Expand Down
3 changes: 1 addition & 2 deletions tests/printer/expr/expected/jsx.res.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,7 @@ let avatarSection =
onMouseLeave={_ => setHoveringAdmin(false)}
onClick={_e => {
stopImpersonating(csrfToken)
}}
>
}}>
<Avatar user={viewer} size={45} />
</div>
: React.nullElement}
Expand Down
6 changes: 2 additions & 4 deletions tests/printer/other/expected/signaturePicker.res.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@ let make = () => {
<div className="pr-2 font-bold text-gray-400 text-lg"> {"Signature"->string} </div>
<select
id="country"
className="transition duration-150 ease-in-out sm:text-sm sm:leading-5 border-none font-bold text-2xl text-gray-600 bg-transparent"
>
className="transition duration-150 ease-in-out sm:text-sm sm:leading-5 border-none font-bold text-2xl text-gray-600 bg-transparent">
{options
->Belt.List.map(option =>
<option key={option->TimeSignature.toString}>
Expand All @@ -48,8 +47,7 @@ let make = () => {
fill="none"
strokeLinecap="round"
strokeLinejoin="round"
className="text-gray-400"
>
className="text-gray-400">
<polyline points="6 9 12 15 18 9" />
</svg>
</label>
Expand Down