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 ability to ignore directories #20

Closed
sindresorhus opened this issue Sep 7, 2015 · 23 comments
Closed

Add ability to ignore directories #20

sindresorhus opened this issue Sep 7, 2015 · 23 comments
Labels
enhancement 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt help wanted

Comments

@sindresorhus
Copy link
Member

sindresorhus commented Sep 7, 2015

Issuehunt badges

joakimbeng/unistyle@4dfa6bc#commitcomment-13087626

Right now you have to do directory/** to ignore all files in a directory, but would be nice, and probably more natural, if you could ignore a directroy directly by using directroy.


IssueHunt Summary

sindresorhus sindresorhus has been rewarded.

Backers (Total: $60.00)

Submitted pull Requests


Tips

@nevilgeorge-zz
Copy link

Could I get some more info about this? I think I could help, but not really sure what needs to be done exactly!

@kevva
Copy link
Contributor

kevva commented Nov 5, 2015

@nevilgeorge, you'd need to check if the items in ignores is a directory or not, and then append /** to it.

@sindresorhus
Copy link
Member Author

Should be done in globby: sindresorhus/globby#33

@pvdlg
Copy link
Contributor

pvdlg commented Jan 20, 2018

Should this issue be closed now that sindresorhus/globby#46 is merged?

@sindresorhus
Copy link
Member Author

sindresorhus commented Jan 20, 2018

Yes, just needs to be mentioned in the docs first.

@pvdlg
Copy link
Contributor

pvdlg commented Jan 20, 2018

After testing it doesn't works because globby expand directory only for the patterns not for the ignores.

That should works if we merge #288 though.

But it probably something that should be fixed in globby?

@sindresorhus
Copy link
Member Author

But it probably something that should be fixed in globby?

Yes. Can you open an issue over there?

@pvdlg
Copy link
Contributor

pvdlg commented Jan 20, 2018

See sindresorhus/globby#62

@IssueHuntBot
Copy link

@issuehuntfest has funded $60.00 to this issue. See it on IssueHunt

@itaisteinherz
Copy link

Is this still an issue? When testing locally using the latest version of XO (v0.23.0), XO seems to correctly ignore a directory given that package.json contains the following config:

{
	...
	"xo": {
		"ignores": [
			"dir"
		]
	},
	...
}

@issuehunt-oss issuehunt-oss bot added the 💵 Funded on Issuehunt This issue has been funded on Issuehunt label May 10, 2019
@pvdlg
Copy link
Contributor

pvdlg commented Feb 17, 2020

It was fixed by sindresorhus/globby#88

@pvdlg pvdlg closed this as completed Feb 17, 2020
@pvdlg pvdlg reopened this Feb 17, 2020
@pvdlg
Copy link
Contributor

pvdlg commented Feb 17, 2020

Didn't mean to closed before someone else confirms

@sindresorhus
Copy link
Member Author

@pvdlg I think we need to update globby first. I haven't done so yet, as it drops support for Windows paths (or rather fast-glob did), so we need to manually handle that here or wait for mrmlnc/fast-glob#240.

@pvdlg
Copy link
Contributor

pvdlg commented Feb 25, 2020

Just to be sure I fully understand.
The problem is about passing a non glob windows path to XO such as:

$ xo C:\my-project\my-file.js

which fast-glob wrongly interprets as a glob.

But it's not related to use globs with windows backslashes such as:

$ xo C:\my-project\*.js

Is that right?
If yes we should be able to add the code you linked in sindresorhus/del@01da91f#diff-168726dbe96b3ce427e7fedce31bb0bcR41-R53 to globby?

@sindresorhus
Copy link
Member Author

No, the inverse. xo C:\my-project\*.js will no longer work as globbing patterns can only have forward slashes now. So we need to manually convert the backward slashes to forward slashes.


If yes we should be able to add the code you linked in sindresorhus/del@01da91f#diff-168726dbe96b3ce427e7fedce31bb0bcR41-R53 to globby?

Yes, but I'm not super excited about having to do that manually in every library that uses globby. I would prefer to solve the problem at the globby side with maybe a separate method that accepts both paths and globs, or an option, which would handle transforming the slashes.

@pvdlg
Copy link
Contributor

pvdlg commented Feb 25, 2020

No, the inverse. xo C:\my-project*.js will no longer work as globbing patterns can only have forward slashes now. So we need to manually convert the backward slashes to forward slashes.

I'm confused then. sindresorhus/del@01da91f#diff-168726dbe96b3ce427e7fedce31bb0bcR45 make the transformation only if the pattern is not a glob.
The way I understand the code is that C:\my-project\*.js will not be transformed while C:\my-project\my-file.js will be.

Yes, but I'm not super excited about having to do that manually in every library that uses globby. I would prefer to solve the problem at the globby side with maybe a separate method that accepts both paths and globs, or an option, which would handle transforming the slashes.

What I meant was to call normalizePatterns inside globby before passing the patterns to fast-glob so libraries using globby wouldn't have to do anything.

@sindresorhus
Copy link
Member Author

Ah, your right. I was just confused by your wording:

which fast-glob wrongly interprets as a glob.

It doesn't "wrongly interpret it as a glob", fast-glob doesn't do anything wrong. Anything you pass it is a glob and the backslashes are interpreted as escapes.


So yes, we're on the same page.


What I meant was to call normalizePatterns inside globby before passing the patterns to fast-glob so libraries using globby wouldn't have to do anything.

👍

@fregante
Copy link
Member

fregante commented Mar 20, 2020

eslint already supports ignoring patterns: https://eslint.org/docs/user-guide/configuring#ignorepatterns-in-config-files

Setting xo.ignorePatterns should work, according to #428 (comment)

@fisker
Copy link
Contributor

fisker commented Aug 8, 2021

This is fixed, we use this feature ourself

xo/package.json

Lines 108 to 114 in 5ff95ad

"xo": {
"ignores": [
"test/fixtures",
"test/temp",
"coverage"
]
},

@sindresorhus
Copy link
Member Author

@fisker Do you know what commit fixed it? No worries if not, I just need to know who to give the bounty to.

@fisker
Copy link
Contributor

fisker commented Aug 8, 2021

I don't know, but both globby and eslint seems
support that, and we run ignore check on both of them. So the one add support on globby, the one update globby, and the one update eslint?

@issuehunt-oss issuehunt-oss bot added 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt and removed 💵 Funded on Issuehunt This issue has been funded on Issuehunt labels Aug 8, 2021
@issuehunt-oss
Copy link

issuehunt-oss bot commented Aug 8, 2021

@sindresorhus has rewarded $54.00 to @sindresorhus. See it on IssueHunt

  • 💰 Total deposit: $60.00
  • 🎉 Repository reward(0%): $0.00
  • 🔧 Service fee(10%): $6.00

@sindresorhus
Copy link
Member Author

I'll just put the bounty on some other issue then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt help wanted
Projects
None yet
Development

No branches or pull requests

8 participants