Skip to content

fix(format): do not quote pipe fields by default - #1253

Closed
11suixing11 wants to merge 1 commit into
C2FO:mainfrom
11suixing11:fix/pipe-field-quoting
Closed

fix(format): do not quote pipe fields by default#1253
11suixing11 wants to merge 1 commit into
C2FO:mainfrom
11suixing11:fix/pipe-field-quoting

Conversation

@11suixing11

@11suixing11 11suixing11 commented Jul 29, 2026

Copy link
Copy Markdown

Summary

  • Build the formatter escape regexp from escaped literal delimiter, row delimiter, CR, and LF alternatives.
  • Stop treating | as an escape trigger when the delimiter is still the default comma.
  • Match multi-character row delimiters as complete strings instead of quoting on each constituent character.
  • Ignore empty regexp alternatives so unsupported empty options cannot make every field match.
  • Add regression coverage for default pipes, a configured pipe delimiter, and a multi-character || row delimiter.

Fixes #787.

Why

The previous character-class regexp included | as a literal character while trying to express alternation. It also reduced multi-character row delimiters to sets of individual characters. As a result, plain pipe data could be quoted even when it did not contain the configured field or row delimiter.

Verification

  • corepack pnpm exec jest packages/format/__tests__/formatter/FieldFormatter.spec.ts --runInBand (24 tests)
  • corepack pnpm test (37 suites, 606 tests, and all examples)
  • corepack pnpm run build (7 workspace projects)
  • corepack pnpm run format:check
  • git diff --check

@c2fo-cibot c2fo-cibot Bot added the size/S Denotes a PR that changes 10-29 lines label Jul 29, 2026
@11suixing11
11suixing11 force-pushed the fix/pipe-field-quoting branch from c90aba4 to 4dc7315 Compare July 30, 2026 13:07
@11suixing11
11suixing11 force-pushed the fix/pipe-field-quoting branch from 4dc7315 to bc91582 Compare July 31, 2026 06:22
@11suixing11

Copy link
Copy Markdown
Author

I re-ran this branch after the upstream rebase.

Local verification:

  • pnpm exec jest packages/format/__tests__/formatter/FieldFormatter.spec.ts --runInBand — 24 tests passed
  • git diff --check — clean

GitHub currently reports the Test and dependency-review workflows as action_required with no jobs, and the PR is blocked on required review. No further code change is needed from my side unless the maintainers identify a different compatibility concern.

@11suixing11

Copy link
Copy Markdown
Author

Additional independent verification for this PR:

  • Clean frozen-lockfile install completed.
  • Workspace build passed.
  • Lint passed after the build.
  • Jest passed: 37 suites / 606 tests.
  • Workspace examples passed.

No code changes were needed in this PR; it remains waiting on the repository's required review/workflow gate.

@11suixing11

Copy link
Copy Markdown
Author

Closing this PR because it has been open for over 30 days without maintainer review or response. The change and validation record remain in the PR; please reopen or comment if the issue is still relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Denotes a PR that changes 10-29 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] The | character causes unnecessary quotation of the value

1 participant