Skip to content

Conversation

gikari
Copy link
Contributor

@gikari gikari commented Jan 6, 2022

I hope, I handled enum right.

Ref #221

@gikari gikari force-pushed the gikari/qjvalue-is-null-undefined branch 2 times, most recently from d4538d5 to 5c3f034 Compare January 6, 2022 13:49
@ratijas
Copy link
Collaborator

ratijas commented Jan 6, 2022

LGTM, except unrelated qml_register_module formatting change.

@ratijas ratijas added A-wrappers Area: Wrappers and bindings C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Jan 6, 2022
@gikari gikari force-pushed the gikari/qjvalue-is-null-undefined branch from 5c3f034 to f5a303b Compare January 8, 2022 19:23
@gikari gikari requested review from ogoffart and ratijas January 8, 2022 19:24
@gikari gikari force-pushed the gikari/qjvalue-is-null-undefined branch from f5a303b to 7ba03be Compare January 8, 2022 19:32
@gikari
Copy link
Contributor Author

gikari commented Jan 8, 2022

It appears, that Default is already used by cpp_class! macros. I do not know if it's fixable.

@ratijas
Copy link
Collaborator

ratijas commented Jan 8, 2022

It appears, that Default is already used by cpp_class! macros. I do not know if it's fixable.

From the link you referenced:

and Default if the class is default constructible

QJSValue does not explicitly define a standalone default constructor, but it does have one with a single optional argument:

QJSValue::QJSValue(QJSValue::SpecialValue value = UndefinedValue)

so a QJSValue::default() constructor should already be the same as calling undefined(). Worth a test though :)

@gikari gikari force-pushed the gikari/qjvalue-is-null-undefined branch from 7ba03be to 9c5ba84 Compare January 10, 2022 14:59
@gikari
Copy link
Contributor Author

gikari commented Jan 10, 2022

so a QJSValue::default() constructor should already be the same as calling undefined(). Worth a test though :)

Added as an assert to is_undefined test.

@gikari
Copy link
Contributor Author

gikari commented Jan 18, 2022

Ping?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-wrappers Area: Wrappers and bindings C-feature-request Category: A feature request, i.e: not implemented / a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants