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

Parser edits #32

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Parser edits #32

wants to merge 3 commits into from

Conversation

Seth-Harlaar
Copy link
Contributor

Updated the way the parser parses strings and handles errors.
Major changes:

  • parser now separates string at new lines and parses each line separately
  • error message indicating where the error was in the line
  • only lines that have an error in them are discarded, the rest of the recipe will parse

@Seth-Harlaar
Copy link
Contributor Author

Seth-Harlaar commented Apr 1, 2022

Things I still have to change before merging:

  • make parser separate lines by all acceptable new line characters
  • - update error message to show user friendly message indicating error
  • - update line counter for error message to properly indicate which line message has an error
  • - remove testing printing messages

@Seth-Harlaar
Copy link
Contributor Author

So far the error message indicates what token trips up the parser. It uses the token definitions from the Cooklang.y file and prints out the token. To me the token names seem pretty clear as to what they represent, but I'm wondering if anyone thinks they should be updated to be more clear. You can look at them here: https://github.com/Seth-Harlaar/cooklang-c/blob/a29c84a6ebc43170e7f562d694367bb2e1c81e6d/src/Cooklang.y#L40

@dubadub
Copy link
Member

dubadub commented Apr 2, 2022

Token names are pretty clear, indeed. Could you please share an error message example?

@Seth-Harlaar
Copy link
Contributor Author

Seth-Harlaar commented Apr 2, 2022

Sure!
Let's say the user forgot the left curly bracket in the example input:

Put @ingredient 4} in pot.

The output error message would be the following 2 lines:
Error, syntax error, expected LCURL.
Error at column 16 on line 1.

@Net-Mist
Copy link
Collaborator

Net-Mist commented Apr 3, 2022

Hello !
If the PR is not ready to be merged, you can convert if to "draft PR", so it is clear for everyone

@Net-Mist
Copy link
Collaborator

Net-Mist commented Apr 3, 2022

also I don't understand the changes in cooklang.l file. We don't want to support UTF-8 anymore ?

@Seth-Harlaar Seth-Harlaar marked this pull request as draft April 4, 2022 12:45
@Seth-Harlaar
Copy link
Contributor Author

Oh, thanks for picking that up, I was making some changes to the parser and removed the UTF-8 to make testing easier. I will revert the changes.

@Net-Mist
Copy link
Collaborator

Net-Mist commented Apr 4, 2022

Oh ! Ok !

@Net-Mist
Copy link
Collaborator

Net-Mist commented Apr 5, 2022

Hello !
Maybe you are aware of this, but the make parser comment doesn't work. Both on main branch and in this PR. As you're working on it, maybe if you fix it you could then add a github action to check that it doesn't break in the future

@Net-Mist
Copy link
Collaborator

Net-Mist commented Apr 5, 2022

parser now separates string at new lines and parses each line separately

How do you plan do manage to parse multi-line block comments ?

@Seth-Harlaar
Copy link
Contributor Author

Was not aware of make parser not working. It seemed to be working perfectly fine on my system. I will look into it.

Maybe in the parseMultipleLineString function we can check a return value from the parser and see if there is an opening and no closing to a block comment. From there we can tell the parser not to parse anything until it has detected a closing tag.

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

Successfully merging this pull request may close these issues.

3 participants