Skip to content

Parser offsets are wrong for files with CRLF line endings - #1282

Merged
theoephraim merged 1 commit into
dmno-dev:mainfrom
voiys:fix-crlf-parser-offsets
Nov 13, 2026
Merged

theoephraim merged 1 commit into
dmno-dev:mainfrom
voiys:fix-crlf-parser-offsets

Conversation

@voiys

@voiys voiys commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor
src  = ["# @defaultRequired=false", "# ---", "PORT=3000", "HOST=localhost", ""].join("\r\n")
item = parse(src).configItems[0]                    // PORT
src.slice(item.start.offset, item.end.offset)

main:  "\r\nPORT=300"
here:  "PORT=3000\r\n"

same for syntax errors: a stray @oops on line 5 is reported at an offset that points into the line above it.

cause: the parser turns \r\n into \n before parsing, so the offsets point into the shortened text. line N is off by N characters.

fix: map offsets back to the original text after parsing. line and column were already right. files without \r\n skip the mapping.

context: found while prototyping a language server on top of the parser (#469) and fuzzing it with generated schemas. the server slices the source with these offsets. nothing inside varlock reads them today, so nobody using the CLI sees this.

heads up: this changes packages/env-spec-parser/src/index.ts, which the parser rewrite in #744 also changes. if #744 is still planned, this would need redoing on top of it, and i'm happy to do that.

test: locations.test.ts, 9 of its cases fail on main.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes: Reviewed the CRLF offset correction, regression tests, and documentation.

  • Original-input locations: Remaps AST and syntax-error offsets after newline normalization, including shared locations and mixed line endings.
  • Regression coverage: Tests node ranges, decorators, unchanged values, and error boundaries; all 289 parser tests and the parser typecheck passed.
  • Documentation: Clarifies newline handling and location coordinates, with a parser patch release entry.

Pullfrog  | View workflow run | Using gpt-6.1-sol | 𝕏

The parser normalizes \r\n to \n before parsing, so every offset it
reported (on nodes and on syntax errors) was an offset into the
normalized text. For a CRLF file the Nth line was off by N characters,
which breaks anything that slices the original source with them.

Map the offsets back to the input after parsing. Line and column are
unchanged, and files without \r\n skip the mapping entirely.
@theoephraim
theoephraim force-pushed the fix-crlf-parser-offsets branch from b7b9600 to ecc2a3c Compare November 13, 2026 00:47
@pkg-pr-new

pkg-pr-new Bot commented Nov 13, 2026

Copy link
Copy Markdown

Open in StackBlitz

@env-spec/parser

npm i https://pkg.pr.new/@env-spec/parser@1282

varlock

npm i https://pkg.pr.new/varlock@1282

@varlock/astro-integration

npm i https://pkg.pr.new/@varlock/astro-integration@1282

@varlock/cloudflare-integration

npm i https://pkg.pr.new/@varlock/cloudflare-integration@1282

@varlock/nextjs-integration

npm i https://pkg.pr.new/@varlock/nextjs-integration@1282

@varlock/nuxt-integration

npm i https://pkg.pr.new/@varlock/nuxt-integration@1282

@varlock/vite-integration

npm i https://pkg.pr.new/@varlock/vite-integration@1282

@varlock/1password-plugin

npm i https://pkg.pr.new/@varlock/1password-plugin@1282

@varlock/aws-secrets-plugin

npm i https://pkg.pr.new/@varlock/aws-secrets-plugin@1282

@varlock/azure-key-vault-plugin

npm i https://pkg.pr.new/@varlock/azure-key-vault-plugin@1282

@varlock/bitwarden-plugin

npm i https://pkg.pr.new/@varlock/bitwarden-plugin@1282

@varlock/keepass-plugin

npm i https://pkg.pr.new/@varlock/keepass-plugin@1282

@varlock/pass-plugin

npm i https://pkg.pr.new/@varlock/pass-plugin@1282

@varlock/proton-pass-plugin

npm i https://pkg.pr.new/@varlock/proton-pass-plugin@1282

commit: ecc2a3c

@theoephraim
theoephraim merged commit e3c0e3c into dmno-dev:main Nov 13, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants