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-path tests #25

Merged
merged 1 commit into from
Feb 2, 2025
Merged

Conversation

43081j
Copy link
Contributor

@43081j 43081j commented Jan 29, 2025

Adds the ignore-path tests from prettier.

Also fixes a bug where non-existent ignore files should be treated as if we loaded an empty file (currently they throw).

Part of #19

Adds the `ignore-path` tests from prettier.

Also fixes a bug where non-existent ignore files should be treated as if
we loaded an empty file (currently they throw).
return await fs.readFile(filePath, "utf8");
} catch {
// Treat a missing file as if it was empty (no ignores)
return "";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

im not a huge fan of creating these empty strings, but if we don't, ignoreManual will end up empty and we'll apply default gitignore etc

prettier doesn't do that, it treats a non-existent file as if it existed (i.e. it turns off the defaults)

Copy link
Collaborator

@fabiospampinato fabiospampinato Feb 2, 2025

Choose a reason for hiding this comment

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

Arguably the behavior in v4 is better, like you point the CLI to something that doesn't exist and we should just silently fail? 🤔 Pretty weird, but ok we can align with that.


I'll change it to be a one-liner in an amending commit though, as that kinda breaks how the code flows visually, which I hate.

@fabiospampinato
Copy link
Collaborator

I see we deleted a test where --debug-check is used, that's ok but best to document anything we change in these files.

@fabiospampinato fabiospampinato merged commit 64d7acb into prettier:main Feb 2, 2025
@fabiospampinato
Copy link
Collaborator

Thanks!

@fabiospampinato
Copy link
Collaborator

It looks like 2 out of 3 tests are not passing. If some tests are not skipped I'm assuming they were passing, were these passing on your end? Maybe I broke something.

@fabiospampinato
Copy link
Collaborator

Ah wait I forgot to compile the thing again 🤣 all good!

@43081j 43081j deleted the ignore-path branch February 2, 2025 19:10
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