Skip to content

Commit 86b092c

Browse files
authored
chore(grainfmt): Consolidate comment logic by removing assumed breaks (#1659)
chore(grainfmt): Consolidate comment_to_loc logic by removing assumed breaks
1 parent b447ced commit 86b092c

File tree

4 files changed

+146
-58
lines changed

4 files changed

+146
-58
lines changed

compiler/src/formatting/comment_utils.re

Lines changed: 99 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -265,23 +265,10 @@ let get_comments_to_end_of_line =
265265

266266
let comment_to_doc = (comment: Parsetree.comment) => {
267267
let comment_string = Comments.get_comment_source(comment);
268-
let newline =
269-
switch (comment) {
270-
| Line(_)
271-
| Shebang(_) => Doc.hardLine
272-
| Doc(_) => Doc.hardLine
273-
| _ => Doc.nil
274-
};
275-
// this is needed for a couple of reasons. cmt_content doesn't include the comment delimiters (// or /*)
268+
// We use cmt_source over cmt_content, which doesn't include the comment delimiters (// or /*)
276269
// if we use cmt_source, it passes through the newline for line comments, which we don't want here
277270
// we want our own line/hardline formatting blocks
278271

279-
Doc.concat([Doc.text(String.trim(comment_string)), newline]);
280-
};
281-
282-
let nobreak_comment_to_doc = (comment: Parsetree.comment) => {
283-
let comment_string = Comments.get_comment_source(comment);
284-
285272
Doc.text(String.trim(comment_string));
286273
};
287274

@@ -329,6 +316,12 @@ let rec comments_inner =
329316
| [] => [Doc.nil]
330317
| [cmt, ...rem] => [
331318
comment_to_doc(cmt),
319+
switch (cmt) {
320+
| Line(_)
321+
| Shebang(_) => Doc.hardLine
322+
| Doc(_) => Doc.hardLine
323+
| _ => Doc.nil
324+
},
332325
...comments_inner(~prev=cmt, rem),
333326
]
334327
}
@@ -347,6 +340,12 @@ let rec comments_inner =
347340
| 0 => [
348341
Doc.space,
349342
comment_to_doc(cmt),
343+
switch (cmt) {
344+
| Line(_)
345+
| Shebang(_) => Doc.hardLine
346+
| Doc(_) => Doc.hardLine
347+
| _ => Doc.nil
348+
},
350349
...comments_inner(~prev=cmt, rem),
351350
]
352351

@@ -356,6 +355,12 @@ let rec comments_inner =
356355
| _ => Doc.hardLine
357356
},
358357
comment_to_doc(cmt),
358+
switch (cmt) {
359+
| Line(_)
360+
| Shebang(_) => Doc.hardLine
361+
| Doc(_) => Doc.hardLine
362+
| _ => Doc.nil
363+
},
359364
...comments_inner(~prev=cmt, rem),
360365
]
361366
| _ => [
@@ -367,6 +372,12 @@ let rec comments_inner =
367372
},
368373
Doc.hardLine,
369374
comment_to_doc(cmt),
375+
switch (cmt) {
376+
| Line(_)
377+
| Shebang(_) => Doc.hardLine
378+
| Doc(_) => Doc.hardLine
379+
| _ => Doc.nil
380+
},
370381
...comments_inner(~prev=cmt, rem),
371382
]
372383
};
@@ -418,33 +429,55 @@ let rec trailing_comments_inner =
418429
| None =>
419430
switch (comments) {
420431
| [] => []
421-
| [cmt] => [nobreak_comment_to_doc(cmt)]
432+
| [cmt] => [comment_to_doc(cmt)]
422433
| [cmt, ...rem] => [
423434
comment_to_doc(cmt),
435+
switch (cmt) {
436+
| Line(_)
437+
| Shebang(_) => Doc.hardLine
438+
| Doc(_) => Doc.hardLine
439+
| _ => Doc.nil
440+
},
424441
...trailing_comments_inner(~prev=cmt, rem),
425442
]
426443
}
427444
| Some(prev_cmt) =>
445+
let (_, prev_line, _, _) =
446+
Locations.get_raw_pos_info(
447+
Locations.get_comment_loc(prev_cmt).loc_end,
448+
);
449+
428450
switch (comments) {
429451
| [] => []
430-
| [cmt, ...rem] =>
431-
let (_, prev_line, _, _) =
432-
Locations.get_raw_pos_info(
433-
Locations.get_comment_loc(prev_cmt).loc_end,
434-
);
452+
| [cmt] =>
435453
let (_, this_line, _, _) =
436454
Locations.get_raw_pos_info(Locations.get_comment_loc(cmt).loc_start);
437455

438-
let comment_printer =
439-
switch (rem) {
440-
| [] => nobreak_comment_to_doc
441-
| _ => comment_to_doc
442-
};
456+
switch (this_line - prev_line) {
457+
| 0 => [Doc.space, comment_to_doc(cmt)]
458+
| 1 => [
459+
switch (prev_cmt) {
460+
| Line(_) => Doc.nil
461+
| _ => Doc.hardLine
462+
},
463+
comment_to_doc(cmt),
464+
]
465+
| _ => [Doc.hardLine, comment_to_doc(cmt)]
466+
};
467+
| [cmt, ...rem] =>
468+
let (_, this_line, _, _) =
469+
Locations.get_raw_pos_info(Locations.get_comment_loc(cmt).loc_start);
443470

444471
switch (this_line - prev_line) {
445472
| 0 => [
446473
Doc.space,
447-
comment_printer(cmt),
474+
comment_to_doc(cmt),
475+
switch (cmt) {
476+
| Line(_)
477+
| Shebang(_) => Doc.hardLine
478+
| Doc(_) => Doc.hardLine
479+
| _ => Doc.nil
480+
},
448481
...trailing_comments_inner(~prev=cmt, rem),
449482
]
450483

@@ -453,16 +486,28 @@ let rec trailing_comments_inner =
453486
| Line(_) => Doc.nil
454487
| _ => Doc.hardLine
455488
},
456-
comment_printer(cmt),
489+
comment_to_doc(cmt),
490+
switch (cmt) {
491+
| Line(_)
492+
| Shebang(_) => Doc.hardLine
493+
| Doc(_) => Doc.hardLine
494+
| _ => Doc.nil
495+
},
457496
...trailing_comments_inner(~prev=cmt, rem),
458497
]
459498
| _ => [
460499
Doc.hardLine,
461-
comment_printer(cmt),
500+
comment_to_doc(cmt),
501+
switch (cmt) {
502+
| Line(_)
503+
| Shebang(_) => Doc.hardLine
504+
| Doc(_) => Doc.hardLine
505+
| _ => Doc.nil
506+
},
462507
...trailing_comments_inner(~prev=cmt, rem),
463508
]
464509
};
465-
}
510+
};
466511
};
467512
};
468513

