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

Upgrade ignore dependency to version 5 #104

Closed
wants to merge 2 commits into from
Closed

Upgrade ignore dependency to version 5 #104

wants to merge 2 commits into from

Conversation

yaodingyd
Copy link

@yaodingyd yaodingyd commented Jan 14, 2019

Fix #92

@sindresorhus
Copy link
Owner

I haven't look into this closely, but I always get suspicious when you have to modify tests to get something working. Are you sure this is the correct way to solve it? I would have thought we needed to do some changes to gitignore.js, like in

return p => ignores.ignores(slash(path.relative(cwd, p)));

gitignore.js Outdated Show resolved Hide resolved
@sindresorhus
Copy link
Owner

@sindresorhus sindresorhus changed the title upgrade ignore to 5 Upgrade ignore dependency to version 5 Feb 15, 2019
@yaodingyd
Copy link
Author

The 5x breaking change is that for every path it uses const REGEX_TEST_INVALID_PATH = /^\.*\/|^\.+$/ this regex to check invalid path and throws errors. So in the test case for "foo.js" after path.relative it returns ../../foo.js which throws errors. I want to confirm if globby is used, all the file paths from fastglob would be relative to current cwd and we would not have this issue, and all I need to do is take care of the test file?

Or we do need to consider this kind of path so in line 48 we need to check if path matches that regex?

@sindresorhus
Copy link
Owner

Or we do need to consider this kind of path so in line 48 we need to check if path matches that regex?

I don't know. Part of the task of fixing #92 is figuring that out.

@sindresorhus
Copy link
Owner

@yaodingyd Did you figure it out?

@yaodingyd
Copy link
Author

I'm checking the input path: if it matches a relative path starting with ./ then I get relative path to cwd; otherwise I would pass it directly to ignore. I think my assumption makes sense: usually when we use globby, we would use it under cwd and any files path looks like the test ones, they should be under cwd, which is the results of path.relative, so that we can directly use them.

@futpib futpib mentioned this pull request Mar 16, 2019
@sindresorhus
Copy link
Owner

Closing in favor of #20

@yaodingyd yaodingyd deleted the ignore branch June 14, 2019 17:54
@fisker fisker mentioned this pull request Feb 26, 2020
4 tasks
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.

Update ignore package to latest version
2 participants