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

Raise exception for reserved words (namely flash) #158

Closed
cllns opened this issue Jun 8, 2016 · 7 comments
Closed

Raise exception for reserved words (namely flash) #158

cllns opened this issue Jun 8, 2016 · 7 comments
Assignees
Milestone

Comments

@cllns
Copy link
Member

cllns commented Jun 8, 2016

This issue started as a request for documentation that flash is a reserved word when using Hanami::Action::Session: hanami/hanami#595

Instead of just documenting it, we should help the developer by raising an exception when they try to use any reserved word.

One reserved word is flash but we should make sure all reserved words are covered.

(Thanks for the original issue @rafaels88)

@nickskalkin
Copy link

If nobody minds, I'll get this issue. But I have no idea how to implement it yet, hah

@jodosha
Copy link
Member

jodosha commented Jun 29, 2016

@Vladimirich Please do.

@akhramov
Copy link

@Vladimirich are you still up for it?

@akhramov
Copy link

Hi everyone. I think we can go with guard module. The idea behind is dead simple:

  • Create a module implementing #expose method, which raises an exception if given list of words contains one or more reserved word. Otherwise it calls super.
  • Prepend this module to an action.
  • Introduce an alias method for #expose (#expose! or #_expose) for internal usage. This alias won't be watched by the guard so no exceptions are raised.

I have created a proof of concept can be found here.

Any thoughts?

Thank you.

@nickskalkin
Copy link

nickskalkin commented Aug 3, 2016

@akhramov , yep, sorry, was on vacation and forget to inform about it in this issue

@nickskalkin
Copy link

@akhramov , but if you already did something you can take this issue. What do you think?

@akhramov
Copy link

akhramov commented Aug 3, 2016

@Vladimirich let us try, thanks.

@jodosha jodosha added this to the v0.8.0 milestone Aug 10, 2016
@jodosha jodosha self-assigned this Aug 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants