search

rtts / djhtml

Django/Jinja template indenter
GNU General Public License v3.0
582 stars 32 forks source link

Indention bug with Stimulus action #57

Closed michael-yin closed 2 years ago

michael-yin commented 2 years ago

If we have a simple test.html file

<div>
  <div data-action="click->modal#closeBackground">
  </div>
</div>

If I run djhtml -i test.html

I got

<div>
    <div data-action="click->modal#closeBackground">
    </div>
    </div>

As you can see, the last </div> indention is not correct.

The click->modal#closeBackground caused this problem.

JaapJoris commented 2 years ago

I thought it was illegal to use unencoded < and > characters inside HTML attributes, but I could only find this paragraph about it in the (outdated) HTML 4 standard:

authors should use "\>" (ASCII decimal 62) in text instead of ">" to avoid problems with older user agents that incorrectly perceive this as the end of a tag (tag close delimiter) when it appears in quoted attribute values.

So I guess until this bug is fixed DjHTML should be considered an "older user agent" :-)

spookylukey commented 2 years ago

Similar problems here, using alpine if and things like x-if="currentStep > step.number"

Past behaviour was different - I narrowed the change down to between 1.4.12 and 1.4.13.

michael-yin commented 2 years ago

I checked djhtml source code and it seems not easy to solve this problem in elegant way.

So here is my solution:

  1. Before djhtml process the HTML, we detect some special attributes and replace the value with UUID, and store the real value in a dict
  2. So some char such as > in the attribute will not be process by djhtml
  3. After djhtml process the HTML, we detect some special attributes and replace the UUID with the orignal value.

I already make it work in my Stimulus project and here is the commit https://github.com/michael-yin/djhtml/commit/90d5ddfba78a059c4c238034767f3dc0113f88d2

Maybe we can add attribute ignore list to djhtml and let it ignore value in the attributes.

Thx.

JaapJoris commented 2 years ago

@michael-yin I appreciate your solution, but I think we have found an even better solution that also fixes this bug. Could you verify that the latest release solves your problem?