This repository has been archived by the owner on Jul 29, 2019. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Network: DRY code in Label for parsing markup #3565
Merged
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
This gets rid of a major eyesore for me. The accumulator object was identical for HTML and Markdown. In addition, the parsing has been refactored. Common elements have been DRY'd and the logic of the parsing has been made more comprehensible.
Suggest pre-compiling regexes as per performance tip on https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions See attached patch. |
OK. Went with this and expanded on it. Also code changes to make it work nice, some code cleanup. This is starting to look a lot like memoization. |
BTW, superfluously, long live unit tests. With the markup testing already in place, changing this was with 100% reliability. |
I'm refraining from further refactoring. I still see some good things to do here, some other time. |
Any other good ideas? Looking forward to your review. |
macleodbroad-wf
approved these changes
Oct 15, 2017
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice refactor! Much more readable.
mojoaxel
pushed a commit
to visjs/vis_original
that referenced
this pull request
Jun 9, 2019
* Network: DRY code in Label for parsing markup This gets rid of a major eyesore for me. The accumulator object was identical for HTML and Markdown. In addition, the parsing has been refactored. Common elements have been DRY'd and the logic of the parsing has been made more comprehensible. * Added suggestion @mbroad wrt regexp precompile * Fixed linting
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This gets rid of a major eyesore for me. The accumulator object was identical for HTML and Markdown.
In addition, the parsing has been refactored. Common elements have been DRY'd and the logic of the parsing has been made more comprehensible.
Unit tests for parsing HTML tags and Markdown are already present. These were used to verify that this refactoring is good.