higgsjs / Higgs

Higgs JavaScript Virtual Machine
875 stars 62 forks source link

Create an Options Parser. #176

Closed sbstp closed 9 years ago

sbstp commented 9 years ago
Features:
 - Argument collection
 - Long options: --long
 - Short options: -s
 - Option description
 - Default values
 - Type checking and error reporting
 - Available types: string, boolean, int, +int, float, +float
 - Type inference using the default value (when provided)
 - Automatic convertion of types (from strings)
 - Mandatory options and error reporting
 - Automatic help and version display (when turned on)

Parsing:
    Values are assigned using an equal sign.
        --opt=val, -o=val
    Short options can be grouped together. The last
    option of a short option group can have a value:
        -abc=def c gets the value 'def'

     -a             => {a: true}
     -a=value       => {a: 'value'}
     -ab            => {a: true, b: true}
     -ab=value      => {a: true, b: 'value'}
     --long=value   => {long: 'value'}

     Anything that is not preceded by an option is considered
     a plain argument.
maximecb commented 9 years ago

Thank you for this contribution :)

maximecb commented 9 years ago

Hi @SBSTP

I decided to make use of your options parser in the turing-turtle example program just to dogfood it a little bit.

I've run into some minor issues and have some suggestions for improvements.

  1. it seems that when the options fail to parse as expected, the option parser crashes, no help or usage is displayed:

./higgs ../examples/turing-turtle.js -- -w 200 -h 200

TypeError: call to non-function "charCodeAt" RegExpContext(1B9F0) ("stdlib/regexp.js"@980:28) RegExp_prototype_test(1C281) ("stdlib/regexp.js"@2675:19) testIntPositive(7759D) ("lib/options.js"@326:16) Options_prototype_parse(776E6) ("lib/options.js"@454:22) ___examples_turing_turtle_js(22AEF) ("../examples/turing-turtle.js"@275:12)

  1. I would suggest adding tests for invalid strings. To at least make sure no exception is thrown.
  2. The lib is imported as such:

var Options = require('lib/options').Options;

It seems to me that there isn't really much of a use case for having multiple options parsers though? Maybe require('lib/options') should produce a "singleton" option parser, avoiding the need to instantiate one. Let me know what you think.

  1. The .add(.., .., ..., ...) syntax with all the null and optional arguments is confusing. I'd prefer something like:

options.add({name: 'width', type: '+int', shortname:'w', desc:'canvas width'});

I'm aware that you already have a syntax to add many options at once this way. I think that .add() should probably work this way as well.

  1. It might be useful to have some optional min and max values for numerical arguments.
sbstp commented 9 years ago

Ok I found why it doesn't work. The format is

./higgs ../examples/turing-turtle.js -- -w=200 -h=200
                                          ^      ^

Using spaces makes the parser much more complex and creates ambiguities as to what is an option value and what's a plain argument, hence why I used =. Still though, the parser shouldn't crash because of that.

maximecb commented 9 years ago

Yes, no complaints on the use of equals, I prefer it this way also. It's only the crashing that seemed an issue.

sbstp commented 9 years ago

Yes, I fixed it now, it yields a type error. I'm working on the API changes.

maximecb commented 9 years ago

Btw, JS also has a SyntaxError: https://github.com/higgsjs/Higgs/blob/master/source/stdlib/error.js#L135