gherkin-ruby: The `rule_type` parameter in `AstBuilder#end_rule` is not used? #208

Closed
drhuffman12 opened this Issue May 12, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@drhuffman12

drhuffman12 commented May 12, 2017

Summary

While working on porting gherkin-ruby to crystal as gherkin-cr, I noticed that the AstBuilder#end_rule method in lib/gherkin/ast_builder.rb looks odd:

    def end_rule(rule_type)
      node = @stack.pop
      current_node.add(node.rule_type, transform_node(node))
    end

Current Behavior

The rule_type parameter passed into end_rule is not used. (The rule_type in node.rule_type is that node's attribute, not the end_rule method's parameter.)

Expected Behavior

I'm not sure which is expected.

Possible Solution

Should def end_rule(rule_type) be just def end_rule, or should rule_type somehow be used to filter the @stack items?

@aslakhellesoy

This comment has been minimized.

Show comment Hide comment
@aslakhellesoy

aslakhellesoy Jul 20, 2017

Owner

Hi @drhuffman12 the parser is generated from a grammar file and a template as described in the README.

Are you writing the parser by hand? That would make it very hard to maintain. You're right that the rule_type isn't used, and we should probably change all the templates for all the languages, but I'm not too worried about it.

Owner

aslakhellesoy commented Jul 20, 2017

Hi @drhuffman12 the parser is generated from a grammar file and a template as described in the README.

Are you writing the parser by hand? That would make it very hard to maintain. You're right that the rule_type isn't used, and we should probably change all the templates for all the languages, but I'm not too worried about it.

@aslakhellesoy

This comment has been minimized.

Show comment Hide comment
@aslakhellesoy

aslakhellesoy Aug 27, 2017

Owner

Closed due to inactivity

Owner

aslakhellesoy commented Aug 27, 2017

Closed due to inactivity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment