fix(max line length): fixes issues around max line length not fully r…

…espected on indented lines Previously this calculation was done before indentation was applied. It is updated to do the update after indentation is applied. closes #580
djlint · Apr 3, 2023 · 1ec6b29 · 1ec6b29
1 parent 6d1cb0e
commit 1ec6b29
Show file tree

Hide file tree

Showing 7 changed files with 71 additions and 77 deletions.
diff --git a/.gitignore b/.gitignore
@@ -3,7 +3,7 @@ node_modules
 source_django
 report.xml
 _site
-
+.ruff_cache
 # Created by https://www.toptal.com/developers/gitignore/api/macos,python
 # Edit at https://www.toptal.com/developers/gitignore?templates=macos,python
 

diff --git a/src/djlint/formatter/condense.py b/src/djlint/formatter/condense.py
@@ -12,7 +12,7 @@
 from ..settings import Config
 
 
-def condense_html(html: str, config: Config) -> str:
+def clean_whitespace(html: str, config: Config) -> str:
     """Compress back tags that do not need to be expanded."""
     # put empty tags on one line
 
@@ -55,50 +55,6 @@ def strip_space(config: Config, html: str, match: re.Match) -> str:
             re.compile(rf"^{line_contents}[{trailing_contents}]*$", re.M), func, html
         )
 
-    def if_blank_line_after_match(config: Config, html: str) -> bool:
-        """Check if there should be a blank line after."""
-        if config.blank_line_after_tag:
-            return not any(
-                re.findall(
-                    re.compile(
-                        rf"((?:{{%\s*?{tag}[^}}]+?%}}\n?)+)",
-                        re.IGNORECASE | re.MULTILINE | re.DOTALL,
-                    ),
-                    html,
-                )
-                for tag in [x.strip() for x in config.blank_line_after_tag.split(",")]
-            )
-        return True
-
-    def if_blank_line_before_match(config: Config, html: str) -> bool:
-        """Check if there should be a blank line before."""
-        if config.blank_line_before_tag:
-            return not any(
-                re.findall(
-                    re.compile(
-                        rf"((?:{{%\s*?{tag}[^}}]+?%}}\n?)+)",
-                        re.IGNORECASE | re.MULTILINE | re.DOTALL,
-                    ),
-                    html,
-                )
-                for tag in [x.strip() for x in config.blank_line_before_tag.split(",")]
-            )
-        return True
-
-    def condense_line(config: Config, match: re.Match) -> str:
-        """Put contents on a single line if below max line length."""
-        if (
-            (
-                len(match.group(1) + match.group(3) + match.group(4))
-                < config.max_line_length
-            )
-            and if_blank_line_after_match(config, match.group(3))
-            and if_blank_line_before_match(config, match.group(3))
-        ):
-            return match.group(1) + match.group(3) + match.group(4)
-
-        return match.group()
-
     def add_blank_line_after(config: Config, html: str, match: re.Match) -> str:
         """Add break after if not in ignored block."""
         if inside_ignored_block(config, html, match):
@@ -145,6 +101,58 @@ def add_blank_line_before(config: Config, html: str, match: re.Match) -> str:
                 html,
             )
 
+    return html
+
+
+def condense_html(html, config):
+    """Put short tags back on a single line."""
+
+    def condense_line(config: Config, match: re.Match) -> str:
+        """Put contents on a single line if below max line length."""
+        if (
+            (
+                len(match.group(1).splitlines()[-1] + match.group(3) + match.group(4))
+                < config.max_line_length
+            )
+            and if_blank_line_after_match(config, match.group(3))
+            and if_blank_line_before_match(config, match.group(3))
+        ):
+            return match.group(1) + match.group(3) + match.group(4)
+
+        return match.group()
+
+    def if_blank_line_after_match(config: Config, html: str) -> bool:
+        """Check if there should be a blank line after."""
+        if config.blank_line_after_tag:
+            return not any(
+                re.findall(
+                    re.compile(
+                        rf"((?:{{%\s*?{tag}[^}}]+?%}}\n?)+)",
+                        re.IGNORECASE | re.MULTILINE | re.DOTALL,
+                    ),
+                    html,
+                )
+                for tag in [x.strip() for x in config.blank_line_after_tag.split(",")]
+            )
+        return True
+
+    def if_blank_line_before_match(config: Config, html: str) -> bool:
+        """Check if there should be a blank line before."""
+        if config.blank_line_before_tag:
+            return not any(
+                re.findall(
+                    re.compile(
+                        rf"((?:{{%\s*?{tag}[^}}]+?%}}\n?)+)",
+                        re.IGNORECASE | re.MULTILINE | re.DOTALL,
+                    ),
+                    html,
+                )
+                for tag in [x.strip() for x in config.blank_line_before_tag.split(",")]
+            )
+        return True
+
+    # add blank lines before tags
+
     func = partial(condense_line, config)
 
     # put short single line tags on one line
@@ -158,10 +166,11 @@ def add_blank_line_before(config: Config, html: str, match: re.Match) -> str:
         re.IGNORECASE | re.MULTILINE | re.DOTALL,
     )
 
