docs: manual traits are in prelude only #1111
Conversation
|
I don't remember if this holds true for GStreamer-rs too... EDIT: It does, but we also reexport these from the crate root. |
|
I did check before submitting the MR; all the manual traits are defined in a prelude module https://gitlab.freedesktop.org/gstreamer/gstreamer-rs/-/blob/master/gstreamer-video/src/lib.rs#L111, https://gitlab.freedesktop.org/gstreamer/gstreamer-rs/-/blob/master/gstreamer-gl/src/lib.rs#L47 & https://gitlab.freedesktop.org/gstreamer/gstreamer-rs/-/blob/master/gstreamer-base/src/lib.rs#L66 for example |
|
@bilelmoussaoui Yup, looks good to me. @sdroege Is it intended that we re-export some (but maybe not all?) traits from the crate root in addition to the prelude? |
The non-manual traits are all exported from the crate root or not? I think we should re-export also the manual ones there... |
|
The non-manual traits are only exported from the prelude, as @bilelmoussaoui says |
|
In that case yes, let's re-export all traits only from the prelude. |
|
@sdroege That seems like quite a significant and breaking change, but perhaps for the better. More consistency when it comes to traits, I guess it doesn't make much sense to get the manual ones in the root and only get the autogenerated ones from the prelude only. |
|
That was easier fixed than writing an actual commit message... https://gitlab.freedesktop.org/gstreamer/gstreamer-rs/-/merge_requests/751 |
|
So that only affects GStreamer? |
From a quick look at the various gtk-rs repositories, seems like it |
|
In gtk-rs Should we adapt |
The discussion is about the manual traits though, no? |
|
Found one,
Correct, but following https://gitlab.freedesktop.org/gstreamer/gstreamer-rs/-/merge_requests/751#note_893338 we're also exploring what's going on with auto- ( EDIT: After all the difference between auto and manual is merely whether we can generate a correct function for it, that should be oblivious to crate users IMO. |
Yes, that's correct
Oh, I see what you mean now. We always re-export the non-manual traits indeed. Well, I don't know then, maybe we can just close this PR & we re-export everything. |
Noticed in [1] that this trait has ManualExt written the wrong way around, and is exported from the crate root instead of the prelude where all other traits are reexported. [1]: gtk-rs/gir#1111 (comment)
This PR is probably correct, but then we should make sure not to export any traits from the crate root. In my limited understanding
What do you think @bilelmoussaoui? @sdroege? |
|
Yes it should be one of the two. What would be the advantage of re-exporting the traits from the crate root? |
Less breakage for crates currently using Gtk/GStreamer and relying on those to be available when importing explicitly or |
Noticed in [1] that this trait has ManualExt written the wrong way around, and is exported from the crate root instead of the prelude where all other traits are reexported. Besides, it has the name the correct way around in the `manual_traits` array inside Gir.toml. [1]: gtk-rs/gir#1111 (comment)
Noticed in [1] that this trait has ManualExt written the wrong way around, and is exported from the crate root instead of the prelude where all other traits are reexported. Besides, it has the name the correct way around in the `manual_traits` array inside Gir.toml. [1]: gtk-rs/gir#1111 (comment) Fixes: ad71d74 ("gio: manually generate get_channel_binding_data")
|
I would go for having everything in prelude. Most of people don't really know the various traits there's and just want to call the method they want; having to look for the right trait & the module to import it from doesn't sound nice in practice. Also not re-exporting every trait in lib.rs would reduce the very large lib.rs docs page. The user can still use a specific trait if they want by importing it from the module it comes from. |
|
On Sun, 2021-04-25 at 09:32 -0700, Bilal Elmoussaoui wrote:
I would go for having everything in prelude. Most of people don't
really know the various traits there's and just want to call the
method they want; having to look for the right trait & the module to
import it from doesn't sound nice in practice. Also not re-exporting
every trait in lib.rs would reduce the very large lib.rs docs page.
The user can still use a specific trait if they want by importing it
from the module it comes from.
I agree. Let's clean that up then
|
That's not the intended usecase anyway, users should get
It would be very nice to have them nicely separated between types and their functions, indeed :)
We have gtk-rs/gtk3-rs#483 for gtk-rs, https://gitlab.freedesktop.org/gstreamer/gstreamer-rs/-/merge_requests/751 for GStreamer-rs, and let's get these exports removed from |
|
Let's merge this one then, I will come with a MR (unless someone wants to take care of that) to remove the exported and only keep them in the |
|
@bilelmoussaoui I think we need more documentation changes because now non-manual traits also come from |
Following a discussion in [1] we have decided to remove all trait exports - both manual and automatic - from the crate root in favour of having them available to crate users in `prelude` only, as per the intended design. It was inconsistent to have ExtManual traits available in `prelude` only (and some randomly in the crate root in GStreamer-rs) whereas all automatic traits were re-exported through the crate root as well. [1]: gtk-rs#1111
Following a discussion in [1] we have decided to remove all trait exports - both manual and automatic - from the crate root in favour of having them available to crate users in `prelude` only, as per the intended design. It was inconsistent to have ExtManual traits available in `prelude` only (and some randomly in the crate root in GStreamer-rs) whereas all automatic traits were re-exported through the crate root as well. [1]: gtk-rs#1111
In gir it was brought up [1] that some traits (in particular
`*ExtManual`) are exported from the crate root in addition to the
prelude, cluttering the environment unnecessarily. This commit removes
all these reexports, leaving those in prelude (that were already there)
only.
After this commit everything matching `Ext(Manual)?\b` in `lib.rs` sits
within `pub mod prelude {};`.
[1]: gtk-rs/gir#1111
fixes #1110