Document entire crate #96
Conversation
|
Help very much wanted here; if you want to add docs before I'm done, please make a PR to this branch! If you spot something confusing or wrong, leave a comment :) |
|
Definitely struggling a bit to get the details right for the |
|
So first readthrough was with perhaps too little understanding of flexbox. Then looking at it again for a second go: When it comes to documentation and naming structs / methods / parameters that relates to flexbox specifically, we should make sure to keep as close as possible with the specs.
This would apply for This might be obvious but I'd thought I'd mention it regardless. |
|
You all rock; thanks for all the feedback. This collaboration is exactly why I wanted to get a draft PR up ASAP. |
|
My PR into this is now ready for review so please head on over there and speak your mind :) alice-i-cecile#2 It attempts to finish up documentation of |
alice-i-cecile
requested review from
Weibye,
TimJentzsch,
colepoirier and
sixfold-origami
|
Oh I see! @Weibye we duplicated work; can you add any relevant changes as comments on this PR? Or perhaps rebase your PR? |
On it :) |
Co-authored-by: Andreas Weibye <13300393+Weibye@users.noreply.github.com>
| @@ -8,6 +8,9 @@ use crate::number::Number; | |||
| use crate::style::Style; | |||
| use crate::sys::{new_vec_with_capacity, ChildrenVec, ParentsVec, Vec}; | |||
|
|
|||
| /// Layout information for a given [`Node`](crate::node::Node) | |||
There was a problem hiding this comment.
This is the smallest nit, but I noticed that you do not include a full stop at the end of the first line of the doc comments, but I did in flexbox.rs. Are there style guidelines/conventions for this in Rust? Do we care enough to make this consistent? (If not it's totally fine to ignore)
There was a problem hiding this comment.
I don't care enough right now; if you'd like feel free to put together a style guide PR.
| @@ -29,6 +33,8 @@ impl NodeData { | |||
| } | |||
| } | |||
|
|
|||
| /// Create the data for a new node | |||
| // TODO: why is this different from new_leaf? | |||
There was a problem hiding this comment.
This function doesn't allow to provide a MeasureFunc, while new_leaf does.
If we want to change something here I think we should also create an issue such that the TODO isn't buried in the code.
There was a problem hiding this comment.
Yeah. I think that MeasureFunc is just a bad design, TBH.
| pub(crate) struct Forest { | ||
| pub(crate) nodes: Vec<NodeData>, | ||
| pub(crate) children: Vec<ChildrenVec<NodeId>>, | ||
| pub(crate) parents: Vec<ParentsVec<NodeId>>, | ||
| } | ||
|
|
||
| impl Forest { | ||
| pub fn with_capacity(capacity: usize) -> Self { | ||
| /// Creates a new [`Forest`] that can store up to `capacity` [`Nodes`](crate::node::Node) before needing to be expanded | ||
| pub(crate) fn with_capacity(capacity: usize) -> Self { |
There was a problem hiding this comment.
I noticed that you changed this from pub to pub(crate), is this a breaking change? To be honest, this crate is the first time I have ever seen pub(crate) so I'm not entirely sure what it does.
There was a problem hiding this comment.
No, it's not. The type is not public, so the method cannot be used outside the crate anyways.
pub(crate) says "anything within this crate can use this type, but nothing else". pub(super) is the other variant you might run across, which restricts visibility to the parent module.
There was a problem hiding this comment.
(pub(crate) would be somewhat similar to internal in C# if you are familiar with that, @TimJentzsch)
Co-authored-by: TimJentzsch <tim-jentzsch@gmx.de>
|
I'm going to merge this now; feel free to make any further corrections in new PRs :) |
* Deny missing docs :3 * Module level docs * Document geometry.rs * Document layout.rs * Document prelude.rs * Document lib.rs * Document node.rs * Document number.rs * Partial docs for style.rs * Warn that padding broken :( * Add docs to forest.rs * Fix typo Co-authored-by: Andreas Weibye <13300393+Weibye@users.noreply.github.com> * Fix docs for geometry.rs * Typo fix Co-authored-by: TimJentzsch <tim-jentzsch@gmx.de> * Typo fix Co-authored-by: TimJentzsch <tim-jentzsch@gmx.de> * Revert non-exhaustive annotation Co-authored-by: TimJentzsch <tim-jentzsch@gmx.de> * Rename unclear method names on Rect * Clarify wording of mark_dirty field Co-authored-by: TimJentzsch <tim-jentzsch@gmx.de> * Clarify wording of mark_dirty field Co-authored-by: TimJentzsch <tim-jentzsch@gmx.de> * Use `parent` as the argument name over `node` in methods on `Forest` * index -> child_index * Fix incorrect comment * More docs for style.rs * Fix merge error * Document JustifyContent * Finish documenting style.rs * Small improvements to style.rs docs Co-authored-by: Andreas Weibye <13300393+Weibye@users.noreply.github.com> * Typo fix Co-authored-by: Andreas Weibye <13300393+Weibye@users.noreply.github.com> * Document sys.rs * Document more of forest.rs * Docs tweaks Co-authored-by: TimJentzsch <tim-jentzsch@gmx.de> * Typos Co-authored-by: TimJentzsch <tim-jentzsch@gmx.de> * Caching notes * Complete internal documentation * Cargo fmt Co-authored-by: Andreas Weibye <13300393+Weibye@users.noreply.github.com> Co-authored-by: TimJentzsch <tim-jentzsch@gmx.de>
Objective
Fixes #22.
Status