[GitHub] [tomcat] kamnani edited a comment on pull request #351: Remove White Spaces from the JSP files
kamnani edited a comment on pull request #351: URL: https://github.com/apache/tomcat/pull/351#issuecomment-760420468 @markt-asf @ChristopherSchultz @rmaucher Since this is laying low for quite a while, can we have updates on this ? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] kamnani edited a comment on pull request #351: Remove White Spaces from the JSP files
kamnani edited a comment on pull request #351: URL: https://github.com/apache/tomcat/pull/351#issuecomment-698430292 > Since there is no new test for this I am not sure but looking at the code I think "Remove white space" is not quite accurate. > It is rather "Remove empty lines". Because BLANK_LINE_PATTERN would match only if there zero or more empty spaces, followed by at least one new line, followed by 0 or more empty spaces. > So the example above: > > ``` > | > | Quick Servlet Demo > | > ``` > > won't produce: > > ``` > | > | Quick Servlet Demo > | > ``` > > In addition: why preserve 1 new line ? HTML minifiers would produce something like `.` I think the node text comes like this `\n` and this will be caught by the BLANK_LINE_PATTERN and thus will later be turned down to `\n` after the processing. I assume you are not taking the node correctly. But on a broader aspect ``` | | Quick Servlet Demo | ``` Will produce ``` | | Quick Servlet Demo | ``` I agree to the minifying part, so we can rather just replace it with an empty character in the end instead of new line character and that will do the job too and will thus be more optimized. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] kamnani edited a comment on pull request #351: Remove White Spaces from the JSP files
kamnani edited a comment on pull request #351: URL: https://github.com/apache/tomcat/pull/351#issuecomment-698430292 > Since there is no new test for this I am not sure but looking at the code I think "Remove white space" is not quite accurate. > It is rather "Remove empty lines". Because BLANK_LINE_PATTERN would match only if there zero or more empty spaces, followed by at least one new line, followed by 0 or more empty spaces. > So the example above: > > ``` > | > | Quick Servlet Demo > | > ``` > > won't produce: > > ``` > | > | Quick Servlet Demo > | > ``` > > In addition: why preserve 1 new line ? HTML minifiers would produce something like `.` I think the node text comes like this `\n` and this will be caught by the BLANK_LINE_PATTERN and thus will later be turned down to `\n` after the processing. I assume you are not taking the node correctly. But on a broader aspect ``` | | Quick Servlet Demo | ``` Will produce ``` | | Quick Servlet Demo | ``` I agree to the minifying part, so we can rather just replace it with an empty character in the end instead of new line character and that will do the job too and will thus be more optimized. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] kamnani edited a comment on pull request #351: Remove White Spaces from the JSP files
kamnani edited a comment on pull request #351:
URL: https://github.com/apache/tomcat/pull/351#issuecomment-694997270
@markt-asf
```
<%@ taglib uri="http://java.sun.com/jsp/jstl/core; prefix="c"%>
<%@ taglib uri="http://java.sun.com/jsp/jstl/functions; prefix="fn" %>
${feature.featureName}:jsp-exec-time
<%-- PLEASE ENSURE ANY CHANGES MADE HERE ARE KEPT IN SYNC WITH
/WEB-INF/views/jsp/ajax/.jsp --%>
${module.content}
${feature.content}
${feature.content}
<%@ include file="criticalFeatureLogging.jsp" %>
${jspError}
${module.moduleError}
${requestScope[feature.featureName]}
<%@ include
file="/WEB-INF/views/jsp/features/debug/includeIsDebugJs.jsp" %>
```
1) This is from a real file in a critical application but we stripped(or
even marked ***) out certain proprietary lines. If there is an error or such
that's why.
2) The indented tags such as line 12 results in white space on the html
output. This white space is eliminated via this change.
3) Our application is filled with neatly-formatted JSPs that output waste
such as that. One public-facing HTML page became 20% smaller after
implementing this.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] kamnani edited a comment on pull request #351: Remove White Spaces from the JSP files
kamnani edited a comment on pull request #351: URL: https://github.com/apache/tomcat/pull/351#issuecomment-694967799 @markt-asf Thanks for replying on this thread. The primary aim of this PR is to trim off the white spaces. I can give you an example here. Optimized ``` | -- | -- | | Quick Servlet Demo | | | Tomcat 9 Test app | This has over 1000 Jar Files | FastStartup Flag: true | OptimizeJSPs Flag: true | To see server Logs: Click Here | To see Optimized Render times of tags: Click Here | | ``` Unoptimized: ``` -- | | Quick Servlet Demo | | | Tomcat 9 Test app | This has over 1000 Jar Files | | FastStartup Flag: false | OptimizeJSPs Flag: false | To see server Logs: Click Here | To see Optimized Render times of tags: Click Here | | | | | | | | ``` For large JSP files these white spaces are tremendous and also useless when passed over the network. The white spaces are good for readability for the user, and are not necessarily needed as source-compiled files. Removing these white spaces result in fewer writes to internal buffers and fewer flushes to the network and thus results in lower latency. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] kamnani edited a comment on pull request #351: Remove White Spaces from the JSP files
kamnani edited a comment on pull request #351: URL: https://github.com/apache/tomcat/pull/351#issuecomment-689824792 @martin-g @rotty3000 Thanks for your time on this : ) I have marked all of the conversations as resolved after the latest commits. Do we have any more feedback on this PR? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
