closure-stylesheets icon indicating copy to clipboard operation
closure-stylesheets copied to clipboard

Adds support for --allow-duplicate-declarations flag

Open BradMclain opened this issue 9 years ago • 8 comments

Adds support for flag --allow-duplicate-declarations.

Please note was originally implmented by @amralassal here https://github.com/google/closure-stylesheets/pull/75 however I needed this feature ASAP so implemented myself.

BradMclain avatar Mar 17 '16 04:03 BradMclain

I'm going to pull this change internally to verify that it doesn't break anything.

iflan avatar Mar 17 '16 08:03 iflan

So, I pulled this change to get it reviewed internally and there were some concerns:

  • It looks like that with this change, setting eliminateDeadStyles to false no longer allows duplicate declarations unless you turn that on as well. This may break existing users.
  • It's unclear whether the combination of eliminateDeadStyles and allowDuplicateDeclarations actually works as intended.

Unfortunately, I don't have time to address these myself if you want it done right away. But if you could, I'd be happy to review the changes.

I think the first thing to do is to create a test for setting eliminateDeadStyles and allowDuplicateDeclarations both to true. You could add it to ClosureCommandLineCompilerTest.java.

If that works, then I'd like the logic for eliminateDeadStyles to default to disallowing duplicate declarations unless allowDuplicateDeclarations is on. Could you do that?

Thanks a bunch!

iflan avatar Mar 17 '16 17:03 iflan

I will try and make these changes when I get a chance.

BradMclain avatar Mar 17 '16 23:03 BradMclain

I'd like this as well, see https://github.com/google/closure-stylesheets/issues/89, is there anyway that I can help to finish this work?

pspeter3 avatar Jul 18 '16 19:07 pspeter3

I can add you to our repo and you can pick up from where I finished. I have been too busy to do anymore on this recently.

Alternatively if you just need it working for a project you can checkout the branch and compile your own version of the compiler to use.

https://github.com/anomaly/closure-stylesheets

BradMclain avatar Jul 19 '16 04:07 BradMclain

I will likely create a fork for Asana and do the work necessary there. I just want to make sure that this will get merged if done. Thanks for the advice!

pspeter3 avatar Jul 19 '16 05:07 pspeter3

Since there has been no further progress on #90 (presumably due to missing CLA) I went ahead and added the test from that PR into this one.

@iflan Do the issues you outlined previously still need addressing?

BradMclain avatar Oct 04 '17 05:10 BradMclain

Will this get merged at some point? Thanks for the work @BradMclain

jorgedavila25 avatar Mar 09 '18 19:03 jorgedavila25