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

fix bug for default value as proc when there is given options values #779

Closed

Conversation

ShPakvel
Copy link
Contributor

@ShPakvel ShPakvel commented Oct 8, 2014

When you have option default as a proc and also any kind of option values (as a proc or normal) the gem is crashed. This small change solves it.

@dblock
Copy link
Member

dblock commented Oct 9, 2014

Can you please update CHANGELOG? For future googling purposes, also, what's 'crashes'? What's the specific error?

@ShPakvel
Copy link
Contributor Author

ShPakvel commented Oct 9, 2014

Using grape you can meet error like 'block (2 levels) in <class:API>': wrong number of arguments (1 for 0) (ArgumentError) with code like:

params do
  optional :type, values: ['valid-type1', 'valid-type2'], default: -> { ['valid-type1', 'valid-type2'].sample }
end
get '/default_lambda' do
  { type: params[:type] }
end

params do
  optional :type, values: -> { ValuesModel.values }, default: -> { ValuesModel.values.sample }
end
get '/default_and_values_lambda' do
  { type: params[:type] }
end

And this PR fixes this bug. The problem was about default option value is proc when used with values. There was not call in place which checks inclusion if default in values.

@ShPakvel
Copy link
Contributor Author

ShPakvel commented Oct 9, 2014

@dblock I've added info to changelog and put some description of the problem here.
Let me know if you need something else.

@dblock
Copy link
Member

dblock commented Oct 10, 2014

Merged via 00e745e, changed the language in the CHANGELOG a bit (and put back the your contribution here note).

@dblock dblock closed this Oct 10, 2014
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

Successfully merging this pull request may close these issues.

2 participants