fix(inspect): allow single-line format when breakLength is Infinity#64238
fix(inspect): allow single-line format when breakLength is Infinity#64238hamidrezaghavami wants to merge 1 commit into
Conversation
nodejs-github-bot
added
needs-ci
ljharb
left a comment
ljharb
left a comment
There was a problem hiding this comment.
this definitely needs tests to cover the changed behavior. ideally, you'd also provide the tests' output absent this change.
|
Hello, @ljharb The single-line formatting behaviour when breakLength is set to Infinity is covered in a test case that I have provided. |
| { // test case | ||
| const obj = { a: 1, b: 2, c: 3, d: 4 }; | ||
| const result = util.inspect(obj, { breakLength: Infinity }); | ||
| assert.strictEqual(result, '{ a: 1, b: 2, c: 3, d: 4 }'); |
There was a problem hiding this comment.
i get this result on node already. we'd need a test case that breaks without the fix.
hamidrezaghavami
force-pushed
the
fix-inspect-infinity-clean
branch
6 times, most recently
from
038a50a to
b91c45e
Compare
| { | ||
| const obj = { long: 'a'.repeat(100), another: 'b'.repeat(100) }; | ||
| const result = util.inspect(obj, { breakLength: Infinity }); | ||
| assert.ok(!result.includes('\n')); |
There was a problem hiding this comment.
this test still passes on node already; your fix isn't related.
hamidrezaghavami
force-pushed
the
fix-inspect-infinity-clean
branch
2 times, most recently
from
31523f9 to
3060df3
Compare
hamidrezaghavami
force-pushed
the
fix-inspect-infinity-clean
branch
3 times, most recently
from
ede2dd7 to
34da682
Compare
|
@ljharb All formatting, linting, and core test suites are now fully passing (29/30 checks successful). The single remaining check (test-macOS) failed on an unrelated debugger timeout (test-debugger-exceptions.js 15000ms exceeded), which appears to be a macOS CI runner flake. Could you please re-run that failed macOS job when you have a moment to review? |
|
@hamidrezaghavami again, your test case already passes, so it's not a regression test. |
|
@ljharb To clarify my intent: this patch is a performance early exit fast path inside isBelowBreakLength when breakLength === Infinity, avoiding unnecessary string width calculation loops. Because observable output is identical to main, standard tests pass on both branches. Should we label/track this strictly as a performance optimization/cleanup rather than a bug fix? |
|
ahh ok, that wasn't clear to me before |
| d: { nested: { deep: true } } | ||
| }; | ||
| const output = util.inspect(obj, { breakLength: Infinity }); | ||
| assert.strictEqual(output.includes('\n'), false); |
There was a problem hiding this comment.
let's put the exact expected output here, not just check for a newline
Signed-off-by: Hamid Reza Ghavami <hamidr.ghavami@gmail.com>
hamidrezaghavami
force-pushed
the
fix-inspect-infinity-clean
branch
from
34da682 to
31719ca
Compare
3 participants
Description
When calling
util.inspect()with abreakLengthconfiguration explicitly set toInfinity, the internal layout formatting logic should bypass multi-line chunking and formatting constraints entirely, allowing the contents to naturally evaluate on a single line.Currently,
isBelowBreakLengthgoes through a character length loop calculation even when length checks are logically unnecessary due to the infinite upper bound. This change introduces an explicit early return branch withinisBelowBreakLengthwhenctx.breakLength === Infinity, properly enabling a clean, un-wrapped single-line string formatting mode.Checklist