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

Handle an edge case with well-known ignores when applying exclusions #141

Merged
merged 6 commits into from
Dec 8, 2024

Conversation

daniilsapa
Copy link
Collaborator

Closes #138

This was a subtle one. The problem was that the folders that were disabled in config but still had diagnostics reported on them in my local env contained .DS_Store files. .DS_Store files prevented a folder from being excluded from VFS. The reason is that minimatch apparently has some registry of ignored files and it always returns false for them. It becomes a problem when we apply inverted glob groups (not(...)) as we invert the result when matching such a group and end up including those files/folders

Let me know if I failed to clearly articulate the problem and you need more explanations

@daniilsapa daniilsapa requested a review from illright December 7, 2024 13:07
Copy link

changeset-bot bot commented Dec 7, 2024

🦋 Changeset detected

Latest commit: 12301b8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
steiger Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Member

@illright illright left a comment

Choose a reason for hiding this comment

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

Let's try to find another solution, because this solution might actually produce unexpected behavior for user-created hidden files (i.e. .eslintrc.js, contrived example, but still). Maybe we can even discuss it in the issue rather than in the PR

* Checks if a file/folder is a well-known ignore.
*
* If a path does not match the "any file/folder" pattern, it is considered a well-known ignore.
* It looks like minimatch has some registry of such files, and it returns false for them regardless of the pattern.
Copy link
Member

Choose a reason for hiding this comment

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

note: I think what's happening here is that hidden files (those whose name starts with a dot) are not matched by globs by default. That's how terminal shells handle globs too.

issue: I don't think ignoring hidden files it's the right approach here, since there are legitimate reasons for why you might want to make a source code file hidden.

suggestion: I'd rather investigate why these files are picked up at all. I was under the impression that we automatically disregard the files that are in .gitignore. If we don't, then we should, I think.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh, yeah, it was so obvious to me that it should have ignored only default hidden FS-specific files that I didn't even checked how it behaves for dot files as such. Yes, it turns out this happens with any dot files, not just a few specific ones. Given these new circumstances, yes, that's not the right approach at all.

Yes, I'll investigate why the files that are in .gitignore are not disregarded by default

@daniilsapa
Copy link
Collaborator Author

I rolled my previous changes back. It turned out you need to supply globby's isGitIgnored with a path to the root of a git repo. It doesn't search it recursively. Adding that function solved the problem

Copy link
Member

@illright illright left a comment

Choose a reason for hiding this comment

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

Thanks for finding the issue and fixing it :)

@illright illright merged commit 5aaf1a3 into master Dec 8, 2024
7 checks passed
@illright illright deleted the bug/insignificant-slice-glob-matching branch December 8, 2024 23:22
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.

Steiger reports insignificant-slice diagnostics on disabled part of file system
2 participants