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

[BUG]: Copyfiles Incorrectly Assigns Up #44

Closed
jd-carroll opened this issue Aug 27, 2018 · 4 comments
Closed

[BUG]: Copyfiles Incorrectly Assigns Up #44

jd-carroll opened this issue Aug 27, 2018 · 4 comments

Comments

@jd-carroll
Copy link

Steps to Reproduce:

$ mkdir ~/folder-with-a-up-in-path && cd $_
$ npm init -y
$ npm install copyfiles
$ # open package.json and add the following line to "scripts"
    "copyfiles": "copyfiles package.json dist/lib"
$ npm run copyfiles
Error: cant go up that far
    at dealWith (/home/user/folder-with-a-up-in-path/node_modules/copyfiles/index.js:30:11)
    at /home/user/folder-with-a-up-in-path/node_modules/copyfiles/index.js:80:39
    at FSReqWrap.oncomplete (fs.js:153:5)

The issue is that the script is being run in a folder with a literal -up in the path

The root of the issue the following block of code:

if (args.argv.$0.indexOf('up') > -1) {
  args.default('u', '1');
}

This looks like it is trying to assign / detect a default value if -u is ever specified without a value, the problem is it isn't. First of all it is incomplete code, it only works with -up and not -u. Second the args.argv.$0 will never contain accurate runtime args, it will only contain the folder.

This code creates a really nasty bug. I know yargs doesn't make it easy to support this functionality, the code above should be removed.

@jd-carroll
Copy link
Author

jd-carroll commented Aug 27, 2018

I'd also note that the recent release of yargs@12.0 may help solve this issue:
https://github.com/yargs/yargs/blob/master/CHANGELOG.md#breaking-changes

  • Options absent from argv (not set via CLI argument) are now absent from the parsed result object rather than being set with undefined

@calvinmetcalf
Copy link
Owner

what we're actually doing is seeing if the script was called as copyfiles or copyup and if it is called as copyup we have a different default value for the u parameter the issue is we are seeing is that I'm just looking for the word up in the results not, that it was called as copy up so I need to fix that logic

@calvinmetcalf
Copy link
Owner

just pushed something up, see if that fixes it for you

@jd-carroll
Copy link
Author

That works great, thank you! I missed that additional logic and get what you are trying to provide.

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

No branches or pull requests

2 participants