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

Finish implementing gdk::Atom #145

Merged
merged 2 commits into from Mar 24, 2017

Conversation

Projects
None yet
3 participants
@Susurrus
Contributor

Susurrus commented Mar 17, 2017

I think this is all that's necessary?

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Mar 18, 2017

Member

Seems you need define GlibPtrDefault to fulfill need of https://github.com/gtk-rs/glib/blob/master/src/translate.rs#L967

impl GlibPtrDefault for Atom {
    type GlibType = *mut ffi::GdkAtom;
}
Member

EPashkin commented Mar 18, 2017

Seems you need define GlibPtrDefault to fulfill need of https://github.com/gtk-rs/glib/blob/master/src/translate.rs#L967

impl GlibPtrDefault for Atom {
    type GlibType = *mut ffi::GdkAtom;
}
@Susurrus

This comment has been minimized.

Show comment
Hide comment
@Susurrus

Susurrus Mar 21, 2017

Contributor

The trait bounds aren't satisfied even with the addition of a GlibPtrDefault impl for Atom. The compiler errors are complaining about a lack of this trait for Vec<Atom>, but I don't think that should actually be implemented. Is there something else missing here?

Contributor

Susurrus commented Mar 21, 2017

The trait bounds aren't satisfied even with the addition of a GlibPtrDefault impl for Atom. The compiler errors are complaining about a lack of this trait for Vec<Atom>, but I don't think that should actually be implemented. Is there something else missing here?

@Susurrus Susurrus referenced this pull request Mar 21, 2017

Merged

Drag targets #472

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Mar 21, 2017

Member
  1. make FromGlibPtrNone and FromGlibPtrFull works from pointers not from value
//src/atom.rs
impl FromGlibPtrNone<*mut ffi::GdkAtom> for Atom {
    #[inline]
    unsafe fn from_glib_none(ptr: *mut ffi::GdkAtom) -> Atom { Atom(*ptr) }
}
  1. Fix value atom errors
//src/event_owner_change.rs
     pub fn get_selection(&self) -> ::Atom {
        unsafe { from_glib_none(self.as_ref().selection as *mut _) }
     }
  1. remove manual display.rs
Member

EPashkin commented Mar 21, 2017

  1. make FromGlibPtrNone and FromGlibPtrFull works from pointers not from value
//src/atom.rs
impl FromGlibPtrNone<*mut ffi::GdkAtom> for Atom {
    #[inline]
    unsafe fn from_glib_none(ptr: *mut ffi::GdkAtom) -> Atom { Atom(*ptr) }
}
  1. Fix value atom errors
//src/event_owner_change.rs
     pub fn get_selection(&self) -> ::Atom {
        unsafe { from_glib_none(self.as_ref().selection as *mut _) }
     }
  1. remove manual display.rs
@Susurrus

This comment has been minimized.

Show comment
Hide comment
@Susurrus

Susurrus Mar 21, 2017

Contributor

I think this corrects the remaining issues.

Contributor

Susurrus commented Mar 21, 2017

I think this corrects the remaining issues.

//}
pub fn list_axes(&self) -> Vec<Atom> {
unsafe {
FromGlibPtrContainer::from_glib_container(ffi::gdk_device_list_axes(self.to_glib_none().0))

This comment has been minimized.

@EPashkin

EPashkin Mar 21, 2017

Member

This function need be tested too.
For my default display it returns 2 devices and this function return empty vector, so I can't test it

let display = gdk::Display::get_default().unwrap();
let devices = display.list_devices();
println!("{}", devices.len());

for device in devices {
    println!("name: {:?}", device.get_name());
    let axes = device.list_axes();
    println!("{}", axes.len());
}
@EPashkin

EPashkin Mar 21, 2017

Member

This function need be tested too.
For my default display it returns 2 devices and this function return empty vector, so I can't test it

let display = gdk::Display::get_default().unwrap();
let devices = display.list_devices();
println!("{}", devices.len());

for device in devices {
    println!("name: {:?}", device.get_name());
    let axes = device.list_axes();
    println!("{}", axes.len());
}

This comment has been minimized.

@Susurrus

Susurrus Mar 21, 2017

Contributor

Indeed, I'm also getting 0's for list_axes for my two devices. Do you know what a GdkDevice is in practice, and output device or an input device? I can't glean much information from various internet searches.

@Susurrus

Susurrus Mar 21, 2017

Contributor

Indeed, I'm also getting 0's for list_axes for my two devices. Do you know what a GdkDevice is in practice, and output device or an input device? I can't glean much information from various internet searches.

Susurrus added some commits Mar 17, 2017

@Susurrus

This comment has been minimized.

Show comment
Hide comment
@Susurrus

Susurrus Mar 21, 2017

Contributor

Pushed a version that ignores the store_clipboard function for now and squash into 2 commits.

Contributor

Susurrus commented Mar 21, 2017

Pushed a version that ignores the store_clipboard function for now and squash into 2 commits.

@Susurrus

This comment has been minimized.

Show comment
Hide comment
@Susurrus

Susurrus Mar 24, 2017

Contributor

@EPashkin Is there anything else left to fix with this or are you waiting for gtk-rs/gtk#472 is in solid shape before you want to merge this?

Contributor

Susurrus commented Mar 24, 2017

@EPashkin Is there anything else left to fix with this or are you waiting for gtk-rs/gtk#472 is in solid shape before you want to merge this?

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Mar 24, 2017

Member

As I don't understand what gdk_display_store_clipboard must do current version good for me and can be merged.
@GuillaumeGomez What you think about merging this now?

Member

EPashkin commented Mar 24, 2017

As I don't understand what gdk_display_store_clipboard must do current version good for me and can be merged.
@GuillaumeGomez What you think about merging this now?

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Mar 24, 2017

Member

I didn't see anything blocking so if @EPashkin says ok, it's good to get merged. Thanks!

Member

GuillaumeGomez commented Mar 24, 2017

I didn't see anything blocking so if @EPashkin says ok, it's good to get merged. Thanks!

@GuillaumeGomez GuillaumeGomez merged commit c2ab311 into gtk-rs:master Mar 24, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Susurrus Susurrus deleted the Susurrus:drag_target branch Mar 24, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment