rewrite parsing of auspiceMainDisplayMarkdown; #1189
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
use string.split instead of regex, which was causing browser to hang (possibly due to https://www.regular-expressions.info/catastrophic.html) despite technically valid paragraph missing both the opening string, "```auspiceMainDisplayMarkdown", and the closing string, "```".
|
Thanks @eharkins -- tested & confirm that it fixes the problematic narrative with no obvious changes to the others i've tested on. I enjoyed learning more about regex's through this! Going forward, I consider it a high priority to merge #1172 which I'll endeavor to do in the next day or so. If you wouldn't mind checking that out with the knowledge you gained from this PR, that would be wonderful! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See https://bedfordlab.slack.com/archives/C116R9PR7/p1595292741008800
This uses string.split instead of regex, which was causing browser to hang (possibly due to https://www.regular-expressions.info/catastrophic.html) despite technically valid paragraph missing both the opening string, "```auspiceMainDisplayMarkdown", and the closing string, "```".
There are many other regexes in the narrative parsing function so this doesn't necessarily prevent the same or similar issues in the future. Maybe there is a way to wrap the whole function to handle any hanging regexes.
Testing
Just tested that the narratives in the auspice repo are correctly parsed and that the browser doesn't hang for the example narrative from slack.