-
Notifications
You must be signed in to change notification settings - Fork 85
Create Tabular package #209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
When this MR is merged, #200 would be obsolete. @Ericson2314 @int-index I believe the one thing left here is renaming, and possibly moving, |
|
Hmm I'd probably prefer documenting this somewhere in the code. |
@knothed That's better! Then I'd say leave where it was in this commit, and then convert to code comments in subsequent commit. |
@@ -417,18 +417,21 @@ the spontaneous lookaheads in the right form to begin with (ToDo). | |||
----------------------------------------------------------------------------- | |||
Merge lookaheads | |||
|
|||
> -- TODO needs better name | |||
> type Lr1State = ([Lr1Item], [(Name, Int)]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should do sth about the todo :D @Ericson2314
Could this be merged? I think we won't need #200 anymore after merging this. |
Oh sorry I lost track of this. I am fine with |
#200 I rebased to just be additional splits and the main part, so I am happy to keep it around for later for when we revisit that sort of stuff :) |
I will review by Monday. |
I have not reviewed by Monday. Shame on me. But I’m doing it right now. |
LGTM. Thank you! |
Create a Tabular package which contains the
Tabular
module and related stuff like LALR.