Skip to content

Conversation

@zephyrtronium
Copy link
Contributor

There probably is a better way to do this, but this is what was easy.

There probably is a better way to do this, but this is what was easy.
This is closer to The Right Way™ to do this. Not sure about adding all
those Repr() methods, though.
@zephyrtronium
Copy link
Contributor Author

I think this is a reasonable approach to solving this problem. My only concern is that adding the extra Repr() method to Term might not be optimal; perhaps a more intelligible approach would be a single function that type switches? There are advantages (cleaner) and disadvantages (does more work, easier to forget to add a new type e.g. disjunction terms) to doing it thus.

@skelterjohn
Copy link
Owner

I'm not a fan of this method of displaying.

Consider having a gopp.ParseError type that encapsulates all this
information - what rule, which token, maybe even some possible parse trees.
The Error() method will return "Expected X and Y" kind of stuff, but if the
programmer digs in or calls a different method, he or she will get the
good stuff.

I like the "Expected X at Y" message for when you're debugging the
document, rather than debugging the grammar.

On Fri, Aug 16, 2013 at 12:33 AM, Branden J Brown
[email protected]:

I think this is a reasonable approach to solving this problem. My only
concern is that adding the extra Repr() method to Term might not be
optimal; perhaps a more intelligible approach would be a single function
that type switches? There are advantages (cleaner) and disadvantages (does
more work, easier to forget to add a new type e.g. disjunction terms) to
doing it thus.


Reply to this email directly or view it on GitHubhttps://github.com//pull/4#issuecomment-22747140
.

@zephyrtronium
Copy link
Contributor Author

This might be better. The Repr methods are still there; they could be useful for things like reconstructing/formatting a grammar, should the desire arise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants