Add positional information to error messages #209
Conversation
|
This seems to cause a stack overflow under some circumstances so that's probably bad |
| @@ -126,21 +133,27 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> { | |||
| V: Visitor<'de>, | |||
| { | |||
| if self.bytes.consume_ident("true") { | |||
| return visitor.visit_bool(true); | |||
| return visitor.visit_bool(true).map_err(|e| self.fix_position(e)); | |||
There was a problem hiding this comment.
why all these cases just don't return positional errors to begin with?
There was a problem hiding this comment.
If I understand correctly, the errors are generated by fn custom<T: fmt::Display>(msg: T) -> Self on Error in error.rs. In serde's json deserializer ( https://github.com/serde-rs/json/blob/master/src/error.rs#L380 ) they've already got the line number and column built into the message, somehow, but I have a hard time following the path.
src/de/error.rs
Outdated
| } else { | ||
| write!(f, "{}", s) | ||
| } | ||
| } | ||
| Error::Parser(_, pos) => write!(f, "{}: {}", pos, self), |
There was a problem hiding this comment.
This line is causing a loop and overflow, somehow. I don't understand how it was working before since I can't find any kind of implementation that's actually doing display for ParseError
| @@ -61,15 +61,21 @@ impl fmt::Display for Error { | |||
| fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | |||
| match *self { | |||
| Error::IoError(ref s) => write!(f, "{}", s), | |||
| Error::Message(ref s) => write!(f, "{}", s), | |||
| Error::Parser(_, pos) => write!(f, "{}: {}", pos, self), | |||
There was a problem hiding this comment.
This line was causing a stack overflow and I don't understand how it was working before
| @@ -126,21 +133,27 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> { | |||
| V: Visitor<'de>, | |||
| { | |||
| if self.bytes.consume_ident("true") { | |||
| return visitor.visit_bool(true); | |||
| return visitor.visit_bool(true).map_err(|e| self.fix_position(e)); | |||
There was a problem hiding this comment.
If I understand correctly, the errors are generated by fn custom<T: fmt::Display>(msg: T) -> Self on Error in error.rs. In serde's json deserializer ( https://github.com/serde-rs/json/blob/master/src/error.rs#L380 ) they've already got the line number and column built into the message, somehow, but I have a hard time following the path.
| @@ -39,6 +39,13 @@ impl<'de> Deserializer<'de> { | |||
| pub fn remainder(&self) -> Cow<'_, str> { | |||
| String::from_utf8_lossy(&self.bytes.bytes()) | |||
| } | |||
|
|
|||
| fn fix_position(&self, e: Error) -> Error { | |||
There was a problem hiding this comment.
Ok, let's just make it so the input error type and the output error type are different. This will still require us to call it, but there will be no way to miss a fix step.
There was a problem hiding this comment.
That sounds smart, I'll give that a try
|
Issue has had no activity in the last 60 days and is going to be closed in 7 days if no further activity occurs |
No description provided.