closure-stylesheets
closure-stylesheets copied to clipboard
Adds support for --allow-duplicate-declarations flag
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.
I'm going to pull this change internally to verify that it doesn't break anything.
So, I pulled this change to get it reviewed internally and there were some concerns:
- It looks like that with this change, setting
eliminateDeadStylestofalseno longer allows duplicate declarations unless you turn that on as well. This may break existing users. - It's unclear whether the combination of
eliminateDeadStylesandallowDuplicateDeclarationsactually 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!
I will try and make these changes when I get a chance.
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?
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
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!
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?
Will this get merged at some point? Thanks for the work @BradMclain