search

ndparker / rjsmin

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

template strings issues #8

Closed aslushnikov closed 5 years ago

aslushnikov commented 8 years ago

Consider feeding the following javascript to rjsmin:

console.log(`  spaces  `);

rjsmin strips the spaces inside the string, but it shouldn't.

ndparker commented 8 years ago

Thanks. Will fix that.

ndparker commented 7 years ago

@allada already went ahead and created this PR: https://github.com/ndparker/rjsmin/pull/10

ndparker commented 7 years ago

Ok, I've added a feature branch here: https://github.com/ndparker/rjsmin/tree/template-strings, incorporating the suggestions from https://github.com/ndparker/rjsmin/pull/10 plus some minor changes plus C changes.

HOWEVER. It's still wrong. First thing is, that template strings allow for literal newlines (fix should be easy), second - I still need to find out if template strings can be nested. If yes, we're kinda screwed :/

Storyyeller commented 6 years ago

Yes, template strings can be nested. For example

`x${`y`}x`

Evaluates to "xyx"

ndparker commented 6 years ago

Yes, I've read the spec in the meantime. This is the worst idea ever. The question is, however, how to fix this. No regex can match that. All ways to fix that I can imagine either are restrictive, or require bigger changes.

davidfstr commented 6 years ago

Having support for template strings that don't have an embedded backticks would still be a significant improvement. Don't let the perfect be the enemy of the good.

My code uses template strings extensively that have embedded newlines (but not backticks) to construct frontend templates for Vue.

davidfstr commented 6 years ago

@ndparker Any issue with simply merging the template-strings branch you have now? If I read its tests correctly it already handles newlines, just (presumably) not embedded backticks.

AlexAndBear commented 6 years ago

Anything new here?

iamareebjamal commented 5 years ago

@davidfstr Sadly, it doesn't handle newlines

ndparker commented 5 years ago

This has finally been fixed in Release 1.1.0. We currently have support for non-nesting template literals. It's a start...

I'll close thise PR for now.