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 comments to ProcLang spec #65

Open
jacobdeery opened this issue Sep 20, 2020 · 4 comments
Open

Add comments to ProcLang spec #65

jacobdeery opened this issue Sep 20, 2020 · 4 comments
Labels
enhancement New feature or request good first issue Good for newcomers procedures Affects procedures or ProcLang code

Comments

@jacobdeery
Copy link
Contributor

Currently, there's no way to comment a ProcLang document, which means we can't add any rationale for any of the decisions we make. It would be nice if we could do something like this:

main:
    1. set series_fill_valve to closed
    2. [5s] set supply_valve to open
        - [p1 < 600] abort_1.1  # if the pressure is too low we need to wait for the tank to heat up
        - [p1 > 1000] abort_2.1
    3. set series_fill_valve to open

In the above snippet, we've added a comment to the first deviation from step 2, explaining why we need to abort.

In order to implement this, we need to add a "comment" rule to the grammar specified in topside/procedures/proclang.py. This might require a bit more work than some of the other rules, because theoretically we would like to be able to add a comment anywhere in a proclang string. Perhaps we will need to drop the %ignore WS directive?

@jacobdeery jacobdeery added enhancement New feature or request procedures Affects procedures or ProcLang code labels Sep 20, 2020
@akmorrison
Copy link
Member

It's very common to have comments be removed by the lexer, not the parser, because adding in the option to have comments anywhere in the program text makes the grammar really complicated.

Lark lets you do this by specifying a terminal to be ignored with the %ignore keyword, and python style comments are the example that they use in the docs: https://lark-parser.readthedocs.io/en/latest/grammar.html?highlight=comment#ignore

TL;DR, add the following two lines somewhere in the grammar spec and it should just work:

COMMENT: "#" /[^\n]/*
%ignore COMMENT

@jacobdeery
Copy link
Contributor Author

Cool, that's simpler than I expected. I'll flag this one as a good first issue.

@jacobdeery jacobdeery added the good first issue Good for newcomers label Sep 20, 2020
@akmorrison
Copy link
Member

Can it be my first issue?

@jacobdeery
Copy link
Contributor Author

If the fix is this straightforward I would prefer to leave it for a new member. It's a good issue for someone new to knock off and will let them learn about unit testing if they're not already familiar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers procedures Affects procedures or ProcLang code
Projects
None yet
Development

No branches or pull requests

2 participants