Skip to content

Conversation

Naoray
Copy link
Contributor

@Naoray Naoray commented Nov 8, 2019

I've no clue if this is what you thought in #2.
I just wanted to give it a try. Maybe you can use some code of it or none at all.


I added a singleton for the Blueprint class and added a config where the generators and lexers can be defined which should be registered by default. I personally don't like that the boot() method on Blueprint is called within the BlueprintServiceProvider@boot() method, but could think of a better alternative.

I also didn't add any tests for the boot() method on the Blueprint class, because I ran into an error that config() method is not defined, which is likely to be caused by how the tests are setup.


If this PR doesn't help you at all please tell me and if you have the time I'd appreciate why and how I could improve things for future contributions.

Thanks!

@jasonmccreary
Copy link
Collaborator

I like it. But will probably sit on it until after v1.0. I want to see what this current pattern feels like as I continue to build out more generators.

I also would want to add tests for this since they are one of the last untested areas. Maybe with orchestra/work-bench or something...

@Naoray
Copy link
Contributor Author

Naoray commented Nov 8, 2019

If you want to change the current test setup to use orchestra, I'd offer to add tests for it. I just can't wrap my head around how I'd test this without loading the configs etc... :)

@jasonmccreary
Copy link
Collaborator

Yeah, me neither. That's another reason to sit on this for just a bit. I'm going to leave it open and allow more time to let the feature unfold.

@owenvoke
Copy link

I was going to ask about this sort of thing in the livestream earlier, was thinking it would be neat to be able to add more lexers and generators!

@jasonmccreary
Copy link
Collaborator

I am closing this in favor of opening another. Thanks for the inspiration, I have applied some of the changes in the new PR.

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.

3 participants