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

test: add ignore-unknown tests #27

Merged
merged 1 commit into from
Feb 2, 2025

Conversation

43081j
Copy link
Contributor

@43081j 43081j commented Jan 30, 2025

Adds the ignore-unknown tests from prettier.

Combining --write and --list-different is not allowed in prettier-cli, so one test was removed which asserted this was possible.

Some snapshots were updated because the error message differs.

Adds the `ignore-unknown` tests from prettier.

Combining `--write` and `--list-different` is not allowed in
prettier-cli, so one test was removed which asserted this was possible.

Some snapshots were updated because the error message differs.
@fabiospampinato
Copy link
Collaborator

If you could mention the name of the test that wasn't ported over it would be useful, in this case I think it was the "ignore-unknown write" one.

I think we should probably keep the test but remove the --list-different flag 🤔

As I'm understanding it the --list-different flag is basically useless here, the --write command will list the differences regardless, so whether --list-different is provided or not doesn't really matter to us in this case, right?

I think it's probably good to be a little stricter here compared to v3 🤔

I'll make the changes on my end for quick iteration's sake. Thanks 👍

@fabiospampinato fabiospampinato merged commit 5f9b315 into prettier:main Feb 2, 2025
@43081j
Copy link
Contributor Author

43081j commented Feb 2, 2025

i think we're doing the right thing and v3 was too lenient (in that you shouldn't be able to combine list-different and write)

@43081j 43081j deleted the ignore-unknown branch February 2, 2025 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants