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

Exclude SSI Comments from HTML minify #670

Merged
merged 3 commits into from
Apr 2, 2020

Conversation

khalwat
Copy link
Contributor

@khalwat khalwat commented Mar 31, 2020

Exclude Nginx & Apache Server Side Include (SSI) comments from being stripped via minification, the same way IE directives are not stripped.

Both Nginx & Apache SSI directives begin with:

<!--#

So we can just look for the # and don’t strip the comment in that case.

Nginx SSI: http://nginx.org/en/docs/http/ngx_http_ssi_module.html

Apache SSI: https://httpd.apache.org/docs/2.4/howto/ssi.html

Resolves: #671

Andrew Welch added 2 commits March 31, 2020 15:43
Exclude Nginx & Apache Server Side Include (SSI) comments from being stripped via minification, the same way IE directives are not stripped.

Both Nginx & Apache SSI directives begin with:

<!—#

So we can just look for the # and don’t strip the comment in that case.

Nginx SSI: http://nginx.org/en/docs/http/ngx_http_ssi_module.html

Apache SSI: https://httpd.apache.org/docs/2.4/howto/ssi.html
Exclude Nginx & Apache Server Side Include (SSI) comments from being stripped via minification, the same way IE directives are not stripped.

Both Nginx & Apache SSI directives begin with:

<!--#

So we can just look for the # and don’t strip the comment in that case.

Nginx SSI: http://nginx.org/en/docs/http/ngx_http_ssi_module.html

Apache SSI: https://httpd.apache.org/docs/2.4/howto/ssi.html
@khalwat khalwat mentioned this pull request Mar 31, 2020
@glensc
Copy link
Collaborator

glensc commented Apr 1, 2020

Can you try to add some testcases?

@khalwat
Copy link
Contributor Author

khalwat commented Apr 1, 2020

Done @glensc !

@glensc
Copy link
Collaborator

glensc commented Apr 2, 2020

Can you squash or reword commits, currently two commits with the same commit message looks weird. Or I'll squash the whole changeset to one commit myself using GitHub Web?

@khalwat
Copy link
Contributor Author

khalwat commented Apr 2, 2020

@glensc yeah if you want to squash the whole changeset, I think that makes the most sense.

Appreciate you working with me on this, I'm hoping to get it tagged & released so I can update the dependency in my plugin: https://github.com/nystudio107/craft-minify

@glensc glensc merged commit 0f607be into mrclay:master Apr 2, 2020
@glensc glensc changed the title Exclude SSI Comments Exclude SSI Comments from HTML minify Apr 2, 2020
@khalwat khalwat deleted the feature/exclude-ssi-comments branch April 2, 2020 20:24
@khalwat
Copy link
Contributor Author

khalwat commented Apr 2, 2020

Awesome, many thanks @glensc

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.

Exclude SSI Comments
2 participants