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

WIP: cxx-qt-gen: use original bridge definition in CXX and remove camel/snake conversion #667

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ahayzen-kdab
Copy link
Collaborator

@ahayzen-kdab ahayzen-kdab commented Aug 24, 2023

Closes #616

Closes #532

@ahayzen-kdab
Copy link
Collaborator Author

This works quite well from a generation side, only the qproperty is potentially fun to sort out syntax wise.

However for qproperty we do not pass it straight through and generate methods from it 🤔

The syntax could be something like

#[qproperty(i32, my_property, cxx_name = "myProperty")]
#[qproperty(i32, myProperty, rust_name = "myProperty")]
#[qproperty(i32, my_property, cxx_name = "myProperty", read = Self::custom_read)]

But note that we might have custom getters and setters in there later too 🤔

Another route could be to require to specify the "name" in C++ as the property name and then specify the field it maps to (Name = Self::Field) eg

#[qproperty(i32, myProperty = Self::my_property)]
#[qproperty(i32, myProperty = Self::my_property, read = Self::custom_read)]

Then it could also fallback to if you don't specify the right side it assumes that they are the same

#[qproperty(i32, myProperty)]  // would look for Self::myProperty

Or a further idea could be to make everything key-values, combine the C++ name with the type

#[qproperty(myProperty = i32, member = Self::myProperty)]
#[qproperty(myProperty = i32, read = Self::custom_read)]

Or if we can use the generic syntax

#[qproperty(myProperty<i32>, member = Self::myProperty)]
#[qproperty(myProperty<i32>, read = Self::custom_read)]

Which could also drop the Self

#[qproperty(myProperty<i32>, member = myProperty)]
#[qproperty(myProperty<i32>, read = custom_read)]

This allows us later to pass through the original method in the
generation.

Related to KDAB#662
This allows for passing through the original signal definition
to CXX and then removes camel <-> snake case conversion.

Related to KDAB#616
@LeonMatthesKDAB
Copy link
Collaborator

Hm, I do like the automatic camel-case/snake-case conversion, as snake-case code does look very out of place in QML/C++.
Though that's potentially just me not being used to it.

I believe we could still make the passthrough work with case conversion, by simply checking whether there's a cxx_name or rust_name associated with the qinvokable and if not, injecting only that attribute and leaving the rest alone, which would still be basically a pass-through, with just one additional attribute.

It's something that we should discuss at the company meeting, whether people think camelCase is important in QML.

This allows for passing through the original method, invokable,
and inherit definition to CXX and then removes camel <-> snake
case conversion.

Related to KDAB#616
@LeonMatthesKDAB
Copy link
Collaborator

Another suggestion for naming of properties:

    extern "RustQt" {
        #[qobject]
        #[qproperty(
            i32, my_property,
            cxx_name="my_prop",
            setter=set_my_prop)]
        type MyThing = super::MyThingRust;

        #[cxx_name="..."]
        fn set_my_prop(self: Pin<&mut MyThing>, value: i32);
    }

It basically boils down to requiring you to declare the actual setter in the bridge yourself, if you don't want to use the auto-generated one.
That way you have full control.

knoxfighter added a commit to binja-tools/cxx-qt that referenced this pull request Apr 4, 2024
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.

Determine if passing through foreign funcs is worth it Consider allowing #[doc] in cxx_qt::inherit blocks
2 participants