Skip to content

Commit

Permalink
Do not overwrite options.comments + cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
@avdg @rvanvelzen
avdg authored and rvanvelzen committed Nov 29, 2016
1 parent 057de57 commit 79b98a9
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 52 deletions.
24 changes: 12 additions & 12 deletions lib/compress.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand All @@ -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";
});
Expand Down Expand Up @@ -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");
Expand All @@ -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)
Expand Down
77 changes: 38 additions & 39 deletions lib/output.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, {
Expand Down Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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 },
Expand Down Expand Up @@ -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 &&
Expand Down
2 changes: 2 additions & 0 deletions lib/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
12 changes: 11 additions & 1 deletion test/mocha/comment-filter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
});
Expand Down Expand Up @@ -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(){}");
});
});

0 comments on commit 79b98a9

Please sign in to comment.