Skip to content

fix(jsonfmt): expand non-empty arrays - #640

Open
sidsri14 wants to merge 3 commits into
Boeing:feat/3.0from
sidsri14:agent/expand-json-arrays
Open

fix(jsonfmt): expand non-empty arrays#640
sidsri14 wants to merge 3 commits into
Boeing:feat/3.0from
sidsri14:agent/expand-json-arrays

Conversation

@sidsri14

@sidsri14 sidsri14 commented Aug 4, 2026

Copy link
Copy Markdown

Summary

  • expand non-empty root and property arrays to one element per line in JSON and JSONC
  • keep empty arrays and short nested array elements compact
  • remove the obsolete numeric fill layout and update canonical fixtures

Fixes #630

Testing

  • go test -count=1 -cover -coverprofile=/tmp/cfv-coverage.out ./... (92.3% total coverage)
  • golangci-lint v2.11.4 run ./... (0 issues)
  • go vet ./...
  • gofmt -s -l -e .
  • go generate ./pkg/filetype/... with a clean generated-file diff
  • CI-equivalent static Linux build

@sidsri14
sidsri14 requested a review from a team as a code owner August 4, 2026 10:04
@kehoecj kehoecj added OSS Community Contribution Contributions from the OSS Community waiting-on-maintainer-review PR is waiting to be reviewed and functionally tested by the maintainers labels Aug 4, 2026

@kehoecj kehoecj left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this @sidsri14! The code is clean and your isInlineArrayElement / inlineArrayValueLength changes work as intended.

When I was running this through the parity test suite, I realized I messed up the issue description. I said "always expand non-empty arrays" but that isn't actually what prettier does. I went and read prettier's source (src/language-js/print/array.js) and here's the real rule:

Short arrays stay on one line when they fit:

Input:  {"items": ["a", "b", "c"]}
prettier: { "items": ["a", "b", "c"] }
This PR: { "items": [\n    "a",\n    "b",\n    "c"\n  ] }

So given that, we need to keep the current collapse behavior here.

Arrays where every element is an array/object force-expand:

Input:  {"brackets": [["{","}"],["[","]"],["(",")"]]}
prettier:
{
  "brackets": [
    ["{", "}"],
    ["[", "]"],
    ["(", ")"]
  ]
}
Current cfv: "brackets": [["{", "}"], ["[", "]"], ["(", ")"]]

This is what needs to change: cfv collapses these when prettier expands them.

The rule from prettier source: force break when (1) more than 1 element, (2) every element is an array or object, (3) all elements are the same type, and (4) each inner element has more than 1 item.

Could you rework this to keep the width-based collapsing for simple arrays and only force-expand when that rule matches? I'll update issue #630 with the corrected spec.

@kehoecj kehoecj added pr-action-requested PR is awaiting feedback from the submitting developer and removed waiting-on-maintainer-review PR is waiting to be reviewed and functionally tested by the maintainers labels Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

OSS Community Contribution Contributions from the OSS Community pr-action-requested PR is awaiting feedback from the submitting developer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants