Implement StyleContext::{get_color, lookup_color} #462

Susurrus · 2017-03-06T00:01:40Z

This is done manually because the RGBA struct isn't generally usable by gir yet.

Closes #454.

EPashkin · 2017-03-06T04:27:11Z

Comment about closing must go to first comment to work.

EPashkin · 2017-03-06T04:43:17Z

src/color_chooser.rs

@@ -55,9 +50,11 @@ impl<O: IsA<ColorChooser>> ColorChooserExt for O {
        }
    }

-    fn add_palette(&self, orientation: Orientation, colors_per_line: i32, colors: Vec<GdkRGBA>) {
+    fn add_palette(&self, orientation: Orientation, colors_per_line: i32, colors: Vec<RGBA>) {


colors: &[RGBA] and as_mut_ptr IMHO won't work

Need something like to_glib_none().0, but its work also need "tested"

Or collecting to Vec<ffi::GdkRGBA> and pass pointer to it.

I don't quite follow, are you saying that the colors argument should not be Vec<RGBA> or are you saying my conversion from Vec<RGBA> to *mut gdk_ffi::GdkRGBA is wrong?

This need colors: &[RGBA] and body changed back

EPashkin · 2017-03-08T07:24:23Z

I add few comments to reminder, it mainly for case when gtk-rs/gdk#140 selected to merge.

EPashkin · 2017-03-08T07:25:03Z

src/color_chooser.rs

-        unsafe { ffi::gtk_color_chooser_get_rgba(self.to_glib_none().0, &mut color) };
+    fn get_rgba(&self) -> RGBA {
+        let mut color = unsafe { RGBA::uninitialized() };
+        unsafe { ffi::gtk_color_chooser_get_rgba(self.to_glib_none().0, color.to_glib_none_mut().0) };


Single unsafe block

EPashkin · 2017-03-08T07:26:07Z

src/color_chooser.rs

        color
    }

-    fn set_rgba(&self, color: GdkRGBA) {
-        unsafe { ffi::gtk_color_chooser_set_rgba(self.to_glib_none().0, &color) };
+    fn set_rgba(&self, color: RGBA) {


Change to color: &RGBA to complement with record usage

EPashkin · 2017-03-08T07:34:02Z

src/style_context.rs

+        }
+    }
+
+    pub fn lookup_color(&self, color_name: &str) -> Option<gdk::RGBA> {


See gtk-rs/gdk#140 (comment) with changing local variable name to color to be equivalent generated version.

color_name is actually what's used in the GTK docs which is why I used it: https://developer.gnome.org/gtk3/stable/GtkStyleContext.html#gtk-style-context-lookup-color

Susurrus · 2017-03-08T16:42:43Z

@EPashkin Just pushed new commits that should resolve all your comments.

EPashkin · 2017-03-08T16:58:18Z

src/color_chooser.rs

@@ -55,9 +52,13 @@ impl<O: IsA<ColorChooser>> ColorChooserExt for O {
        }
    }

-    fn add_palette(&self, orientation: Orientation, colors_per_line: i32, colors: Vec<GdkRGBA>) {
+    fn add_palette(&self, orientation: Orientation, colors_per_line: i32, colors: &mut [RGBA]) {


No &mut just &

It needs to be mut for the as_mut_ptr call.

Whoops, looks like that should have been an as_ptr() call, in which case you're correct.

EPashkin · 2017-03-08T17:13:58Z

@Susurrus Sorry, but after RGBA have glib::Value support you need use generated code for it.
So manual style_context.rs no longer needed.

Susurrus · 2017-03-08T17:23:33Z

@EPashkin Yeah, that makes more sense! Cool, that enables a bunch more functions too, including one that was already manually implemented, so that's nice. Hopefully this is now correct.

EPashkin · 2017-03-08T17:29:21Z

src/color_button.rs

-        unsafe { 
-            let mut rgba = mem::uninitialized();
-            ffi::gtk_color_button_get_rgba(self.to_glib_none().0, &mut rgba);
+        unsafe {


This function and set_rgba also not needed as it implemented by ColorChooserExt
I don't know why rust now don't nags about multiple cases even on use.

Susurrus · 2017-03-08T17:46:41Z

I've removed the set_rgba and get_rgba functions as you asked.

Also, with the addition of the RGBA type, the auto-generated code now includes StyleContext::GetBackgroundColor and StyleContext::GetBorderColor, both of which are deprecated as of 3.16. Should I disable their autogeneration or leave them?

EPashkin · 2017-03-08T17:51:05Z

Thanks.
Ignoring not needed.
We ignore only functions, that deprecated by v3.4.
It controlled by Gir.toml parameters min_cfg_version and deprecate_by_min_version

Susurrus · 2017-03-08T18:10:03Z

Okay, so I don't need to do anything about those two functions then?

EPashkin · 2017-03-08T18:37:48Z

Yes, this functions stays as is, deprecation will noted on docs as for http://gtk-rs.org/docs/gtk/prelude/trait.WidgetExt.html#tymethod.drag_source_set_icon_stock

EPashkin · 2017-03-11T14:54:24Z

@GuillaumeGomez Maybe merge this before PR's related to gtk-rs/examples#112 ?

GuillaumeGomez · 2017-03-11T14:56:54Z

And another one I forgot... Thanks @EPashkin for the reminder and thanks a lot @Susurrus for the PR!

Susurrus · 2017-03-11T16:41:04Z

Awesome, thanks for helping through implementing this @GuillaumeGomez and @EPashkin!

Add property doc generation

Susurrus mentioned this pull request Mar 6, 2017

First draft implementation of RGBA gtk-rs/gdk#139

Closed

EPashkin reviewed Mar 6, 2017

View reviewed changes

EPashkin reviewed Mar 8, 2017

View reviewed changes

Susurrus force-pushed the master branch from 9e31af3 to 4a11ee1 Compare March 8, 2017 16:42

Susurrus mentioned this pull request Mar 8, 2017

Alternate rgba implementation gtk-rs/gdk#140

Merged

EPashkin reviewed Mar 8, 2017

View reviewed changes

Susurrus force-pushed the master branch from 4a11ee1 to 08b6710 Compare March 8, 2017 17:08

Susurrus force-pushed the master branch from 08b6710 to d975631 Compare March 8, 2017 17:22

EPashkin reviewed Mar 8, 2017

View reviewed changes

Susurrus added 2 commits March 8, 2017 09:41

Update for new gdk::RGBA type

0b16fe1

Regen

75e6a7f

Susurrus force-pushed the master branch from d975631 to 75e6a7f Compare March 8, 2017 17:43

EPashkin mentioned this pull request Mar 11, 2017

Use crates with git replacements in pending branch gtk-rs/examples#112

Merged

GuillaumeGomez merged commit 974759d into gtk-rs:master Mar 11, 2017

Susurrus mentioned this pull request Mar 12, 2017

Builds failing since #464 #465

Closed

EPashkin mentioned this pull request Mar 21, 2017

Remove manual duplicate #473

Merged

alex179ohm pushed a commit to alex179ohm/gtk that referenced this pull request Oct 21, 2019

Merge pull request gtk-rs#462 from GuillaumeGomez/property-doc

955fbe7

Add property doc generation

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement StyleContext::{get_color, lookup_color} #462

Implement StyleContext::{get_color, lookup_color} #462

Susurrus commented Mar 6, 2017 •

edited

Loading

EPashkin commented Mar 6, 2017

EPashkin Mar 6, 2017

EPashkin Mar 6, 2017

EPashkin Mar 6, 2017

Susurrus Mar 6, 2017

EPashkin Mar 8, 2017

EPashkin commented Mar 8, 2017

EPashkin Mar 8, 2017

EPashkin Mar 8, 2017

EPashkin Mar 8, 2017

Susurrus Mar 8, 2017

Susurrus commented Mar 8, 2017

EPashkin Mar 8, 2017

Susurrus Mar 8, 2017

Susurrus Mar 8, 2017

EPashkin commented Mar 8, 2017

Susurrus commented Mar 8, 2017

EPashkin Mar 8, 2017

Susurrus commented Mar 8, 2017

EPashkin commented Mar 8, 2017

Susurrus commented Mar 8, 2017

EPashkin commented Mar 8, 2017

EPashkin commented Mar 11, 2017

GuillaumeGomez commented Mar 11, 2017

Susurrus commented Mar 11, 2017

Implement StyleContext::{get_color, lookup_color} #462

Implement StyleContext::{get_color, lookup_color} #462

Conversation

Susurrus commented Mar 6, 2017 • edited Loading

EPashkin commented Mar 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EPashkin commented Mar 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Susurrus commented Mar 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EPashkin commented Mar 8, 2017

Susurrus commented Mar 8, 2017

Choose a reason for hiding this comment

Susurrus commented Mar 8, 2017

EPashkin commented Mar 8, 2017

Susurrus commented Mar 8, 2017

EPashkin commented Mar 8, 2017

EPashkin commented Mar 11, 2017

GuillaumeGomez commented Mar 11, 2017

Susurrus commented Mar 11, 2017

Susurrus commented Mar 6, 2017 •

edited

Loading