Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(formatter): add expand #4819

Merged
merged 7 commits into from
Jan 3, 2025
Merged

feat(formatter): add expand #4819

merged 7 commits into from
Jan 3, 2025

Conversation

ematipico
Copy link
Member

@ematipico ematipico commented Jan 1, 2025

Summary

This PR closes #4370

I added a new option called expand. It was suggested expandLiterals, but it doesn't seem very beginner-friendly:

  • it's not very clear what "literal" means to a beginner
  • strings, booleans and numbers are also literals, so it could be confusing

However, feel free to suggest some other name.

Also, package.json is always expanded, unless the user decides to change the defaults.

Test Plan

I added new tests

@github-actions github-actions bot added A-CLI Area: CLI A-Project Area: project A-Formatter Area: formatter L-JSON Language: JSON and super languages labels Jan 1, 2025
Copy link

codspeed-hq bot commented Jan 1, 2025

CodSpeed Performance Report

Merging #4819 will not alter performance

Comparing feat/json-formatting (53a9018) with next (17cf8f7)

Summary

✅ 97 untouched benchmarks

@ematipico ematipico linked an issue Jan 1, 2025 that may be closed by this pull request
@ematipico ematipico marked this pull request as ready for review January 1, 2025 22:35
@ematipico ematipico requested review from a team January 1, 2025 22:35
Copy link
Contributor

@arendjr arendjr left a comment

Choose a reason for hiding this comment

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

I do admit I’m not really happy with the naming. My first thought was the option was something to configure lists of something to expand (kinda like allowLists), and I only understood what it actually meant after reading the description. But maybe that’s just me 😅

But even after reading it, I understood it would apply to arrays, but I was still surprised it applied to objects too. I don’t think I’ve ever seen objects called lists in any terminology before, so this doesn’t really make sense to me.

I actually think there’s less confusion with expandLiterals, because even though the term literals applies to primitives too, I don’t think anyone is surprised it doesn’t expand literals that cannot be expanded.

I also think “literal” is not really a beginner-unfriendly term. It’s surely better than calling things incorrectly to avoid the actual correct words.

I also considered expandObjects. At least there’s the precedent that JavaScript considers arrays to be objects, but I still prefer expandLiterals overall, since both “object literal” and “array literal” are commonly accepted in JavaScript terminology and expandObjects may just introduce confusion in the opposite direction that expandLists does.

@ematipico
Copy link
Member Author

ematipico commented Jan 2, 2025

What do you think of just expand? We can document it by saying:

  • that Biome will try to format the document by using less width as possible;
  • or, that Biome will try to format lists like arrays and objects on multiple lines

@arendjr
Copy link
Contributor

arendjr commented Jan 2, 2025

Hm, yeah. Given it’s only for JSON, where there aren’t many other explanations possible, I think that could work:)

Introduce the ability to configure JSON list formatting with an "expand lists" option. This ensures arrays and objects can always be expanded across multiple lines when specified. Defaults are applied differently for `package.json` files unless overridden.
@ematipico ematipico changed the title feat(formatter): add expandLists feat(formatter): add expand Jan 3, 2025
@ematipico ematipico requested a review from arendjr January 3, 2025 09:31
@ematipico ematipico force-pushed the feat/json-formatting branch from d58887d to daa03d6 Compare January 3, 2025 09:31
@ematipico ematipico requested a review from a team January 3, 2025 09:31
.changeset/added_a_json_format_option_expandslists.md Outdated Show resolved Hide resolved
.changeset/the_file_packagejson.md Outdated Show resolved Hide resolved
ematipico and others added 2 commits January 3, 2025 09:40
@ematipico ematipico merged commit 78c8910 into next Jan 3, 2025
12 checks passed
@ematipico ematipico deleted the feat/json-formatting branch January 3, 2025 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-CLI Area: CLI A-Formatter Area: formatter A-Project Area: project L-JSON Language: JSON and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Format package.json like Prettier does, by default
2 participants