Continuation PR for #256: Implement std::error::Error for errors #262
Conversation
Please implement |
|
☔ The latest upstream changes (presumably #259) made this pull request unmergeable. Please resolve the merge conflicts. |
50ed753 to
51312ea
Compare
@nox |
|
☔ The latest upstream changes (presumably #264) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@nox @SimonSapin Does one of you have time to review this? |
procedural-masquerade/lib.rs
Outdated
| } | ||
| } | ||
| } | ||
| ($proc_macro_name: ident ! $paren: tt) => { |
There was a problem hiding this comment.
This seems a rustfmt bug that should be fixed and undone.
src/tokenizer.rs
Outdated
| ), | ||
| QuotedString(value) => format!("The value '{}' does not include the quotes", value), | ||
| UnquotedUrl(value) => format!( | ||
| "The value '{}' does not include the `url(` `)` markers.", |
There was a problem hiding this comment.
This doesn't look right... UnquotedUrl is just url(string-without-quotes), not any kind of error.
src/tokenizer.rs
Outdated
| "The value '{}' does not include the `#` marker, but has a valid ID selector", | ||
| value | ||
| ), | ||
| QuotedString(value) => format!("The value '{}' does not include the quotes", value), |
There was a problem hiding this comment.
Same for this, I don't know what this is about?
src/parser.rs
Outdated
| fn description(&self) -> String { | ||
| match self { | ||
| BasicParseErrorKind::UnexpectedToken(token) => { | ||
| format!("Unexpected token: '{}'", token.description()) |
There was a problem hiding this comment.
I would just do: format!("Unexpected token: {:?}", token), and remove Token::description().
src/tokenizer.rs
Outdated
| SubstringMatch => String::from("*="), | ||
| CDO => String::from("<!--"), | ||
| CDC => String::from("-->"), | ||
| Function(name) => format!("The value ({}) does not include the `(` marker", name), |
There was a problem hiding this comment.
To be clear, these are legit token types. If the input is foo() and I do input.expect_ident(), I get an unexpected token error with the function, but there's nothing the "doesn't include the ( marker" or anything like that.
|
Want to review the new commit @emilio? |
| @@ -53,6 +55,20 @@ pub enum BasicParseErrorKind<'i> { | |||
| QualifiedRuleInvalid, | |||
| } | |||
|
|
|||
| impl<'i> BasicParseErrorKind<'i> { | |||
| fn description(&self) -> String { | |||
| match self { | |||
There was a problem hiding this comment.
Can we move this to Display::fmt?
src/parser.rs
Outdated
| @@ -62,6 +78,18 @@ pub struct BasicParseError<'i> { | |||
| pub location: SourceLocation, | |||
| } | |||
|
|
|||
| impl<'i> fmt::Display for BasicParseError<'i> { | |||
| fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { | |||
| formatter.write_str(self.kind.description().as_str()) | |||
There was a problem hiding this comment.
That way this doesn't need the extra allocation.
There was a problem hiding this comment.
Also, it may be nice to expose self.location, but that may be fine as a follow-up.
src/parser.rs
Outdated
| { | ||
| fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { | ||
| match &self.kind { | ||
| ParseErrorKind::Basic(basic_kind) => formatter.write_str(&basic_kind.description()), |
There was a problem hiding this comment.
This can just use basic_kind.fmt(formatter)
|
☔ The latest upstream changes (presumably #288) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Is there a way I can help with this? |
|
@categulario If you would like to review and or make changes, please do. I wanted to finish this, but did not manage to. You have push access to my fork. |
|
Thanks @dutchmartin . I addressed @emilio 's concerns in c962d4a . |
|
☔ The latest upstream changes (presumably #332) made this pull request unmergeable. Please resolve the merge conflicts. |
This PR is the continuation of #256.
Some changes are due to me running cargo
fmtI have implemented description, but I can not satisfy the borrow checker with a reference of a String that will get deleted after the function ends. (see image). Therefore I used a static string pointing the user in the right direction.
Closes #256
Fixes #247