fix(psl): preserve schema line endings during reformat#5814
fix(psl): preserve schema line endings during reformat#5814barobaonguyen wants to merge 2 commits into
Conversation
`prisma format` previously emitted LF for every line of its output but ended the file with a CRLF when the input used CRLF anywhere. That left Windows users with a stray `\r\n` on the last line of an otherwise-LF file (or vice versa) and broke editors that disallow mixed line endings. Fix it natively in the renderer instead of post-processing the output (the maintainer rejected post-processing in prisma#5812). The reformatter now: * Detects the input's line-ending style (first newline wins; mixed inputs fall back to LF) via a new `NewlineType::detect` helper. * Threads the chosen ending through `Renderer` so each `end_line` writes the configured separator. The trailing-newline guard in `reformat` is updated to check for and append the configured ending too. * Exposes a `reformat_with_line_ending` entry point alongside the existing `reformat` for callers that want to pin a specific style. Closes prisma/prisma#8548
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Summary by CodeRabbit
WalkthroughThe PR detects input newline style (LF vs CRLF), threads it into the schema Renderer, exposes ChangesLine-ending preservation in schema reformatting
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR updates the PSL reformatter to preserve (or explicitly control) output line endings (LF vs CRLF), addressing a regression where output could end with a different newline style than the body.
Changes:
- Add
NewlineType::detect()and threadNewlineTypethroughRendererso all emitted newlines are consistent. - Introduce
reformat_with_line_ending()and havereformat()auto-detect the input’s line-ending style. - Add regression tests ensuring LF/CRLF consistency and correct trailing newline behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| psl/schema-ast/src/renderer.rs | Renderer now emits newlines using a configured NewlineType instead of always \n. |
| psl/schema-ast/src/reformat.rs | Adds auto-detection and a public API to force line endings; ensures trailing newline matches chosen ending. |
| psl/schema-ast/src/lib.rs | Re-exports reformat_with_line_ending as part of the public API surface. |
| psl/schema-ast/src/ast/newline_type.rs | Adds NewlineType::detect() to infer newline style from an input string. |
| psl/psl/tests/reformat/reformat.rs | Adds regression tests for line-ending preservation and consistency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pub fn detect(input: &str) -> NewlineType { | ||
| let bytes = input.as_bytes(); | ||
| for (i, &b) in bytes.iter().enumerate() { | ||
| if b == b'\n' { | ||
| if i > 0 && bytes[i - 1] == b'\r' { | ||
| return NewlineType::Windows; | ||
| } | ||
| return NewlineType::Unix; | ||
| } | ||
| } | ||
| NewlineType::Unix | ||
| } |
| /// The output preserves the line-ending style detected from `input` (LF or | ||
| /// CRLF). Mixed-ending inputs fall back to LF. See `NewlineType::detect`. |
| #[test] | ||
| fn mixed_input_defaults_to_lf() { | ||
| // First line uses LF, later lines use CRLF: the first newline wins | ||
| // and the reformatter falls back to LF for the whole output. | ||
| let input = format!( | ||
| "{}\n{}\r\n{}\r\n{}\r\n", | ||
| SCHEMA_LINES[0], SCHEMA_LINES[1], SCHEMA_LINES[2], SCHEMA_LINES[3] | ||
| ); | ||
| let out = reformat(&input); | ||
| assert!(!out.contains('\r'), "mixed input should normalize to LF: {out:?}"); | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@psl/psl/tests/reformat/reformat.rs`:
- Around line 1285-1294: Add a second test variant that mirrors
mixed_input_defaults_to_lf but with CRLF on the first line(s) and LF later to
ensure normalization behavior is symmetric; duplicate the test logic in a new
function (e.g., mixed_input_with_crlf_first) using the same SCHEMA_LINES
sequence but build input so the first newline is "\r\n" and later lines use "\n"
(or construct format with CRLF-first then LF), call reformat(&input) and assert
no '\r' in out exactly like the original test to lock the mixed-ending contract
from both directions.
In `@psl/schema-ast/src/ast/newline_type.rs`:
- Around line 36-47: The detect function currently returns on the first newline
and misclassifies mixed CRLF+LF input as Windows; update NewlineType::detect to
scan the entire byte slice and track two booleans (e.g., seen_crlf and seen_lf)
while iterating bytes, setting seen_crlf when a newline has a preceding '\r' and
seen_lf when a newline does not; after the loop return NewlineType::Unix if both
flags are set (mixed -> prefer LF), otherwise return Windows if only seen_crlf,
Unix if seen_lf, or Unix by default—adjust the logic inside detect accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3c5ba2f3-013b-4cd4-86aa-32c56f7f3c0a
📒 Files selected for processing (5)
psl/psl/tests/reformat/reformat.rspsl/schema-ast/src/ast/newline_type.rspsl/schema-ast/src/lib.rspsl/schema-ast/src/reformat.rspsl/schema-ast/src/renderer.rs
Per Copilot + coderabbitai review on PR prisma#5814: detect() previously returned on the first newline seen, so CRLF-first-then-LF inputs were classified as Windows even though the doc contract says mixed-ending inputs should fall back to LF. Now any bare LF anywhere in the input forces NewlineType::Unix; an all-CRLF input still returns Windows. Adds a regression test covering the CRLF-first-then-LF direction (mirroring the existing LF-first-then-CRLF test) so both mixing orders are locked down.
|
Addressed in b699af9.
What changed
Local |
|
CodeRabbit chat interactions are restricted to organization members for this repository. Ask an organization member to interact with CodeRabbit, or set |
Summary
Fixes prisma/prisma#8548 —
prisma formatemitted LF for every line of its output but added a CRLF as the trailing newline when the input used CRLF anywhere, leaving Windows users with mixed line endings (CRLF only on the last line).Per maintainer guidance from @SevInf (prisma/prisma#8548 comment), this fix lives in the Rust formatter (
psl/schema-ast/src/reformat.rs) rather than the TS CLI, and the line-ending decision is part of the renderer state — no post-process string replace.Changes
NewlineType::detect(&str) -> NewlineTypeinpsl/schema-ast/src/ast/newline_type.rs— first newline in the input wins; mixed-ending inputs fall back to LF per maintainer note.Renderernow carries aline_ending: NewlineTypefield;end_line()writes the configured separator natively (not LF + later replace).reformat()auto-detects the input line ending and delegates to a new siblingreformat_with_line_ending()that exposes explicit control. The trailing-newline guard appends the configured ending too.How tested
Added
psl/psl/tests/reformat/reformat.rs::line_endingscovering:reformat_with_line_endingLF / CRLF.Differs from prior PRs
prisma/prismaTS CLI (#29245, #29299, #29373, #29460, #29475, #29561, #29569, #29570 — closed for wrong target).Rendererstate, so each emitted line writes the configured separator rather than the formatter emitting LF and then string-replacing post-hoc (the rejection reason for fix(psl): preserve schema line endings during reformat #5812).Closes prisma/prisma#8548
/claim prisma/prisma#8548