Skip to content
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

Lexer- and parser-relative require resolution #33

Merged
merged 1 commit into from
Aug 2, 2020

Conversation

jgellin-sf
Copy link
Contributor

@jgellin-sf jgellin-sf commented Jul 15, 2020

This PR fixes the vmRequire functions for reading the lexer and parser during Typescript declaration generation. The git history indicates that problems in this area are due to slight misuse of the require.resolve function which returns a path string, not a module. Later changes worked around this problem in the case of the parser but limited the require to 'antlr4/index', 'antlr4', or the parserFile itself. Therefore any custom imports in the antlr grammar (for example, any require statement in the @header section) could not be loaded, leading to generation problems.
This PR simplifies require resolution by using require.resolve relative to the lexer and parser file to return the module path, and then require-ing that path. This should work in a general way, eliminating the need for the various conditions in vmRequire.

@jgellin-sf
Copy link
Contributor Author

This PR should partially fix #32. The Typescript will generate, but the resulting parser declaration extends Parser, not the class name specified in the superClass option.

@jgellin-sf
Copy link
Contributor Author

@mcchatman8009 I have some work dependent on this PR. I was wondering if you could comment on the timeline for merging/publishing it. If you won't have a chance for awhile that's ok with me. I'll publish a version to my local NPM registry, but I'd like to know when I can plan to integrate a version of the real tool. Thanks for this tool.

@mcchatman8009
Copy link
Owner

@jgellin-sf I'll review later today.

@jgellin-sf
Copy link
Contributor Author

@mcchatman8009 Checking in on this PR.

@mcchatman8009 mcchatman8009 merged commit 9f2a702 into mcchatman8009:master Aug 2, 2020
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.

None yet

2 participants