googlearchive / polyup

A helpful assistant for migrating from Polymer v0.5 to 1.0. Does many of the boring mechanical parts for you.
http://polymerlabs.github.io/polyup/
BSD 3-Clause "New" or "Revised" License
38 stars 9 forks source link

polyup removes backslashes from regexp string. #69

Closed davidmaxwaterman closed 9 years ago

davidmaxwaterman commented 9 years ago

Polyup makes a complete mess of comments, but they're still comments and so are quite inert.

However, I consider the following change to be more worrisome - it seems that polyup has removed backslashes from regular expressions, which surely completely changes their effect :

$ git diff master cp-formatter.js 
diff --git a/example-app/app/content_push/cp-formatter.js b/example-app/app/content_push/cp-formatter.js
index 916c4b2..b3667b7 100644
--- a/example-app/app/content_push/cp-formatter.js
+++ b/example-app/app/content_push/cp-formatter.js
@@ -27,10 +27,10 @@

 (function () {
   /* regexp for HTML tags */
-  var HTML_TAG_REGEX = new RegExp('<\/?[^<>]+?\/?>', 'gm');
+  var HTML_TAG_REGEX = new RegExp('</?[^<>]+?/?>', 'gm');

   /* regexp to retrieve the domain from a URL */
-  var DOMAIN_REGEX = new RegExp('http[s]?:\/\/([^\/]+)\/');
+  var DOMAIN_REGEX = new RegExp('http[s]?://([^/]+)/');

   var CpFormatter = {
     /* clean HTML tags out of text */

Any thoughts on this? Perhaps there is some validity to removing them that I'm not seeing...

rictic commented 9 years ago

Those slashes are safe to remove because they're in strings, not RegExp literals. In a string you can backslash-escape any ascii character and it will give that character. E.g.

'\/'.length === 1
'/' === '\/'
'<\/?[^<>]+?\/?>' === '</?[^<>]+?/?>'  // true!