Parse fields in classes and generate correct class structs #276
Conversation
| @@ -72,6 +72,7 @@ fn generate_lib(w: &mut Write, env: &Env) -> Result<()>{ | |||
|
|
|||
| try!(writeln!(w, "\n}}")); | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
Honestly, I have no idea what is going on here. My local repository doesn't have that line and git says "nothing to commit" and "everything up to date"...
There was a problem hiding this comment.
Actually, my whole file is different... wth...
There was a problem hiding this comment.
Okay, this was stupid, but forgot to switch branches :)
| try!(writeln!(w, "{}#[repr(C)]\n{0}pub struct {} {{", comment, klass.c_type)); | ||
|
|
||
| for line in lines { | ||
| try!(writeln!(w, "{}{}", comment, line)); |
There was a problem hiding this comment.
Isn't a comma missing in here?
There was a problem hiding this comment.
Aren't you writing struct's fields? (It's totally possible that I misunderstood what you were doing here.)
There was a problem hiding this comment.
Not quite. The fields are in generate_fields, which generates indented lines that end with a comma. I could move the comma out of generate_fields, though.
There was a problem hiding this comment.
Also, the original code wasn't written by me (I think @EPashkin wrote it), I simply modified it to work with classes as well as records, as I didn't see any reason it shouldn't.
There was a problem hiding this comment.
Ok, so it confirms what I thought. My bad. :p
|
|
||
| let comment = if commented { "//" } else { "" }; | ||
| if lines.is_empty() { | ||
| try!(writeln!(w, "{}#[repr(C)]\n{0}pub struct {}(c_void);\n", comment, klass.c_type)); |
There was a problem hiding this comment.
Could you use named parameters instead of {} and {0} please?
There was a problem hiding this comment.
Sure. FYI, though, I copied the style from generate_records.
| try!(writeln!(w, "{}#[repr(C)]\n{0}pub struct {}(c_void);\n", comment, klass.c_type)); | ||
| } | ||
| else { | ||
| try!(writeln!(w, "{}#[repr(C)]\n{0}pub struct {} {{", comment, klass.c_type)); |
|
Changes made by this PR in sys crate: EPashkin/rust-gnome-sys@be6f7c5 |
|
May be this need be separated as special mode. |
|
Hmm... seeing as how the same types of fields are randomly private or public, I can force them all to be private, until someone actually raises an issue... |
|
I'm coming round to making all fields public as requested in gtk-rs/sys#25. Given the advances in bindgen and apparent intention of the Rust project to integrate more C interoperability in 2017, it seems undesirable to diverge from C headers too much, and C doesn't have private fields. |
Which doesn't seem to be much of a problem for the GTK developers who create them... |
|
@GuillaumeGomez, how you think, better make all fields public? |
|
On other side, if we find, that all pub is wrong and unneeded changing it back it's be |
|
I think that all FFI struct's fields should be public. Otherwise, it depends if it is an internal mechanism or not. |
|
@GuillaumeGomez, seems it works, as you say. |
|
@GuillaumeGomez, about extra empty lines from https://github.com/gtk-rs/gir/blob/master/src/codegen/sys/lib_.rs#L245 etc: I think that better be replaced later with something like |
|
@EPashkin, @GuillaumeGomez |
|
Sure, no worries. |
This pull request makes it possible for the generator to generate proper class structs, which is very useful for creating custom widgets for example. I haven't tested it much, mostly because I'm not very familiar with gtk-rs (or gtk+, for that matter), so if you see any problems with it, please point them out to me.