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

add code config option to enable alternative rule DSL #451

Closed
wants to merge 1 commit into from

Conversation

kristianmandrup
Copy link

This little tweak will allow for alternative DSLs where the resulting rule code and the string generated can differ.

// could be generated by DSL
let selectClauseStr = `() => {
    this.CONSUME(Select)
    this.AT_LEAST_ONE_SEP({
        SEP: Comma, DEF: () => {
            this.CONSUME(Identifier)
        }
    })
}`

export class SelectParser extends Parser {
  registry = {}

  constructor(input) {
    super(input, allTokens)
    this.rules()

    // must be called at the end of the constructor!    
    Parser.performSelfAnalysis(this)
  }

  rules() {
    let def = {
      SEP: Comma,
      DEF: () => {
        this.CONSUME(Identifier)
      }
    }

    let execRule = () => {
      this.CONSUME(Select)
      this.AT_LEAST_ONE_SEP(def)
    }

    this.selectClause = this.RULE("selectClause", execRule, {
      // using code string override to make regExp parser work!
      code: selectClauseStr
    })

    this.selectStatement = this.RULE("selectStatement", () => {
      this.SUBRULE(this.selectClause)
    })
  }

Already tested and works with my DSL experiment in red-dragon :)

@CLAassistant
Copy link

CLAassistant commented Apr 15, 2017

CLA assistant check
All committers have signed the CLA.

@bd82
Copy link
Member

bd82 commented Apr 15, 2017

I like the concept.

But not the reliance on strings.
This means that any custom API must also generate strings compatible with the original API.
This is not a very robust approach.

Instead I would go with a more generic approach:

  • modify the Parser's config object (passed at construction) not each Rule's config object.
    to accept a custom function that given some params (rule name / rule impel func/ ...) will create the GAST structure.
  • By default it will use the built in GAST Builder.
  • Custom use cases can implement their own logic, but won't have to be forced to generate strings.

@bd82
Copy link
Member

bd82 commented Apr 15, 2017

From code review perspective:

  • 100% code coverage is mandatory, so if you add a new code branch a new test must also be added.
  • Your IDE auto formatted the whole file, please only commit your changes 😄
  • An example (in examples/parser) directory could be nice, but not mandatory.

@kristianmandrup
Copy link
Author

Cool, sounds great :)

Yeah well, using strings was the easiest approach since I don’t know the internals of the GAST model. I would much welcome a mixed approach,
where the code option is allowed as an override but where the recommended approach is to use a custom GAST builder!

Would love to see a minimal example of this next week!

Cheers! Kristian

PS: Just managed to make the DSL example work with my String based approach for now

@bd82
Copy link
Member

bd82 commented Apr 15, 2017

I would much welcome a mixed approach.

There is no need for a mixed approach.
If the GastBuilder string --> gast method is exposed.
Than a single generic function on the parser level can perform the conversion from
a string you generated to the GAST using some ID (ruleName).

Would love to see a minimal example of this next week!

You would if you code it 😄
This is a nice to have feature, but not a high priority.
unfortunately I have some higher priority items:

  • CST Visitor + Docs
  • POCing using Chevrotain in the CoffeeScript compiler

So it would take a while before I get around to this...

@kristianmandrup
Copy link
Author

Excited to see what you come up with next. I have this rule DSL for now.

Will look into CST/GAST next.
Cheers!

@bd82
Copy link
Member

bd82 commented Apr 16, 2017

new CST related capabilities (CST Visitor) will be available in a day or two.
It will make working with CST much easier. 😄

I'll look into these extensibility options in a week or two...
It nice that ppl can create "plugins" to Chevrotain to customize something so internal as
the DSL itself. 👍

@bd82
Copy link
Member

bd82 commented Jun 23, 2017

I will attempt to implement an official GAST --> Chevrotain DSL transformer.
This will be the basis of supporting both a built-in Combinator API (probably similar to myna's APIs) and to enable users to create their own custom APIs for Chevrotain.

Relevant issue: #480

This could even enable using Chevrotain as a parser generator 😄

This is a huge issue. But the first thing on the agenda is the
** GAST --> Chevrotain DSL transformer. **
Which should at least enable your custom API.

@bd82 bd82 closed this Jun 23, 2017
@kristianmandrup
Copy link
Author

AweZome!

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.

3 participants