OvermindDL1 / ex_spirit

27 stars 1 forks source link

Error Messages #9

Closed tmbb closed 7 years ago

tmbb commented 7 years ago

Currently error messages are a little bit crude:

iex> %{error: error} = parse("b", alt([char(?x), char(?y)]))
%ExSpirit.Parser.Context{column: 1,
 error: %ExSpirit.Parser.ParseException{context: %ExSpirit.Parser.Context{column: 1,
   error: nil, filename: "<unknown>", line: 1, position: 0, rest: "b",
   result: nil, rulestack: [:e2], skipper: nil, state: %{}, userdata: nil},
  extradata: nil,
  message: "Tried parsing out any of the the characters of `'y'` but failed due to the input character not matching"},
 filename: "<unknown>", line: 1, position: 0, rest: "b", result: nil,
 rulestack: [], skipper: nil, state: %{}, userdata: nil}

The error message is accurate, but I think we can do better. This is inside an alt() parser, so we should make it clearer to the user that we tried to parser not only ?y but also x. Merely concatenating the messages of the child parsers and reporting the error at the level of the alt() parser would be a great improvement.

What do you think?

Obviously not a priority right now. I think the priority should be documenting the undocumented parts of the parser and maybe add some tests.

OvermindDL1 commented 7 years ago

Hmm, yeah something like alt could easily do something like that, and though it would slow it down 'slightly' I don't think it would be enough to really matter (only an extra binding and Cons call per failed case). This would not be at all hard to do, hmm...

OvermindDL1 commented 7 years ago

Added it in, might need refining, but it shows it all for now. :-)