Fix missing line break at end tag

[ang-zeyu]

Description

• Source branch in your fork has meaningful name (not master)

Fixes Issue: #1718, #1365

Before Merge Checklist

These items can be completed after PR is created.

(Check any items that are not applicable (NA) for this PR)

• JavaScript implementation
• Python implementation (NA)
• Added Tests to data file(s)
• Added command-line option(s) (NA)
• README.md documents new feature/option(s) (NA)

[ang-zeyu]

First commit (#1718)

Details

there's an explicit check against this in set_tag_position for inline elements (svg), it would work correctly without this PR if the svg in #1718 was switched out for a div

Current code:

} else if (parser_token.is_end_tag) { //this tag is a double tag so check for tag-ending
    /*
     if (either):
     1. start tag is multiline_content
     2. not inline el and
        previous tag token not inline and
        (previous tok not TAG_CLOSE or (ie. has some intermediate stuff in between)
         start_tag_token !== last_tag_token (ie. has some intermediate tags in between)) and
        previous tok is not text
     print a newline
     */
    if ((parser_token.start_tag_token && parser_token.start_tag_token.multiline_content) ||
      !(parser_token.is_inline_element || // this is causing the problem
        (last_tag_token.is_inline_element) ||
        (last_token.type === TOKEN.TAG_CLOSE &&
          parser_token.start_tag_token === last_tag_token) ||
        (last_token.type === 'TK_CONTENT')
      )) {
      printer.print_newline(false);
    }
  }

Originally introduced in 0e9ad61 (#1407)

It's an important check though, newlines shouldn't be printed before inline elements.

Fix

Currently multiline_content (correct me if im wrong) is set on the basis that some child of it is a 'block' (non-inline, non-text) element for start tags. This fails for the example in the issue as it's an
'empty element'.

This pr just sets multiline_content correctly for the above case.

[ang-zeyu]

Expanded the scope of this pr to include (#1365) since the symptoms are similar;

Second commit (#1365)

As mentioned here #1365 (comment), this commit adds newline detection for starting inline tags. If the parent is a non-inline element, we set multiline_content to true here as well.

I'm not sure of the expected behaviour for ending inline tag tokens with newlines though (left as is):

<div class="col-xs-2"><label for="coli" class="control-label">Collision
</label></div>

currently beautifies into:

<div class="col-xs-2"><label for="coli" class="control-label">Collision
    </label></div>

if we add a similar check,

<div class="col-xs-2"><label for="coli" class="control-label">Collision
    </label>
</div>

which should be the correct behaviour?

---

the below force push is just me forgetting to run make ci 🙊

[bitwiseman]

This looks great. I'm going to do a few manual tests before merging.

[bitwiseman]

@ang-zeyu

To answer your question, while the second output feels correct, I think the first output is actually correct. The new line occurs inside the <label> meaning it is multiline, but that doesn't not make the parent <div> multiline:

<div class="col-xs-2"><label for="coli" class="control-label">Collision
    </label></div>

However, I did find a different case that is not correct.

Input

<div class="col-xs-2">
    <input type="radio" class="control-label" ng-disabled="!col" ng-model="col" value="2" class="form-control" id="coli" name="coli" /></div>

Current output is same as above.

Expected output:

<div class="col-xs-2">
    <input type="radio" class="control-label" ng-disabled="!col" ng-model="col" value="2" class="form-control" id="coli" name="coli" />
</div>

Don't worry about fixing this issue, I'll make some tweaks to what you did and merge. Thanks!

[ang-zeyu]

To answer your question, while the second output feels correct, I think the first output is actually correct. The new line occurs inside the meaning it is multiline, but that doesn't not make the parent

multiline:

agreed, that makes sense.

Don't worry about fixing this issue, I'll make some tweaks to what you did and merge. Thanks!

neat, thanks for looking at this as well!

[bitwiseman]

This is turned into a significantly more complex change. Adding tests for edge cases exposed more issues (most of the preexisting) that needed fixing.

The result is an implementation with fewer code paths, but more churn in this PR.