Fix simple errors in regex

ember-cli / broccoli-asset-rewrite

Broccoli plugin to rewrite a source tree from an asset map.

MIT License

10 stars 53 forks source link

Fix simple errors in regex #45

Closed pwfisher closed 8 years ago

pwfisher commented 8 years ago

remove backslash from character classes where it was unintentionally included (per comments)
remove unnecessary character class defined for a single character
remove {1} because it has no effect

codecov-io commented 8 years ago

Current coverage is 100% (diff: 100%)

Merging #45 into master will not change coverage

@@           master   #45   diff @@
===================================
  Files           1     1          
  Lines          84    84          
  Methods        12    12          
  Messages        0     0          
  Branches       19    19          
===================================
  Hits           84    84          
  Misses          0     0          
  Partials        0     0

Powered by Codecov. Last update bf3c323...a8e6502

pwfisher commented 8 years ago

In my app, this change appears to have significantly improved performance. (Like 30-50% for JS files.)

I attribute it primarily to the more restrictive matching for the very first symbol in the pattern (no longer processing a potential match for each backslash).

stefanpenner commented 8 years ago

can you add tests?

rickharrison commented 8 years ago

Why would you say it was unintentionally included? Although, I can not recall why, I would not have put \\ there without a reason.

pwfisher commented 8 years ago

I say unintentional because the comments describe the intended behavior and it makes no sense to repeat the same character several times within a character class. You must have thought that characters which need escaping outside of a character class also needed escaping inside a character class. It's a very understandable mistake.

Superseded by #46, which includes a failing test for JS processing performance in a real-world scenario.