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 support for defining new interfaces and other minor things #438

Merged
merged 8 commits into from Feb 2, 2019

Conversation

Projects
None yet
3 participants
@sdroege
Copy link
Member

sdroege commented Feb 1, 2019

See individual commits

}
}

/// Add a new action signal with accumulator to the interface.

This comment has been minimized.

@EPashkin

EPashkin Feb 1, 2019

Member

With accumulator?

This comment has been minimized.

@sdroege

sdroege Feb 2, 2019

Author Member

What do you mean? :)

Also this is the same documentation as we already had for subclasses before

This comment has been minimized.

@EPashkin

EPashkin Feb 2, 2019

Member

I missed that handler actually GSignalAccumulator accumulator, sorry

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Feb 1, 2019

Thanks, found only small nit in docs

ret_type: Type,
accumulator: F,
) where
F: Fn(&mut Value, &Value) -> bool + Send + Sync + 'static,

This comment has been minimized.

@EPashkin

EPashkin Feb 2, 2019

Member

Maybe use enum { None, TrueHandled, FirstWins, .., Custom(F:Fn())} for accumulator?
Not sure how to handle g_application_handle_local_options_accumulator etc,
maybe define trait to get function pointer?

This comment has been minimized.

@sdroege

sdroege Feb 2, 2019

Author Member

An enum would have the same problem that we had with Option for optional callbacks, and Fn is a trait for giving a function pointer and passing some data around ;)

What exactly do you see wrong with the way how the function is defined? Also this is how we had it before for subclasses already, no change here.

If you'd like to have a different API for that, maybe let's handle that in a separate PR?

This comment has been minimized.

@sdroege

sdroege Feb 2, 2019

Author Member

g_application_handle_local_options_accumulator would be

|return_acc, handler_return| {
  let value = handler_return.get::<u32>().unwrap();
  *return_acc = value.to_value();
  value < 0
}

This comment has been minimized.

@EPashkin

EPashkin Feb 2, 2019

Member

Yes can handled later, just note about extension

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Feb 2, 2019

👍

@sdroege

This comment has been minimized.

Copy link
Member Author

sdroege commented Feb 2, 2019

Ok, let's get this in then? :) @GuillaumeGomez @EPashkin?

I added another small commit on top.

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Feb 2, 2019

👍

@sdroege sdroege force-pushed the sdroege:interfaces branch from 31b33df to c670524 Feb 2, 2019

sdroege added some commits Feb 1, 2019

Add bindings for GInitiallyUnowned
This will never directly appear anywhere in use apart from defining new
subclasses that inherit from GInitiallyUnowned directly, i.e. classes
that use floating references.

This should only really ever be used for compatibility with C code.
Floating references cause hard to debug problems and are meaningless for
bindings.
Add accessor for getting the interface implementation from an instance
Mirrors the exactly same API from the ObjectSubclass trait.

@sdroege sdroege force-pushed the sdroege:interfaces branch from c670524 to 08f04dc Feb 2, 2019

@GuillaumeGomez

This comment has been minimized.

Copy link
Member

GuillaumeGomez commented Feb 2, 2019

All good for me!

@EPashkin

This comment has been minimized.

Copy link
Member

EPashkin commented Feb 2, 2019

Still 👍
Thanks @sdroege

@sdroege sdroege merged commit 14d3bb3 into gtk-rs:master Feb 2, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.