separate keep_classnames & keep_fnames

[alexlamsl]

fixes #2418

[kzc]

This one is a bit odd:

$ cat names.js 
class C0 {}
function F0() {}
console.log(class C1{}, class C2{}, function F1(){}, function F2(){});

$ cat names.js | bin/uglifyjs -mc --toplevel
console.log(class c{},class c{},function(){},function(){});

I would have expected the same output as:

$ cat names.js | bin/uglifyjs -mc keep_classnames=0 --toplevel
console.log(class{},class{},function(){},function(){});

[kzc]

I see what's going on - this should default to false.

[kzc]

I bet that extra character is annoying you to no end. :-)

[alexlamsl]

Thanks for catching this - I waited till just before bedtime to make this PR to alleviate my pain.

[kzc]

I'll make a doc PR after this is merged.

[kzc]

In another PR we could have keep_fnames and keep_classnames in mangle, compress and their corresponding top level minify options accept either a regex or an array of valid names as per #2416 (comment). This has been requested a few times over the years.

[kzc]

documented in #2511

[kzc]

Do we want some test/compress/ tests for this?

Using same code for all tests:

class C1 {}
function F1() {}
console.log(class C2{}, function F2(){});

[alexlamsl]

Done in https://github.com/mishoo/UglifyJS2/pull/2510/files#diff-7a02108b11616cdd07d36b9a138a71cbR1578

I use the subtlety of parentheses instead, just to make me feel better 😈

[kzc]

Cool. I don't know how I missed that.

issue_2418_1 can be removed - identical to the more explicit/descriptive issue_2418_2.

[kzc]

I use the subtlety of parentheses instead, just to make me feel better

Yeah - I go overboard with console.log in tests.

[alexlamsl]

issue_2418_1 is testing the default values from Compressor, to cover #2510 (comment)

[kzc]

Compressor default values are not applied in test/compress/ tests.

[alexlamsl]

They are if they are not associated with false_by_default (though that does bring up the question on whether keep_fnames should be false_by_default instead of false)

[kzc]

I see. false_by_default is only useful in testing at the moment?

The next release will be 3.2.0 because of this new option and hoist_props defaulting to true, I'm assuming?

[alexlamsl]

Yes ✖️ 2