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

challenge(formatter): respect prettier Range options #774

Merged

Conversation

Gumichocopengin8
Copy link
Contributor

@Gumichocopengin8 Gumichocopengin8 commented Nov 19, 2023

Summary

Some test cases in prettier-snap files did not respect Prettier's Range options (formatted the entire code instead of the partial code in the range). This is the fix to respect the Range options.

I updated test cases with Prettier repo as of 093745f0ec429d3db47c1edd823357e0ef24e226 commit hash

Test Plan

  • Make sure prepare_tests.js changes are valid and updated test cases are valid.
  • cargo test -p biome_js_formatter --test prettier_tests

Note

When running node prepare_tests.js <<Prettier Repo Path>>, the following files are updated, but looks like these are not compatible with the current Biome, so ignored these changes.

    specs::prettier::js::comments::function::between_parentheses_and_function_body_js
    specs::prettier::js::comments::return_statement_js
    specs::prettier::js::for_::continue_and_break_comment_without_blocks_js
    specs::prettier::js::sequence_expression::parenthesized_js
    specs::prettier::js::strings::escaped_js
    specs::prettier::js::switch::comments2_js
    specs::prettier::typescript::as_::expression_statement_ts
    specs::prettier::typescript::prettier_ignore::mapped_types_ts
    specs::prettier::typescript::typeparams::print_width_120::issue_7542_tsx
    specs::prettier::typescript::union::consistent_with_flow::single_type_ts
    specs::prettier::typescript::union::inlining_ts
    specs::prettier::typescript::union::single_type::single_type_ts

@github-actions github-actions bot added A-Formatter Area: formatter L-JavaScript Language: JavaScript and super languages labels Nov 19, 2023
@Gumichocopengin8 Gumichocopengin8 marked this pull request as ready for review November 19, 2023 00:25
@Conaclos
Copy link
Member

Conaclos commented Nov 19, 2023

Thanks for your contribution :)
I will be happy to merge once my comment addressed :)

EDIT: I removed myself report.md

@Conaclos Conaclos force-pushed the fix/update-prettier-test-cases branch from 192f532 to cf03855 Compare November 19, 2023 17:36
@Conaclos Conaclos merged commit 6ce1352 into biomejs:main Nov 19, 2023
12 checks passed
@Gumichocopengin8 Gumichocopengin8 deleted the fix/update-prettier-test-cases branch November 19, 2023 19:52
@Gumichocopengin8
Copy link
Contributor Author

@Conaclos Thank you for reviewing the PR, and sorry that I didn't notice there was a similar PR

@ematipico
Copy link
Member

@Conaclos Thank you for reviewing the PR, and sorry that I didn't notice there was a similar PR

Not your fault, don't worry. We failed to communicate the work. That's on us. Thank you for your contribution! 🙏

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants