cats-parse icon indicating copy to clipboard operation
cats-parse copied to clipboard

Parser error prettyprinting

Open re-xyr opened this issue 3 years ago • 9 comments

As title, it is good if we can format the Expectations into Strings. Ideally, Trifecta's output style (with line numbers and offending lines) will be very nice.

re-xyr avatar Jun 13 '21 02:06 re-xyr

Yes. See the LocationMap class that can compute the line and column.

It would be nice to have some utility that allows you to easily print.

I have some in another project but I use typelevel/paiges to produce the result and I didn't want to add that dependency here.

But we could do something with String or StringBuilder perhaps.

johnynek avatar Jun 13 '21 06:06 johnynek

I wrote a simple prettyprinting utility: gist link.

I tried to test with the JSON parser in README. It gave these errors:

(repl):2:7: error: expected one of 'false', 'null', 'true'
2 | {"x": except}
  |       ^
(repl):2:7: error: expected '"'
2 | {"x": except}
  |       ^
(repl):2:7: error: expected '0' ~ '9'
2 | {"x": except}
  |       ^
(repl):2:7: error: expected '['
2 | {"x": except}
  |       ^
(repl):2:7: error: expected '{'
2 | {"x": except}
  |       ^

Should these errors (all at one position) be combined in some way?

re-xyr avatar Jun 13 '21 09:06 re-xyr

Also, LocationMap does not support getting the EOF position (offset == length) which is inconvenient.

re-xyr avatar Jun 13 '21 11:06 re-xyr

Yeah, that is an issue.

If you get None for the line/col, it might be EOL. It would have been nicer if I had noticed that earlier and I could have made an ADT return type:

sealed abstract class RowColResult {
  def toValid: Option[(Int, Int)]
}

object RowColResult {
  case object OutOfBounds extends RowColResult {
    def toValid = None
  }
  case class Valid(row: Int, col: Int) extends RowColResult {
    def toValid = Some((row, col))
  }
  case object EOF extends RowColResult {
    def toValid = None
  }
}

something like that...

We could probably add a method like that, and then have the current method just call toValid on that.

Want to make a PR?

johnynek avatar Jun 14 '21 18:06 johnynek

About this: https://github.com/typelevel/cats-parse/issues/239#issuecomment-860181108

yes, I would group by location, and then aggregate all the expecations: expected false, true, null, ", [, or {

something like that.

johnynek avatar Jun 14 '21 18:06 johnynek

This comment:

I was thinking about also giving the EOF a position, which is one column after the last character. This way the offending line can be at the right position.

re-xyr avatar Jun 15 '21 05:06 re-xyr

I think that's right. The EOF should be on the same row as the last real character, unless that character is an end of line itself.

Put another way, EOF should be at the position that would be the same if we did: str + "e" and asked for the position of that final "e".

johnynek avatar Jun 15 '21 18:06 johnynek

I made some pretty printing in bosatsu here:

https://github.com/johnynek/bosatsu/pull/808

Maybe something general could make it in here, but people could also copy that code and tweak to suit them.

johnynek avatar Nov 11 '21 23:11 johnynek

@re-xyr I implemented a sketch for a Show[Error] at

https://github.com/typelevel/cats-parse/pull/410

you can see it in action in the test suite: https://github.com/typelevel/cats-parse/blob/03cc03cdfe9fe7e326ee640fcfdff9eb5674b0f5/core/shared/src/test/scala/cats/parse/ErrorShowTest.scala

MasseGuillaume avatar Apr 29 '22 02:04 MasseGuillaume