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

lib/rodauth/features/base.rb: add throw_status method that throws :rodauth_error without needing to set_field_error #418

Closed
wants to merge 2 commits into from

Conversation

jf
Copy link
Contributor

@jf jf commented Apr 26, 2024

as per #412

…rodauth_error without needing to set_field_error
@jeremyevans
Copy link
Owner

The method I said I would accept in #412 (reply in thread) would just call throw :rodauth_error. Something like:

def throw_error
  throw :rodauth_error
end

You could then write your own throw_status method that calls this method. I would not accept a throw_status method, because there is nothing internal to Rodauth that uses it, as I mentioned in #412 (comment).

@jf
Copy link
Contributor Author

jf commented Apr 27, 2024

I see. Thanks for the clarification. I'll update the PR

@jf
Copy link
Contributor Author

jf commented Apr 27, 2024

hm, just realized that there is a throw_error method already in lib/rodauth/features/base.rb that has 2 arguments (field, and error). I could call the new method throw_rodauth_error... but I'm not sure I like this name / approach (throw_rodauth_error as a name means you are sort of made aware of the specific error being thrown; but throw_error does not). What do you think of making field and error optional for throw_error like so?

def throw_error(field=nil, error=nil)
  set_field_error(field, error) unless field.nil? and error.nil?
  throw :rodauth_error
end

This way you can simply call throw_error to just throw :rodauth_error.. but if you want to also set_field_error, you can do that as well, with throw_error(field, error)

@jeremyevans
Copy link
Owner

I already made it clear in #412 (reply in thread) that
I don't want to make the throw_error arguments optional. I would be OK adding throw_rodauth_error as a separate method.

@jf
Copy link
Contributor Author

jf commented Apr 27, 2024

My apologies. In my mind we were talking about modifying throw_error_status (#412 (reply in thread)); I thought things might be different for throw_error.

I've updated the PR to add throw_rodauth_error.

@jeremyevans
Copy link
Owner

Squash merged at b44b441

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