kdillane commented on pull request #351: URL: https://github.com/apache/tomcat/pull/351#issuecomment-698458135
> > 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: > > ``` > > | <head> > > | <title>Quick Servlet Demo</title> > > | </head> > > ``` > > > > > > won't produce: > > ``` > > |<head> > > | <title>Quick Servlet Demo</title> > > | </head> > > ``` > > > > > > In addition: why preserve 1 new line ? HTML minifiers would produce something like `...<head><title>...</title></head>...` > > I think the node text comes like this `<head>\n<indentations>` and this will be caught by the BLANK_LINE_PATTERN and thus will later be turned down to `<head>\n` after the processing. I assume you are not taking the node correctly. > > But on a broader aspect > > ``` > | <head> > | <title>Quick Servlet Demo</title> > | </head> > ``` > > Will produce > > ``` > | <head> > | <title>Quick Servlet Demo</title> > | </head> > ``` > > 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. I'd recommend adding a test to cover this use-case. ---------------------------------------------------------------- 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
