fix(tomlfmt): keep arrays inside inline tables on a single line - #642
fix(tomlfmt): keep arrays inside inline tables on a single line#642MsfPablo wants to merge 3 commits into
Conversation
printArray expanded a nested array whenever the enclosing inline table was
estimated to exceed column_width, so an array written across several lines
inside an inline table stayed multiline:
reqwest = { workspace = true, default-features = false, features = [
"rustls",
] }
An inline table is meant to occupy a single line (TOML v1.0 §5.2), so the
width of the line cannot justify breaking one apart. Force single-line
printing for any array nested in an inline table and drop the
inlineTableLineLen bookkeeping, which existed only to drive that expansion.
Arrays holding comments keep expanding: collapsing them would move a
comment onto the same line and comment out the rest of the table.
Test_TOML_InlineTableExpandsAtColumnWidth asserted the old behavior and is
replaced by Test_TOML_InlineTableArrayStaysSingleLine, plus a regression
test for the reported input and fixture coverage for both a multiline
array and a multiline array nested one inline table deeper.
Fixes Boeing#631
|
Hi @kehoecj — just a friendly follow-up on this PR. It's been about a week with no review activity. Is there anything I can clarify or adjust to help move it forward? Happy to rebase, split the change, or rework the approach if there's a preferred style for this repo. Thanks! |
kehoecj
left a comment
There was a problem hiding this comment.
Thanks for working on this @MsfPablo!
The fix is right for the short case but goes a bit too far for the long case. Right now it sets multiline = false unconditionally inside inline tables, which means arrays never expand regardless of line length. taplo (our reference tool) still expands when the total line exceeds column_width:
Short inline table — should stay on one line (your PR gets this right ✅):
# 73 chars total, fits fine
reqwest = { workspace = true, default-features = false, features = ["rustls"] }Long inline table — taplo expands the array (your PR keeps it inline ❌):
# Input: 127 chars total
criterion = { version = "0.5", default-features = false, features = ["async_tokio", "html_reports", "plotters"] }
# taplo output:
criterion = { version = "0.5", default-features = false, features = [
"async_tokio",
"html_reports",
"plotters",
] }
# This PR keeps it all on one line regardless of widthCould you change the multiline = false to something like multiline = (prefixLen + singleLineLen) > p.opts.ColumnWidth so it still respects the width threshold? That way short inline tables stay compact but long ones get the array expanded — matching what taplo does.
Summary
Fixes #631.
printArrayexpanded a nested array whenever the enclosing inline table was estimated to exceedcolumn_width, so an array written across several lines inside an inline table stayed multiline:Per TOML v1.0 §5.2, an inline table is meant to occupy a single line — the line's width can't justify breaking one apart.
Fix
Force single-line printing for any array nested in an inline table, and dropped the
inlineTableLineLenbookkeeping that existed only to drive the old expansion behavior. Arrays holding comments still expand, since collapsing them would move a comment onto the same line and comment out the rest of the table.Test_TOML_InlineTableExpandsAtColumnWidthasserted the old (buggy) behavior and is replaced byTest_TOML_InlineTableArrayStaysSingleLine, plus a new regression test (Test_TOML_InlineTableArrayCollapsesFromMultiline) for the exact reported input, and fixture coverage (inline_table.input.toml/.expected.toml) for both a multiline array and a multiline array nested one inline table deeper.Test plan
go test ./pkg/formatter/tomlfmt/...— all pass.