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 tail rec #175

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Parser tail rec #175

wants to merge 8 commits into from

Conversation

ThunderMikey
Copy link
Collaborator

Make parser tail recursive in order to parse longer Markdown files.

@ThunderMikey
Copy link
Collaborator Author

I tested the JS in VS-Code, but when I tried to preview the example.fmark, it showed "resource is not available".

fmark-resource_not_available

@ThunderMikey ThunderMikey added the help wanted Extra attention is needed label Apr 14, 2018
@ps-george
Copy link
Collaborator

It's a problem with file includes, think it is windows only? It changes every time they update Vs code :(

@ThunderMikey
Copy link
Collaborator Author

What should we doooooo?

@ps-george
Copy link
Collaborator

For testing your PR you can see if fmark.example works without a file include! And we'll fix that issue later :(

@ThunderMikey
Copy link
Collaborator Author

No luck. Even with only one line of normal text.

@ymherklotz
Copy link
Member

Hmm, did you actually managed to make it tail recursive though? I can test it out later if it works on my linux

@ThunderMikey
Copy link
Collaborator Author

I tried it on Linux, the debug console said js-beauty not found.

@ymherklotz
Copy link
Member

ymherklotz commented Apr 16, 2018

Ahh ok, yeah, when testing you have to use yarn to update and install the missing packages. I think yarn install at the base of the repo should do. Then you'll be able to test it.

This is in the VSCode repo though.

@ThunderMikey
Copy link
Collaborator Author

I did yarn install, as the result, no error showed up in the console this time. However, the FMark Preview stays blank no matter what I do in a .fmark file.

@ymherklotz
Copy link
Member

Did you check the debug log? Were there still no errors there? That is very weird. I ll have a look at it as soon as I have time, which should be end of next week.

@ps-george
Copy link
Collaborator

@ThunderMikey parser is now tail recursive? nice

@ThunderMikey
Copy link
Collaborator Author

Yeah, just don't know whether the other issues has been solved or not.

@ps-george
Copy link
Collaborator

ps-george commented Sep 4, 2019

Did we get anywhere with figuring out the bug with this? Does the master branch work in its current state?

@ps-george ps-george reopened this Sep 4, 2019
@ps-george ps-george mentioned this pull request Sep 4, 2019
6 tasks
Copy link
Member

@ymherklotz ymherklotz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to add a test that parses a large markdown file, like 10000 lines, and hopefully completes under 1sec and does not crash.

#180

Copy link
Collaborator

@ps-george ps-george left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ThunderMikey, you removed a lot of |> Ok, result.map, & result.bind/makeOk in the code.

Could you explain why? Did you replace with an equivalent, or are results no longer needed because the functions "won't fail" / don't have a fail condition?

@ThunderMikey
Copy link
Collaborator Author

@ps-george If I recall correctly, I did not want the function to fail, at the very least, fmark will spit something even though the text is malformatted.

We can probably improve this by having different states (error states will be used to inform the user) while spitting the text.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants