Repository navigation
Format a blank line around multi-line module items - #8961
rolandpeelen wants to merge 2 commits into
Conversation
Semantic Diff1 commit · Full page Format a blank line around module items that span several linesd4fe753 · Roland Peelen · Oct 22, 2026
Format modified (structure items) module Details = {
let make = () => React.string("details")
}
module Output = Tool.TextResult.Output
let failed = Tool.TextResult.failed
module Details = {
let make = () => React.string("details")
}
+
module Output = Tool.TextResult.Output
let failed = Tool.TextResult.failedFormat unchanged (one-line items with comments): let a = 1
// about b
let b = 2 // trailing
let c = 3
let a = 1
// about b
let b = 2 // trailing
let c = 3Review
|
rolandpeelen
changed the title
Semantic Diff1 new commit (2 total) · Full page Make multi-line item spacing the experimental feature MultilineItemSpacingfcda8d7 · Roland Peelen · Oct 23, 2026
Format modified (multi-line item spacing, opt-in) module A = {
let x = 1
}
let y = 2
module A = {
let x = 1
}
let y = 2Format unchanged (default): module A = {
let x = 1
}
let y = 2
module A = {
let x = 1
}
let y = 2Diagnostic modified (unknown experimental feature) "experimental-features": {"FooBar": true}
-Unknown experimental feature 'FooBar'. Available features: LetUnwrap
+Unknown experimental feature 'FooBar'. Available features: LetUnwrap, MultilineItemSpacingReview
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8961 +/- ##
==========================================
+ Coverage 81.46% 81.48% +0.01%
==========================================
Files 460 460
Lines 64675 64728 +53
==========================================
+ Hits 52689 52741 +52
- Misses 11986 11987 +1
🚀 New features to boost your workflow:
|
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
|
Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-8961 |
Semantic Diff1 new commit (4 total) · Full page Cover multi-line item spacing with unit tests2af5880 · Roland Peelen · Oct 23, 2026 Codecov reported 15 changed lines without coverage: branches of the separator's measurement that the fixtures never reach first, the trailing comments it moves, the new cases of will_break, is_nil and debug, and Experimental_features.to_string. tests/syntax_tests/res_test.ml now prints documents built for each case and checks the output, and formats a module with the feature off and on through the printer API. The fixture adds a multi-line template literal and a trailing comment on a multi-line item. print_list matched an accumulator that cannot be empty after the first item; it now matches the one case that adds a separator and treats every other one as the first item. |
Semantic Diff1 new commit (5 total) · Full page Check is_nil on every document constructorf209b66 · Roland Peelen · Oct 23, 2026 Codecov still reported the last line of the is_nil match: its Group, CustomLayout and BreakParent alternatives share the line with the new ones, and no test passed them. The test now checks every constructor. |
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
f209b66 to
02a1b82
Compare
Changelog💅 Polish
Semantic Diff2 commits · Full page Format a blank line around module items that span several linesbc537f4 · rolandpeelen · Oct 22, 2026, 14:14 UTC
Format modified (structure items) module Details = {
let make = () => React.string("details")
}
module Output = Tool.TextResult.Output
let failed = Tool.TextResult.failed
module Details = {
let make = () => React.string("details")
}
+
module Output = Tool.TextResult.Output
let failed = Tool.TextResult.failedFormat unchanged (one-line items with comments): let a = 1
// about b
let b = 2 // trailing
let c = 3
let a = 1
// about b
let b = 2 // trailing
let c = 3Review
✏️ Format the repository with multi-line item spacing3487c86 · rolandpeelen · Oct 28, 2026, 16:37 UTC · edited The previous commit makes the formatter put a blank line around module items that span several lines. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 02a1b823f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
4afd717 to
800e290
Compare
rolandpeelen
changed the title
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Review notes (Fable) The layout-aware measurement and the trailing-comment relocation are both needed for idempotency:
No bugs found. Possible simplifications:
Behaviour worth a conscious decision, and maybe fixture coverage:
|
800e290 to
2aea23c
Compare
Why:
Dense code, often written by AI agents, runs multi-line modules and
functions together, which makes a file hard to scan (#8960). The
formatter already collapses several blank lines into one; it now also
separates an item that prints on more than one line from its
neighbours in structures and signatures. Blocks inside expressions
keep their layout.
Format modified (structure items):
Example:
module Details = {
let make = () => React.string("details")
}
module Output = Tool.TextResult.Output
let failed = Tool.TextResult.failed
Before (rescript):
module Details = {
let make = () => React.string("details")
}
module Output = Tool.TextResult.Output
let failed = Tool.TextResult.failed
After (rescript):
module Details = {
let make = () => React.string("details")
}
module Output = Tool.TextResult.Output
let failed = Tool.TextResult.failed
Format unchanged (one-line items with comments):
Example:
let a = 1
// about b
let b = 2 // trailing
let c = 3
After (rescript):
let a = 1
// about b
let b = 2 // trailing
let c = 3
Review: Is "multi-line" decided on the source or on the output? On the output: the new Res_doc.item_separator measures the items on each side with the printer's own layout decisions, stopping at the first line break, so an item that only breaks at the line width counts and formatting twice gives the same text. The source line span (`let x = someLongCall(a, b, c, d)` is one source line but prints on several) and Doc.will_break (a record that wraps by width is force-broken only on the second pass) would both add the blank line only on the second format. The measurement costs at most one output line per item. The roundtrip test formats every printer fixture twice and gets the same text.
Review: Why a Res_doc.comment marker? Leading comments print inside the item's document (a value binding carries them), so a `//` line above a one-line item would make it multi-line, even when the item is measured before print_list adds its own comments. The marker keeps comments out of the measurement; doc comments are attributes and still count.
Review: Why move own-line trailing comments below the blank line? A comment inside the previous item's source, such as `f(a // c => 1)`, prints on its own line after the item. Formatting again makes it a leading comment of the next item, after the blank line, so the first pass puts it there too.
Review: How are the measurement's branches tested? tests/syntax_tests/res_test.ml prints documents built for each case (line breaks, groups that fit and break, if_breaks, comments, trailing comments) and checks the output, as the printer fixtures do not reach every branch first.
Review: What else prints through it? The editor's code actions and `rescript-tools format-codeblocks` for code blocks in docstrings, so the Xform and FormatDocstringsTest2 snapshots gain a blank line.
Review: Do attributes count? Yes, an attribute or doc comment on its own line makes an item print on two lines, so runs of `@genType` or documented one-line items are spaced; an inline `@val external` is not. The fixtures cover both, and nested module bodies.
Review: Is it opt-in? No: every `rescript format` uses it. The next commit formats this repository with it.
Signed-off-by: Roland Peelen <[email protected]>
The previous commit makes the formatter put a blank line around module items that span several lines. `make format` applies it to the ReScript sources it covers: 347 files gain 1151 blank lines and lose nothing. rewatch/testrepo/packages/dep02/src/Array.res gains one too: the rewatch format test expects exactly the testrepo's 8 deliberately unformatted files to change. The outputs that record source line numbers follow: the super_errors snapshots, the `assert` locations in the generated test JavaScript, the doc-extraction snapshots of tests/tools_tests, and the locations in React.res and JsxEvent.res that the completion and reanalyze snapshots of tests/analysis_tests point to. Signed-off-by: Roland Peelen <[email protected]>
2aea23c to
3487c86
Compare
Closes #8960.
rescript formatnow separates a structure or signature item that prints on more than one line from its neighbours with a blank line. One-line items stay together, and blocks inside expressions keep their layout.This applies to every project; there is no flag. Existing code gains blank lines the first time it is formatted.
Commits
make formaton this repository (blank lines only), plus the test outputs and snapshots whose line numbers move with it.How
Res_doc.item_separator ~prev ~nextis a hard line, plus a blank line when either neighbour spans several lines in the output. It is measured with the printer's own layout decisions, stopping at the first line break, so an item that only wraps at the line width counts, and formatting twice gives the same text.Res_doc.commentkeeps leading//comments out of that measurement; doc comments are attributes and still count.print_structureandprint_signatureuse the separator; nothing else changes.Testing
printer/structure/multilineSpacing.resandprinter/signature/multilineSpacing.resi; existing snapshots only gain blank lines.tests/syntax_tests/res_test.mlchecks the separator's measurement case by case.make test,make test-syntax+ roundtrip,make test-tools,make test-gentype,make test-analysis,make test-reanalyze,make test-rewatch,make test-rewatch-rustandmake checkformatpass.Checklist
changes/README.md), or no release notes are needed and a maintainer should applychangelog:skip.git commit -s).