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

Fixes license header and adds stable sort #1267

Closed
wants to merge 2 commits into from

Conversation

RReverser
Copy link
Contributor

For #1234.

@RReverser RReverser changed the title Fixes license header. Fixes license header and adds stable sort for #1234 Mar 19, 2014
@RReverser RReverser changed the title Fixes license header and adds stable sort for #1234 Fixes license header and adds stable sort Mar 19, 2014
@RReverser
Copy link
Contributor Author

@zpao @benjamn Ping.

@RReverser
Copy link
Contributor Author

Since there was no response from @zpao and others, this seems to be not relevant (most likely it even won't merge now), so I'm closing it.

@RReverser RReverser closed this May 9, 2014
@zpao
Copy link
Member

zpao commented May 9, 2014

Sorry for letting it hang. I may still come back to it, it's just been lower pri. I know @vjeux is still interested.

I was actually thinking about this again the other day when I was thinking of some other transform stuff. I'd be curious if we could use recast instead of escodegen (I vaguely remember you did at one point). That would result in leaving comments and such intact...

@RReverser
Copy link
Contributor Author

Yes, that would be possible and I mentioned that in #1234 (comment), however you didn't reply anything at that point, so wasn't sure. I switched from recast to esprima+escodegen in pure-cjs as it improved performance in 4-5 times, but if you'd say this is the thing that you need, I could add using recast as separate option.

@RReverser
Copy link
Contributor Author

@zpao With esprima+escodegen upgrades, added support for preserving comments in output via comments option in pure-cjs@1.10.

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

Successfully merging this pull request may close these issues.

2 participants