Skip to content
This repository has been archived by the owner on Jun 8, 2021. It is now read-only.

Add container Variant API #651

Merged
merged 11 commits into from
Sep 3, 2020
Merged

Add container Variant API #651

merged 11 commits into from
Sep 3, 2020

Conversation

zeenix
Copy link
Contributor

@zeenix zeenix commented Jun 5, 2020

This PR will add container Variant API. It already adds basic array support.

@zeenix
Copy link
Contributor Author

zeenix commented Jun 5, 2020

@sdroege Marked as WIP as I'm sure I'll be adding more API soon. But have a look already if you like the direction or not.

src/variant.rs Outdated Show resolved Hide resolved
src/variant.rs Outdated Show resolved Hide resolved
src/variant.rs Outdated Show resolved Hide resolved
src/variant.rs Outdated Show resolved Hide resolved
src/variant.rs Show resolved Hide resolved
@sdroege
Copy link
Member

sdroege commented Jun 5, 2020

Goes into the right direction IMHO, thanks :)

@sdroege
Copy link
Member

sdroege commented Jun 6, 2020

So only thing missing here is the FromVariant / ToVariant impls for Vec/slice/tuples/hashmap or also something else?

@zeenix
Copy link
Contributor Author

zeenix commented Jun 6, 2020

So only thing missing here is the FromVariant / ToVariant impls for Vec/slice/tuples/hashmap or also something else?

Yeah, I'll try to add those but for my own needs, the only thing that I know is missing is g_variant_new_dict_entry wrapper.

@sdroege
Copy link
Member

sdroege commented Jun 6, 2020

Yeah, I'll try to add those

Well without these, the new trait impls are not really useful yet :)

@zeenix
Copy link
Contributor Author

zeenix commented Jun 6, 2020

Well without these, the new trait impls are not really useful yet :)

They are, you can create the Variant from bytes or manually create it (like the array example in my patches). But yeah, w/o those conversions, the API will still be very incomplete.

@sdroege
Copy link
Member

sdroege commented Jul 8, 2020

Any updates here?

@zeenix
Copy link
Contributor Author

zeenix commented Jul 8, 2020

Any updates here?

no. :( If you want to merge what we already have here, let's unmark it as WIP and merge. I can always add additional PRs.

@sdroege
Copy link
Member

sdroege commented Jul 8, 2020

Without the remaining trait impls it's not really useful yet, so let's just wait :) Too late for this release now anyway, it happened already.

@zeenix
Copy link
Contributor Author

zeenix commented Jul 8, 2020

Without the remaining trait impls it's not really useful yet, so let's just wait :) Too late for this release now anyway, it happened already.

Sure. :)

@sdroege
Copy link
Member

sdroege commented Aug 18, 2020

Any updates here?

@zeenix
Copy link
Contributor Author

zeenix commented Aug 18, 2020

Any updates here?

No, for me it's not an isolated effort but rather something I've been doing as part of the gvariant addition in zvariant (so it gets some real testing) but there has been more important zbus bugs/issues that I've been looking into, in my limited sparetime before I could continue finishing this work.

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 like good progress :)

src/variant.rs Outdated Show resolved Hide resolved
src/variant.rs Outdated Show resolved Hide resolved
src/variant.rs Show resolved Hide resolved
src/variant.rs Outdated Show resolved Hide resolved
src/variant.rs Outdated Show resolved Hide resolved
src/variant.rs Show resolved Hide resolved
src/variant.rs Outdated Show resolved Hide resolved
src/variant.rs Show resolved Hide resolved
src/variant.rs Outdated Show resolved Hide resolved
src/variant.rs Outdated Show resolved Hide resolved
@zeenix zeenix changed the title WIP: Add container Variant API Add container Variant API Sep 2, 2020
src/variant.rs Show resolved Hide resolved
src/variant.rs Outdated Show resolved Hide resolved
src/variant.rs Show resolved Hide resolved
src/variant.rs Show resolved Hide resolved
src/variant.rs Show resolved Hide resolved
@sdroege
Copy link
Member

sdroege commented Sep 3, 2020

Now just needs to merge cleanly :) Can you remove those "Revert Revert" commits and the gir update commit?

@zeenix
Copy link
Contributor Author

zeenix commented Sep 3, 2020

Now just needs to merge cleanly :) Can you remove those "Revert Revert" commits and the gir update commit?

Rebased. Although I failed to test the results because:

$ cargo test
error: failed to select a version for `glib-sys`.
    ... required by package `glib v0.10.0 (/home/zeenix/checkout/gnome/glib-rs)`
versions that meet the requirements `=0.9.1` are: 0.9.1

the package `glib` depends on `glib-sys`, with features: `v2_64` but `glib-sys` does not have these features.


failed to select a version for `glib-sys` which could resolve this conflict

Cargo clean didn't help, neither did removal of selected folders under ~.cargo.

@zeenix
Copy link
Contributor Author

zeenix commented Sep 3, 2020

Rebased. Although I failed to test the results because:

Nm, needed cargo update. All tests pass still.

@sdroege
Copy link
Member

sdroege commented Sep 3, 2020

Needs a run of cargo fmt, seems otherwise the CI is happy :)

@sdroege sdroege merged commit 4516a75 into gtk-rs:master Sep 3, 2020
@zeenix zeenix deleted the complete-variant branch September 3, 2020 20:13
ids1024 added a commit to ids1024/glib that referenced this pull request Sep 3, 2020
This is useful when combined with the container variant functions and
trait implementations from gtk-rs#651.

Implementing `ToVariant` would be useful as well, but isn't currently
possible: gtk-rs#678
ids1024 added a commit to ids1024/glib that referenced this pull request Sep 4, 2020
This is useful when combined with the container variant functions and
trait implementations from gtk-rs#651.

BREAKING CHANGE: This remove `impl<T: ToVariant> From<T> for Variant`,
which made it impossible to implement `ToVariant` since it conflicts
with the standard library `impl<T> From<T> for T`:
gtk-rs#678
ids1024 added a commit to ids1024/glib that referenced this pull request Sep 4, 2020
This is useful when combined with the container variant functions and
trait implementations from gtk-rs#651.

BREAKING CHANGE: This remove `impl<T: ToVariant> From<T> for Variant`,
which made it impossible to implement `ToVariant` since it conflicts
with the standard library `impl<T> From<T> for T`:
gtk-rs#678
ids1024 added a commit to ids1024/glib that referenced this pull request Sep 4, 2020
This is useful when combined with the container variant functions and
trait implementations from gtk-rs#651.

BREAKING CHANGE: This remove `impl<T: ToVariant> From<T> for Variant`,
which made it impossible to implement `ToVariant` since it conflicts
with the standard library `impl<T> From<T> for T`:
gtk-rs#678
ids1024 added a commit to ids1024/glib that referenced this pull request Sep 6, 2020
This is useful when combined with the container variant functions and
trait implementations from gtk-rs#651.

BREAKING CHANGE: This remove `impl<T: ToVariant> From<T> for Variant`,
which made it impossible to implement `ToVariant` since it conflicts
with the standard library `impl<T> From<T> for T`:
gtk-rs#678
GuillaumeGomez pushed a commit to gtk-rs/gtk3-rs that referenced this pull request Oct 28, 2020
This is useful when combined with the container variant functions and
trait implementations from gtk-rs/glib#651.

BREAKING CHANGE: This remove `impl<T: ToVariant> From<T> for Variant`,
which made it impossible to implement `ToVariant` since it conflicts
with the standard library `impl<T> From<T> for T`:
gtk-rs/glib#678
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

2 participants