search

ndparker / rjsmin

Fast javascript minifier for Python
http://opensource.perlig.de/rjsmin/
Apache License 2.0
60 stars 15 forks source link

Spaces being removed from regex #17

Closed rowanseymour closed 2 years ago

rowanseymour commented 5 years ago 
>>> from rjsmin import jsmin
>>> jsmin(r'for(f=0;f<z;f++)/^ *-+: *$/.test(x)')
'for(f=0;f<z;f++)/^*-+:*$/.test(x)'

Which in this particular case results in an invalid regex.

Using latest rjsmin 1.1.0

ndparker commented 5 years ago

Thanks, confirmed. I'll look into it.

ndparker commented 5 years ago

Lovely.

There's a test for this issue now: https://github.com/ndparker/rjsmin/blob/2f5fb652a6d3a21127a64194544116f8db0a6959/tests/test_issue17.py#L72

I've added a test for another case, which looks exactly like it for rjsmin (but it's not): https://github.com/ndparker/rjsmin/blob/2f5fb652a6d3a21127a64194544116f8db0a6959/tests/test_issue17.py#L40

It's easy enough to change rjsmin to make one of them successful, but not both at the same time. Any suggestions?

rowanseymour commented 4 years ago

Alas not.. this might be as far as one can go with just regexes

BoPeng commented 3 years ago

I agree that fixing both

(f++)/ 4 + 3 / 2)
for(f=0;f<z;f++)/^ *-+: *$/.test(x)

could be extremely difficult but I would argue that correctness is more important than effectiveness here, meaning that when in doubt, we should certain make the second case work even if we do not minify the first case as efficiently as we should.

ndparker commented 2 years ago

Good news. Found a way to improve regex detection: https://github.com/ndparker/rjsmin/tree/issue-17. The file presented in #23 is left unchanged now (modulo comments, of course), which is a good thing. Need to follow up with the C implementation yet.

BoPeng commented 2 years ago

Looking forward to a new release. Right now I have to keep a few .min.js outside of the bundle due to this issue.

ndparker commented 2 years ago

Fixed in 1.2.0 (just released)/.