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 --wrap-attributes doesn't respect --wrap-line-length #1238

Closed
ms1111 opened this issue Sep 3, 2017 · 6 comments
Closed

HTML --wrap-attributes doesn't respect --wrap-line-length #1238

ms1111 opened this issue Sep 3, 2017 · 6 comments

Comments

@ms1111
Copy link

ms1111 commented Sep 3, 2017

Description

The HTML attribute formatter doesn't seem to wrap attributes before the value set for --wrap-line-length. It does wrap them eventually, just not at the correct position.

Input

<span uib-tooltip="{{firstName}} {{lastName}}" tooltip-enable="showToolTip" >
      <ng-letter-avatar charCount="2" data="{{data}}"
       shape="round" fontsize="{{font}}" height="{{height}}" width="{{width}}" avatarcustombgcolor="{{bgColor}}" dynamic="true"></ng-letter-avatar>
</span>

Expected Output

<span uib-tooltip="{{firstName}} {{lastName}}" tooltip-enable="showToolTip">
    <ng-letter-avatar charCount="2" data="{{data}}" shape="round" fontsize="{{font}}" height="{{height}}"
        width="{{width}}" avatarcustombgcolor="{{bgColor}}" dynamic="true"></ng-letter-avatar>
</span>

Actual Output

<span uib-tooltip="{{firstName}} {{lastName}}" tooltip-enable="showToolTip">
    <ng-letter-avatar charCount="2" data="{{data}}" shape="round" fontsize="{{font}}" height="{{height}}" width="{{width}}"
        avatarcustombgcolor="{{bgColor}}" dynamic="true"></ng-letter-avatar>
</span>

Steps to Reproduce

html-beautify -f input.html --wrap-line-length 110 \
    --wrap-attributes auto \
    --unformatted "abbr, acronym, b, bdo, big, br, cite, code, dfn, em, i, kbd, map, object, q, samp, small, strong, sub, sup, textarea, tt, var" > output.html 

I played with other values for --wrap-line-length. A value of 101 gives the expected output. Not sure why as that's partway through the height attribute, just before the wrap should occur.

Environment

html-beautify 1.6.14

Settings

--wrap-line-length 110
--wrap-attributes auto
--unformatted "abbr, acronym, b, bdo, big, br, cite, code, dfn, em, i, kbd, map, object, q, samp, small, strong, sub, sup, textarea, tt, var"
@triamazikamno
Copy link

The problem is that it's checking whether it needs to wrap the attribute while going character by character ( https://github.com/beautify-web/js-beautify/blob/master/js/src/html/beautifier.js#L406 ) and it's not checking if next attribute will make the line longer than the threshold.
So, by the time it's on width attribute, it's on the position 107 < 110. And next time it checks for wrapping is position 125 (avatarcustombgcolor).
In order to solve it, it would require some kind of peek_next_attribute function and check for its length in space_or_wrap()

@triamazikamno
Copy link

I fixed it in a hacky way in this fork: triamazikamno@e935fbb
It breaks many tests, because it handles indentation too now, but seems to work fine for me so far

@bitwiseman
Copy link
Member

@triamazikamno
Wow, you're right, that is hacky.
How do the performance test result compare before and after?

@RenaldasK
Copy link

Is this getting implemented? It's very annoying when the following:
<meta content="IE=edge" http-equiv="X-UA-Compatible">

becomes:

<meta content="IE=edge"
      http-equiv="X-UA-Compatible">

Even though there's plenty of space to keep it in one line

@bitwiseman
Copy link
Member

@RenaldasK
It looks like @triamazikamno implemented a self-described "hacky" fix. We need someone to take this up implement tests, submit a PR, and drive this to completion. Perhaps you?

@bitwiseman bitwiseman added this to the v1.8.x milestone Aug 11, 2018
@MacKLess
Copy link
Collaborator

This is another issue that will be easier to resolve once a html parser has been developed.

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

5 participants