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

Proposal: rust signals should be done differently #100

Open
1 task
ratijas opened this issue Jun 3, 2020 · 7 comments
Open
1 task

Proposal: rust signals should be done differently #100

ratijas opened this issue Jun 3, 2020 · 7 comments
Labels
A-callbacks Area: Passing code back and forth between Rust and C++/Qt/QML C-enhancement Category: An issue proposing an enhancement or a PR with one.

Comments

@ratijas
Copy link
Collaborator

ratijas commented Jun 3, 2020

This is a tracking issue for signal / slot mechanism wrappers in Rust.

From Rust side, RustSignal and other satellite types should be done differently, in a more type-safe and convenient way.

TODO:

  • Research how other qt binding crates are doing it, write a sum-up of their approaches.

Origin of this discussion: #97 (comment)

@rubdos
Copy link
Contributor

rubdos commented Jun 10, 2020

Just chipping in on this discussion! While looking at other Qt bindings is useful, it's also useful to keep in mind that proc_macro's are getting quite a bit of stability lately, and maybe the other bindings don't use that to their full extent yet.

What would be cool is the ability to annotate otherwise normal Rust methods:

use qmetaobject::QObject;

#[derive(QObject)]
#[qmetaobject::signal(contentsChanged)]
struct Foo {
    inner: Vec<u32>,

    #[qmetaobject::property]
    #[notify(contentsChanged)]
    is_active: bool,
}

impl Foo {
    #[qmetaobject::method(Foo::addInteger)]
    fn add(&mut self, i: u32) { self.contents_changed() } // note that it is possible to auto-map the casing!
}

EDIT: I've also added some slot and property syntax idea's.

@rubdos
Copy link
Contributor

rubdos commented Jun 10, 2020

Note that I'm only talking from an ergonomic point of view, I don't know whether this is physically possible to implement.

@ogoffart
Copy link
Member

this particular suntax is indeed not possible because the #[derive(QObject)] do not have any knowledge of the other impls.
But there is certainly room for improvement. When i first developed this crate, only the derive macro was stable. Also i was not familiar with the rust convention, and laybe qt_property! would have been better as a #[property] and there could be better ways to register methods.
Especially signals could be done as associated const or something like that instead of fields.

@rubdos
Copy link
Contributor

rubdos commented Jun 11, 2020

I suppose methods could get registered as an attribute under the derive too, then? Like

#[derive(QObject)]
#[qmetaobject::method(fooBar)]
struct Foo {}

@rubdos
Copy link
Contributor

rubdos commented Jun 11, 2020

Another comment that can be taken into account in this rework: handling the renaming/capitalization of methods, we're putting #[allow(non_snake_case) all over the place right now.

@ratijas
Copy link
Collaborator Author

ratijas commented Sep 6, 2020

I've been out for a while. Came back after two new minor Rust versions has already shipped.

Looking at the release notes of 1.45.0, they even took an example from the gnome-class crate. I don't know much about GObject and Gnome concepts, but it looks somewhat reminiscent to something familiar. Maybe worth investigating deeper.

Also, note how GNOME project took initiative to create and support Rust wrappers on their own. (I'm looking at you, The Qt Company 👀)

https://blog.rust-lang.org/2020/07/16/Rust-1.45.0.html#stabilizing-function-like-procedural-macros-in-expressions-patterns-and-statements
https://gitlab.gnome.org/federico/gnome-class

@iovxw
Copy link
Contributor

iovxw commented Jan 6, 2021

How about this? Just add two keywords:

// Just a normal struct
struct Foo {
    inner: Vec<u32>,
    the_real_is_active: bool,
}

#[qmetaobject(rename_all_signal = "camelCase")]
impl QObject for Foo { // QObject is the qt_base_class!
    signal contents_changed;

    #[notify(contents_changed)]
    #[bind(the_real_is_active)] // Or #[read(func)] #[write(func)]
    property is_active: bool;
  
    fn add(&mut self, i: u32) { self.contents_changed() }
}

@ratijas ratijas assigned ratijas and unassigned ratijas Jun 30, 2021
@ratijas ratijas added A-callbacks Area: Passing code back and forth between Rust and C++/Qt/QML C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Jun 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-callbacks Area: Passing code back and forth between Rust and C++/Qt/QML C-enhancement Category: An issue proposing an enhancement or a PR with one.
Projects
None yet
Development

No branches or pull requests

4 participants