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

Clarification for CaseStmt and DefaultStmt #4

Open
Holt59 opened this issue Oct 27, 2020 · 8 comments
Open

Clarification for CaseStmt and DefaultStmt #4

Holt59 opened this issue Oct 27, 2020 · 8 comments

Comments

@Holt59
Copy link

Holt59 commented Oct 27, 2020

Me, again....

The specification+ for the SelectOne / SelectMany + Case / Default is not clear for me.

Is each Case / Default followed by exactly one Break? For instance, is this valid (like in many language):

SelectOne ...

Case A
Case B
Break
  • If it is valid, what is the expected behavior? In particular for SelectMany? Case A implies A + B?

  • If it is not valid, then I think it would be a good idea to explicitly add the 'Break' in the grammar in the selectCaseList rule.

Also, is Default allowed to not be the last case? And is Default allowed in SelectMany, in which case, is it run when nothing is selected?

Edit: Well, actually, what is the purpose of Default? I assume with SelectOne, the user has to select an option, so no default, and for SelectMany... If none of the option was selected?

None of the test wizards in tests/ have Default and all uses a Break after Case, so having this kind of rule:

selectCaseList: (caseStmt 'Break')* (defaultStmt 'Break')?;

Would be very nice to write an interpreter :D


+ I am assuming this is the official specification : https://wrye-bash.github.io/docs/Wrye%20Bash%20Technical%20Readme.html, let me know if it's not !

@Infernio
Copy link
Member

Best idea is to write a wizard and test it in the old parser 🤷‍♀️
The parser code isn't exactly nice to navigate and it's very, very loose with what it allows. I think the intent of Breaks is to exit the whole Select* as soon as one is hit, otherwise it would fall through to the next cases afterwards, similar to a switch with fall-through in most programming languages.
Defaults look like they can go anywhere, and they're probably available in SelectOnes because the two share pretty much the exact same implementation (e.g. they both use the Select flow type). The intention is probably to put them at the end of SelectMany statements, to catch cases where users selected nothing.
Also, don't put too much stock on the wizards in the tests directory - the most complex wizards are missing from there, since almost all of them are under copyright.

@Infernio
Copy link
Member

Calling the Technical Readme a specification is a bit of a strong term - plus the parser itself doesn't obey it. For example, it allows this:

hw = "Hello World".len

... even though the Technical Readme claims that function calls need parentheses.

@Holt59
Copy link
Author

Holt59 commented Oct 27, 2020

The parser code isn't exactly nice to navigate and it's very, very loose with what it allows. I think the intent of Breaks is to exit the whole Select* as soon as one is hit, otherwise it would fall through to the next cases afterwards, similar to a switch with fall-through in most programming languages.

Well, if this was the case, then the SelectMany would not work I guess? Since there are Break in SelectMany. I'll check with the old parser if I have time.

Also, don't put too much stock on the wizards in the tests directory - the most complex wizards are missing from there, since almost all of them are under copyright.

Even if you don't put the wizards in the repository, could you put links to them somewhere (if these are available online) so that we can check them?

@Infernio
Copy link
Member

Infernio commented Oct 27, 2020

Even if you don't put the wizards in the repository, could you put links to them somewhere (if these are available online) so that we can check them?

Added that to the TODO list in #1

Edit: Beermotor also posted a link to a bunch of his wizards here. I haven't gotten around to adding them to the test suite, but none of them seem to use 'Default` either.

@Holt59
Copy link
Author

Holt59 commented Oct 28, 2020

Thanks for the extra wizards, I'll try them when I can.

I checked a few things with the current release of Wrye Bash and

  • The Break statement is not mandatory and will perform fallthrough, both in SelectOne and SelectMany.
  • The Default statement is used when nothing is selected in SelectMany, and if the selected option does not have a Case statement in SelectOne.

Also, I noticed that the Case label can be written without quotes... I know you are fixing bad stuff in this parser, I am not sure if this was intentional. If so, maybe indicates this somewhere?

@Infernio
Copy link
Member

Also, I noticed that the Case label can be written without quotes... I know you are fixing bad stuff in this parser, I am not sure if this was intentional. If so, maybe indicates this somewhere?

What exactly do you mean by that? Something like this?

Case foo
    Note 'bar'
    Break

I'm not sure how the current parser handles this - does it read this as one of its weird bare strings? And what if foo is defined as a variable beforehand, i.e. would this work?

foo = "Option 1"
; ...
Case foo
    Note 'bar'
    Break

The bare strings thing is definitely a bad idea and should already not work anymore, all those bare strings will now get parsed as identifiers instead. The question here is then only if we want to allow the new parser to use variables for Cases, or if I should restrict it to only literal strings.

@Holt59
Copy link
Author

Holt59 commented Oct 28, 2020

Both works actually...

Case foo
    Note 'bar'
    Break

If there is a foo variable, then the content of the foo variable will be substituted, but if there is no variable foo, the literal "foo" is considered.

Also, the Default in SelectOne is actually used if the user select nothing... I am not familiar with Wrye Bash so I assumed SelectOne meant "Select exactly one", but it seems to be "Select at most one"?

@Infernio
Copy link
Member

I think SelectOne is at least supposed to be exactly one. It's possible that it's bugged, of course.

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