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

normalize seems to override coerce callback result #56

Closed
rpl opened this issue Aug 24, 2016 · 5 comments
Closed

normalize seems to override coerce callback result #56

rpl opened this issue Aug 24, 2016 · 5 comments
Labels

Comments

@rpl
Copy link

rpl commented Aug 24, 2016

It seems that when the two property are combined, normalize overrides the coerced value with the normalized version of the original value of the option.

Minimal nodejs snippet to reproduce the issue:

var yargs = require('yargs');
var path = require('path');

var argv = yargs.option('source-dir', {
  normalize: true,
  coerce: path.resolve
}).argv;

console.log("result", {
  sourceDir: argv.sourceDir,
  resolvedSourceDir: path.resolve(argv.sourceDir)
});
$ node test-normalize-coerce.js --source-dir ../src/../src
result { sourceDir: '../src',
  resolvedSourceDir: '/resolved/dir/src' }
@maxrimue
Copy link
Member

Hey, thanks for reporting! I suppose for now you can instead do something like this:

var argv = yargs.option('source-dir', {
  coerce: val => path.resolve(path.normalize(val))
}).argv;

@maxrimue
Copy link
Member

I think #57 squashed the bug found here

@bcoe
Copy link
Member

bcoe commented Sep 15, 2016

@maxrimue @rpl yeah, #57 should fix this; let's work hard to get an update out to yargs this weekend; I think we might need to make it a major, since our coercion is significantly different.

@maxrimue
Copy link
Member

@bcoe I'd say we add a part about the handling of objects in coercion to the readme at yargs when Greenkeeper sends a pull request for yargs-parser once we published the change in the next major 👍

@bcoe
Copy link
Member

bcoe commented Sep 26, 2016

@maxrimue @rpl #57 is released in yargs-parser@4.0.0; @maxrimue feel free to add that doc update 👍

@bcoe bcoe closed this as completed Sep 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants