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

Depreciate regular expressions for copy's filter #239

Closed
martinheidegger opened this issue Apr 21, 2016 · 6 comments
Closed

Depreciate regular expressions for copy's filter #239

martinheidegger opened this issue Apr 21, 2016 · 6 comments

Comments

@martinheidegger
Copy link

martinheidegger commented Apr 21, 2016

Regular Expression filters in ncp.js:38 are run against the full source path, i.e.:

/Users/martin/Private/projects/nodeschool/git-it-electron/node_modules/eslint/node_modules/uglify-js/node_modules/source-map/lib/util.js

If I want in the folder git-it-electron to exclude the eslint folder I need to know that I am in the /Users/martin/Private/projects/nodeschool/git-it-electron folder when I do that. If I test for /eslint/ then It would exclude the folder properly but If by some accident I would put the project in a folder called:

/Users/eslint/Private/projects/nodeschool/git-it-electron

It would ignore all of the files in my project 😢 . This means that someone would need to implement the file matching with that in mind. Given that ncp.js already considers the working directory I think it would be specially awesome if

A. there would be the opportunity to have filters that work relatively to the working directory
B. there would be the opportunity to specify where the working directory is (overriding the good process.cwd() default)

Additionally it would be super awesome if the filter would allow glob file patterns instead of regexp patterns because they are made for files.

Note: I am thinking about the API a little and think it would be good to have an include and exclude option instead of filters. (Considering #65)

@jprichardson
Copy link
Owner

What if the RegExp's were deprecated and you just passed a function {filter: function() { }}... this way you could use glob?

@jprichardson
Copy link
Owner

FYI, functions are supported now.

@martinheidegger
Copy link
Author

They are supported but they don't know the base path. A deprecation might
be worth thinking about. Also it might be worth to specify the filter API
to be directly compatible with glob in order for the filter to work like
this: include: glob.bind(null, "**/*.js"). I could also think an API like
include: require("fs-extra/pattern")("**/*.js") might be good.

@RyanZim
Copy link
Collaborator

RyanZim commented Oct 26, 2016

@jprichardson ping?

@jprichardson
Copy link
Owner

I'd prefer only functions, but I'm afraid that it'll break a lot of code. Long-term, this should use file glob patterns with another module like glob. So we should at least deprecate the Regex version for 1.0.0.

Thoughts?

@RyanZim
Copy link
Collaborator

RyanZim commented Oct 26, 2016

@jprichardson Migration from regexes is simple:

- /somereg.*/
+ /somereg.*/.test

Edit: Incorrect, you will lose the this binding for test. Must do (using arrow functions):

s => /somereg.*/.test(s)

If you want to depreciate, go ahead.

@RyanZim RyanZim changed the title Rethinking the ignore Regexp Filters in ncp.js Depreciate regular expressions for copy's filter Oct 26, 2016
@martinheidegger martinheidegger changed the title Depreciate regular expressions for copy's filter Deprecate regular expressions for copy's filter Oct 26, 2016
@martinheidegger martinheidegger changed the title Deprecate regular expressions for copy's filter Depreciate regular expressions for copy's filter Oct 26, 2016
@RyanZim RyanZim added this to the 1.0.0 milestone Oct 26, 2016
RyanZim added a commit that referenced this issue Oct 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants