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

HTML - Line breaks changing code output #1304

Closed
valtism opened this issue Dec 20, 2017 · 11 comments
Closed

HTML - Line breaks changing code output #1304

valtism opened this issue Dec 20, 2017 · 11 comments

Comments

@valtism
Copy link

valtism commented Dec 20, 2017

Description

I have this problem with the html formatter in VSCode, which I believe is done with js-beautify.

In this case I have a label and an input. I don't want any space between them so I can style them as a single box.

When there is whitespace or a line break between the elements, spacing is automatically added between the elements.

When I put the elements together in this way, the formatter automatically breaks them up onto separate lines, changing the output. I don't think that the formatter should be changing the meaning or output of the code it is formatting.

Input

The code looked like this before beautification:

<label>Every</label><input class="scheduler__minutes-input" type="text">

Expected Output

The code should have looked like this after beautification:

<label>Every</label><input class="scheduler__minutes-input" type="text">

Actual Output

The code actually looked like this after beautification:

<label>Every</label>
<input class="scheduler__minutes-input" type="text">

Environment

OS: Windows 10
Editor: VSCode

Settings

Default VSCode settings.

@bitwiseman
Copy link
Member

Yes, this is an issue with the way the beautifier understands HTML. It is a bug and it would be great to get it fixed.

The short term workaround would be to wrap those two elements in a <span>. Then they'd stay together.

@andreyvolokitin
Copy link
Contributor

But beautifier actually needs to break such tags in case of minified HTML — it consists of them. Can these use cases be distinguished and handled separately? At first look, it seems like pretty hard deal

@andreyvolokitin
Copy link
Contributor

This line makes this, so it is always a new line at a tag start, if this tag gets formatted. I think this is the place for some additional logic, but not sure how this logic should behave...

@andreyvolokitin
Copy link
Contributor

andreyvolokitin commented Jan 17, 2018

This probably should be an option, because in case of aggressive minification all whitespace between tags gets removed, and then there is no way to distinguish where this whitespace should remain removed after beautification. With an option like keepCollapsedWhitespace: true beautifier would not add a new line at the start of a tag which has a direct left sibling tag and has no left whitespace. Then the user can beautify minified HTML with this option turned off, and even with this option turned on if HTML will be minified in "conservative" way with 1 space left between uncollapsed tags

@bitwiseman
Copy link
Member

There's a bunch of discussion around this in #840, #841.

@andreyvolokitin
Copy link
Contributor

I am planning a pull request, what file should I edit? This one (a lot of contributions but it says "AUTO-GENERATED. DO NOT MODIFY.")? Or this?

@andreyvolokitin
Copy link
Contributor

Despite current beautifier logic for overall formatting, on top of it I made this option keep_collapsed_whitespace: keep it off (it is off by default) if you want beautifier to keep breaking tags with no whitespace between them (this is unchanged old behavior, useful if you want to beautify aggressively minified HTML). But turn it on if you want to keep collapsed tags without wrapping them. Also with this option turned on beautifier does not wrap tags if there is a string with no spaces between them:

<tag></tag>string_with_no_spaces<tag></tag>
or
<tag></tag>&nbsp;<tag></tag>

I think these 2 use cases also should be useful, because if someone writes HTML like that: <tag></tag>&nbsp;<tag></tag> she probably needs to keep it that way. With another example I am less confident: <tag></tag>string_with_no_spaces<tag></tag>, it is quite uncommon use case but if it appears then probably it was meant to be that way, and if not — it is not a big deal. Optionally the string length limit could be set, so if string_with_no_spaces is too long, then tag wrapping happens. What do you think?

@bitwiseman
Copy link
Member

Edit the file under src and run ./build to generate the file under lib.

Rather than add an option, I would prefer that you add a new list of tags that should be treated as "phrasing content" - https://www.w3.org/TR/html5/dom.html#phrasing-content.

For instance, the first to examples here will be displayed the same way. The third will have extra spaces.

<p><div>Hello</div><div>again</div> to <span>you</span>, <b>sir</b>!</p>
<p>
  <div>
    Hello
  </div>
  <div>
    again
  </div>
  to <span>you</span>, <b>sir</b>!
</p>
<p>
  <div>
    Hello
  </div>
  <div>
    again
  </div>
  to 
  <span>
    you
  </span>,
  <b>
    sir
  </b>!
</p>

See https://jsfiddle.net/tuyqxm17/

If we treat "phrasing content" elements as text that must remain with any non-whitespace on either side of it, we get correct formatting and correct displayed output.

@andreyvolokitin
Copy link
Contributor

The thing with this issue (as I understood it, and it certainly the case for me) is that it is about purely layout/presentational needs and not content semantics. There are cases when we just need any combination of sibling tags, located anywhere in the tree, to be with no whitespace between them. This may be needed for layout purposes, or just to keep some tags rendered on a single line. Beautifier breaks this when it adding a whitespace at the spot where it was intentionally removed. These tags not necessarily should represent a "phrasing content". It even makes sense mostly presentationally: you care about tags properly rendering in the browser, but not necessarily about where and how exactly they are printed in HTML code (which is what "phrasing content" is about, if I understood it correctly). I think that this is quite different from what you suggesting. The option is solving just that: if there are some collapsed tags — they kept that way

@bitwiseman
Copy link
Member

@andreyvolokitin I would be happy to consider a PR implementing this option. Please make sure to add a significant number of tests showing how the functionality differs from regular behavior.

@andreyvolokitin
Copy link
Contributor

andreyvolokitin commented Jan 21, 2018

Almost there. I also think that currently unformatted option actually works, so if it present with a default value, then HTML from initial issue stays unformatted as expected. Can you confirm? Very strange to me, I thought it was not working (as well as you I guess) but apparently I just had unformatted: [] set in my config.

Though this option has a different meaning than issue itself

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants