fix(jsonfmt): expand non-empty arrays - #640
Conversation
kehoecj
left a comment
There was a problem hiding this comment.
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.
Summary
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