Unify comment formatting inside the empty blocks #647
Conversation
| @@ -28,6 +28,10 @@ type t = /* c0 */ string /* c1 */ as 'x // after | |||
| // Ptyp_poly | |||
| type fn = {f: /* c0 */ 'a /* c1 */ 'b /* c2 */. /* c3 */ string /* c4 */} | |||
|
|
|||
| type fn = {/* comment1 */ | |||
There was a problem hiding this comment.
Can we tweak the output to look more like the input:
type fn = {
/* comment1 */
// comment2
}
| @@ -107,6 +107,11 @@ let user = /* before */ { | |||
| /* c4 */ age /* c5 */: /* c6 */ 31 /* c7 */, | |||
| } // after | |||
|
|
|||
| // Pexp_record with empty record | |||
| let user = /* before */ {/* comment1 */ | |||
There was a problem hiding this comment.
The comments inside look a bit suboptimal, can we go for:
{
/* comment1 */
// comment2
}
|
Also, I just fixed CI, please rebase on latest master to get CI to run. |
ah-yu
changed the title
| <A | ||
| // Comment | ||
| /> | ||
| <A> |
There was a problem hiding this comment.
Fine with me! That would make it easier to add code after the comment anyway.
|
Has the scope of this PR changed? |
|
Yes. Now this PR is for adjusting the comments format inside an empty block(list, record, etc) |
Would you add an entry in the CHANGELOG.md as this is user-visible. |
There was a problem hiding this comment.
If a user wrote [/* a */], can we keep it on one line? Similar for the other "empty nodes".
We tend to look at the style of the author: did he write it over multiple lines or not?
- If it is on one line, keep it on one line (unless it breaks the column width).
- If it is written over multiple lines, force break it over multiple lines.
|
That sounds reasonable, will fix it later. |
src/res_printer.ml
Outdated
| Doc.rbrace; | ||
| ]) | ||
| let doc = printCommentsInside cmtTbl modType.pmty_loc in | ||
| if Doc.isNil doc then |
There was a problem hiding this comment.
This does the job but looks like tight coupling at a distance.
When is it that printCommentsInside currently produces Nil, and can we get hold of that information instead (e.g. return an optional).
There was a problem hiding this comment.
Yes, in an ideal world, we shouldn't inspect the contents of a doc to make formatting decisions. The information we go off is the AST (+ comments).
@ah-yu is the isNil absolutely necessary?
There was a problem hiding this comment.
isNil is only used in the development phase, then I could do a quick test without too many code changes.
Forgot to clean it up and will do it later.
src/res_printer.ml
Outdated
| @@ -113,6 +113,12 @@ let hasNestedJsxOrMoreThanOneChild expr = | |||
| in | |||
| loop false expr | |||
|
|
|||
| let hasCommentsInside tbl loc = | |||
| match Hashtbl.find tbl.CommentTable.inside loc with | |||
|
@ah-yu just ping when this is ready. |
|
@cristianoc done |
Thanks a lot for contributing this! |
Unify the format of comments inside the empty block