Provide nested parser class for Zeitwerk compat#52
Merged
cjheath merged 1 commit intocjheath:masterfrom Jan 22, 2025
Merged
Conversation
Zeitwerk follows the general convention that the file name implies the constant the file will load. Since the compiled Treetop files define two constant one will not end up matching the filename, making it incompatible with reloaders such as Zeitwerk. To maintain compatibility with existing uses of Treetop we want to maintain the <Grammar>Parser constant. But to enable compatibility with the convention that Zeitwerk follows we also want a <Grammar>::Parser constant. Therefore, the compiled code now defines both. First, it defines the nested class within the grammar module. Then it aliases that constant to the existing constant. This should provide the best of both worlds. Users already using the old constant can continue to do so. But users using auto-loaders like Zeitwerk can now assume the standard naming conventions. --- In addition to changing the output, I also rebuilt the Treetop gramar so the new constant can be used on the included grammar. I also added a test to verify the new nested class can be used as a parser. --- This closes cjheath#51.
Owner
|
Published to RubyGems as 1.6.14 |
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Zeitwerk follows the general convention that the file name implies the constant the file will load. Since the compiled Treetop files define two constant one will not end up matching the filename, making it incompatible with reloaders such as Zeitwerk.
To maintain compatibility with existing uses of Treetop we want to maintain the
<Grammar>Parserconstant. But to enable compatibility with the convention that Zeitwerk follows we also want a<Grammar>::Parserconstant.Therefore, the compiled code now defines both. First, it defines the nested class within the grammar module. Then it aliases that constant to the existing constant.
This should provide the best of both worlds. Users already using the old constant can continue to do so. But users using auto-loaders like Zeitwerk can now assume the standard naming conventions.
In addition to changing the output, I also rebuilt the Treetop grammar so the new constant can be used on the included grammar. I also added a test to verify the new nested class can be used as a parser.
My editor auto-removed some unnecessary white-space.
This closes #51.