Pp refresh #3
Pp refresh #3
Conversation
| input, | ||
| regex, | ||
| } | ||
| pub const fn new(input: &'de str, regex: Regex) -> Deserializer { |
There was a problem hiding this comment.
Do you have a good use-case for making this const?
AFAIK creating a Regex isn't possible in a const context anyways.
| @@ -2,6 +2,7 @@ use std::fmt::{Display, Formatter}; | |||
|
|
|||
| /// An error that occurred during deserialization. | |||
| #[derive(Debug)] | |||
| #[non_exhaustive] | |||
There was a problem hiding this comment.
i don't think we should do this. Marking an error enum as non exhaustive makes some devs more lazy about error handling and there should be a good reason for doing this. The error handling of this crate is already quite weak and in would be better to improve this instead of allowing users to ignore errors.
| fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { | ||
| use Error::*; | ||
| match self { |
There was a problem hiding this comment.
This make the code more difficult to read without any benefit.
| pub fn from_str<'a, T>(input: &'a str, regex: &str) -> std::result::Result<T, Error> where T: Deserialize<'a> { | ||
| let regex = Regex::new(®ex).map_err(Error::BadRegex)?; | ||
| from_str_regex(input, regex) | ||
| #[inline] |
There was a problem hiding this comment.
Do you have any benchmarks that show noticeable performance improvements marking this and other functions as inline?
| where | ||
| T: Deserialize<'input>, | ||
| { | ||
| let rex = Regex::new(regex).map_err(Error::BadRegex)?; |
There was a problem hiding this comment.
Name shadowing can be confusing, but i don't think this applies to regex here and replacing it with rex doesn't improve this.
| struct Value { | ||
| name: String, | ||
| value: String, | ||
| } | ||
|
|
||
| impl Value { | ||
| fn parse<T>(&self) -> Result<T> where T: FromStr { | ||
| #[allow(clippy::map_err_ignore)] |
There was a problem hiding this comment.
It's better to not ignore this error and make the parser error available in the caller instead of ignoring this problem...
Hi,
What do you think about the attached (mostly trivial) commits that address some style issues reported by
cargo fmtandcargo clippywith a couple of non-default Clippy categories allowed? (most of the clippy::restriction ones, some clippy::pedantic ones, and clippy::nursery) Of course, as the author of the de-regex crate the decision as to which of these make sense and which are just busy-work lies entirely with you :)Thanks again, and keep up the great work!
G'luck,
Peter