search

wrye-bash / wizparse

ANTLR-based parser for BAIN wizards
MIT License
4 stars 0 forks source link

body should be part of forStmt #3

Closed Holt59 closed 3 years ago

Holt59 commented 4 years ago

body is present at the end of both forRangeLoop and forInLoop but it would make more sense to have it within the forStmt rule:

// A for loop. There are two possible types of for loop.
forStmt: 'For' (forRangeLoop | forInLoop) body 'EndFor';

// A for loop of the form 'For a from b to c [by d]', where: a is a variable b is the start value c
// is the end value d (optional) is the step size
forRangeLoop: Identifier 'from' expr 'to' expr ('by' expr)?;

// A for loop of the form 'For a in b', where: a is a variable b is a value to iterate over
forInLoop: Identifier In expr;

Edit: Actually it would also make sense for the Identifier to be in forStmt.

Note: I am trying to write an interpreter, so I am probably going to open quite a few issues regarding potential improvements to the parser or questions regarding the language itself. I'll probably send you a PR for the parser when I'm done. I hope questions can help improve the specification.

Infernio commented 4 years ago

I pushed two commits addressing this and #2 to inf-2-3-tweaks. Feel free to try them and leave feedback.

Holt59 commented 4 years ago

I pushed two commits addressing this and #2 to inf-2-3-tweaks. Feel free to try them and leave feedback.

Thanks, appreciate the changes.

I think you made a typo here

https://github.com/wrye-bash/wizparse/blob/inf-2-3-tweaks/wizards/wizard.g4#L146

Should probably be "Exponentiation" :)