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

Before running the sketch, check if all references are valid #17

Open
fdb opened this issue Feb 19, 2018 · 11 comments
Open

Before running the sketch, check if all references are valid #17

fdb opened this issue Feb 19, 2018 · 11 comments

Comments

@fdb
Copy link
Member

fdb commented Feb 19, 2018

Currently resolving references happens at runtime, which means sketches like this only fail 50% of the time:

root:
- This line is fine
- This line contains an invalid {{ reference }}

To avoid these errors happening at runtime I suggest creating a validation step in parsePhraseBook that checks if all references in the script are valid.

@kunal-mohta
Copy link
Contributor

This would require another loop to be run after the already running loop, because once the first loop is completed then only we can determine whether a reference is defined or not.
If I am going in the right direction, can I try this?

@fdb
Copy link
Member Author

fdb commented Feb 20, 2018

Actually we just need to check all references we encounter during parsing. We don't need to run the code "normally" at all.

Give it a try if you want!

@kunal-mohta
Copy link
Contributor

Okay, I will give it a try.

@kunal-mohta
Copy link
Contributor

Wait, I don't think I got this right.
Say for example, we have

root:
- This is {{ reference1 }} //(*)
- This is {{ reference2 }} //(**)

//(***)
reference1:
- Some stuff

reference3:
- Some stuff

How will we know at line (*) and (**) that reference1 and reference2 are defined after (***) or not, without running the code "normally"

@stebanos
Copy link
Member

stebanos commented Feb 21, 2018 via email

@kunal-mohta
Copy link
Contributor

Oh! Okay. I thought you were planning to implement this inside parsePharsebook() function.

@kunal-mohta
Copy link
Contributor

Should the check be included here

@fdb
Copy link
Member Author

fdb commented Feb 22, 2018

No idea. @stebanos ?

@stebanos
Copy link
Member

stebanos commented Feb 22, 2018

No, it should be checked after everything is parsed, before returning the phraseBook, so yes, inside the parsePhrasebook function, but at the end. Because it is only at this point we know for sure what all the phrases and references are.

@kunal-mohta
Copy link
Contributor

Won't that mean rewriting the code that extracts the references from {{ ... }}, that already has been written?

@stebanos
Copy link
Member

No, the check needs to perform a similar action as what happens in the Interpreter class. For each phrase, the Parser class returns a tree of nodes of a certain type, some of these nodes are of type NODE_KEY, it's these ones that need to be checked against the whole of the phraseBook. Since these nodes are "buried" within the tree, the tree needs to be traversed, like what happens in the visit(node) function.

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

3 participants