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

Generate Debug for sys types #488

Merged
merged 2 commits into from Nov 11, 2017
Merged

Conversation

@GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Nov 10, 2017

Fixes #486.

cc @EPashkin @sdroege

@GuillaumeGomez
Copy link
Member Author

@GuillaumeGomez GuillaumeGomez commented Nov 10, 2017

All green. :)

try!(writeln!(
w,
"{comment}#[repr(C)]\n{comment}pub struct {name} {{",
"{comment}#[repr(C)]\n{debug}{comment}pub struct {name} {{",

This comment has been minimized.

@EPashkin

EPashkin Nov 10, 2017
Member

This line seems generated struct with extra line:

#[repr(C)]

pub struct AtkAttribute {
    pub name: *mut c_char,
    pub value: *mut c_char,
}

This comment has been minimized.

@GuillaumeGomez

GuillaumeGomez Nov 10, 2017
Author Member

Wo. Where does this backline comes from? Oo

{fields_name})\n\
\t}}\n\
}}\n",
name = klass.c_type,

This comment has been minimized.

@EPashkin

EPashkin Nov 10, 2017
Member

Sorry, but this unreadable, and seems repeated at minimum twice.

This comment has been minimized.

@GuillaumeGomez

GuillaumeGomez Nov 10, 2017
Author Member

True. I'll write a function and maybe comment or create temporary variables in order to make this less horrible.

@@ -456,15 +572,26 @@ fn generate_records(w: &mut Write, env: &Env, records: &[&Record]) -> Result<()>
}
try!(writeln!(
w,
"{}#[repr(C)]\n{0}pub struct {} {{",
"{}#[repr(C)]\n{}\n{0}pub struct {} {{",

This comment has been minimized.

@GuillaumeGomez

GuillaumeGomez Nov 10, 2017
Author Member

@EPashkin: It comes from here actually.

@GuillaumeGomez GuillaumeGomez force-pushed the GuillaumeGomez:debug-impl branch from 6f120d0 to 3283e71 Nov 10, 2017
\t\twrite!(f, \"{name} @ {{:?}}\", self as *const _)\n\
\t}}\n\
}}\n",
name=record.c_type));

This comment has been minimized.

@EPashkin

EPashkin Nov 10, 2017
Member

IMHO this better extract as function too and maybe use raw string for format

@EPashkin
Copy link
Member

@EPashkin EPashkin commented Nov 10, 2017

This intended that inner fields not comma-separated?

impl ::std::fmt::Debug for AtkObject {
    fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
        write!(f, "AtkObject @ {:?} {{ {:?} {:?} {:?} {:?} {:?} {:?} {:?} }}",
               self as *const _,
               self.parent,
               self.description,
               self.name,
               self.accessible_parent,
               self.role,
               self.relation_set,
               self.layer)
    }
}
@GuillaumeGomez
Copy link
Member Author

@GuillaumeGomez GuillaumeGomez commented Nov 10, 2017

Hum yes. Do you want comma?

@EPashkin
Copy link
Member

@EPashkin EPashkin commented Nov 10, 2017

Then its normal, just strange.

@GuillaumeGomez
Copy link
Member Author

@GuillaumeGomez GuillaumeGomez commented Nov 11, 2017

I'll add the comma tomorrow then. ;)

@GuillaumeGomez GuillaumeGomez force-pushed the GuillaumeGomez:debug-impl branch from 3283e71 to 5a15289 Nov 11, 2017
@GuillaumeGomez
Copy link
Member Author

@GuillaumeGomez GuillaumeGomez commented Nov 11, 2017

Updated.

@sdroege
Copy link
Member

@sdroege sdroege commented Nov 11, 2017

For printing things struct-like, please use the functions from Formatter like debug_struct()

w,
name,
&format!(
"write!(f, \"{name} @ {{:?}} {{{{ {fields}}}}}\",\n\

This comment has been minimized.

@EPashkin

EPashkin Nov 11, 2017
Member

IMHO extra space needed {{{{ {fields} }}}}

impl_content)
}