-    # put short template tags back on one line
+    # put short template tags back on one line. must have leading space
+    # jinja +%} and {%+ intentionally omitted.
     html = re.sub(
         re.compile(
-            rf"({{%-?[ ]*?({config.optional_single_line_template_tags})[^\n(?:%}})]*?%}})\s*?([ ]*?[^%\n]*?[ ]*?)\s*?({{%-?[ ]+?end(\2)[ ]*?%}})",
+            rf"((?:\s|^){{%-?[ ]*?({config.optional_single_line_template_tags})[^\n(?:%}})]*?%}})\s*([^%\n]*?)\s*?({{%-?[ ]+?end(\2)[ ]*?%}})",
             flags=re.IGNORECASE | re.MULTILINE | re.VERBOSE,
         ),
         func,

diff --git a/src/djlint/reformat.py b/src/djlint/reformat.py
@@ -7,7 +7,7 @@
 from pathlib import Path
 
 from .formatter.compress import compress_html
-from .formatter.condense import condense_html
+from .formatter.condense import clean_whitespace, condense_html
 from .formatter.css import format_css
 from .formatter.expand import expand_html
 from .formatter.indent import indent_html
@@ -24,9 +24,10 @@ def reformat_file(config: Config, this_file: Path) -> dict:
 
     expanded = expand_html(compressed, config)
 
-    condensed = condense_html(expanded, config)
+    condensed = clean_whitespace(expanded, config)
 
     beautified_code = indent_html(condensed, config)
+    beautified_code = condense_html(beautified_code, config)
 
     if config.format_css:
         beautified_code = format_css(beautified_code, config)

diff --git a/tests/test_config/test_format_attribute_template_tags/html-one.html b/tests/test_config/test_format_attribute_template_tags/html-one.html
@@ -8,9 +8,7 @@
      src="{% static '/img/loader.gif' %}"
      alt="report image"/>
 <a class="asdf
-          {% if favorite == "yes" %}
-              favorite
-          {% endif %}
+          {% if favorite == "yes" %}favorite{% endif %}
           has-tooltip-arrow has-tooltip-right"
    data-tooltip="{% if favorite == "yes" %}
                      Remove from Favorites
@@ -29,14 +27,10 @@
          comments-msg
      {% else %}
          comments-newMsgReply
-     {% endifchanged %}>
-</div>
+     {% endifchanged %}></div>
 <a class="piwik_download"
-   href="{% static activity_version.get_win_document_with_images_file_path %}?{% now "jSFYHi" %}">
-</a>
-<span {% if a %}
-          required
-      {% endif %}
+   href="{% static activity_version.get_win_document_with_images_file_path %}?{% now "jSFYHi" %}"></a>
+<span {% if a %}required{% endif %}
       title="{% if eev.status == eev.STATUS_CURRENT %}
                  {% trans 'A' %}
              {% elif eev.status == eev.STATUS_APPROVED %}
@@ -45,14 +39,9 @@
                  {% trans 'C' %}
              {% endif %}"
       class="asdf
-             {% if a %}
-                 b
-             {% endif %}
+             {% if a %}b{% endif %}
              asdf"
-      {% if a %}
-          checked
-      {% endif %}>
-</span>
+      {% if a %}checked{% endif %}></span>
 {% block body %}
     <form action="{% if gpg -%}asdf something pretty long. can't beat this length{%- endif %}"
           method="POST">
@@ -68,7 +57,5 @@
            {% endfor %}
            is-collapsible">
 </ul>
-<div {% if true %}
-         class="test"
-     {% endif %}
+<div {% if true %}class="test"{% endif %}
      {% include "django/forms/widgets/attrs.html" %}></div>
diff --git a/tests/test_django/test_comments.py b/tests/test_django/test_comments.py
@@ -144,8 +144,7 @@ def test_nested_inline_comment(runner: CliRunner, tmp_file: TextIO) -> None:
         <div class="{% if 1 %}class {% else %} class {% endif %}">
             <div class="class"
                  data-parameters="{#?@ViewBag.DefaultFilters#}"
-                 data-target="profile-{{ profile_type }}-{{ profile_id }}">
-            </div>
+                 data-target="profile-{{ profile_type }}-{{ profile_id }}"></div>
         </div>
     {% endif %}
 """,

diff --git a/tests/test_html/test_alpinejs.py b/tests/test_html/test_alpinejs.py
@@ -25,8 +25,7 @@ def test_alpine_js(runner: CliRunner, tmp_file: TextIO) -> None:
      x-show="show"
      x-transition.duration.500ms
      :disabled="!$store.userPreferences.deleteConfirm"
-     @click="clicked=true">
-</div>""",
+     @click="clicked=true"></div>""",
     )
 
     assert output.exit_code == 0

diff --git a/tests/test_html/test_attributes.py b/tests/test_html/test_attributes.py
@@ -104,8 +104,7 @@ def test_long_attributes(runner: CliRunner, tmp_file: TextIO) -> None:
                 font-weight: bold;
                 font-size: 1.5rem"
          data-attr="stuff"
-         class="my long class goes here">
-    </div>
+         class="my long class goes here"></div>
 </div>
 """,
     )
@@ -146,7 +145,7 @@ def test_ignored_attributes(runner: CliRunner, tmp_file: TextIO) -> None:
      id="something_meaning_less_is_here"
      required
      checked="checked"
-     json-data='{"menu":{"header":"SVG Viewer","items":[{"id":"Open"}]}}'>\n</div>
+     json-data='{"menu":{"header":"SVG Viewer","items":[{"id":"Open"}]}}'></div>
 """
     )