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

Increase DSL methods suffix indices. #606

Closed
CMCDragonkai opened this issue Nov 21, 2017 · 7 comments
Closed

Increase DSL methods suffix indices. #606

CMCDragonkai opened this issue Nov 21, 2017 · 7 comments
Milestone

Comments

@CMCDragonkai
Copy link

It appears that there's a limit to the occurrence index from 1 to 5. While this is useful most of the time. It should be possible to make this number dynamic by moving it into a parameter instead, or perhaps using overloaded getters and setters to refere to an arbitrary number of indices.

What was the reason to stop at 5?

@bd82
Copy link
Member

bd82 commented Nov 21, 2017

What was the reason to stop at 5?

I personally never needed more. It can be fairly easily increased.
I supposed it could also be turned into an argument, but in that case it would be more difficult to parse by Chevrotain, more verbose and may be inconsistent with out APIs using configuration objects.

// existing API
CONSUME1(SemiColon ,{errMsg:"missing semicolon at end of statement"})

// using argument
CONSUME(1, SemiColon ,{errMsg:"missing semicolon at end of statement"})

// cannot be "compiled" as it the index is resolved at runtime
var x = 2
CONSUME(x, SemiColon ,{errMsg:"missing semicolon at end of statement"})

// using argument in config object - forces always using config object
CONSUME(SemiColon ,{idx:2)
  • Did you encounter a scenario where more were needed?

@CMCDragonkai
Copy link
Author

It's also possible to use overloaded getters as well. The ES6 proxy system supports this, so it's possible to do CONSUMEX. I got close to at using CONSUME5.

@bd82 bd82 changed the title Occurrence index of the parser rules Max Occurrence index of the parser rules Nov 22, 2017
@bd82
Copy link
Member

bd82 commented Nov 22, 2017

It's also possible to use overloaded getters as well. The ES6 proxy system supports this, so it's possible to do CONSUMEX.

I am guessing the proxy approach would have performance implications.
Regardless, Does it work on ES5 (polyfills)? This is a base requirement to consider it.

@bd82 bd82 changed the title Max Occurrence index of the parser rules Increase Max Occurrence index of the parser rules Nov 22, 2017
@bd82
Copy link
Member

bd82 commented Dec 5, 2017

Regardless, Does it work on ES5 (polyfills)? This is a base requirement to consider it.

To the best of my understanding the proxy dynamic property getter does not work on IE11
and cannot be polyfilled.

I guess that I would just add additional methods to reduce the chance of running out.

Also in the longer term when Chevrotain would hopefully have declarative apis #480
Meaning it would also be a parser generator, the approach using an additional argument

CONSUME(1, StringLiteral)

Could be used to enable unlimited suffixes as the added verbosity would be less meaningful
in generated code.

@bd82 bd82 added this to the V2.00 milestone Feb 5, 2018
bd82 added a commit that referenced this issue Feb 8, 2018
bd82 added a commit that referenced this issue Feb 8, 2018
bd82 added a commit that referenced this issue Feb 8, 2018
bd82 added a commit that referenced this issue Feb 8, 2018
bd82 added a commit that referenced this issue Feb 9, 2018
bd82 added a commit that referenced this issue Feb 9, 2018
bd82 added a commit that referenced this issue Feb 9, 2018
@bd82 bd82 closed this as completed in #659 Feb 9, 2018
bd82 pushed a commit that referenced this issue Feb 9, 2018
@CMCDragonkai
Copy link
Author

So the fix increased it to 9?

@bd82
Copy link
Member

bd82 commented Feb 10, 2018

Yes. will be released in 2.0 soon.
I will also change the way the grammar is analyzed so end users can setup
an ES6 proxy to support an infinite number if they know all expected run-times support it...

@bd82
Copy link
Member

bd82 commented Feb 10, 2018

I will re-open this issue as a reminder to update to grammar analysis code too.

@bd82 bd82 reopened this Feb 10, 2018
@bd82 bd82 changed the title Increase Max Occurrence index of the parser rules Increase DSL methods suffix indices. Feb 10, 2018
@bd82 bd82 closed this as completed in cebd75d Feb 10, 2018
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

No branches or pull requests

2 participants