Closed knothed closed 2 years ago
In https://github.com/simonmar/happy/pull/200#issuecomment-965845352 @int-index wrote
Before moving the GLR backend though, I think it would be better to make it a pure function (like the normal backend).
I think that is a good idea too. Would you mind making that a separate commit that does that?
I will take a stab at that; I feel bad there seemingly always is some sort of gotcha with these.
Oh, I think it would be good to give happy-backend
a more descriptive name that doesn't assume it's the default. Any ideas (or did we talk about this in the original mega-PR)?
glr
and lalr
probably?
I've renamed it.
Regarding the CLI: Am I right in assuming that there is still interest in splitting up the Main file with the accompanying CLI declarations and functions? Now, after we've done a relatively clean splitting of the existing functionality into packages, I would be happy to create additional *-cli
packages and split CLI functionality into the respective packages to allow extending happy with new packages (which was, after all, the original motive of the modularisation process).
Yes, having -cli
packages is fine with me.
Great, then I‘ll do that next. Is there something I should take extra care of?
I am not sure, we'll see if @int-index has anything for this PR.
I think after we are done with all the splits it would be a good to look over the code again before trying to make a Hackage release, but I can't predict what we might find or think.
Oh, somehow I missed that you removed IO from the GLR backend so quickly. I’ll take a better look tomorrow.
@int-index friendly reminder :)
In terms of code structure, everything LGTM. In terms of behavior, I trust you were careful not to change anything :-)
This splits off backend and backend-glr into their own packages.
Main is left mainly as is, just some functions are moved into backend and backend-glr.
I have also moved the three template files from
data/
topackages/backend/data
andpackages/backend-glr/data
. Does that make sense? I believe we have to do this at the latest when adding CLI-stuff.@Ericson2314 @int-index