feat: add breakLength option for single-line output#50
feat: add breakLength option for single-line output#50abhu85 wants to merge 2 commits intoinspect-js:mainfrom
Conversation
When breakLength is set to Infinity, output will always be single-line regardless of the indent option.
This matches the behavior of Node.js `util.inspect({ breakLength: Infinity })`.
Fixes inspect-js#47
- Add validation for breakLength option (positive integer or Infinity)
- When breakLength is Infinity, skip indentation to force single-line output
- Add tests for validation and functionality
- Update README documentation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Friendly ping for review when you have a chance. Note: This PR was generated with Claude Code. If you're not comfortable with AI-generated contributions, please let me know and I'll close it - no hard feelings. The implementation is straightforward enough that a maintainer could do it directly if preferred. |
|
Maintainer edits are now enabled. Could you re-run the CI checks? Thanks! |
ljharb
left a comment
There was a problem hiding this comment.
I don’t see any tests for a finite breakLength. Also, why is zero disallowed?
|
Thanks for the review @ljharb! Re: Why is zero disallowed? Re: Tests for finite breakLength
Note: The current implementation focuses on the |
|
If there’s only two modes - multi and single line - then surely it should just be a boolean? |
|
@Ijharb Good point. My implementation currently only distinguishes:
I don't actually measure output length against a finite threshold. The original issue #47 requested What's your preference?
|
|
We should indeed be matching util.inspect. We shouldn't add the option until we have full compatibility. |
|
@ljharb Updated in 959da36 — How it works:
Examples: inspect({a: {b: 1}, c: 2}, {indent: 2, breakLength: 30})
// '{ a: { b: 1 }, c: 2 }' (22 chars <= 30, fits)
inspect({a: {b: 1}, c: 2}, {indent: 2, breakLength: 10})
// '{\n a: { b: 1 },\n c: 2\n}' (outer breaks, inner 8 chars <= 10 fits)
inspect({a: {b: 1}, c: 2}, {indent: 2, breakLength: 5})
// both levels break (inner 8 > 5)Backward compatible: When All 256 tests pass. |
ljharb
left a comment
There was a problem hiding this comment.
The feature works and the tests are comprehensive, but there are some issues to address before merging.
|
Thanks for the detailed review! I've addressed all 6 issues: 1. Performance regression (fixed)Gated if (indent && !singleLineValues(xs)) {
return '[' + indentedJoin(xs, indent) + ']';
}
var singleLine = '[ ' + $join.call(xs, ', ') + ' ]';
if (indent && hasBreakLength && singleLine.length > breakLength) {
return '[' + indentedJoin(xs, indent) + ']';
}
return singleLine;2. Documentation of semantic change (fixed)Added explicit note in README:
3. Function parameters (fixed)Refactored function collectionOf(type, size, entries, opts)
// opts: { breakOpts: { has, len }, indent }4 & 5. Comment typos (fixed)
6. README formatting (fixed)Reformatted as bulleted list similar to other options. All 260 tests pass! |
4236618 to
5832fbb
Compare
|
I've rebased this with a few tweaks. Note that the Map test checks breakLength: 0 and breakLength: Infinity but doesn't test a finite threshold boundary (eg, the exact length where single-line transitions to multi-line). The object and array tests do this already, so it'd be nice to mirror that here (probably for Set also) |
Add tests that verify the exact transition point from single-line to multi-line output for Map and Set, mirroring the existing object and array threshold tests: - Map (28 chars): breakLength 28 → single-line, 27 → multi-line - Set (17 chars): breakLength 17 → single-line, 16 → multi-line Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Added threshold boundary tests for Map and Set in 27e8f42:
These now mirror the object and array threshold tests. All 264 tests pass. |
Summary
Fixes #47
Adds support for the
breakLengthoption, matching Node.jsutil.inspectbehavior. WhenbreakLengthis set toInfinity, output will always be single-line regardless of theindentoption.This allows users to get compact, single-line output like:
Changes
breakLengthoption validation and logicInfinityInfinity, forces single-line output by setting indent to nullbreakLengthoptionTest Results
All 237 tests pass.