Fallback values for datetime errors #4851
Conversation
robertbastian
changed the title
| ds.get_symbol_for_era(l, &year.era) | ||
| .map(|e| e.unwrap_or(&year.era.0)) | ||
| }) { |
There was a problem hiding this comment.
Nit: I think you don't need the extra map here. It should use the Err path in the match
| ds.get_symbol_for_era(l, &year.era) | |
| .map(|e| e.unwrap_or(&year.era.0)) | |
| }) { | |
| ds.get_symbol_for_era(l, &year.era) | |
| }) { |
There was a problem hiding this comment.
get_symbol_for_era returns Result<Option<&str>>, so I was under the impression that the Option is an optimisation and not an error. If it's an error, why doesn't it return Result<&str>?
There was a problem hiding this comment.
See the test I had to change. It was a mechanism to write out the era name fallback. TryWriteable is a better solution. Hmm, I think this trait is internal, so we could probably change it to return Result<&str>.
There was a problem hiding this comment.
Will address this in a follow-up
There was a problem hiding this comment.
Will wait for @robertbastian to review my changes and merge.
| ds.get_symbol_for_era(l, &year.era) | ||
| .map(|e| e.unwrap_or(&year.era.0)) | ||
| }) { |
There was a problem hiding this comment.
get_symbol_for_era returns Result<Option<&str>>, so I was under the impression that the Option is an optimisation and not an error. If it's an error, why doesn't it return Result<&str>?
| .and_then(|ns| { | ||
| // We only support fixed field length for fractional seconds. | ||
| let FieldLength::Fixed(p) = length else { | ||
| return Err(Error::Pattern( |
There was a problem hiding this comment.
Nit: Avoid using return in this function. Unclear what it returns from
No description provided.