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

Add gclosure macro #338

Merged
merged 2 commits into from
Nov 14, 2021
Merged

Add gclosure macro #338

merged 2 commits into from
Nov 14, 2021

Conversation

jf2048
Copy link
Member

@jf2048 jf2048 commented Nov 9, 2021

Fixes #19, probably also good enough for #23

I tried to copy the syntax of clone! as much as I could. This should be a convenient improvement on connect_object since this can take weak references on multiple objects in one call.

@sdroege
Copy link
Member

sdroege commented Nov 9, 2021

Thanks, this looks really promising. I'll review it in detail later.

probably also good enough for #23

If you pass the object into the closure as a weak reference then for normal usage yes. However GtkBuilder signal handlers can also use this flag, and then connection to the signal must happen slightly differently (I forgot the details).

glib-macros/src/gclosure.rs Outdated Show resolved Hide resolved
glib-macros/src/gclosure.rs Outdated Show resolved Hide resolved
glib/src/closure.rs Outdated Show resolved Hide resolved
glib/src/object.rs Outdated Show resolved Hide resolved
glib-macros/tests/test.rs Outdated Show resolved Hide resolved
@sdroege sdroege added this to the 0.15.0 milestone Nov 9, 2021
@sdroege
Copy link
Member

sdroege commented Nov 9, 2021

probably also good enough for #23

If you pass the object into the closure as a weak reference then for normal usage yes. However GtkBuilder signal handlers can also use this flag, and then connection to the signal must happen slightly differently (I forgot the details).

Which you implement differently in the GtkBuilder case in the other PR, so that's indeed covered then :)

glib/src/object.rs Outdated Show resolved Hide resolved
glib/src/object.rs Outdated Show resolved Hide resolved
glib/src/object.rs Show resolved Hide resolved
glib-macros/src/gclosure.rs Outdated Show resolved Hide resolved
@jf2048 jf2048 force-pushed the connect-closure branch 2 times, most recently from 2df4964 to c0ba00a Compare November 10, 2021 04:13
@jf2048
Copy link
Member Author

jf2048 commented Nov 10, 2021

Which you implement differently in the GtkBuilder case in the other PR, so that's indeed covered then :)

Indeed, I thought I would use this macro there initially but it turned out to not be very practical to do it that way, since BuilderRustScope needs to take callbacks and only creates the closure objects on demand.

glib-macros/src/lib.rs Outdated Show resolved Hide resolved
glib-macros/src/lib.rs Outdated Show resolved Hide resolved
glib-macros/src/lib.rs Outdated Show resolved Hide resolved
glib/src/object.rs Outdated Show resolved Hide resolved
glib/src/object.rs Outdated Show resolved Hide resolved
@A6GibKm
Copy link
Contributor

A6GibKm commented Nov 10, 2021

Is there a reason it is called glib::gclosure rather than glib::closure? In my opinion it goes against how namespaces work in Rust.

@jf2048
Copy link
Member Author

jf2048 commented Nov 11, 2021

It was closure! first, then I named it gclosure! because I thought maybe it made sense because the macro returns a Closure, not a rust native type. It probably should be changed back to closure! though now that you mention it.

@sdroege
Copy link
Member

sdroege commented Nov 11, 2021

It was closure! first, then I named it gclosure! because I thought maybe it made sense because the macro returns a Closure, not a rust native type. It probably should be changed back to closure! though now that you mention it.

See also #341 (comment) . My reasoning would've been the same for calling this one gclosure!, but OTOH it's glib::Closure and not glib:GClosure. I have no strong opinion here, there are arguments for both.

@bilelmoussaoui @GuillaumeGomez ?

@sdroege
Copy link
Member

sdroege commented Nov 11, 2021

Let's keep this one as gclosure! and worry about it in general in #343

glib-macros/src/lib.rs Outdated Show resolved Hide resolved
@jf2048 jf2048 force-pushed the connect-closure branch 2 times, most recently from 83ed78e to 9ca2b8c Compare November 14, 2021 04:33
@jf2048 jf2048 force-pushed the connect-closure branch 4 times, most recently from 8e15324 to dfc604a Compare November 14, 2021 04:56
glib/src/lib.rs Outdated Show resolved Hide resolved
sdroege
sdroege previously approved these changes Nov 14, 2021
@sdroege sdroege merged commit 27948e1 into gtk-rs:master Nov 14, 2021
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.

Provide proc macro attribute to generate generic signal handler functions
5 participants