Skip to content
This repository has been archived by the owner on Mar 4, 2024. It is now read-only.

glib/translate: take advantage of Rust Option & Result types #11

Merged
merged 14 commits into from Nov 6, 2020
Merged

glib/translate: take advantage of Rust Option & Result types #11

merged 14 commits into from Nov 6, 2020

Conversation

fengalin
Copy link
Contributor

@fengalin fengalin commented Nov 1, 2020

This is the continuation of gtk-rs/glib#718.

Requires:

glib/src/translate.rs Outdated Show resolved Hide resolved
gtk/src/entry.rs Outdated Show resolved Hide resolved
gtk/src/text_iter.rs Outdated Show resolved Hide resolved
Copy link
Member

@sdroege sdroege left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks generally good to me. Please also rebase on latest master when fixing the remaining comments as that will then give us CI :)

@sdroege
Copy link
Member

sdroege commented Nov 4, 2020

@GuillaumeGomez Your opinion?

@GuillaumeGomez
Copy link
Member

Looks good to me. Just the remaining unwrap() to be replaced and the other comments you made. ;)

Thanks @fengalin !

Copy link
Member

@sdroege sdroege left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. @GuillaumeGomez please first merge the gir PR, and then this here needs to get gir updated and can go in too

@sdroege
Copy link
Member

sdroege commented Nov 5, 2020

@fengalin Please update with gtk-rs/gir#977 and then let's get this in :)

@fengalin
Copy link
Contributor Author

fengalin commented Nov 5, 2020

It's much more convenient to contribute with the new single repo 👍

@sdroege
Copy link
Member

sdroege commented Nov 5, 2020

It's much more convenient to contribute with the new single repo

Please use generator.py to regenerate everything :)

@sdroege
Copy link
Member

sdroege commented Nov 5, 2020

@fengalin Good to go? I'll merge once the gstreamer-rs MR is also ready so we don't have things broken for too long :)

@fengalin
Copy link
Contributor Author

fengalin commented Nov 5, 2020

@fengalin Good to go?

No, it's not. Using generator.py showed 2 issues: gobject versions / features mapping are not generated in glib-sys's Cargo.toml and I believe system-deps is buggy. The lib name is not inherited when overriding with a feature. I'm on those issues ATM...

@fengalin
Copy link
Contributor Author

fengalin commented Nov 5, 2020

Just pushed a commit so that you can see what I had to do so far to workaround some issues since running generator.py (probably not directly related to the script :)). See the commit message for details. I'll investigate some more.

@fengalin
Copy link
Contributor Author

fengalin commented Nov 5, 2020

I was able to build the examples with all features using last commit (and system-deps patch).

However, the glib/sys tests don't pass.

@sdroege
Copy link
Member

sdroege commented Nov 5, 2020

However, the glib/sys tests don't pass.

How does it fail?

@fengalin
Copy link
Contributor Author

fengalin commented Nov 5, 2020

However, the glib/sys tests don't pass.

How does it fail?

Nevermind, I think it's related to mingw not being installed. It failed on master with the same errors.

@fengalin
Copy link
Contributor Author

fengalin commented Nov 6, 2020

@GuillaumeGomez @sdroege I think it's finally good to go!

gdkx11/sys/Cargo.toml Outdated Show resolved Hide resolved
@sdroege
Copy link
Member

sdroege commented Nov 6, 2020

Otherwise seems good to me

@fengalin
Copy link
Contributor Author

fengalin commented Nov 6, 2020

All green!

@GuillaumeGomez
Copy link
Member

@sdroege It's good for me, so whenever you want.

@fengalin
Copy link
Contributor Author

fengalin commented Nov 6, 2020

@sdroege It's good for me, so whenever you want.

No way! I'm doing the doc changes first :)

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Nov 6, 2020

Oh wow. Please do so, don't want to be in the middle XD

This commit introduces OptionToGlib and TryFromGlib to help with
translations from and to Glib types which encode a value with a
sentinel to indicate a None value and/or for which some values
are invalid.

These traits allow auto-implementing Option<T>::to_glib,
Option<T>::from_glib or Result::<Option<T>, _>::from_glib from
dependent crates when appropriate despite the orphan rule.
This is a fix for issues that appeared since previous
commit:

- gobject version table is not properly generated.
- features dependencies not properly generated for gdkx11-sys.
  See gtk-rs/gir#979.
@sdroege sdroege merged commit 70d65b7 into gtk-rs:master Nov 6, 2020
@fengalin fengalin deleted the translate-option-result branch November 6, 2020 13:36
elmarco pushed a commit to elmarco/gtk-rs that referenced this pull request Feb 10, 2021
elmarco pushed a commit to elmarco/gtk-rs that referenced this pull request Feb 10, 2021
elmarco pushed a commit to elmarco/gtk-rs that referenced this pull request Feb 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants