-
-
Notifications
You must be signed in to change notification settings - Fork 132
Add gio::Vfs subclass #1726
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
Add gio::Vfs subclass #1726
Conversation
63b4266 to
6427fa2
Compare
2484ba8 to
8a65537
Compare
|
Looks good to me except for the two open discussions about the string array. |
8a65537 to
ea7130d
Compare
ea7130d to
b831e69
Compare
b831e69 to
056b4a5
Compare
056b4a5 to
bcba69c
Compare
glib/src/collections/strv.rs
Outdated
| /// It can be constructed safely from a `&StrV` and unsafely from a pointer to a C array. | ||
| /// This type is very similar to `[GStringPtr]`, but with one added constraint: the underlying C array must be `NULL`-terminated. | ||
| #[repr(transparent)] | ||
| pub struct CStrV { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name seems inconsistent with everything else. This is basically the String/PathBuf (StrV) vs. str/Path (CStrV) duality. Maybe this one should've actually been StrV and the other one StringV. Using Buf here seems wrong.
In other places (e.g. in gstreamer) there's a pattern of Foo and FooRef, so maybe we can just call this one StrVRef? @bilelmoussaoui what do you think?
Also GStringPtr is an ugly name, should probably reconsider that before the next release. Its difference with GStr is that it doesn't store the length and is equivalent with the raw C pointer. GStrLite or so... doesn't have to be solved as part of this PR but in case someone has ideas while we talk about naming anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CStrV because of std::ffi::CStr. However I agree that name should be consistent with others gtk-rs types. I think also that CStrV should be StrV and StrV should be StringV or if you don't want breaking changes, maybe renameCStrV to GStrV and rename StrV to GStringV and create a type alias (deprecated) StrV = GStringV.
Anyway let me know for CStrV new name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd still vote for StrV (stays as it is) and StrVRef. But waiting for @bilelmoussaoui to opine :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
StrvRef / Strv sounds good to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
bcba69c to
c333df3
Compare
This type represents a borrowed reference to a `NULL`-terminated array of `NULL`-terminated UTF-8 strings. Signed-off-by: fbrouille <[email protected]>
Signed-off-by: fbrouille <[email protected]>
c333df3 to
7cf9052
Compare
Allow custom implementation of gio::Vfs