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

Fixed determining Boolean from false #378

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ktimothy
Copy link

This is a small hack, that fixes incorrect behaviour in #377.

The problem, as I see it, is that Virtus::Attribute::Boolean has only TrueClass as a primitive. This makes Virtus::TypeLookup to determine correctly only true, because it executes TrueClass >= true.class. The same expression with false - TrueClass >= false.class will not allow TypeLookup to think that false is somehow connected to Virtus::Attribute::Boolean.

I replaced TrueClass primitive with BooleanPrimitive, that returns true when comparing with both TrueClass and FalseClass, thus allowing TypeLookup to connect true and false values with Virtus::Attribute::Boolean.

@ktimothy
Copy link
Author

@elskwid could you please take a look?

@ktimothy
Copy link
Author

@elskwid any thoughts? =)

@dblock
Copy link

dblock commented Jul 5, 2019

This got re-raised in ruby-grape/grape#1577. Maybe someone cares to merge this? Thank you!

@dblock
Copy link

dblock commented Jul 5, 2019

I just read the discontinued note. Sad to see Virtus go, thanks @solnic for your hard work. We'll move off it eventually.

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