Pile improvements 1 #233
Pile improvements 1 #233
Conversation
| @@ -185,6 +192,12 @@ impl std::ops::Deref for PendingPiledWidget { | |||
| } | |||
|
|
|||
| /// A widget that has been added to a [`Pile`]. | |||
| /// | |||
| /// If this is dropped, the widget will be removed from the Pile. | |||
There was a problem hiding this comment.
| /// If this is dropped, the widget will be removed from the Pile. | |
| /// When the last clone of a `PiledWidget` is dropped, the widget will be removed from the Pile. |
| @@ -218,6 +231,7 @@ struct PiledWidgetData { | |||
|
|
|||
| impl Drop for PiledWidgetData { | |||
| fn drop(&mut self) { | |||
| println!("drop called"); | |||
There was a problem hiding this comment.
| println!("drop called"); |
| @@ -67,6 +69,9 @@ impl Pile { | |||
| /// When the last clone of the returned [`PiledWidget`] is dropped, `widget` | |||
| /// will be removed from the pile. If it is the currently visible widget, | |||
| /// the next widget in the pile will be made visible. | |||
| /// | |||
| /// See [`PiledWidget`] for important lifetime information. | |||
There was a problem hiding this comment.
Doesn't the paragraph directly before this statement explain what the summary added to PiledWidget states?
| @@ -49,6 +49,8 @@ impl PileData { | |||
| impl Pile { | |||
| /// Returns a placeholder that can be used to show/close a piled widget | |||
| /// before it has been constructed. | |||
| /// | |||
| /// See [`PiledWidget`] for important lifetime information. | |||
There was a problem hiding this comment.
The return type of this function isn't a PiledWidget but a PendingPIledWidget, so this tip seems wrong.
| @@ -164,6 +169,8 @@ pub struct PendingPiledWidget(Option<PiledWidget>); | |||
|
|
|||
| impl PendingPiledWidget { | |||
| /// Place `widget` in the pile and returns a handle to the placed widget. | |||
| /// | |||
| /// See [`PiledWidget`] for important lifetime information. | |||
There was a problem hiding this comment.
| /// See [`PiledWidget`] for important lifetime information. | |
| /// When the last clone of the returned [`PiledWidget`] is dropped, `widget` | |
| /// will be removed from the pile. If it is the currently visible widget, | |
| /// the next widget in the pile will be made visible. |
I prefer not making the user jump through links to read information like this. I've pasted the summary from push as the suggestion here.
| @@ -164,6 +169,8 @@ pub struct PendingPiledWidget(Option<PiledWidget>); | |||
|
|
|||
| impl PendingPiledWidget { | |||
| /// Place `widget` in the pile and returns a handle to the placed widget. | |||
| /// | |||
| /// See [`PiledWidget`] for important lifetime information. | |||
| #[allow(clippy::must_use_candidate)] | |||
There was a problem hiding this comment.
| #[allow(clippy::must_use_candidate)] | |
| #[must_use] |
This seems like an oversight by me.
| /// Lifetimes: | ||
| /// | ||
| /// Due to how this macro works variables in the closure will be dropped at the end of the | ||
| /// method body, and *NOT* when the application terminates. |
There was a problem hiding this comment.
Did you test that this documentation shows up when running cargo doc? I ask because this macro already has documentation in another location:
Lines 46 to 47 in f6a4704
I linked to the paragraph where I tried to explain what is happening. It seems like that should be updated to be more explicit.
Improvements based on discoveries made in #231