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

Minification of a + +a, x - --x... #73

Closed
wants to merge 7 commits into from
Closed

Conversation

acatton
Copy link

@acatton acatton commented Jan 26, 2015

Hey @rspivak ,

I started working on this because there was a bug when minifying a + +a. It was becoming a++a which is invalid.

But I weren't very excited about how the minifying tests were done, so if you don't I added more than just fixing this bug:

  • I added the ability to compare ast object. (This is the big part of the diff. I didn't want to do something generic, because in python explicit is better than generic. So I went over each ast types and added equality and __repr__)
  • I didn't like how classes were monkey-patched. I felt that Metaclasses would be a better way to do that. (And this is why there were intended) so I refactored the test case creation into a big metaclasses generating the tests.
  • I created tests which asserted that parse(source) == parse(minified(source))

Let me know what you think, or if you want me to change anything.

Thanks!

metatoaster added a commit to calmjs/calmjs.parse that referenced this pull request Jun 2, 2017
- Addressed the __repr__ "fix" requested by rspivak/slimit#73, but done
  at the correct level (commit acatton/fork--rspivak--slimit@dcd49dbb6f), i.e.  without
  modifying all the Node subclasses.
- If needed, equality check should be possible by comparing the repr.
- More tests will be needed, but this will be done in conjunction with
  the parser tests to be done.
metatoaster added a commit to calmjs/calmjs.parse that referenced this pull request Jun 8, 2017
- The ES5Program class no longer has pretty printing capabilities by
  default, and now inherits from Program
- The parser no longer imports directly from asttypes, but rather use
  the factory to create a new version with the desired pretty printing
  methods for the str/repr representations.
- Naturally, update the rest of the code
- Also ensure that the es5.PrettyPrinter class don't attempt to invoke
  a str version of its own, but use the repr instead.
- Have the ReprVisitor take a depth parameter - a depth of 0 means no
  traversal, 1 for a single level of depth, etc.
- This completes the better implementation of the repr for each node in
  a generic, extensible and non-invasive way for rspivak/slimit#73
metatoaster added a commit to calmjs/calmjs.parse that referenced this pull request Jun 8, 2017
- Also note that the following issues were addressed, where applicable
  to the lexer or parser.

  - rspivak/slimit#52
  - rspivak/slimit#54
  - rspivak/slimit#57
  - rspivak/slimit#59
  - rspivak/slimit#62
  - rspivak/slimit#65
  - rspivak/slimit#70
  - rspivak/slimit#73
  - rspivak/slimit#79
  - rspivak/slimit#81
  - rspivak/slimit#82
  - rspivak/slimit#90

- Will get the release out when I get some sleep.
@acatton
Copy link
Author

acatton commented Jun 12, 2019

Apparently this is now unmaintained, and my fix was applied to the https://github.com/calmjs/calmjs.parse fork.

Closing.

@acatton acatton closed this Jun 12, 2019
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.

1 participant