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

Regenerate autogenerated bindings with latest GIR #509

Merged
merged 4 commits into from May 16, 2017

Conversation

Projects
None yet
3 participants
@sdroege
Member

sdroege commented May 15, 2017

Also remove the GIO prelude re-export as discussed in gtk-rs/gir#364 (comment)

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin May 15, 2017

Member
  1. Update Gir submodule, better in separate commit before regen.
  2. Many files in src need traits with reexport it in prelude: ex. https://github.com/gtk-rs/gtk/blob/master/src/assistant.rs -> AssistantExtManual
  3. Doctest errors need fix in https://raw.githubusercontent.com/gtk-rs/lgpl-docs/master/gtk/docs.md
    with something like # use LabelExt;
Member

EPashkin commented May 15, 2017

  1. Update Gir submodule, better in separate commit before regen.
  2. Many files in src need traits with reexport it in prelude: ex. https://github.com/gtk-rs/gtk/blob/master/src/assistant.rs -> AssistantExtManual
  3. Doctest errors need fix in https://raw.githubusercontent.com/gtk-rs/lgpl-docs/master/gtk/docs.md
    with something like # use LabelExt;
@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege May 15, 2017

Member

Ok, thanks!

Member

sdroege commented May 15, 2017

Ok, thanks!

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege May 15, 2017

Member

Is the plan/intention to add all traits to the prelude? Should do that in the other pull requests too then for consistency at least.

Member

sdroege commented May 15, 2017

Is the plan/intention to add all traits to the prelude? Should do that in the other pull requests too then for consistency at least.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin May 15, 2017

Member

As currently all object traits reexported in prelude adding manual trait need for consistency for classes that both manual and auto. Not sure that it need for only manual classes.
Gio don't have manual part for objects (only for record Resource)
Pango and Gdk seems also need change.
Pango currently don't have prelude and don't have manual+auto classes, so IMHO only prelude with reexport from auto::trait need be added.
Gdk need manual trait at minimum for DragContext

Member

EPashkin commented May 15, 2017

As currently all object traits reexported in prelude adding manual trait need for consistency for classes that both manual and auto. Not sure that it need for only manual classes.
Gio don't have manual part for objects (only for record Resource)
Pango and Gdk seems also need change.
Pango currently don't have prelude and don't have manual+auto classes, so IMHO only prelude with reexport from auto::trait need be added.
Gdk need manual trait at minimum for DragContext

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin May 15, 2017

Member

*forgot to say: Currently prelude reexport all trait, that user can use. And IMHO it good.

Member

EPashkin commented May 15, 2017

*forgot to say: Currently prelude reexport all trait, that user can use. And IMHO it good.

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege May 15, 2017

Member

Ok, so if I see this correctly nothing is need about the prelude. GIR submodule is updated.
And for doctests, it seems you fixed lgpl-docs earlier already?

Anything missing here or in the other pull requests? :)

Member

sdroege commented May 15, 2017

Ok, so if I see this correctly nothing is need about the prelude. GIR submodule is updated.
And for doctests, it seems you fixed lgpl-docs earlier already?

Anything missing here or in the other pull requests? :)

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin May 15, 2017

Member

#509 (comment)

At minimum please add manual trait for all classes that also generated.

Member

EPashkin commented May 15, 2017

#509 (comment)

At minimum please add manual trait for all classes that also generated.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin May 15, 2017

Member

and gdk too.

Member

EPashkin commented May 15, 2017

and gdk too.

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege May 15, 2017

Member

At minimum please add manual trait for all classes that also generated.

I don't understand what you mean. You want manual traits to be added for every automatically generated trait? Why?

Member

sdroege commented May 15, 2017

At minimum please add manual trait for all classes that also generated.

I don't understand what you mean. You want manual traits to be added for every automatically generated trait? Why?

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin May 15, 2017

Member

Some classes (ex. Assistant) has both generated and manual code.
After you moved all methods to trait in generated code, you need do it to manual part too.
Or for future children will have "access" only to generated traits method as it have implementation for IsA but not for manual part that implemented only for parent (with exception of constructors and class functions - this need implemented on parent directly).

Member

EPashkin commented May 15, 2017

Some classes (ex. Assistant) has both generated and manual code.
After you moved all methods to trait in generated code, you need do it to manual part too.
Or for future children will have "access" only to generated traits method as it have implementation for IsA but not for manual part that implemented only for parent (with exception of constructors and class functions - this need implemented on parent directly).

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege May 15, 2017

Member

Ah yes of course, thanks for pointing that out :) Will do

Member

sdroege commented May 15, 2017

Ah yes of course, thanks for pointing that out :) Will do

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege May 15, 2017

Member

Should all be good now

Member

sdroege commented May 15, 2017

Should all be good now

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin May 16, 2017

Member

assistant.rs, tree_model_filter.rs, tree_store.rs not changed.
Also I see old bug: none of manual trait is reexported on gtk crate level,
IMHO just pub use prelude::*; in lib.rs will fix it.

Member

EPashkin commented May 16, 2017

assistant.rs, tree_model_filter.rs, tree_store.rs not changed.
Also I see old bug: none of manual trait is reexported on gtk crate level,
IMHO just pub use prelude::*; in lib.rs will fix it.

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege May 16, 2017

Member

Done. However prelude::* is also not exported from gdk and pango.

Member

sdroege commented May 16, 2017

Done. However prelude::* is also not exported from gdk and pango.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin May 16, 2017

Member

No actual problem in pango as only manual trait Matrix reexported explicitly.
But gdk really has same problem.

👍 Thanks, @sdroege

Member

EPashkin commented May 16, 2017

No actual problem in pango as only manual trait Matrix reexported explicitly.
But gdk really has same problem.

👍 Thanks, @sdroege

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez May 16, 2017

Member

Thanks!

Member

GuillaumeGomez commented May 16, 2017

Thanks!

@GuillaumeGomez GuillaumeGomez merged commit 9678b00 into gtk-rs:master May 16, 2017

2 checks passed

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

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege May 16, 2017

Member

But gdk really has same problem.

Great, re-exported prelude::* from gdk too in gtk-rs/gdk#163

Member

sdroege commented May 16, 2017

But gdk really has same problem.

Great, re-exported prelude::* from gdk too in gtk-rs/gdk#163

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