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

Make enums comparable #5

Merged
merged 1 commit into from
May 19, 2020
Merged

Conversation

buschmann23
Copy link
Collaborator

I thought enums were already comparable, but this only applied to the equality operator, not for less than etc. My unit tests for the comparison operators were successfully completed, but this was apparently rather coincidental.

This now adds the missing comparison operators to MetaEnumVariable and registers them with QMetaType::registerComparators().

I thought enums were already comparable, but this only applied to the
equality operator, not for less than etc. My unit tests for the comparison
operators were successfully completed, but this was apparently rather
coincidental.

This now adds the missing comparison operators to MetaEnumVariable and
registers them with QMetaType::registerComparators().
@dantti dantti merged commit 35e7dbf into cutelyst:master May 19, 2020
@dantti
Copy link
Member

dantti commented May 19, 2020

Looks, fine, while you are at it do you think adding support for QJson types would be difficult?
I tried to use them one time but looks like it didn't work

buschmann23 added a commit to buschmann23/cutelee that referenced this pull request May 20, 2020
This adds support for Qt’s JSON types as proposed in the comment of PR cutelyst#5.

The difficulty in using Qt’s JSON data types was that they can be
casted to QVariantList/QVariantHash if they are inside a QVariant and
e.g. `QVariant::canConvert<QVariantList>` returns true for a QJsonArray,
but the next step by Cutelee is to cast them into QAssociativeIterable
or QSequentialIterable. That only works for template based containers.

So, my solution is to “intercept“ this types before the QVariant cast is
tried and convert them directly into QVariantList or QVariantHash. If
the JSON type is direct part of the context, this happens in
`Cutelee::Variable::resolve()`, if the JSON type is stored in a context
variable’s property or further down the road, the lookup will be done in
`Cutelee::MetaType::lookup()`.

Unit tests are currently written to test “normal“ usage as variable and
to be used in a `{% for %}` loop tag. Maybe we should add some more tests
to see if they play nicely with filters and the rest, even though I
think that this implementations should make this possible without
further changes.
@buschmann23 buschmann23 mentioned this pull request May 20, 2020
dantti pushed a commit that referenced this pull request May 21, 2020
* Qt JSON types support

This adds support for Qt’s JSON types as proposed in the comment of PR #5.

The difficulty in using Qt’s JSON data types was that they can be
casted to QVariantList/QVariantHash if they are inside a QVariant and
e.g. `QVariant::canConvert<QVariantList>` returns true for a QJsonArray,
but the next step by Cutelee is to cast them into QAssociativeIterable
or QSequentialIterable. That only works for template based containers.

So, my solution is to “intercept“ this types before the QVariant cast is
tried and convert them directly into QVariantList or QVariantHash. If
the JSON type is direct part of the context, this happens in
`Cutelee::Variable::resolve()`, if the JSON type is stored in a context
variable’s property or further down the road, the lookup will be done in
`Cutelee::MetaType::lookup()`.

Unit tests are currently written to test “normal“ usage as variable and
to be used in a `{% for %}` loop tag. Maybe we should add some more tests
to see if they play nicely with filters and the rest, even though I
think that this implementations should make this possible without
further changes.

* Use QJsonValue::toVariant() in Variable::resolve()

Make the switch a bit simpler and convert directly to QVariant.

* Use QJson data types in JSON lookup functions
@buschmann23 buschmann23 deleted the comparableenums branch May 22, 2020 10:13
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