@@ -480,7 +525,7 @@ let single_line_of_comments = (comments: list(Parsetree.comment)) =>
480525
Doc.space,
481526
Doc.join(
482527
~sep=Doc.space,
483-
List.map(c => {nobreak_comment_to_doc(c)}, comments),
528+
List.map(c => {comment_to_doc(c)}, comments),
484529
),
485530
])
486531
};
@@ -496,6 +541,12 @@ let rec new_comments_inner =
496541
| [] => []
497542
| [cmt, ...rem] => [
498543
comment_to_doc(cmt),
544+
switch (cmt) {
545+
| Line(_)
546+
| Shebang(_) => Doc.hardLine
547+
| Doc(_) => Doc.hardLine
548+
| _ => Doc.nil
549+
},
499550
...new_comments_inner(~prev=cmt, rem),
500551
]
501552
}
@@ -514,6 +565,12 @@ let rec new_comments_inner =
514565
| 0 => [
515566
Doc.space,
516567
comment_to_doc(cmt),
568+
switch (cmt) {
569+
| Line(_)
570+
| Shebang(_) => Doc.hardLine
571+
| Doc(_) => Doc.hardLine
572+
| _ => Doc.nil
573+
},
517574
...new_comments_inner(~prev=cmt, rem),
518575
]
519576

@@ -524,6 +581,12 @@ let rec new_comments_inner =
524581
| _ => Doc.hardLine
525582
},
526583
comment_to_doc(cmt),
584+
switch (cmt) {
585+
| Line(_)
586+
| Shebang(_) => Doc.hardLine
587+
| Doc(_) => Doc.hardLine
588+
| _ => Doc.nil
589+
},
527590
...new_comments_inner(~prev=cmt, rem),
528591
]
529592
| _ => [
@@ -535,6 +598,12 @@ let rec new_comments_inner =
535598
},
536599
Doc.hardLine,
537600
comment_to_doc(cmt),
601+
switch (cmt) {
602+
| Line(_)
603+
| Shebang(_) => Doc.hardLine
604+
| Doc(_) => Doc.hardLine
605+
| _ => Doc.nil
606+
},
538607
...new_comments_inner(~prev=cmt, rem),
539608
]
540609
};

compiler/src/formatting/format.re

