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

Add flag to remove newline before tag close #85

Merged
merged 2 commits into from
Nov 22, 2021
Merged

Add flag to remove newline before tag close #85

merged 2 commits into from
Nov 22, 2021

Conversation

lucamattiazzi
Copy link
Contributor

@lucamattiazzi lucamattiazzi commented Sep 22, 2021

creates a new flag that allows the user to avoid adding newlines before the closing tag in order to both solve issue #84 and not change how the library works

Fixes #84

@sibiraj-s sibiraj-s self-assigned this Nov 5, 2021
@sibiraj-s
Copy link
Collaborator

I have some mixed opinions about this option. Let me get back on this.

@sibiraj-s
Copy link
Collaborator

sibiraj-s commented Nov 13, 2021

I think this is fine. This is the shortest and non breaking fix can be done I guess.

Just thinking out loud.

  • maxLineLength is a nice to have option, but I am not sure a minifier must have it. It makes sense for a formatter like prettier.
  • I am also having thoughts about switching to some well maintained parser. like parse5 or htmlparser2 or some other parsers, so that this package can let alone focus on minifying html.

@DanielRuf
Copy link
Contributor

DanielRuf commented Nov 14, 2021

  • I am also having thoughts about switching to some well maintained parser. like parse5 or htmlparser2 or some other parsers, so that this package can let alone focus on minifying html.

Definitely a good idea. I am currently unsure but afaik there were discussions regarding this for html-minifier.
See kangax/html-minifier#155

@sibiraj-s
Copy link
Collaborator

Definitely a good idea. I am currently unsure but afaik there were discussions regarding this for html-minifier.
See kangax/html-minifier#155

Thanks, will check it out.

@sibiraj-s
Copy link
Collaborator

I would prefer this

const endTagMatch = results[0].match(endTag)
const forceSameLine = endTagMatch && inlineTextTags(endTagMatch[1])

Its breaking, but it is the correct behaviour I think. May be for v7.

@DanielRuf
Copy link
Contributor

DanielRuf commented Nov 22, 2021

CI - Node 16 (ubuntu-latest) Expected — Waiting for status to be reported

I'm wondering what happened with the CI build.

@sibiraj-s
Copy link
Collaborator

CI - Node 16 (ubuntu-latest) Expected — Waiting for status to be reported

I'm wondering what happened with the CI build.

Yeah I was waiting for the same. I think it is the branch protection rule. checking

@sibiraj-s
Copy link
Collaborator

sibiraj-s commented Nov 22, 2021

Okay. Found it I guess. the workflow was waiting for approval to run for first time contributors. Since it is not approved for more than 30 days, It got expired.

@DanielRuf
Copy link
Contributor

Okay. Found it I guess. the workflow was waiting for approval to run for first time contributors. Since it is not approved for more than 30 days, It got expired.

Ah, can we somehow trigeger a new build? Optionally we can maybe push a commit Trigger CI without any changes (--allow-empty).

@sibiraj-s
Copy link
Collaborator

Ah, can we somehow trigeger a new build? Optionally we can maybe push a commit Trigger CI without any changes (--allow-empty).

Pushed a small change. we can squash merge the PR.

@DanielRuf DanielRuf changed the title adds flag to remove newline before tag close Add flag to remove newline before tag close Nov 22, 2021
@DanielRuf DanielRuf merged commit 35e034e into terser:master Nov 22, 2021
@sibiraj-s
Copy link
Collaborator

Making a release now.

@sibiraj-s
Copy link
Collaborator

Published in v6.1.0

@DanielRuf
Copy link
Contributor

@lucamattiazzi thank you very much for your contribution 👍

@lucamattiazzi
Copy link
Contributor Author

thank you for your library! and thanks for approving&merging!

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.

using maxLineLength can introduce newlines that might create bugs in links underline
3 participants