Skip to content

Conversation

jf2048
Copy link
Member

@jf2048 jf2048 commented Mar 1, 2023

Also changes strings with transfer=full to impl Into<GString>

I added new Bounds but not a new Transformation, if anything else requires this stack-callback approach then one could be added

Also added Transformation too to correspond to the new chunk type, so IntoStrV works as well

@sdroege
Copy link
Member

sdroege commented Mar 1, 2023

Same question here, can you create a draft PR with the changes this causes? :)

@sdroege
Copy link
Member

sdroege commented Mar 1, 2023

There's also impl Into<GString> for the transfer-full case and impl IntoStrV for the "array of strings" case that would be useful to consider here.

@jf2048 jf2048 force-pushed the into-gstr branch 2 times, most recently from 59d6829 to 2a45ed4 Compare March 2, 2023 17:47
@sdroege
Copy link
Member

sdroege commented Mar 2, 2023

analysis: Don't use IntoGStr for string params with length

Why not?

@jf2048
Copy link
Member Author

jf2048 commented Mar 2, 2023

I added IntoStrV but I think we want to do gtk-rs/gtk-rs-core#978 first, I don't want to make gir subtract one from len() only to remove that later

@jf2048
Copy link
Member Author

jf2048 commented Mar 2, 2023

Why not?

Those don't need to be nul terminated, it's fine for them to just be &str. I mean, I guess it could be AsRef<str> if we wanted

@sdroege
Copy link
Member

sdroege commented Mar 2, 2023

I added IntoStrV but I think we want to do gtk-rs/gtk-rs-core#978 first, I don't want to make gir subtract one from len() only to remove that later

gtk-rs/gtk-rs-core#1037

Those don't need to be nul terminated, it's fine for them to just be &str. I mean, I guess it could be AsRef<str> if we wanted

Oh right, that makes sense.

@sdroege
Copy link
Member

sdroege commented Mar 11, 2023

I think the futures change would be worth its own PR. That can be merged already :)

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.

2 participants