Skip to content
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

Set argument of SetProperty to Variant #239

Merged
merged 1 commit into from
Dec 17, 2022

Conversation

WhyNotHugo
Copy link
Contributor

This function strictly takes a Variant, and other types will fail with error:

No such interface “org.freedesktop.DBus.Properties” on object at path ...

Closes #202

object.go Show resolved Hide resolved
This function fails for anything that is not a Variant with error:

    No such interface “org.freedesktop.DBus.Properties” on object at path ...

Convert non-Variant into Variants.

Closes godbus#202
@WhyNotHugo WhyNotHugo force-pushed the setproperty-signature branch 3 times, most recently from b08ac61 to 677653d Compare November 1, 2022 16:38
@WhyNotHugo
Copy link
Contributor Author

I'm not entirely convinced by the results here; it someone passes an int then we're trying to set a property of type int, but if someone passes a Variant<int>, we're also trying to set a property of type int, and not Variant<int>.

I'm not sure which is the best behaviour with the current signature.

@guelfey
Copy link
Member

guelfey commented Dec 4, 2022

But that would still be compatible to existing code - if you have a property that already is an actual Variant, you need to pass e.g. a Variant<Variant<Int>> here. This just makes this function a bit nicer to use if your property is not a Variant, which I'm fine with.

@WhyNotHugo
Copy link
Contributor Author

Yeah, the main upside is that it maintains compatibility with existing code, but it's really unexpected behaviour and not a great API.

Maybe it's best to merge this as-is and refine it before a major release (at which point a breaking change might be acceptable).

@guelfey guelfey merged commit 3e8deb0 into godbus:master Dec 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to Set a Property Value?
3 participants