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

feat: normalize patterns to handle "./" prefix in files and ignores #162

Merged
merged 8 commits into from
Mar 23, 2025

Conversation

Pixel998
Copy link
Contributor

Prerequisites checklist

What is the purpose of this pull request?

Addresses issue where patterns starting with "./" were not correctly matched in files and ignores.

What changes did you make? (Give an overview)

Strip "./" from the start of strings in files and ignores so that they match as expected.

Related Issues

eslint/eslint#18757

Is there anything you'd like reviewers to focus on?

@eslint-github-bot eslint-github-bot bot added the bug Something isn't working label Mar 11, 2025
Copy link
Member

@nzakas nzakas 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 taking a look at this. Overall, it looks good.

I think we have a question here: Is it safe to overwrite files in an existing object or should we be creating a new object? I think we may need to create a new object to be safe, but that would also increase the number of objects we create. And if we do that, perhaps we should only do so when files has at least one pattern starting with "./" and reuse every other config as-is?

@eslint/eslint-tsc what do you think?

@mdjermanovic
Copy link
Member

I think we should create a new object when we need to modify its content. In this case, where at least one of its files / ignores patterns starts with "./" or "!./".

@mdjermanovic mdjermanovic changed the title fix: normalize patterns to handle "./" prefix in files and ignores feat: normalize patterns to handle "./" prefix in files and ignores Mar 18, 2025
@mdjermanovic
Copy link
Member

I changed the tag to feat as this enables patterns that are currently no-op.

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Would like @mdjermanovic to approve before merging.

Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@mdjermanovic mdjermanovic merged commit bbbe909 into eslint:main Mar 23, 2025
18 checks passed
@github-actions github-actions bot mentioned this pull request Mar 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working contributor pool feature
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

3 participants