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

Unable to indicate option should be a string #894

Closed
dougharris opened this issue Sep 10, 2013 · 7 comments
Closed

Unable to indicate option should be a string #894

dougharris opened this issue Sep 10, 2013 · 7 comments

Comments

@dougharris
Copy link

When passing a release number to a task which has a trailing zero, e.g. grunt -r 3.10, the code which grabs this, grunt.option("r") treats this as a number and forever after, this is 3.1, not 3.10.

Simple, sample gruntfile to demonstrate:

module.exports = function(grunt) {
  grunt.registerTask('default', 'Release preparation', function () {
    var rel = grunt.option("r").toString();

    grunt.log.writeln("Release data type:" + typeof rel);
    grunt.log.writeln("release (" + rel + ")");
  });
};

Output:

$ grunt -r 3.10
Running "default" task
Release data type:string
release (3.1)

Done, without errors.

(Also asked on stackoverflow for a workaround)

@shama
Copy link
Member

shama commented Sep 10, 2013

That is an interesting issue. Even when you quote it, grunt -r "3.10" it still doesn't work. I guess as a work around do grunt -r 3.10.0 until we can address this. Thanks!

@cowboy
Copy link
Member

cowboy commented Sep 11, 2013

I'm not sure how to handle this. Grunt uses nopt@~1.0.10 for parsing CLI options. The nopt README says:

When parsing unknown fields, "true", "false", and "null" will be interpreted as their JavaScript equivalents, and numeric values will be interpreted as a number.

The problem is that nopt doesn't check to see if the value, once converted to a number and then back to a string matches the original string.

This seems like a bug in how nopt parses numbers, I've filed an issue here npm/nopt#24.

I'm not sure that there's a fix, other than to force nopt to treat the value as a string by prefixing it with a non-number, like v1.10.

@dougharris
Copy link
Author

I don't think that always preserving the trailing zero and/or keeping the "number" as a string is the right answer either. There are likely grunt users who depend on the option value coming in as a number.

nopt has the ability to define expected data types for known options. What about adding an option to the grunt configuration in which the Gruntfile can similarly define data types for options specific to the project. If this optional configuration doesn't exist, then option handling proceeds as it does currently.

@explunit
Copy link

@dougharris the invoke of nopt for parsing happens in grunt-cli, but your Gruntfile will not be processed by grunt until later. If I'm reading the code correctly, a change like this would mean using nopt twice: once by grunt-cli to get the options for grunt itself, and once by grunt to get any project-specific options.

@dougharris
Copy link
Author

@explunit I didn't say it'd be easy, I'm just suggesting a possible solution from the point of view of the end user. :-)

@cowboy
Copy link
Member

cowboy commented Sep 11, 2013

This is unfortunately not possible with the current architecture of Grunt and options parser that Grunt uses, but is definitely a reasonable consideration for the future.

At some point soon, we're going to be releasing a survey where you'll be able to request features for a future Grunt. I strongly recommend requesting this feature there.

@cowboy cowboy closed this as completed Sep 11, 2013
@cowboy
Copy link
Member

cowboy commented Sep 11, 2013

Ok, here's an updated issue for nopt npm/nopt#24. If they can address both that and npm/nopt#23, It should be possible to upgrade to a new version of nopt in Grunt.

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

4 participants