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

Have a way to allow starting the parsing from any rule #234

Open
krisnye opened this issue Jan 29, 2014 · 6 comments
Open

Have a way to allow starting the parsing from any rule #234

krisnye opened this issue Jan 29, 2014 · 6 comments
Labels
Milestone

Comments

@krisnye
Copy link

krisnye commented Jan 29, 2014

Or else allowing any start rule should be the default if that property isn't specified.

@grob
Copy link

grob commented Jan 29, 2014

👍 - one of my projects (ringo-sqlstore) has lots of unit tests for various rules, and to explicitly define them as start rules is cumbersome.

@dmajda
Copy link
Contributor

dmajda commented Apr 16, 2014

I'm OK with having a way to allow starting the parsing from any rule — the unit tests use case is especially convincing. However the proposed way doesn't sound right to me. I'd rather have a more meaningful special value, e.g. allowedStartRules = "all".

Or else allowing any start rule should be the default if that property isn't specified.

This would make the parser bigger (see the relevant part of the code generator to see why) and break future optimizations such as rule inlining. Both is undesirable, especially given that most parsers are fine with just one start rule.

If anyone wants to implement this, I'll gladly take a patch. If no patch appears, I'll probably implement this myself sometime later in the 0.9.0 timeframe.

@andreineculau
Copy link
Contributor

Talking about meaningful special value, I went for allowedStartRules = "*" in my PR.

@dmajda
Copy link
Contributor

dmajda commented Aug 8, 2015

Hmm, * as a special value has two unfortunate properties:

  • It needs to be escaped on the command-line.
  • It may lead to an impression that it is possible to specify allowed rules using globs.

A special string value (like allowedStartRules = "all" I proposed above) is fine in the API, but could be confusing in the CLI (does --allowed-start-rules all mean “all rules” or “the rule named all”?).

I’m not sure what the right solution is right now. I’m also starting to doubt this feature would be really that useful. The unit tests use case can be solved without this by parsing whole inputs, not just parts, in the tests — and this is probably the right way, because you generally don’t want to test pieces of a parser or a parser generated differently than the production one (where you would presumably not allow starting from any rule).

In any case, I don’t want to sped time on this now ⇒ moving to post-1.0.0. Comments describing real-world situations where this feature would enable/simplify something are welcome.

@dmajda dmajda modified the milestones: post-1.0.0, 0.9.0 Aug 8, 2015
@jamestalmage
Copy link

I’m also starting to doubt this feature would be really that useful. The unit tests use case can be solved without this by parsing whole inputs, not just parts, in the tests

I disagree. Testing an entire input makes it difficult to troubleshoot rules that are deeply nested, and moves you closer to integration tests vs unit tests.

I solved this slightly differently. I used a RegExp to parse my grammar for rule names, and added helper functions.

var ruleNames = extractAllRuleNames(grammarSource);
// returns an array of rule names: ["RULE1", "RULE2"];

var parser = PEG.buildParser(grammarSource, {allowedStartRules: ruleNames});

// for each rule in ruleNames, generate a helper function that matches the rule name
parser.RULE1 = function(input, options) {
  options = options || {};
  options.startRule = "RULE1";
  parser.parse(input, options);
};
parser.RULE2 = function(input, options) {
  options = options || {};
  options.startRule = "RULE2";
  parser.parse(input, options);
};
// ... etc

This has the advantage of allowing you to explicitly specify which rule you expect to be the start rule. I think that is a bit better than just allowing it to start with any rule.

The only downside to this approach is that the regular expression that backs extractAllRuleNames is fairly brittle (especially when trying to handle the initializer portion). It would be much nicer to have all this automatically exported. Perhaps just by using a --debug flag when generating the parser. None of this makes much sense to me outside of testing.

@dmajda dmajda changed the title (allowedStartRules = true) should allow you to parse starting from any expression Have a way to allow starting the parsing from any rule Aug 14, 2015
@felipellrocha
Copy link

What's the status on this? As previously mentioned, this would come in really handy when it comes to testing my rules...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants