Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade outdated dependency to commander #3743

Closed
guimard opened this issue Mar 5, 2020 · 5 comments
Closed

Upgrade outdated dependency to commander #3743

guimard opened this issue Mar 5, 2020 · 5 comments

Comments

@guimard
Copy link

guimard commented Mar 5, 2020

wishlist

Uglify version (uglifyjs -V) ≥ 2.8.29

Hi,

here is a simple patch that permits to use uglifyjs without behavior change with commander > 4 (tests succeeds). Change that affects uglifyjs is explained here.

  • option with optional argument : replace -cme by -c -m -e (else it becomes -c me)
--- a/bin/uglifyjs
+++ b/bin/uglifyjs
@@ -5,6 +5,15 @@

 require("../tools/exit");

+var argv = [];
+process.argv.forEach(function(arg){
+  if(arg.match(/^-([cebmo]+)$/)) {
+    argv = argv.concat(RegExp.$1.split('').map(s => { return '-'+s }));
+  }
+  else argv.push(arg);
+});
+process.argv = argv;
+
 var fs = require("fs");
 var info = require("../package.json");
 var path = require("path");
  • --rename default value (change order) :
--- a/bin/uglifyjs
+++ b/bin/uglifyjs
@@ -44,8 +53,8 @@
 program.option("--ie8", "Support non-standard Internet Explorer 8.");
 program.option("--keep-fnames", "Do not mangle/drop function names. Useful for code relying on Function.prototype.name.");
 program.option("--name-cache <file>", "File to hold mangled name mappings.");
-program.option("--rename", "Force symbol expansion.");
 program.option("--no-rename", "Disable symbol expansion.");
+program.option("--rename", "Force symbol expansion.");
 program.option("--self", "Build UglifyJS as a library (implies --wrap UglifyJS)");
 program.option("--source-map [options]", "Enable source map/specify source map options.", parse_js());
 program.option("--timings", "Display operations run time on STDERR.");
@alexlamsl
Copy link
Collaborator

Thanks for the suggestion, though there is a different reason why we decided to stay on the 2.x series.

But if we do need to upgrade in the future, your patch would come in handy 👍

@jonassmedegaard
Copy link

@alexlamsl could you elaborate/reference the reasoning for staying with the 2.x series?

Reason I ask is that Debian currently carries only one version of the commander module (to limit what the distribution need to care about), and we now consider moving to 4.x but perhaps you have some insight as to why we maybe shouldn't - or why we perhaps should consider maintaining 2 versions concurrently.

@jonassmedegaard
Copy link

ah, I guess the reason is compatibility with Nodejs v0.10 as indicated in #3450 - we do not have that need for Debian, but I admire your strong focus on backwards compatibity!

Please do share if you are aware of other reasons to stay with commander v2.0.

@alexlamsl
Copy link
Collaborator

Yes that is indeed the reason − at some point we could internalise the command line functionality (just as we did for source-map) since we use a very limited subset of what commander provides. Would that alleviate your concern?

@jonassmedegaard
Copy link

jonassmedegaard commented Jun 5, 2020 via email

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

No branches or pull requests

3 participants