Skip to content
This repository has been archived by the owner on Jun 8, 2021. It is now read-only.

Publish event members #21

Merged
merged 4 commits into from
May 10, 2015
Merged

Publish event members #21

merged 4 commits into from
May 10, 2015

Conversation

lucaswerkmeister
Copy link
Contributor

No description provided.

@gkoz
Copy link
Member

gkoz commented May 9, 2015

It would be nice to have some safe solution to ensure that the event record we receive in the signal handler is of the correct type (probably implement FromGlibPtr for events).

@lucaswerkmeister
Copy link
Contributor Author

I’m sorry, I don’t understand – is there some problem with this PR? Should some members remain unpublished?

@gkoz
Copy link
Member

gkoz commented May 9, 2015

These belong in gdk-sys actually, as gdk shouldn't expose the foreign types obviously. So a proper approach is to move them there (publishing the fields) and then most likely create wrapper structs with accessor methods that would take care of any conversions.

@lucaswerkmeister
Copy link
Contributor Author

Okay, but in the meantime, is there any reason why some events (e. g. EventTouch, EventScroll, EventKey) have public members, and others (e. g. EventMotion, EventButton) don’t? I assumed this was simply an oversight. AFAICT, there’s currently no way to get, for instance, the coordinates of a click event, and that seems… inconvenient :D

@gkoz
Copy link
Member

gkoz commented May 10, 2015

Can't answer for why some of the stuff has been done the way it is but it's way unfinished for sure.
I guess there's no harm in publishing scalar fields. Do you want raw pointers as well?

@lucaswerkmeister
Copy link
Contributor Author

Do you want raw pointers as well?

Not really… I’ll add a commit to unpublish them (including those that were public before).

@gkoz
Copy link
Member

gkoz commented May 10, 2015

What, the event structs are missing #[repr(C)]? :/
Please add that and it will allow you to remove allow(dead_code).

@@ -72,6 +72,8 @@ pub trait Event: Sized {
}
}

// TODO unfisished, see some discussion in #21
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this #25

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done (squash + rebase), also fixed the typo in “unfi_s_ished”

@lucaswerkmeister
Copy link
Contributor Author

Done.

@gkoz
Copy link
Member

gkoz commented May 10, 2015

👍
@GuillaumeGomez

@GuillaumeGomez
Copy link
Member

Here I am ! @lucaswerkmeister: Nice job and thanks for your work !

GuillaumeGomez added a commit that referenced this pull request May 10, 2015
@GuillaumeGomez GuillaumeGomez merged commit 6aed5d9 into gtk-rs:master May 10, 2015
@lucaswerkmeister lucaswerkmeister deleted the pub_event branch May 10, 2015 14:44
@lucaswerkmeister
Copy link
Contributor Author

Cool, thanks for merging!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants