Repository navigation
Experimental formatter feature MultilineItemSpacing: blank lines around multi-line module items - #8961
Experimental formatter feature MultilineItemSpacing: blank lines around multi-line module items#8961rolandpeelen wants to merge 5 commits into
Conversation
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. All 1025 printer and idempotency fixtures are stable across two passes.
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. 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: Does this reformat existing code? Yes: 346 of the 1870 .res/.resi files `make format` covers gain blank lines. They are not reformatted in this commit, so `make checkformat` reports them.
Signed-off-by: Roland Peelen <roland@magic.dev>
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
|
…acing
Why:
Spacing multi-line module items changes the output of `rescript
format` for most existing projects (341 files in this repository
alone), so it is opt-in, like `let?` (LetUnwrap). Without the feature
the formatter prints exactly as before, and the snapshots of the
previous commit return to their master state.
Format modified (multi-line item spacing, opt-in):
Example:
module A = {
let x = 1
}
let y = 2
Before (rescript): always, since the previous commit
module A = {
let x = 1
}
let y = 2
After (rescript): "experimental-features": {"MultilineItemSpacing": true}
module A = {
let x = 1
}
let y = 2
Format unchanged (default):
Example:
module A = {
let x = 1
}
let y = 2
After (rescript): without the feature
module A = {
let x = 1
}
let y = 2
Diagnostic modified (unknown experimental feature):
Example:
"experimental-features": {"FooBar": true}
Before (diagnostic): Unknown experimental feature 'FooBar'. Available features: LetUnwrap
After (diagnostic): Unknown experimental feature 'FooBar'. Available features: LetUnwrap, MultilineItemSpacing
Review: How does `rescript format` learn about the feature? Both build systems pass the root project's experimental flags, the same ones a build passes, to `bsc` before `-format`; `-format` runs as soon as bsc reads it, so the order matters. Outside a project, format runs without them, as before. Covered by rewatch/tests/format/05-format-experimental-multiline-item-spacing.sh, which passes for both rescript.exe and rescript-rust.exe.
Review: How do the syntax tests enable it? `res_parser` accepts `-enable-experimental`, and scripts/test_syntax.sh prints and round-trips each file under tests/syntax_tests/data/experimental/<Feature>/ with that feature enabled.
Review: What about tools that call `bsc -format` directly instead of `rescript format`? They format without the feature unless they pass `-enable-experimental MultilineItemSpacing` before `-format`.
Signed-off-by: Roland Peelen <roland@magic.dev>
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✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #8961 +/- ##
==========================================
+ Coverage 81.21% 81.31% +0.09%
==========================================
Files 457 456 -1
Lines 63853 63886 +33
==========================================
+ Hits 51859 51947 +88
+ Misses 11994 11939 -55
🚀 New features to boost your workflow:
|
The formatter writes CRLF on Windows, so the new rewatch format test failed there on line endings alone; it now strips them before comparing. Signed-off-by: Roland Peelen <roland@magic.dev>
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 |
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. Signed-off-by: Roland Peelen <roland@magic.dev>
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. |
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. Signed-off-by: Roland Peelen <roland@magic.dev>
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". |
Closes #8960.
Adds the experimental feature
MultilineItemSpacing. When a project enables it,rescript formatseparates 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. Likelet?(LetUnwrap), it is opt-in:Without the feature the formatter output is byte-for-byte unchanged: every existing syntax snapshot matches master, and no repository files need reformatting.
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, so formatting twice gives the same text.Res_doc.commentkeeps leading comments out of that measurement.print_structure/print_signatureturn it on whenExperimental_features.MultilineItemSpacingis enabled.Experimental_features.MultilineItemSpacing.bsc -enable-experimental MultilineItemSpacing -format file.resformats with it; the flag must come before-format.rewatchand OCamlrewatch-ocaml): the config accepts the feature, andrescript format(files and--stdin) passes the root project's experimental flags tobsc -format. Outside a project, format runs without them, as before.build-schema.jsonandCompilerConfigurationSpec.mdlist the feature.Testing
tests/syntax_tests/data/experimental/MultilineItemSpacing/.res_parseraccepts-enable-experimental, andscripts/test_syntax.shprints and round-trips eachexperimental/<Feature>/directory with that feature enabled.rewatch/tests/format/05-format-experimental-multiline-item-spacing.sh(opt-in check);01-invalid-experimental-key.shnow expects both features in the error.make test,make test-syntax+ roundtrip,make checkformat(ocamlformat 0.29), the rewatch suite against bothrescript.exeandrescript-rust.exe,dune runtestfortests/ounit_testsandtests/rewatch_ounit_tests, andcargo clippy -D warningsall pass.Checklist
changes/README.md), or no release notes are needed and a maintainer should applychangelog:skip.git commit -s).