Skip to content

fix: highlight escaped JSON strings correctly - #188

Merged
NuroDev merged 8 commits into
mainfrom
NuroDev/fix-json-string-highlighting
Oct 2, 2026
Merged

NuroDev merged 8 commits into
mainfrom
NuroDev/fix-json-string-highlighting

Conversation

@NuroDev

@NuroDev NuroDev commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

Fixes #187.

JSON string highlighting currently stops at an escaped quote, leaving the rest of the value unstyled. Consume JSON escape sequences as pairs so the complete string value stays highlighted.

Add regression coverage for escaped quotes, backslashes, Windows paths, and other JSON escapes. Confirm non-TTY and NO_COLOR output remains plain, parseable JSON even with FORCE_COLOR set. Include a patch changeset.

Validated on macOS:

  • pnpm check
  • pnpm --filter cf test: 1,425 passed
  • pnpm --filter @cloudflare/wrangler-tests test: 637 passed, with existing skipped and todo cases

@pkg-pr-new

pkg-pr-new Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
npm i https://pkg.pr.new/cf@188

commit: af500bb

@ask-bonk

ask-bonk Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

LGTM

github run

@NuroDev NuroDev self-assigned this Oct 2, 2026
@github-actions

github-actions Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

cf --help startup

mean stddev min max
PR 321.0 ms 6.1 ms 313.9 ms 342.2 ms
base 326.4 ms 12.3 ms 319.8 ms 377.3 ms

Δ (PR vs base): ↓ -5.4 ms (-1.6%)

Measured with hyperfine on ubuntu-latest (3 warmup runs, 20 timed runs). CI numbers carry ±a few ms of runner jitter; treat small deltas as noise.

@NuroDev
NuroDev marked this pull request as ready for review October 2, 2026 08:20
@NuroDev
NuroDev enabled auto-merge October 2, 2026 08:21
@ask-bonk

ask-bonk Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

LGTM

github run

Comment thread packages/cli/src/lib/output.ts
@ask-bonk

ask-bonk Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

🤖 I've done a thorough review of your PR.

Fixes escaped JSON-string highlighting and adds regression coverage.

  1. Warning: packages/cli/src/lib/output.ts:83 still mis-highlights escaped quotes followed by : as a JSON key. Posted an inline suggestion.

github run

@ask-bonk

ask-bonk Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

LGTM

github run

Comment thread packages/cli/src/__tests__/lib/output.test.ts Outdated
Comment thread packages/cli/src/__tests__/lib/output.test.ts
Comment thread packages/cli/src/lib/output.ts
@ask-bonk

ask-bonk Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

LGTM

github run

@ask-bonk

ask-bonk Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

LGTM

github run

@NuroDev
NuroDev added this pull request to the merge queue Oct 2, 2026
Comment thread packages/cli/src/lib/output.ts
Merged via the queue into main with commit 2d8007d Oct 2, 2026
11 checks passed
@NuroDev
NuroDev deleted the NuroDev/fix-json-string-highlighting branch October 2, 2026 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JSON string highlighting breaks on escaped quotation marks

3 participants