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
Feature: Run-time requiring of pegjs files #631
Comments
I’d rather not allow this. |
@ExE-Boss : For our use case, it'd just be for testing (also, not sure if you noted this is the pegjs repo.) |
I know that this is the PEG.js repo, which is exactly why I’m opposed to implementing this here. |
Ok, cool... (I can sometimes follow a link to another repo without noticing it is not a separate issue, so just making sure) :) |
No more features until the 2017 two-liners that let us use es6 modules and arrows are out. |
Also, it looks like what you actually want is testing You don't put the coverage hooks in the library. You put them in the test file. If you want coverage, you can install This parser generator should not suddenly become tied to the Besides, coverage is a false desire for a parser. Consider the following:
Now, just write a test for rice. Notice you only wrote a test for The way a packrat parser (honestly the way ever parser I can think of) works means coverage is fake Don't waste your time |
As far as putting coverage hooks in the library, normally one puts them in one's tests, yes, but when a parser is auto-generated with some branches as relevant and some as not, then it makes sense for the auto-generator to produce this so that those projects which do not wish to ignore their parser file can do so. It is not related to the coverage of the pegjs library (which has its own coverage requirements) but to coverage of the generated parser's use of the grammar. In your example, the generated parser has lines of code to check for "Peanuts", "Shellfish" and "Rice", and if they aren't covered, they can indeed be reported. It becomes more of an open question whether a rule like As far as this issue, there could be an entrance file which added the feature, and another which didn't. |
Issue type
Prerequisites
Description
While there might be other use cases for being able to require PEGJS files directly in Node at run-time, the main intent of this issue is for code coverage (e.g., for Mocha tests).
This latter use case of code coverage would first need #93 (providing source maps) to be first implemented. If implemented, one could avoid the need for a separate instrumented copy of one's tests + peg code.
I see that Node's
require.extensions
is used to accomplish this forts-node
, and thoughrequire.extensions
is technically deprecated in Node, as per the discussions around https://stackoverflow.com/questions/28884377/better-way-to-require-extensions-with-node-js (particularly in the vein of this answer), it seems to be the only practical mechanism here, and as per the comment, I don't think we have to worry about Node breaking Babel.The text was updated successfully, but these errors were encountered: