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

handle line-breaks around custom fragments gracefully #720

Merged
merged 2 commits into from
Sep 22, 2016

Conversation

alexlamsl
Copy link
Collaborator

@gzzhanghao this is what I have in mind with regards to #716 and #719. Does this address all your cases?

@alexlamsl alexlamsl force-pushed the minify-custom-fragments branch from 1b75a7a to 0418998 Compare September 21, 2016 13:24
@gzzhanghao
Copy link

It works like a charm! Can't wait to merge this 😃

@gzzhanghao
Copy link

The issue in #716 still exists in this branch. I think we should restore all substitutions in the escapeFragments method.

@alexlamsl
Copy link
Collaborator Author

@gzzhanghao thanks for the review - I'll go fix it up now... 😉

@alexlamsl
Copy link
Collaborator Author

Just tried your example in #716 and it seems to work:

> minify(`<script>
... function a() {
.....   b()
.....   <?php if (false): ?>
.....     c()
.....   <?php endif; ?>
..... }
... </script>`, {minifyJS:true})
'<script>function a(){b(),\n<?php if (false): ?>\n,c(),\n<?php endif; ?>\n}</script>'

What other options have you specified which produce the incorrect results as you described?

@gzzhanghao
Copy link

@alexlamsl It becomes <script>function a(){b(),\n\n}</script> after running in php runtime. The trailing comma leads to a syntax error :(

Custom fragments are not valid javascript stuff, I think we can't simply treat it as a identifier in javascript.

@alexlamsl
Copy link
Collaborator Author

@gzzhanghao now I see what you mean.

Yeah, this one is inherently tricky, and the same issue would arise with minifyCSS as well. I guess I'll merge this for now and think about #716 some more, as they are separate issues...

@alexlamsl alexlamsl merged commit 3640d84 into gh-pages Sep 22, 2016
@alexlamsl alexlamsl deleted the minify-custom-fragments branch September 22, 2016 20:56
alexlamsl added a commit that referenced this pull request Sep 22, 2016
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