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

📎 Implement format option singleAttributePerLine #1706

Closed
ematipico opened this issue Jan 30, 2024 · 5 comments · Fixed by #1712
Closed

📎 Implement format option singleAttributePerLine #1706

ematipico opened this issue Jan 30, 2024 · 5 comments · Fixed by #1712
Assignees
Labels
A-Formatter Area: formatter L-JavaScript Language: JavaScript and super languages S-Enhancement Status: Improve an existing feature S-Help-wanted Status: you're familiar with the code base and want to help the project

Comments

@ematipico
Copy link
Member

ematipico commented Jan 30, 2024

Description

From the discussion #698

It would be great if someone wanted to implement this option.

Here's the prettier reference: https://prettier.io/docs/en/options#single-attribute-per-line

I'm happy to reply to any questions you might have.

@ematipico ematipico added S-Help-wanted Status: you're familiar with the code base and want to help the project A-Formatter Area: formatter L-JavaScript Language: JavaScript and super languages S-Enhancement Status: Improve an existing feature labels Jan 30, 2024
@octoshikari
Copy link
Contributor

Hi @ematipico ! I've start working on this issue and have 1 question about running specific jsx tests with newly added configuration option singleAttributePerLine. For now add this in crates/biome_js_formatter/tests/prettier_tests.rs for options variable, but it apply for all tests and i have a lot of snapshot changes. Can you help with how i should do it correctly for testing purpose

@ematipico
Copy link
Member Author

ematipico commented Jan 30, 2024

That's expected because we track the value of the options in the snapshot tests. Just make sure that the snapshots show the new option. Then, commit all these new snapshot changes, and start creating new test cases. With the new test cases, you should be able to see the difference between the default value and the new value.

@octoshikari
Copy link
Contributor

octoshikari commented Jan 30, 2024

I mean , how i can apply new optionsfor specific tests cases not for all existed. It my first contribution to biome and i not a familiar with all processes

@ematipico
Copy link
Member Author

Hi @octoshikari , sorry for the lack of context, I didn't have much time yesterday.

Creating new tests with new options in the formatter is a bit hectic and outdated, and I hope to renovate it soon, maybe with some help.

So, usually, you can find an options.json file, like this one:

{
"cases": [
{
"trailing_comma": "ES5"
},
{
"trailing_comma": "None"
}
]
}

Since we are adding a new option, we have to tell the testing infrastructure to understand and read the new option. You'll have to add a new option here:

pub struct JsSerializableFormatOptions {
/// The indent style.
pub indent_style: Option<JsSerializableIndentStyle>,
/// The indent width.
pub indent_width: Option<u8>,
/// The type of line ending.
pub line_ending: Option<JsSerializableLineEnding>,
/// What's the max width of a line. Defaults to 80.
pub line_width: Option<u16>,
/// The style for quotes. Defaults to double.
pub quote_style: Option<JsSerializableQuoteStyle>,
/// The style for JSX quotes. Defaults to double.
pub jsx_quote_style: Option<JsSerializableQuoteStyle>,
/// When properties in objects are quoted. Defaults to as-needed.
pub quote_properties: Option<JsSerializableQuoteProperties>,
/// Print trailing commas wherever possible in multi-line comma-separated syntactic structures. Defaults to "all".
pub trailing_comma: Option<JsSerializableTrailingComma>,
/// Whether the formatter prints semicolons for all statements or only in for statements where it is necessary because of ASI.
pub semicolons: Option<JsSerializableSemicolons>,
/// Whether to add non-necessary parentheses to arrow functions. Defaults to "always".
pub arrow_parentheses: Option<JsSerializableArrowParentheses>,
/// Whether to insert spaces around brackets in object literals. Defaults to true.
pub bracket_spacing: Option<bool>,
/// Whether to hug the closing bracket of multiline HTML/JSX tags to the end of the last line, rather than being alone on the following line. Defaults to false.
pub bracket_same_line: Option<bool>,

And then you have to apply this option here, by adding a new function called with_*

.with_bracket_same_line(
self.bracket_same_line
.map_or_else(BracketSameLine::default, |value| value.into()),
)

This should be enough to prepare the testing infrastructure.

Then, I suggest creating a new folder here: https://github.com/biomejs/biome/tree/main/crates/biome_js_formatter/tests/specs/jsx

As you can see, there are already two folder that test a particular option. Follow the same pattern. Create a new JSX file with what you want to test. Then create options.json file and add the new option you created before. You must use underscores.

@octoshikari
Copy link
Contributor

@ematipico Hi! Thank you for good clarification. I'm already created PR for this issue and added new test case with options.json usage. Will be glad for any feedback when you will have time for that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Formatter Area: formatter L-JavaScript Language: JavaScript and super languages S-Enhancement Status: Improve an existing feature S-Help-wanted Status: you're familiar with the code base and want to help the project
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants