After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 709620 - ClutterEvent: Mention _get_source_device() in docs
ClutterEvent: Mention _get_source_device() in docs
Status: RESOLVED FIXED
Product: clutter
Classification: Platform
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: clutter-maint
clutter-maint
Depends on:
Blocks:
 
 
Reported: 2013-10-08 09:41 UTC by Bastien Nocera
Modified: 2013-10-08 12:58 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
ClutterEvent: Mention _get_source_device() in docs (1.42 KB, patch)
2013-10-08 09:41 UTC, Bastien Nocera
needs-work Details | Review
ClutterEvent: Mention _get_source_device() in docs (3.52 KB, patch)
2013-10-08 12:28 UTC, Bastien Nocera
committed Details | Review

Description Bastien Nocera 2013-10-08 09:41:45 UTC
.
Comment 1 Bastien Nocera 2013-10-08 09:41:48 UTC
Created attachment 256704 [details] [review]
ClutterEvent: Mention _get_source_device() in docs

It's too easy getting bitten by the ->device red herring, thinking
that it's the original input device the event originated from.
Comment 2 Emmanuele Bassi (:ebassi) 2013-10-08 11:07:21 UTC
Review of attachment 256704 [details] [review]:

I agree, it should be clearer; thanks for the patch.

the same should probably be done for all ClutterEvent structures, not just for StageState and Key events.

to be fair, the entirety of ClutterEvent should be sealed, and you should use the accessor functions.
Comment 3 Bastien Nocera 2013-10-08 11:45:21 UTC
(In reply to comment #2)
> Review of attachment 256704 [details] [review]:
> 
> I agree, it should be clearer; thanks for the patch.
> 
> the same should probably be done for all ClutterEvent structures, not just for
> StageState and Key events.

StageStage? I'm guessing you mean Button. Those are the only ones where the field wasn't marked as "for future expansion". I know it's used in ClutterKeyEvent, which is why I changed the docs there.

> to be fair, the entirety of ClutterEvent should be sealed, and you should use
> the accessor functions.

I need to mention it in clutter_event_get_device() too.
Comment 4 Emmanuele Bassi (:ebassi) 2013-10-08 12:00:15 UTC
(In reply to comment #3)
> (In reply to comment #2)
> > Review of attachment 256704 [details] [review] [details]:
> > 
> > I agree, it should be clearer; thanks for the patch.
> > 
> > the same should probably be done for all ClutterEvent structures, not just for
> > StageState and Key events.
> 
> StageStage? I'm guessing you mean Button.

ugh, Splinter highlighted the wrong structure.

> Those are the only ones where the
> field wasn't marked as "for future expansion". I know it's used in
> ClutterKeyEvent, which is why I changed the docs there.

ClutterCrossingEvent, ClutterMotionEvent, ClutterScrollEvent, and ClutterTouchEvent have a `device` field. they should all mention clutter_event_get_source_device().

> > to be fair, the entirety of ClutterEvent should be sealed, and you should use
> > the accessor functions.
> 
> I need to mention it in clutter_event_get_device() too.

yeah, that would be good.
Comment 5 Bastien Nocera 2013-10-08 12:28:12 UTC
Created attachment 256729 [details] [review]
ClutterEvent: Mention _get_source_device() in docs

It's too easy getting bitten by the ->device red herring, thinking
that it's the original input device the event originated from.
Comment 6 Emmanuele Bassi (:ebassi) 2013-10-08 12:51:16 UTC
Review of attachment 256729 [details] [review]:

looks good.
Comment 7 Bastien Nocera 2013-10-08 12:58:50 UTC
Attachment 256729 [details] pushed as 676a7cd - ClutterEvent: Mention _get_source_device() in docs