feat(curl,task): highlight + pretty-print responses - #1432
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
commit: |
|
Warning Review limit reached
Next review available in: 59 seconds You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe CLI adds shared multi-language syntax highlighting and an HTML formatter. The curl command now formats headers and response bodies by stream and content type. Task output uses the shared JSON highlighter. The old JSON-only utility and tests are removed. Unit tests cover highlighting, HTML formatting, and curl response handling. The build configuration bundles the highlighting dependency. Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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.
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 `@packages/nuxt-cli/src/commands/curl.ts`:
- Line 237: Update the output flow around formatBody so the final-newline check
uses the formatted body returned by formatBody, rather than the original
text/content. Preserve the existing behavior while ensuring responses whose
formatted HTML ends with a newline keep the shell prompt separated.
In `@packages/nuxt-cli/src/utils/format-html.ts`:
- Line 7: Update NODE_RE in format-html.ts so quoted single- and
double-attribute values are consumed atomically before recognizing a tag-ending
`>`, preserving values such as `data-x="a>b"` without inserting formatting
inside them. Add a regression test covering a quoted attribute followed by a
block element and verify the attribute content remains unchanged.
🪄 Autofix
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: CHILL
Plan: Pro Plus
Run ID: 552584f9-ff97-470b-a198-52d6f2329363
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
packages/nuxt-cli/package.jsonpackages/nuxt-cli/src/commands/curl.tspackages/nuxt-cli/src/commands/task/_utils.tspackages/nuxt-cli/src/utils/format-html.tspackages/nuxt-cli/src/utils/highlight.tspackages/nuxt-cli/src/utils/json-highlight.tspackages/nuxt-cli/test/unit/utils/format-html.spec.tspackages/nuxt-cli/test/unit/utils/highlight.spec.tspackages/nuxt-cli/test/unit/utils/json-highlight.spec.tspackages/nuxt-cli/tsdown.config.ts
💤 Files with no reviewable changes (2)
- packages/nuxt-cli/test/unit/utils/json-highlight.spec.ts
- packages/nuxt-cli/src/utils/json-highlight.ts
|
Could be nice to highlight markdown as it becomes a new standard for LLM to send back Markdown with specific Accept header |
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 `@packages/nuxt-cli/src/commands/curl.ts`:
- Line 22: Update packages/nuxt-cli/src/commands/curl.ts at lines 22 and 303-304
to distinguish HTML from generic XML and SVG content types, invoking formatHtml
only for HTML while preserving XML/SVG text and applying highlighting without
rewriting it. Update packages/nuxt-cli/test/unit/commands/curl.spec.ts at lines
285-293 to assert that XML output remains unchanged after VT control characters
are removed.
- Line 25: Update NDJSON_CONTENT_TYPE_RE to recognize the application/ndjson
media type alongside the existing supported subtypes, and add a regression test
covering this content type so the NDJSON highlighting branch is exercised.
🪄 Autofix
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: CHILL
Plan: Pro Plus
Run ID: f67eb2e0-6f6c-43dd-8b70-be40245b7f47
📒 Files selected for processing (4)
packages/nuxt-cli/src/commands/curl.tspackages/nuxt-cli/src/utils/highlight.tspackages/nuxt-cli/test/unit/commands/curl.spec.tspackages/nuxt-cli/test/unit/utils/highlight.spec.ts
| const HAS_SCHEME_RE = /^[a-z][a-z\d+.-]*:\/\//i | ||
| const JSON_CONTENT_TYPE_RE = /^application\/(?:[\w.+-]+\+)?json\b/i | ||
| /** Markup reindented as HTML: the HTML rules cover XML documents too. */ | ||
| const MARKUP_CONTENT_TYPE_RE = /^(?:text\/(?:html|xml)|(?:application|image)\/(?:[\w.+-]+\+)?xml)\b/i |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not reindent generic XML or SVG responses.
formatHtml inserts newlines and indentation between elements. In XML, including SVG, that whitespace can be significant character data. A terminal response copied after formatting can therefore differ semantically from the received response.
packages/nuxt-cli/src/commands/curl.ts#L22-L22: distinguish HTML from generic XML and SVG content types.packages/nuxt-cli/src/commands/curl.ts#L303-L304: callformatHtmlonly for HTML. Highlight generic XML and SVG without rewriting their text.packages/nuxt-cli/test/unit/commands/curl.spec.ts#L285-L293: assert that XML remains unchanged after VT control characters are removed.
📍 Affects 2 files
packages/nuxt-cli/src/commands/curl.ts#L22-L22(this comment)packages/nuxt-cli/src/commands/curl.ts#L303-L304packages/nuxt-cli/test/unit/commands/curl.spec.ts#L285-L293
🤖 Prompt for 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.
In `@packages/nuxt-cli/src/commands/curl.ts` at line 22, Update
packages/nuxt-cli/src/commands/curl.ts at lines 22 and 303-304 to distinguish
HTML from generic XML and SVG content types, invoking formatHtml only for HTML
while preserving XML/SVG text and applying highlighting without rewriting it.
Update packages/nuxt-cli/test/unit/commands/curl.spec.ts at lines 285-293 to
assert that XML output remains unchanged after VT control characters are
removed.
| const MARKUP_CONTENT_TYPE_RE = /^(?:text\/(?:html|xml)|(?:application|image)\/(?:[\w.+-]+\+)?xml)\b/i | ||
|
|
||
| /** Newline-delimited JSON keeps one record per line, so each line is highlighted on its own. */ | ||
| const NDJSON_CONTENT_TYPE_RE = /^application\/(?:x-ndjson|jsonl|json-seq)\b/i |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Recognize application/ndjson.
The pattern accepts application/x-ndjson, application/jsonl, and application/json-seq, but not application/ndjson. Responses with that content type skip the NDJSON branch and are emitted without highlighting.
Add the subtype and add a regression case.
Proposed fix
-const NDJSON_CONTENT_TYPE_RE = /^application\/(?:x-ndjson|jsonl|json-seq)\b/i
+const NDJSON_CONTENT_TYPE_RE = /^application\/(?:x-ndjson|ndjson|jsonl|json-seq)\b/i🤖 Prompt for 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.
In `@packages/nuxt-cli/src/commands/curl.ts` at line 25, Update
NDJSON_CONTENT_TYPE_RE to recognize the application/ndjson media type alongside
the existing supported subtypes, and add a regression test covering this content
type so the NDJSON highlighting branch is exercised.
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 `@packages/nuxt-cli/src/commands/curl.ts`:
- Line 25: Update NDJSON_CONTENT_TYPE_RE to recognize application/json-seq
alongside the existing NDJSON media types, restoring per-record JSON
highlighting. Add a regression test covering application/json-seq rendering.
- Around line 280-281: Update the body decoding immediately before the renderer
invocation to parse the response Content-Type charset, decode the buffer with
that charset, and fall back to UTF-8 when the charset is missing or unsupported;
preserve the existing renderer and piping behavior. Add a regression test
covering a non-UTF-8 response charset such as iso-8859-1.
🪄 Autofix
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: CHILL
Plan: Pro Plus
Run ID: e9941bb8-f376-400d-97c4-66e0bb5f7d01
📒 Files selected for processing (5)
packages/nuxt-cli/src/commands/curl.tspackages/nuxt-cli/src/utils/format-html.tspackages/nuxt-cli/src/utils/highlight.tspackages/nuxt-cli/test/unit/commands/curl.spec.tspackages/nuxt-cli/test/unit/utils/format-html.spec.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/nuxt-cli/test/unit/commands/curl.spec.ts
- packages/nuxt-cli/src/utils/highlight.ts
- packages/nuxt-cli/src/utils/format-html.ts
- packages/nuxt-cli/test/unit/utils/format-html.spec.ts
| const MARKUP_CONTENT_TYPE_RE = /^(?:text\/(?:html|xml)|(?:application|image)\/(?:[\w.+-]+\+)?xml)\b/i | ||
|
|
||
| /** Newline-delimited JSON keeps one record per line, so each line is highlighted on its own. */ | ||
| const NDJSON_CONTENT_TYPE_RE = /^application\/(?:(?:x-)?ndjson|jsonl)\b/i |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 5 'NDJSON_CONTENT_TYPE_RE|CONTENT_TYPE_LANGUAGES|json-seq|resolveRenderer' \
packages/nuxt-cli/src/commands/curl.ts \
packages/nuxt-cli/test/unit/commands/curl.spec.tsRepository: nuxt/cli
Length of output: 4093
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== curl.ts relevant sections =="
sed -n '1,120p' packages/nuxt-cli/src/commands/curl.ts
sed -n '310,345p' packages/nuxt-cli/src/commands/curl.ts
echo "== json-seq occurrences =="
rg -n 'json-seq|NDJSON|JSON_CONTENT_TYPE_RE|CONTENT_TYPE_LANGUAGES|highlight\(' packages/nuxt-cli/src packages/nuxt-cli/test || trueRepository: nuxt/cli
Length of output: 8921
Restore application/json-seq rendering.
CONTENT_TYPE_LANGUAGES does not map application/json-seq, so per-record JSON highlighting is lost for that media type. Add json-seq back to NDJSON_CONTENT_TYPE_RE and cover it with a regression test.
🤖 Prompt for 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.
In `@packages/nuxt-cli/src/commands/curl.ts` at line 25, Update
NDJSON_CONTENT_TYPE_RE to recognize application/json-seq alongside the existing
NDJSON media types, restoring per-record JSON highlighting. Add a regression
test covering application/json-seq rendering.
🔗 Linked issue
📚 Description
this follows up on #1417 to highlight more responses (currently JSON + HTML). we use https://github.com/speed-highlight/core for this. it's a well-maintained library that's tiny (when inlined - 9.3 kB). only issue is that it doesn't expose
tokenize- opening a PR shortly to add this feature...we also pretty-print html, only rewriting whitespace at block-level boundaries to avoid changing the meaning of a request.