[Pkg-javascript-commits] [uglifyjs] 38/49: Do not overwrite options.comments + cleanup

Jonas Smedegaard dr at jones.dk
Fri Dec 9 11:43:32 UTC 2016


This is an automated email from the git hooks/post-receive script.

js pushed a commit to branch master
in repository uglifyjs.

commit 79b98a9fe87f950607c601a45a3566a46c32f425
Author: Anthony Van de Gejuchte <anthonyvdgent at gmail.com>
Date:   Wed Oct 26 17:34:30 2016 +0200

    Do not overwrite options.comments + cleanup
---
 lib/compress.js              | 24 +++++++-------
 lib/output.js                | 77 ++++++++++++++++++++++----------------------
 lib/utils.js                 |  2 ++
 test/mocha/comment-filter.js | 12 ++++++-
 4 files changed, 63 insertions(+), 52 deletions(-)

diff --git a/lib/compress.js b/lib/compress.js
index 1845717..5879b93 100644
--- a/lib/compress.js
+++ b/lib/compress.js
@@ -916,7 +916,7 @@ merge(Compressor.prototype, {
     (function (def){
         var unary_bool = [ "!", "delete" ];
         var binary_bool = [ "in", "instanceof", "==", "!=", "===", "!==", "<", "<=", ">=", ">" ];
-        def(AST_Node, function(){ return false });
+        def(AST_Node, return_false);
         def(AST_UnaryPrefix, function(){
             return member(this.operator, unary_bool);
         });
@@ -934,16 +934,16 @@ merge(Compressor.prototype, {
         def(AST_Seq, function(){
             return this.cdr.is_boolean();
         });
-        def(AST_True, function(){ return true });
-        def(AST_False, function(){ return true });
+        def(AST_True, return_true);
+        def(AST_False, return_true);
     })(function(node, func){
         node.DEFMETHOD("is_boolean", func);
     });
 
     // methods to determine if an expression has a string result type
     (function (def){
-        def(AST_Node, function(){ return false });
-        def(AST_String, function(){ return true });
+        def(AST_Node, return_false);
+        def(AST_String, return_true);
         def(AST_UnaryPrefix, function(){
             return this.operator == "typeof";
         });
@@ -1197,11 +1197,11 @@ merge(Compressor.prototype, {
 
     // determine if expression has side effects
     (function(def){
-        def(AST_Node, function(compressor){ return true });
+        def(AST_Node, return_true);
 
-        def(AST_EmptyStatement, function(compressor){ return false });
-        def(AST_Constant, function(compressor){ return false });
-        def(AST_This, function(compressor){ return false });
+        def(AST_EmptyStatement, return_false);
+        def(AST_Constant, return_false);
+        def(AST_This, return_false);
 
         def(AST_Call, function(compressor){
             var pure = compressor.option("pure_funcs");
@@ -1221,13 +1221,13 @@ merge(Compressor.prototype, {
         def(AST_SimpleStatement, function(compressor){
             return this.body.has_side_effects(compressor);
         });
-        def(AST_Defun, function(compressor){ return true });
-        def(AST_Function, function(compressor){ return false });
+        def(AST_Defun, return_true);
+        def(AST_Function, return_false);
         def(AST_Binary, function(compressor){
             return this.left.has_side_effects(compressor)
                 || this.right.has_side_effects(compressor);
         });
-        def(AST_Assign, function(compressor){ return true });
+        def(AST_Assign, return_true);
         def(AST_Conditional, function(compressor){
             return this.condition.has_side_effects(compressor)
                 || this.consequent.has_side_effects(compressor)
diff --git a/lib/output.js b/lib/output.js
index 63a78c6..50e5aa4 100644
--- a/lib/output.js
+++ b/lib/output.js
@@ -45,6 +45,20 @@
 
 var EXPECT_DIRECTIVE = /^$|[;{][\s\n]*$/;
 
+function is_some_comments(comment) {
+    var text = comment.value;
+    var type = comment.type;
+    if (type == "comment2") {
+        // multiline comment
+        return /@preserve|@license|@cc_on/i.test(text);
+    }
+    return type == "comment5";
+}
+
+function is_comment5(comment) {
+    return comment.type == "comment5";
+}
+
 function OutputStream(options) {
 
     options = defaults(options, {
@@ -72,46 +86,30 @@ function OutputStream(options) {
     }, true);
 
     // Convert comment option to RegExp if neccessary and set up comments filter
-    if (typeof options.comments === "string" && /^\/.*\/[a-zA-Z]*$/.test(options.comments)) {
-        var regex_pos = options.comments.lastIndexOf("/");
-        options.comments = new RegExp(
-            options.comments.substr(1, regex_pos - 1),
-            options.comments.substr(regex_pos + 1)
-        );
-    }
-    if (options.comments instanceof RegExp) {
-        options.comments = (function(f) {
-            return function(comment) {
-                return comment.type == "comment5" || f.test(comment.value);
-            }
-        })(options.comments);
-    }
-    else if (typeof options.comments === "function") {
-        options.comments = (function(f) {
-            return function(comment) {
-                return comment.type == "comment5" || f(this, comment);
-            }
-        })(options.comments);
-    }
-    else if (options.comments === "some") {
-        options.comments = function(comment) {
-            var text = comment.value;
-            var type = comment.type;
-            if (type == "comment2") {
-                // multiline comment
-                return /@preserve|@license|@cc_on/i.test(text);
-            }
-            return type == "comment5";
+    var comment_filter = options.shebang ? is_comment5 : return_false; // Default case, throw all comments away except shebangs
+    if (options.comments) {
+        var comments = options.comments;
+        if (typeof options.comments === "string" && /^\/.*\/[a-zA-Z]*$/.test(options.comments)) {
+            var regex_pos = options.comments.lastIndexOf("/");
+            comments = new RegExp(
+                options.comments.substr(1, regex_pos - 1),
+                options.comments.substr(regex_pos + 1)
+            );
         }
-    }
-    else if (options.comments){ // NOTE includes "all" option
-        options.comments = function() {
-            return true;
+        if (comments instanceof RegExp) {
+            comment_filter = function(comment) {
+                return comment.type == "comment5" || comments.test(comment.value);
+            };
+        }
+        else if (typeof comments === "function") {
+            comment_filter = function(comment) {
+                return comment.type == "comment5" || comments(this, comment);
+            };
         }
-    } else {
-        // Falsy case, so reject all comments, except shebangs
-        options.comments = function(comment) {
-            return comment.type == "comment5";
+        else if (comments === "some") {
+            comment_filter = is_some_comments;
+        } else { // NOTE includes "all" option
+            comment_filter = return_true;
         }
     }
 
@@ -421,6 +419,7 @@ function OutputStream(options) {
         with_square     : with_square,
         add_mapping     : add_mapping,
         option          : function(opt) { return options[opt] },
+        comment_filter  : comment_filter,
         line            : function() { return current_line },
         col             : function() { return current_col },
         pos             : function() { return current_pos },
@@ -503,7 +502,7 @@ function OutputStream(options) {
                 }));
             }
 
-            comments = comments.filter(output.option("comments"), self);
+            comments = comments.filter(output.comment_filter, self);
 
             // Keep single line comments after nlb, after nlb
             if (!output.option("beautify") && comments.length > 0 &&
diff --git a/lib/utils.js b/lib/utils.js
index 8ef6193..d0a21ac 100644
--- a/lib/utils.js
+++ b/lib/utils.js
@@ -112,6 +112,8 @@ function merge(obj, ext) {
 };
 
 function noop() {};
+function return_false() { return false; }
+function return_true() { return true; }
 
 var MAP = (function(){
     function MAP(a, f, backwards) {
diff --git a/test/mocha/comment-filter.js b/test/mocha/comment-filter.js
index 7916275..01580c8 100644
--- a/test/mocha/comment-filter.js
+++ b/test/mocha/comment-filter.js
@@ -2,7 +2,7 @@ var UglifyJS = require('../../');
 var assert = require("assert");
 
 describe("comment filters", function() {
-    it("Should be able to filter comments by passing regex", function() {
+    it("Should be able to filter comments by passing regexp", function() {
         var ast = UglifyJS.parse("/*!test1*/\n/*test2*/\n//!test3\n//test4\n<!--test5\n<!--!test6\n-->test7\n-->!test8");
         assert.strictEqual(ast.print_to_string({comments: /^!/}), "/*!test1*/\n//!test3\n//!test6\n//!test8\n");
     });
@@ -62,4 +62,14 @@ describe("comment filters", function() {
         var ast = UglifyJS.parse("#!foo\n//foo\n/*@preserve*/\n/* please hide me */");
         assert.strictEqual(ast.print_to_string({comments: "some"}), "#!foo\n/*@preserve*/\n");
     });
+
+    it("Should have no problem on multiple calls", function() {
+        const options = {
+            comments: /ok/
+        };
+
+        assert.strictEqual(UglifyJS.parse("/* ok */ function a(){}").print_to_string(options), "/* ok */function a(){}");
+        assert.strictEqual(UglifyJS.parse("/* ok */ function a(){}").print_to_string(options), "/* ok */function a(){}");
+        assert.strictEqual(UglifyJS.parse("/* ok */ function a(){}").print_to_string(options), "/* ok */function a(){}");
+    });
 });

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-javascript/uglifyjs.git



More information about the Pkg-javascript-commits mailing list