Repository navigation
[formatting] Unify delimiter alignment and block continuation layouts - #636
purefunctor wants to merge 6 commits into
Conversation
Granite ReviewCouncil
FindingsNo actionable findings. |
Compatibility regression reportPackage set ✅ The candidate introduces no compatibility errors.
Introduced errorsNone. Fixed errors (0)None. Warning changes (0 introduced, 0 fixed)Introduced None. Fixed None. Candidate errors (0)None. Candidate warnings (36)
|
Review summaryI found no issues I'm confident are real, so there are no inline comments. The change is in the layer that owns it. The delimiter-alignment choice moved into
Note, not a blocker: parenthesised Checks run
I didn't rerun the full workspace suite because CI's Cargo Build & Test covers it. |
![purefunctor[bot]](https://avatars.githubusercontent.com/in/4468429?s=60&v=4){kind=small}
Granite ReviewCouncil review superseded by a newer revision Progress
|
Granite ReviewCouncil
FindingsNo actionable findings. |
0da7d37 to
ad517a8
Compare
Granite ReviewCouncil
FindingsNo actionable findings. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| ⇐ Ord value | ||
|
|
||
| class (Eq value, Show value) ⇐ Thing | ||
| value |
There was a problem hiding this comment.
This accepted expectation breaks the class head Thing value itself rather than the superclass list:
class (Eq value, Show value) ⇐ Thing
valueThe flat line class (Eq value, Show value) ⇐ Thing value is 42 columns, over the 40-column limit. The commit message for this change says broken superclass lists move below class "while allowing the closing parenthesis, constraint arrow, and class head to fit together". That would give:
class
( Eq value
, Show value
) ⇐ Thing valueAt width 30 / indent 2, this same declaration does take that layout (lines 45–49). At width 40 / indent 4 it doesn't. The likely cause is that the prefix group built by self.delimited(constraints, head_context, Some(keyword)) in printer.rs:475 decides whether to break by looking only as far as the next soft break inside the class head. class (Eq value, Show value) ⇐ Thing fits in 40 columns, so the list stays flat and the head application breaks instead.
AGENTS.md: "Snapshots record observed behavior; passing or accepting them does not establish semantic correctness. Check the result against the intended behavior." This snapshot contradicts the stated intent, so either the layout decision should take the whole class head into account, or the expectation should be explained.
Review checksRan against the PR merge commit (head
Tests are at the level AGENTS.md assigns: the source-level formatter behavior is covered by There is one inline finding. In I couldn't build the CLI in this environment to probe further cases by hand, so the finding rests on the committed snapshot. 🤖 Generated with Claude Code |
purefunctor
changed the title
Granite ReviewCouncil
FindingsNo actionable findings. |
Review checks (head
|
5869c9c to
ece7842
Compare
Granite ReviewCouncil
FindingsNo actionable findings. |
Review checks (head
|
Use opener-relative, comma-leading layout for imports, exports, parenthesized terms, types and binders, and constraint lists. Preserve compact constructor-member spelling and fully expand wrapped exports. Fit complete superclass headers before breaking their lists below class, keeping the closing parenthesis, arrow, and class head together when possible. Preserve comment boundaries and exclude class bodies from the fit decision. Include width-boundary, exact class-head fit, comment, constructor-member, and binder fixtures with regenerated expectations. Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-01a11570-3ec2-7539-bc19-3d4825503ee1
Place a broken data equation’s leading equals sign alongside its alternative pipes. Anchor constructor arguments to the constructor column instead of the declaration margin. Regenerate declaration, record-constructor, and commented-separator expectations at the existing widths and indentation settings. Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-01a11570-3ec2-7539-bc19-3d4825503ee1
Place parentheses, arrays, records, and record updates containing do or ado blocks below their prefixes. Apply the same rule to bindings, arguments, and operator operands instead of letting block bodies drift rightward. Keep expression access and block detection on Tree, and include regenerated delimited-block and continuation-boundary fixtures. Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-01a11570-3ec2-7539-bc19-3d4825503ee1
Treat case expressions as inline blocks and measure complete headers independently of their branches. Keep a fitting header beside its prefix, move it intact when necessary, and split case and of only when the scrutinees require continuation layout. Cover width boundaries, multiple and nested scrutinees, comments, parentheses, operator operands, and nested block continuations. Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-01a11570-3ec2-7539-bc19-3d4825503ee1
Keep clauses at one margin and decide compact versus multiline layout for the complete chain. Reuse continuation formatting for wrapped conditions, branch expressions, and blocks. Preserve parentheses and line-forcing comment boundaries, and retain nested then conditionals. Destructure validated conditional syntax with slice patterns. Cover compact and broken chains, wrapped conditions, block branches, comment boundaries, nested conditionals, parentheses, and operator operands. Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-01a11570-3ec2-7539-bc19-3d4825503ee1
Replace positional syntax accesses for case, let/ado, where, and forall with named structural patterns. Bind nonempty chain tails and leading else declarations directly instead of repeating indexing assumptions. Preserve formatting behavior, including empty ado bodies and optional where bindings. Keep position-driven delimiter and operator iteration unchanged. Existing formatting expectations require no updates. Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-01a11570-3ec2-7539-bc19-3d4825503ee1
ece7842 to
aad73df
Compare
Granite ReviewCouncil
FindingsNo actionable findings. |
This branch has not been deployed
1 participant
Summary
Make multiline delimiter layouts consistent across terms and types, and keep block continuations readable without unnecessary breaks or excessive rightward indentation.
The initial layout changes are organized into four behavior-focused commits, each with its corresponding fixtures and generated expectations:
Box(First, Second)andBox(..), adding a space before multiline member lists. Move broken superclass lists belowclass, keeping) <= Thingtogether when it fits.=with alternative|separators in broken data declarations. Indent constructor arguments, including record arguments, relative to the constructor rather than the declaration margin.dooradobelow their prefixes in bindings, applications, and operator operands. Keep the first expression beside its opening delimiter and place expression access and block detection onTree.case … ofheader independently of its branches. Keep a fitting header beside its prefix; otherwise move the intact header to a continuation line; split alignedcaseandofonly when the scrutinees require it. Use leading commas for broken multiple scrutinees and preserve comment boundaries.Follow-up regression and fix commits cover complete-class-head width boundaries: expand the superclass list before wrapping the class parameters, preserve the compact exact-width header, and use the structured list when comments prevent a flat header.
Retain the structured multiline
if/then/elselayout, the separatelet/bindings/in/result lines, and the existingadoresult layout (in value).Fixtures cover exact width boundaries, alternate indentation and Unicode, nested blocks, comments, constructor lists, constraints, and operator continuations. The history rewrite preserves the exact final tree.
Verification
just t formatting --verbose: 1,063 passed, no pending snapshots.cargo check -p formatting --testsjust format --checkgit diff --checkjust formatandjust licensescompleted without tracked changes.Discussion
https://ampcode.com/threads/T-01a11570-3ec2-7539-bc19-3d4825503ee1