understory_virtual_list: Doc improvements
#84
Conversation
| /// Given a scroll offset, find an index `i` such that: | ||
| /// | ||
| /// - the item at `i` is at or before that offset, | ||
| /// - and `i` is clamped into `0..=len()`. | ||
| /// - and `i` is clamped into the valid index range. | ||
| /// | ||
| /// Concretely: | ||
| /// - if `len() == 0`, this must return `0`, | ||
| /// - otherwise it must return an index in `0..len()`. |
There was a problem hiding this comment.
The wording here can perhaps be improved a bit more. For example,
and
iis clamped into the valid index range.
implies the index returned is always valid, but 0 is arguably invalid for an empty list. Maybe this should return Option<usize>? That's a slight ergonomics cut, but would prevent things like accidentally writing do_stuff(list.index_at_offset(offset)) and have that crashing, if do_stuff assumes the index is valid.
| /// Given a scroll offset, find an index `i` such that: | |
| /// | |
| /// - the item at `i` is at or before that offset, | |
| /// - and `i` is clamped into `0..=len()`. | |
| /// - and `i` is clamped into the valid index range. | |
| /// | |
| /// Concretely: | |
| /// - if `len() == 0`, this must return `0`, | |
| /// - otherwise it must return an index in `0..len()`. | |
| /// Given a scroll offset, find the index `i` such the item at that index is at the offset | |
| /// or, if the offset is beyond the last item, the item at `i` is the last item. | |
| /// | |
| /// This returns `None` if `len() == 0`. |
| /// Implementations may truncate as needed (the `f32`/`f64` impls truncate | ||
| /// toward zero). Callers are expected to clamp the result to a valid index | ||
| /// range afterwards. | ||
| fn floor_to_isize(self) -> isize; |
There was a problem hiding this comment.
Perhaps the method should be renamed to truncate_to_isize, and we can have the behavior of truncation then be required for implementations.
We may also want to state the behavior for out-of-range values:
/// If the value of the scalar does not fit in the range of an `isize`,
/// the returned `isize` is saturated as [`isize::MIN`] or [`isize::MAX`].
| /// This does not modify the underlying track model; it only affects how | ||
| /// the flat cell indices are mapped onto tracks. | ||
| /// This also resizes the underlying track model to the number of tracks | ||
| /// implied by the current `len` and the new `cells_per_track`. |
There was a problem hiding this comment.
/// implied by the current
lenand the newcells_per_track.
maybe as determined by would be better here. Below as well
3 participants
No description provided.