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

subclass/container: widget in set_focu_child should be Nullable #922

Merged

Conversation


imp.set_focus_child(&wrap, &widget)
imp.set_focus_child(&wrap, widget)

This comment has been minimized.

Copy link
@sdroege

sdroege Dec 6, 2019

Member
Suggested change
imp.set_focus_child(&wrap, widget)
imp.set_focus_child(&wrap, widget.as_ref())

We must never pass owned references to a from_glib_borrow() return value anywhere

@@ -24,7 +24,7 @@ pub trait ContainerImpl: ContainerImplExt + WidgetImpl + 'static {
self.parent_check_resize(container)
}

fn set_focus_child(&self, container: &Container, widget: &Widget) {
fn set_focus_child(&self, container: &Container, widget: Option<Widget>) {

This comment has been minimized.

Copy link
@sdroege

sdroege Dec 6, 2019

Member
Suggested change
fn set_focus_child(&self, container: &Container, widget: Option<Widget>) {
fn set_focus_child(&self, container: &Container, widget: Option<&Widget>) {

And similar for the other places

@sdroege

This comment has been minimized.

Copy link
Member

sdroege commented Dec 6, 2019

@alatiera alatiera force-pushed the alatiera:alatiera/container-set-focus-child branch from 4f0affd to 3cdb118 Dec 6, 2019
@sdroege

This comment has been minimized.

Copy link
Member

sdroege commented Dec 6, 2019

Thanks, looks good to me! @GuillaumeGomez please merge after CI :)

@sdroege

This comment has been minimized.

Copy link
Member

sdroege commented Dec 6, 2019

@GuillaumeGomez CI green

@GuillaumeGomez

This comment has been minimized.

Copy link
Member

GuillaumeGomez commented Dec 6, 2019

Thanks!

@GuillaumeGomez GuillaumeGomez merged commit 14b34cf into gtk-rs:master Dec 6, 2019
2 checks passed
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
Projects
None yet
3 participants
You can’t perform that action at this time.