Skip to content
This repository has been archived by the owner on Jul 29, 2019. It is now read-only.

Network: DRY code in Label for parsing markup #3565

Merged
merged 4 commits into from
Oct 20, 2017

Conversation

wimrijnders
Copy link
Contributor

@wimrijnders wimrijnders commented Oct 13, 2017

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.

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.
@macleodbroad-wf
Copy link
Contributor

Suggest pre-compiling regexes as per performance tip on https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions

See attached patch.

precompile-regex.diff.zip

@wimrijnders
Copy link
Contributor Author

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.

@wimrijnders
Copy link
Contributor Author

BTW, superfluously, long live unit tests. With the markup testing already in place, changing this was with 100% reliability.

@wimrijnders
Copy link
Contributor Author

wimrijnders commented Oct 15, 2017

I'm refraining from further refactoring. I still see some good things to do here, some other time.

@wimrijnders
Copy link
Contributor Author

Any other good ideas? Looking forward to your review.

Copy link
Contributor

@macleodbroad-wf macleodbroad-wf left a 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.

@yotamberk yotamberk merged commit 7110549 into almende:develop Oct 20, 2017
@wimrijnders wimrijnders deleted the refactorMarkupParsing branch October 21, 2017 04:57
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.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants