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

Improve parsing logic for set_variable action types #371

Merged
merged 1 commit into from
Feb 23, 2022

Conversation

t-hale
Copy link
Contributor

@t-hale t-hale commented Jan 19, 2022

Fixes coinbase/mesh-cli#248

Motivation

To improve the parsing logic for set_variable action types

Solution

Before splitting the string to determine if the remaining line contains an arithmetic operator, we first check to see if the remaining line (sans ; suffix) is valid JSON

Open questions

Is there any possibility of arithmetic and JSON in the same line?

@t-hale t-hale changed the title fixes https://github.com/coinbase/rosetta-cli/issues/248 Improve parsing logic for set_variable action types Jan 19, 2022
@shrimalmadhur
Copy link
Contributor

shrimalmadhur commented Jan 19, 2022

Is there any possibility of arithmetic and JSON in the same line?

@t-hale Can you give me an example of what it will look like. I did not get this case?

@t-hale
Copy link
Contributor Author

t-hale commented Jan 20, 2022

Is there any possibility of arithmetic and JSON in the same line?

@t-hale Can you give me an example of what it will look like. I did not get this case?

I take it by your confusion that it's not possible. I'm pretty new to the Rosetta DSL, so I wasn't sure if there was a possibility of these two cases math and set_variable occurring on the same line (this case would make the new logic I added in this PR invalid)

@t-hale
Copy link
Contributor Author

t-hale commented Feb 20, 2022

@shrimalmadhur bumping this one back up

@shrimalmadhur
Copy link
Contributor

@t-hale sorry about this delay. I think there's not possibility of JSON and math op in same line. so this should be good. I see some build failures - Possibly due to stale code. Could you please rebase? Thank you and again apologies for the delays.

@t-hale
Copy link
Contributor Author

t-hale commented Feb 23, 2022

@t-hale sorry about this delay. I think there's not possibility of JSON and math op in same line. so this should be good. I see some build failures - Possibly due to stale code. Could you please rebase? Thank you and again apologies for the delays.

No worries - I've gone ahead and rebased

@shrimalmadhur shrimalmadhur merged commit 73e590a into coinbase:master Feb 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Rosetta Constructor DSL: error parsing a string containing a "-" character
2 participants