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

fixes for removeOptionalTags & ignoreCustomFragments #501

Closed
wants to merge 13 commits into from
Closed

fixes for removeOptionalTags & ignoreCustomFragments #501

wants to merge 13 commits into from

Conversation

alexlamsl
Copy link
Collaborator

No description provided.

@alexlamsl
Copy link
Collaborator Author

So the trouble is with these lines, when both removeStartTag() and removeEndTag() are triggered.

While fixing #500 I have also noticed the empty content cases for optional start tags are missing, so added those as well.

@alexlamsl
Copy link
Collaborator Author

@kangax I know it's been around forever, but should we be unconditionally trim whitespace of input to minify() before we even start to parse it, or do you think it should depend on collapseWhitespace?

@alexlamsl
Copy link
Collaborator Author

@kangax btw, it's been bugging me that grunt test runs 9 more tests than the online version, until I realise the existence of lint-tests.html 😅

@alexlamsl alexlamsl changed the title fix terminal cases in removeOptionalTags fixes for removeOptionalTags Feb 23, 2016
@alexlamsl alexlamsl changed the title fixes for removeOptionalTags fixes for removeOptionalTags & ignoreCustomFragments Feb 23, 2016
@XhmikosR
Copy link
Collaborator

@alexlamsl: you shouldn't mix unrelated commits. You should merge the unrelated patches to master.

@kangax
Copy link
Owner

kangax commented Feb 23, 2016

@XhmikosR @alexlamsl I agree that large-ish PR's are usually harder to land, since I tend to postpone reviewing them (when I don't have much time to look through all of it and understand; I just keep moving it for later). Small ones are definitely easier to understand and merge so if at all possible, unrelated changes are best extracted. And, as always, thanks for all the continuous help, folks!

@alexlamsl
Copy link
Collaborator Author

Wasn't certain about the process - will do for future maintenance commits that are trivial...

Speaking of trivial commits, shall we land #498 before it gets really stale?

@alexlamsl
Copy link
Collaborator Author

Thanks a lot for the reviews so far - I know how hard it can be even for me to read my own code sometimes!

@alexlamsl
Copy link
Collaborator Author

Let's see what I can do with git here... 😅

@alexlamsl
Copy link
Collaborator Author

Closing for new PR(s)

@alexlamsl alexlamsl closed this Feb 25, 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.

3 participants