Improve WidgetMut documentation #663
Conversation
There was a problem hiding this comment.
This is a very neat hack. I do think that it is necessary to improve the documentation here substantially.
However, this makes Masonry's code significantly harder to understand, and it also makes implementing external widgets harder (as, for example, there is no longer a reference to WidgetMut in the implementations, to even allow the docs about extension traits to be found).
My order of preferred options are:
- Move all of the widgets to extension traits, ensuring a uniform but bad experience for all users.
- Keep things as-is until arbitrary self types is stable. Re-evaluate on (approx) 2025-01-09 if new blockers for arbitrary self types v2 appear
- This solution
| @@ -100,6 +100,8 @@ | |||
| #![cfg_attr(not(debug_assertions), allow(unused))] | |||
| // False-positive with dev-dependencies only used in examples | |||
| #![cfg_attr(not(test), warn(unused_crate_dependencies))] | |||
| // Needed because of the SelfMut macro | |||
| #![allow(clippy::needless_arbitrary_self_type)] | |||
|
|
|||
There was a problem hiding this comment.
It's not clear how this can work without a #![feature(arbitrary_self_types)] (probably in a cfg_attr)
There was a problem hiding this comment.
To be clear, this lint doesn't need #![feature(arbitrary_self_types)]. It applies to cases like:
fn foobar(self: Self) {}which have a more concise version.
There was a problem hiding this comment.
I wasn't commented on the added line - I was commenting on "the top of masonry/src/lib.rs", because it isn't clear to me how the new docs can work without enabling the feature for std::ops::Receiver.
Like, they do seem to work based on local testing, but I'm really struggling to work out how.
I think it's a rustdoc bug.
If we go for a uniform bad experience, I'd rather convert all methods to free functions, the other solution we discussed. I still don't like the Ext trait solution. I think it produces worse documentation, and I suspect it would produce worse error messages and a worse learning curve as well. But maybe we need to have a broader discussion about this. |
|
Associated functions wouldn't be world-endingly terrible, yeah. So basically, each block is: impl Button {
pub fn label_mut(this: &mut WidgetMut<Self>) -> WidgetMut<Label>;
}or whatever. Replacing |
|
Closing this, since we've agreed we won't pursue this doc trick. |
See discussion here #663 and [on zulip](https://xi.zulipchat.com/#narrow/channel/317477-masonry/topic/Improving.20docs.20for.20WidgetMut). Basically, previous code was using `WidgetMut<MyWidget>` as a receiver. This can be done when WidgetMut is a local type, but external users wouldn't be able to do it. The result would be that users importing Masonry as a dependency but copy-pasting code from one of our widgets would get compile errors. This PR switches to a syntax that external crates will be able to use when declaring widgets. It's a more verbose, less readable syntax, but it's unambiguous and doesn't require clever tricks. We can consider switching back to WidgetMut-as-a-receiver when `#![feature(arbitrary_self_types)]` or some equivalent gets stabilized. See rust-lang/rust#44874 (comment) for current progress.
This PR introduces an
ImplMutmacro and aSelfMutmacro which resolve to different types depending on whether we're generating doc. The doc version uses the unstable receiver trait.For details, see this zulip thread.
I'm not quite sure how to document this yet. It's a pretty far-ranging change.
Also, because I want to avoid merge conflicts with other PRs in the pipeline, imports in this PR are still messy.
Leaving this PR as a draft for now.