sindresorhus / globby

User-friendly glob matching
MIT License
2.53k stars 129 forks source link

Don't pass `options.ignore` to `isGitIgnored` #223

Closed fisker closed 2 years ago

fisker commented 2 years ago

ignore option in isGitIgnored() mean to ignore .gitignore files, but when running globby(), ignore mean to ignore matched files.

For example:

globbySync('**/*', {gitignore: true, ignore: ['.gitignore']})

This mean to glob all files that's not gitignored, but except .gitignore file, not mean don't read the .gitignore file.

Before:

> globbySync('**/*', {gitignore: true, ignore: ['.gitignore']}).filter(file => file.startsWith('node_modules')).length
11662

After:

> globbySync('**/*', {gitignore: true, ignore: ['.gitignore']}).filter(file => file.startsWith('node_modules')).length
0

This is a little hard to test, I didn't add.

fisker commented 2 years ago

@sindresorhus After second thought, this ignore option for isGitIgnored doesn't make sense to me either, why someone want to skip reading a specific .gitignore file and let the result be true?

Do you think we should remove it?

sindresorhus commented 2 years ago

No idea. It was added a long time ago. I do agree with your reasoning though.

fisker commented 2 years ago

It was added in https://github.com/sindresorhus/globby/pull/48, I think it's added for xo, maybe mean to check file is ignored by ignore option, but it never works that way.

sindresorhus commented 2 years ago

Yeah, let's remove it.