fn generate_fields_with_debug(w: &mut Write, name: &str, lines: &[String]) -> Result<()> {

This comment has been minimized.

@EPashkin

EPashkin Nov 11, 2017
Member

Not generate_debug_with_fields?

@GuillaumeGomez
Copy link
Member Author

@GuillaumeGomez GuillaumeGomez commented Nov 11, 2017

Updated and improved. Thanks for the suggestion @sdroege!

} else {
let field_name = field.split(":").next().unwrap().trim().replace("pub ", "");
Some(if field.replace(",", "").trim().ends_with("c_void") {
format!("\t\t .field(\"{name}\", &\"c_void\")\n", name=field_name)

This comment has been minimized.

@EPashkin

EPashkin Nov 11, 2017
Member

Is really need write constant string in debug result?

This comment has been minimized.

@EPashkin

EPashkin Nov 11, 2017
Member

I meant: maybe better just skip this field?
Current results:

//gobject
impl ::std::fmt::Debug for GParamSpecString {
    fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
        f.debug_struct(&format!("GParamSpecString @ {:?}", self as *const _))
         .field("parent_instance", &self.parent_instance)
         .field("default_value", &self.default_value)
         .field("cset_first", &self.cset_first)
         .field("cset_nth", &self.cset_nth)
         .field("substitutor", &self.substitutor)
         .field("_truncated_record_marker", &"c_void")
         .finish()
    }
}

//gtk
impl ::std::fmt::Debug for GtkRcStyle {
    fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
        f.debug_struct(&format!("GtkRcStyle @ {:?}", self as *const _))
         .field("parent_instance", &self.parent_instance)
         .field("name", &self.name)
         .field("bg_pixmap_name", &self.bg_pixmap_name)
         .field("font_desc", &self.font_desc)
         .field("color_flags", &self.color_flags)
         .field("fg", &self.fg)
         .field("bg", &self.bg)
         .field("text", &self.text)
         .field("base", &self.base)
         .field("xthickness", &self.xthickness)
         .field("ythickness", &self.ythickness)
         .field("rc_properties", &self.rc_properties)
         .field("rc_style_lists", &self.rc_style_lists)
         .field("icon_factories", &self.icon_factories)
         .field("_truncated_record_marker", &"c_void")
         .finish()
    }
}

This comment has been minimized.

@GuillaumeGomez

GuillaumeGomez Nov 11, 2017
Author Member

I want all fields to be there. So better having a useless "c_void" string than nothing from my point of view.

This comment has been minimized.

@EPashkin
Copy link
Member

@EPashkin EPashkin commented Nov 11, 2017

@sdroege, is good for you now?

w,
&klass.c_type,
&format!("write!(f, \"{name} @ {{:?}}\", self as *const _)",
name=klass.c_type)

This comment has been minimized.

@sdroege

sdroege Nov 11, 2017
Member

You could use debug_tuple(name).field(the_pointer_string).build() here

w,
&interface.c_type,
&format!("write!(f, \"{name} @ {{:?}}\", self as *const _)",
name=interface.c_type)

This comment has been minimized.

@sdroege

sdroege Nov 11, 2017
Member

And here

w,
&record.c_type,
&format!("write!(f, \"{name} @ {{:?}}\", self as *const _)",
name=record.c_type)

This comment has been minimized.

@sdroege

sdroege Nov 11, 2017
Member

And here

@sdroege
Copy link
Member

@sdroege sdroege commented Nov 11, 2017

Feel free to ignore that comment, it's all good to go for me.

@GuillaumeGomez
Copy link
Member Author

@GuillaumeGomez GuillaumeGomez commented Nov 11, 2017

To give @EPashkin a bit of context: @sdroege and I discussed about it and globally what came out was:

me: I don't see the use for the debug_tuple. If we have the need of it some day, I'll add it but for now it's not really useful.
@sdroege: Ok.

@GuillaumeGomez
Copy link
Member Author

@GuillaumeGomez GuillaumeGomez commented Nov 11, 2017

I'll let you handle the merging @EPashkin.

@GuillaumeGomez
Copy link
Member Author

@GuillaumeGomez GuillaumeGomez commented Nov 11, 2017

Sorry but I want to quickly move towards the next release so I'll merge this and make a PR for the sys repository.

@GuillaumeGomez GuillaumeGomez merged commit e58a8db into gtk-rs:master Nov 11, 2017
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
@GuillaumeGomez GuillaumeGomez deleted the GuillaumeGomez:debug-impl branch Nov 11, 2017
@EPashkin
Copy link
Member

@EPashkin EPashkin commented Nov 11, 2017

Thanks, @GuillaumeGomez

vhdirk pushed a commit to vhdirk/gir that referenced this pull request Jul 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants