Allow at most 2 unclosed nodes of the same type (open/open or close/close) in Liquid branches

[charlespwd]

In this PR

• Add HtmlDanglingMarker{Open,Close} nodes
• Pretty print HtmlDanglingMarker{Open,Close} while maintaining lack of whitespace sensitivity

Rationale

#90 has been opened for a while. A lot of folks intentionally add unclosed nodes inside Liquid if statements. This PR allows this to happen while still throwing errors for the cases that should be errors.

That is, we're being slightly more lenient with what we allow to be parsed as a "full tree."

It should pretty print dangling open tags inside if statements
printWidth: 1
{% if condition %}
  <div
    attr="value"
    other="{{ product.id }}"
  >
{% else -%}
  <product-wrapper
    id="{{ product.id }}"
  >
{%- endif %}

It should pretty print dangling close tags inside if statements
printWidth: 1
{% if condition %}
  </div>
{% else -%}
  </product-wrapper>
{%- endif %}

It should pretty print dangling close tags inside unless statements
printWidth: 1
{% unless condition %}
  </div>
{% else -%}
  </product-wrapper>
{%- endunless %}

It should allow two nodes as well, without indenting
printWidth: 1
{% if condition %}
  <div
    attr="value"
    other="{{ product.id }}"
  >
  <p
    attr="value"
  >
{% endif %}

It should format as expected, Dawn example...
printWidth: 80
<div>
  {%- if block.settings.make_collapsible_row -%}
    <details id="Details-{{ block.id }}-{{ section.id }}" open>
    <summary>
  {%- endif %}
  ...
  {%- if block.settings.make_collapsible_row -%}
    </summary>
  {%- endif %}
  ...
  {%- if block.settings.make_collapsible_row -%}
    </details>
  {%- endif %}
</div>

Heuristic used

The logic used is as follows:

• Throw error UNLESS we have
  • at most 2 dangling close nodes as children of a liquid tag branch
  • at most 2 dangling open nodes as children of a liquid tag branch

👍 Now allowed:

{% if cond %}<a>{% endif %}
{% if cond %}<a><b>{% endif %}
{% if cond %}<a><b>{% else %}<c>{% endif %}
{% if cond %}<a><b>{% elsif cond %}<c><d>{% else %}<e>{% endif %}
{% if cond %}</a></b>{% elsif cond %}</c></d>{% else %}</e>{% endif %}
{% if cond %}</a></b>{% else %}</c>{% endif %}
{% if cond %}</a></b>{% endif %}
{% if cond %}</a>{% endif %}

👎 Still recognized as error:

{% if cond %}<a>text{% endif %}
{% if cond %}<a>{{ 'output' }}{% endif %}
{% if cond %}<a>{{ 'output' }}</a><b>{% endif %}
{% if cond %}<b><a>{{ 'output' }}</a>{% endif %}

{% # 3 feels like a strech %}
{% if cond %}<a><b><c>{% endif %}

{% # etc. %}

Style choice

I decided to treat the unclosed nodes the same way we treat nodes that do not have a closing tag (e.g. <img>). That is, we won't mess with the indentation in reaction to them.

Consequences:

• If you open two tags in an if statement, there's no indentation hint about it.

<div>
  {% if summary %}
    <detail attr=...>
    <summary>
  {% endif %}

  {{ summary }}

  {% if summary %}
    </summary>
  {% endif %}

  {{ content }}

  {% if summary %}
    </detail>
  {% endif %}
</div>

Which feels awkward as hell, but tbh it already was 😅

Reasoning:

If someone does something funky like the above, but there's also an if branch with only one child, deciding which indentation of the siblings should have would get wonky real fast.

<div>
  {% if summary %}
    <detail attr=...>
      <summary>
  {% else %}
    <content-wrapper>
  {% endif %}

    # here?
    {{ summary }}

      # or here?
      {{ summary }}

    # what do we do with this?
    {% if summary %}
      </summary>
    {% endif %}

      # oh my...
      {{ content }}

  {% if summary %}
    </detail>
  {% else %}
    </content-wrapper>
  {% endif %}
</div>

Fixes #90

[albchu]

LGTM!

[karreiro]

Thank you, @charlespwd! Awesome stuff! I've left only two minor suggestions/questions.

[karreiro]

Personally, as a parser user, I find this scenario a bit unexpected:

👍 the parser doesn't throw any error:

{% if cond %}<b>{% endif %}

👎 the parser throws a LiquidHTMLParsingError: Attempting to close LiquidTag \'if\' before HtmlElement \'b\' was closed error

{% if cond %}<a>hi</a><b>{% endif %}

As the a gets successfully closed, it doesn't seem like it should implies in a side effect on b.

(still, please, feel free to ignore if our goal is handling this is a different PR)

[charlespwd]

That's the fine line I'm trying to navigate. I'm trying to do this:

• Report problems when there might be one
• Don't report problems for something that is intentional

The strategy I went with was if there's two open or two close dangling markers, then it's intentional and we allow it.

<a>hi</a><b> seems unintentional. Either you forgot the <b> there or it should have been closed. Not so much with <a><b>.

[karreiro]

Awesome we're covering unless as well :)

I wonder if we could include include test cases with the tags for, tablerow, and form as well (to document how we're handling blocks with those tags). Wdyt?