Lines changed: 9 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -834,13 +834,9 @@ let print_trailing_comments = (~separator, ~itemloc: Location.t, comments) => {
834834
let (_, code_line, _, _) =
835835
Locations.get_raw_pos_info(itemloc.loc_end);
836836
if (this_comment_line > code_line) {
837-
[
838-
Doc.hardLine,
839-
Comment_utils.nobreak_comment_to_doc(comment),
840-
...acc,
841-
];
837+
[Doc.hardLine, Comment_utils.comment_to_doc(comment), ...acc];
842838
} else {
843-
[Comment_utils.nobreak_comment_to_doc(comment), ...acc];
839+
[Comment_utils.comment_to_doc(comment), ...acc];
844840
};
845841
| Some(next) =>
846842
let (_, next_comment_line, _, _) =
@@ -851,13 +847,9 @@ let print_trailing_comments = (~separator, ~itemloc: Location.t, comments) => {
851847
next_comment := Some(comment);
852848

853849
if (this_comment_line <= next_comment_line) {
854-
[
855-
Doc.hardLine,
856-
Comment_utils.nobreak_comment_to_doc(comment),
857-
...acc,
858-
];
850+
[Doc.hardLine, Comment_utils.comment_to_doc(comment), ...acc];
859851
} else {
860-
[Comment_utils.nobreak_comment_to_doc(comment), ...acc];
852+
[Comment_utils.comment_to_doc(comment), ...acc];
861853
};
862854
};
863855
},
@@ -884,7 +876,7 @@ let mix_comments_and_separator =
884876
[
885877
separator,
886878
Doc.space,
887-
Comment_utils.nobreak_comment_to_doc(comment),
879+
Comment_utils.comment_to_doc(comment),
888880
Doc.breakParent, // forces the lines to break, and so make this line comment force a new line
889881
...acc,
890882
];
@@ -906,11 +898,7 @@ let mix_comments_and_separator =
906898
Locations.get_raw_pos_info(item_location.loc_end);
907899

908900
if (this_comment_line > code_line) {
909-
[
910-
Doc.hardLine,
911-
Comment_utils.nobreak_comment_to_doc(comment),
912-
...acc,
913-
];
901+
[Doc.hardLine, Comment_utils.comment_to_doc(comment), ...acc];
914902
} else {
915903
force_break_for_comment(comment, acc);
916904
};
@@ -924,11 +912,7 @@ let mix_comments_and_separator =
924912
next_comment := Some(comment);
925913

926914
if (this_comment_line < next_comment_line) {
927-
[
928-
Doc.hardLine,
929-
Comment_utils.nobreak_comment_to_doc(comment),
930-
...acc,
931-
];
915+
[Doc.hardLine, Comment_utils.comment_to_doc(comment), ...acc];
932916
} else {
933917
force_break_for_comment(comment, acc);
934918
};
@@ -1261,10 +1245,7 @@ and resugar_list =
12611245
let trailing_comments =
12621246
List.map(
12631247
(cmt: Parsetree.comment) =>
1264-
Doc.concat([
1265-
Doc.space,
1266-
Comment_utils.nobreak_comment_to_doc(cmt),
1267-
]),
1248+
Doc.concat([Doc.space, Comment_utils.comment_to_doc(cmt)]),
12681249
item_comments,
12691250
);
12701251

@@ -3323,7 +3304,7 @@ and print_expression_inner =
33233304
(index, c) =>
33243305
Doc.concat([
33253306
Doc.space,
3326-
Comment_utils.nobreak_comment_to_doc(c),
3307+
Comment_utils.comment_to_doc(c),
33273308
switch (c) {
33283309
| Line(_) => Doc.breakParent
33293310
| _ => Doc.nil

compiler/test/formatter_inputs/comments.gr

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,3 +138,22 @@ Result.expect(
138138
File.fdWrite(File.stdout, "Status: 500\n\nInternal Server Error")
139139
)
140140
// this comment stays outside
141+
142+
// For debugging:
143+
144+
// provide let getRefCount = (value) => {
145+
// let userPtr = WasmI32.fromGrain(value)
146+
// let ret = if (WasmI32.eqz(userPtr & Tags._GRAIN_GENERIC_TAG_MASK) && WasmI32.ne(userPtr, 0n)) {
147+
// WasmI32.toGrain((getRefCount(userPtr) * 2n) + 1n) : Number
148+
// } else {
149+
// 0
150+
// }
151+
// decRef(userPtr)
152+
// ret
153+
// }
154+
155+
// provide let rec setDebug = (enabled: Bool) => {
156+
// _DEBUG = enabled
157+
// decRef(WasmI32.fromGrain(setDebug))
158+
// void
159+
// }

compiler/test/formatter_outputs/comments.gr

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,3 +128,22 @@ Result.expect(
128128
File.fdWrite(File.stdout, "Status: 500\n\nInternal Server Error")
129129
)
130130
// this comment stays outside
131+
132+
// For debugging:
133+
134+
// provide let getRefCount = (value) => {
135+
// let userPtr = WasmI32.fromGrain(value)
136+
// let ret = if (WasmI32.eqz(userPtr & Tags._GRAIN_GENERIC_TAG_MASK) && WasmI32.ne(userPtr, 0n)) {
137+
// WasmI32.toGrain((getRefCount(userPtr) * 2n) + 1n) : Number
138+
// } else {
139+
// 0
140+
// }
141+
// decRef(userPtr)
142+
// ret
143+
// }
144+
145+
// provide let rec setDebug = (enabled: Bool) => {
146+
// _DEBUG = enabled
147+
// decRef(WasmI32.fromGrain(setDebug))
148+
// void
149+
// }

0 commit comments

Comments
 (0)