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

Gen more things #239

Merged
merged 3 commits into from Sep 7, 2018

Conversation

Projects
None yet
3 participants
@GuillaumeGomez
Member

GuillaumeGomez commented Sep 4, 2018

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Sep 4, 2018

Member

👍

Member

sdroege commented Sep 4, 2018

👍

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Sep 5, 2018

Member

gdk_property_change has wrong Gdk-3.0.gir definition, so gir-files and sys need updated first,
or gir need fixed.
Current

        <parameter name="data" transfer-ownership="none">
          <doc xml:space="preserve">the data (a `guchar *`
  `gushort *`, or `gulong *`,
  depending on @format), cast to a `guchar *`.</doc>
          <type name="guint8" c:type="const guchar*"/>
        </parameter>

IMHO Type need be something like

          <array length="6" zero-terminated="0" c:type="const guint8*">
            <type name="guint8" c:type="guint8"/>
          </array>
Member

EPashkin commented Sep 5, 2018

gdk_property_change has wrong Gdk-3.0.gir definition, so gir-files and sys need updated first,
or gir need fixed.
Current

        <parameter name="data" transfer-ownership="none">
          <doc xml:space="preserve">the data (a `guchar *`
  `gushort *`, or `gulong *`,
  depending on @format), cast to a `guchar *`.</doc>
          <type name="guint8" c:type="const guchar*"/>
        </parameter>

IMHO Type need be something like

          <array length="6" zero-terminated="0" c:type="const guint8*">
            <type name="guint8" c:type="guint8"/>
          </array>

@EPashkin EPashkin referenced this pull request Sep 6, 2018

Closed

Fix #25

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Sep 6, 2018

Member

Updated. Bound the function manually.

Member

GuillaumeGomez commented Sep 6, 2018

Updated. Bound the function manually.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin
Member

EPashkin commented Sep 7, 2018

@GuillaumeGomez Thanks.

ULongs(&'a [libc::c_ulong]),
UChar(u8),
UShort(u16),
ULong(libc::c_ulong),

This comment has been minimized.

@EPashkin

EPashkin Sep 7, 2018

Member

Inner types don't need have pub ?

@EPashkin

EPashkin Sep 7, 2018

Member

Inner types don't need have pub ?

This comment has been minimized.

@EPashkin

EPashkin Sep 7, 2018

Member

And here no u32 cases

@EPashkin

EPashkin Sep 7, 2018

Member

And here no u32 cases

This comment has been minimized.

@sdroege

sdroege Sep 7, 2018

Member

u32/guint is not mentioned in the documentation of the function

@sdroege

sdroege Sep 7, 2018

Member

u32/guint is not mentioned in the documentation of the function

This comment has been minimized.

@sdroege

sdroege Sep 7, 2018

Member

Going through uses of it, there seem to be more types possible than documented though:
https://sources.debian.org/src/gtk+2.0/2.24.31-2/gtk/gtkselection.c/?hl=2466#L2466
https://sources.debian.org/src/gtk+3.0/3.24.0-1/gdk/mir/gdkmirwindowimpl.c/?hl=1934#L1934
https://sources.debian.org/src/nautilus/3.26.3.1-1/src/nautilus-canvas-dnd.c/?hl=503#L503

This seems to allow arbitrary types, I'm not sure how this function is supposed to work safely.

@sdroege

sdroege Sep 7, 2018

Member

Going through uses of it, there seem to be more types possible than documented though:
https://sources.debian.org/src/gtk+2.0/2.24.31-2/gtk/gtkselection.c/?hl=2466#L2466
https://sources.debian.org/src/gtk+3.0/3.24.0-1/gdk/mir/gdkmirwindowimpl.c/?hl=1934#L1934
https://sources.debian.org/src/nautilus/3.26.3.1-1/src/nautilus-canvas-dnd.c/?hl=503#L503

This seems to allow arbitrary types, I'm not sure how this function is supposed to work safely.

This comment has been minimized.

@GuillaumeGomez

GuillaumeGomez Sep 7, 2018

Member

This seems to allow arbitrary types, I'm not sure how this function is supposed to work safely.

By limiting the number of entries. If we need more, we can always add them afterwards.

Inner types don't need have pub ?

Woups... Thanks!

@GuillaumeGomez

GuillaumeGomez Sep 7, 2018

Member

This seems to allow arbitrary types, I'm not sure how this function is supposed to work safely.

By limiting the number of entries. If we need more, we can always add them afterwards.

Inner types don't need have pub ?

Woups... Thanks!

This comment has been minimized.

@sdroege

sdroege Sep 7, 2018

Member

Ok then. Do we know of any code actually using this binding? How do they use it?

@sdroege

sdroege Sep 7, 2018

Member

Ok then. Do we know of any code actually using this binding? How do they use it?

This comment has been minimized.

@GuillaumeGomez

GuillaumeGomez Sep 7, 2018

Member

At least I don't. I'll ask around.

@GuillaumeGomez

GuillaumeGomez Sep 7, 2018

Member

At least I don't. I'll ask around.

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Sep 7, 2018

Member

So? Is it ok for you?

Member

GuillaumeGomez commented Sep 7, 2018

So? Is it ok for you?

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Sep 7, 2018

Member

Go for it :)

Member

sdroege commented Sep 7, 2018

Go for it :)

@GuillaumeGomez GuillaumeGomez merged commit c7ae4cf into gtk-rs:master Sep 7, 2018

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:gen-more-things branch Sep 7, 2018

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