Add enum to string arguments #237

Merged
merged 1 commit into from Aug 13, 2012

Conversation

Projects
None yet
3 participants
@sferik
Owner

sferik commented Jul 8, 2012

This gives users the ability to define a method option like this:

method_option "fruit", :type => :string, :enum => %w(apple banana)

which constrains the string value to the values in the array.

@acesuares

This comment has been minimized.

Show comment Hide comment
@acesuares

acesuares Jul 15, 2012

+1

+1

@sferik

This comment has been minimized.

Show comment Hide comment
@sferik

sferik Jul 15, 2012

Owner

@acesuares I was able to get this working in the enum branch. I'd be interested to get your feedback before merging it into master. I would also like to use the same logic to select unambiguous substrings of tasks to select unambiguous substrings of enums. What do you think about that? Good or bad idea?

Owner

sferik commented Jul 15, 2012

@acesuares I was able to get this working in the enum branch. I'd be interested to get your feedback before merging it into master. I would also like to use the same logic to select unambiguous substrings of tasks to select unambiguous substrings of enums. What do you think about that? Good or bad idea?

@sferik

This comment has been minimized.

Show comment Hide comment
@sferik

sferik Jul 15, 2012

Owner

FYI, here's my main use case: sferik/t@9d9d38c. In this case, resolving unambiguous substrings to their full values would be nice but I could imagine cases where you'd want an enum to be a strict constraint. Maybe this should be another flag on on string types. Something like strict_enum which could be true or false (probably true by default)?

Owner

sferik commented Jul 15, 2012

FYI, here's my main use case: sferik/t@9d9d38c. In this case, resolving unambiguous substrings to their full values would be nice but I could imagine cases where you'd want an enum to be a strict constraint. Maybe this should be another flag on on string types. Something like strict_enum which could be true or false (probably true by default)?

@acesuares

This comment has been minimized.

Show comment Hide comment
@acesuares

acesuares Jul 15, 2012

Fine work, is it really needed to do all the extra parsing, or just get the first version out the door, the one that does nothing else then take the full strings (no aliases, no unambiguity) and then maybe later make it more versatile? Don't be mad, just asking. I couldnt follow the link to your use case, could you paste it again? thx

Fine work, is it really needed to do all the extra parsing, or just get the first version out the door, the one that does nothing else then take the full strings (no aliases, no unambiguity) and then maybe later make it more versatile? Don't be mad, just asking. I couldnt follow the link to your use case, could you paste it again? thx

@acesuares

This comment has been minimized.

Show comment Hide comment
@acesuares

acesuares Jul 15, 2012

Would the use of :enum => %w(apple banana) also automatically create a :banner => "apple|banana" by default?

    DATABASE_OPTIONS = %w(sqlite mysql)
    method_option :database, :aliases => "-d", :default => DATABASE_OPTIONS.first, :banner => DATABASE_OPTIONS.join('|'), :desc => 'specify development database'

Would the use of :enum => %w(apple banana) also automatically create a :banner => "apple|banana" by default?

    DATABASE_OPTIONS = %w(sqlite mysql)
    method_option :database, :aliases => "-d", :default => DATABASE_OPTIONS.first, :banner => DATABASE_OPTIONS.join('|'), :desc => 'specify development database'
@sferik

This comment has been minimized.

Show comment Hide comment
@sferik

sferik Jul 15, 2012

Owner

The default banner for a string option is the name of the option. The possible values (as well as the default value) are displayed in the second column, below the description. Like this:

Owner

sferik commented Jul 15, 2012

The default banner for a string option is the name of the option. The possible values (as well as the default value) are displayed in the second column, below the description. Like this:

@acesuares

This comment has been minimized.

Show comment Hide comment
@acesuares

acesuares Jul 15, 2012

That is also a good solution, +1 for what it's worth. Could you send me the use case, in my browser it show up as sferik/t@9d9d38c. and if I click it, it opens an email...

That is also a good solution, +1 for what it's worth. Could you send me the use case, in my browser it show up as sferik/t@9d9d38c. and if I click it, it opens an email...

@sferik

This comment has been minimized.

Show comment Hide comment
@sferik

sferik Jul 15, 2012

Owner

Ah, that appears to be a bug in GitHub-Flavored Markdown. Here's the full link: sferik/t@9d9d38c

Owner

sferik commented Jul 15, 2012

Ah, that appears to be a bug in GitHub-Flavored Markdown. Here's the full link: sferik/t@9d9d38c

@acesuares

This comment has been minimized.

Show comment Hide comment
@acesuares

acesuares Jul 15, 2012

Personally I would go for the strict implementation of enum in this case. I think typing

t -d s -p m -t p

is less useful then

t --development sqlite --production mysql --test postgresql
t -d sqlite -p mysql -t postgresql

and it there really a need to be able to do this:

t --deve sql --pr my --t pos

But, if it can do all that, well, that's good, there is no such thing as too much features...

Personally I would go for the strict implementation of enum in this case. I think typing

t -d s -p m -t p

is less useful then

t --development sqlite --production mysql --test postgresql
t -d sqlite -p mysql -t postgresql

and it there really a need to be able to do this:

t --deve sql --pr my --t pos

But, if it can do all that, well, that's good, there is no such thing as too much features...

@acesuares

This comment has been minimized.

Show comment Hide comment
@acesuares

acesuares Jul 15, 2012

On second thought, debugging a script when someone adds an option that makes a previously unambiguous option ambiguous might be problematic...

On second thought, debugging a script when someone adds an option that makes a previously unambiguous option ambiguous might be problematic...

sferik added a commit that referenced this pull request Aug 13, 2012

Merge pull request #237 from wycats/enum
Add enum to string arguments

@sferik sferik merged commit 802545d into master Aug 13, 2012

1 check was pending

default The Travis build is in progress
Details
@travisbot

This comment has been minimized.

Show comment Hide comment
@travisbot

travisbot Aug 13, 2012

This pull request fails (merged 0150b86 into 5523b5f).

This pull request fails (merged 0150b86 into 5523b5f).

@rafaelfranca rafaelfranca deleted the enum branch Aug 12, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment