Skip to content

Conversation

CheloXL
Copy link

@CheloXL CheloXL commented Feb 9, 2021

Fixes #28. The original RegExp split method was not working as it needs to properly escape rx delimiters, etc.
Added RX escape method and replaced the old string.split with the new RX split.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 81

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 56 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-74.9%) to 3.175%

Files with Coverage Reduction New Missed Lines %
index.ts 1 50.0%
src/minifyHTMLLiterals.ts 55 2.42%
Totals Coverage Status
Change from base Build 77: -74.9%
Covered Lines: 4
Relevant Lines: 64

💛 - Coveralls

@asyncliz
Copy link
Owner

Thanks for the PR! It looks like I had originally investigated this but never finished the implementation.

The problem with my original idea (just passing the string to RegExp to make the last semicolon optional) as you found out is that the parens are special characters and are not escaped. While this approach works, I'd like to try and stick with simple string splitting, rather than introduce any new potential complexity with the RegExp class (especially if others are overriding the getPlaceholder method).

I've pushed a fix to the main branch and will release an update shortly. Thanks again for the report!

@asyncliz asyncliz closed this Feb 12, 2021
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.

Error: splitHTMLByPlaceholder() must return same number of strings as template parts

3 participants