Add StrftimeItems::{parse, parse_to_owned} and more documentation#1184
Add StrftimeItems::{parse, parse_to_owned} and more documentation#1184pitdicker merged 4 commits intochronotope:mainfrom
StrftimeItems::{parse, parse_to_owned} and more documentation#1184Conversation
d9b4faf to
bd2ae15
Compare
a031fa3 to
51d009e
Compare
9b2325f to
135fd7d
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1184 +/- ##
==========================================
- Coverage 93.41% 91.87% -1.55%
==========================================
Files 34 38 +4
Lines 16754 17614 +860
==========================================
+ Hits 15651 16182 +531
- Misses 1103 1432 +329 ☔ View full report in Codecov by Sentry. |
src/format/strftime.rs
Outdated
| #[cfg(any(feature = "alloc", feature = "std"))] | ||
| pub fn parse(self) -> Result<Vec<Item<'a>>, ParseError> { | ||
| let result: Vec<_> = self.collect(); | ||
| if result.iter().any(|i| i == &Item::Error) { |
There was a problem hiding this comment.
Instead of first collecting and then iterating again to find Error values, we can do both in one iteration by relying on the FromIter implementation for Result. Something like this:
self.into_iter().map(|item| match item == Item::Error {
false => Ok(item),
true => Err(BAD_FORMAT),
}).collect()There was a problem hiding this comment.
Cool, thank you!
| /// # Ok::<(), ParseError>(()) | ||
| /// ``` | ||
| #[cfg(any(feature = "alloc", feature = "std"))] | ||
| pub fn parse_to_owned(self) -> Result<Vec<Item<'static>>, ParseError> { |
There was a problem hiding this comment.
Here, too: why should this be added to the public API? What use case does it address that is currently underserved?
There was a problem hiding this comment.
The Vec returned by StrftimeItems::parse borrows from the input string. While we have items that can take owned strings they were really not easy to use, or even discover.
Somewhat suggested in #852 (comment) (#1183).
a35592c to
f2b0580
Compare
f2b0580 to
31db358
Compare
|
This is a smallish PR that improves the documentation of @djc you already reviewed a lot the past few days, but how to you feel about getting this one over the finish line? Also because you already gave it one round of review. |
src/format/strftime.rs
Outdated
| /// # Errors | ||
| /// | ||
| /// While iterating [`Item::Error`] will be returned if the format string contains an invalid | ||
| /// or unrecognised formatting specifier. |
src/format/strftime.rs
Outdated
| /// # Errors | ||
| /// | ||
| /// While iterating [`Item::Error`] will be returned if the format string contains an invalid | ||
| /// or unrecognised formatting specifier. |
31db358 to
ccde6a2
Compare
Adds
StrftimeItems::parseandStrftimeItems::parse_to_owned.parsereturnsVec<Item<'_>>and can return an error if the format string is invalid.With the new formatting API the
Vechopefully gets more use, and then it may become difficult that is contains references to the format string. That is what theOwnedLiteralandOwnedSpacevariants ofItemare for, but we had no easy way to make those.parse_to_ownedreturnsVec<Item<'static>>.And I added some documentation and doctests.