-
Notifications
You must be signed in to change notification settings - Fork 38
fix: Avoid tail comments inside tag from being eaten #664
Changes from all commits
ffc7a35
60add8d
fc28cce
8fc5794
6efe3a5
8c4a6a9
6df4f3d
01d4475
5613ee8
7c9991c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
module Comment = Res_comment | ||
module Doc = Res_doc | ||
module ParsetreeViewer = Res_parsetree_viewer | ||
|
||
type t = { | ||
leading: (Location.t, Comment.t list) Hashtbl.t; | ||
|
@@ -1310,9 +1311,43 @@ and walkExpression expr t comments = | |
walkExpression callExpr t inside; | ||
after) | ||
in | ||
let afterExpr, rest = partitionAdjacentTrailing callExpr.pexp_loc after in | ||
attach t.trailing callExpr.pexp_loc afterExpr; | ||
walkList (arguments |> List.map (fun (_, e) -> ExprArgument e)) t rest | ||
if ParsetreeViewer.isJsxExpression expr then ( | ||
let props = | ||
arguments | ||
|> List.filter (fun (label, _) -> | ||
match label with | ||
| Asttypes.Labelled "children" -> false | ||
| Asttypes.Nolabel -> false | ||
| _ -> true) | ||
in | ||
let maybeChildren = | ||
arguments | ||
|> List.find_opt (fun (label, _) -> | ||
label = Asttypes.Labelled "children") | ||
in | ||
match maybeChildren with | ||
(* There is no need to deal with this situation as the children cannot be NONE *) | ||
| None -> () | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this not walking the props at all? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would you give me a hint under what circumstances this would happen? I tested some cases and found children were always there If there are no children elements inside, there will be
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about adding a comment saying just that. |
||
| Some (_, children) -> | ||
let leading, inside, _ = partitionByLoc after children.pexp_loc in | ||
if props = [] then | ||
(* All comments inside a tag are trailing comments of the tag if there are no props | ||
<A | ||
// comment | ||
// comment | ||
/> | ||
*) | ||
let afterExpr, _ = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happen to the second argument, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because it is an empty list. <A
// comment
// comment
>
</A> If there are no props, all comments after There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about adding a comment saying that. |
||
partitionAdjacentTrailing callExpr.pexp_loc after | ||
in | ||
attach t.trailing callExpr.pexp_loc afterExpr | ||
else | ||
walkList (props |> List.map (fun (_, e) -> ExprArgument e)) t leading; | ||
walkExpression children t inside) | ||
else | ||
let afterExpr, rest = partitionAdjacentTrailing callExpr.pexp_loc after in | ||
attach t.trailing callExpr.pexp_loc afterExpr; | ||
walkList (arguments |> List.map (fun (_, e) -> ExprArgument e)) t rest | ||
| Pexp_fun (_, _, _, _) | Pexp_newtype _ -> ( | ||
let _, parameters, returnExpr = funExpr expr in | ||
let comments = | ||
|
Uh oh!
There was an error while loading. Please reload this page.