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

Add test for issue when excluding a nested dir from a higher level gitignore file #4301

Closed
wants to merge 1 commit into from

Conversation

jakemac53
Copy link
Contributor

@jakemac53 jakemac53 commented Jun 5, 2024

Repro test for #4300

I don't see a good mechanism for skipping individual parts of a test so this can just land after the issue is fixed (or close this and write different tests)

@sigurdm
Copy link
Contributor

sigurdm commented Jun 10, 2024

As mentioned in the issue I believe you are "holding it wrong".

I think we implement the correct rules here. They are just somewhat obscure.


// Check that you can unignore certain files too.
await d.dir('myapp', [
d.file('.gitignore', '*.txt\n!foo.txt'),
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this is tested well enough in the general ignore-rules tests. Eg. here:

'.': ['f*', '!file.txt'],


// Check that you can unignore certain dirs from a higher level gitignore.
await d.dir('reporoot', [
d.file('.gitignore', '*.txt\n!myapp/'),
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this does whatever you think it does.

myapp/ is not ignored here, so it cannot meaningfully be negated/unignored.

We could test that "/*\n!myapp/" leads to no warnings. That is already the case.

Suggested change
d.file('.gitignore', '*.txt\n!myapp/'),
d.file('.gitignore', '/*\n!myapp/'),

@jakemac53
Copy link
Contributor Author

Sounds good, that behavior seems bizarre to me but as long as we are consistent with git, that is what matters :)

@jakemac53 jakemac53 closed this Jun 10, 2024
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