Fix: do not indent multiline tags unless black actually ran (#180)

* fix: do not indent multiline tags unless black actually ran to completion

* chore: update sqlfmt stats to reflect fmt stability

* chore: update changelog

CHANGELOG.md

@@ -4,13 +4,22 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
 
+### Fixes
+
+-   fixes issue causing unstable formatting of multiline jinja tags when black is unable to parse the tag ([#176](https://github.com/tconbeer/sqlfmt/issues/176))
+-   fixes issue for developers where pre-commit hooks would not install
+
+### Features
+
+-   sqlfmt_primer now runs against forked (formatted) repos to make changes easier to detect
+
 ## [0.8.0] - 2022-05-04
 
 ### Formatting Changes
 
--   sqlfmt is now more conservative about preserving whitespace around jinja expressions when we remove newlines ([#162](https://github.com/tconbeer/sqlfmt/issues/162), [#165](https://github.com/tconbeer/sqlfmt/issues/165))
--   Jinja blocks are now dedented before line merging, instead of after. This results in small changes to formatted output in some cases where jinja blocks are used
--   Fixes an issue where jinja else and elif statements could cause unstable formatting. May impact whitespace for the tokens following `{% else %}` and `{% elif %}` statements
+-   sqlfmt is now more conservative about preserving whitespace around jinja expressions when we remove newlines ([#162](https://github.com/tconbeer/sqlfmt/issues/162), [#165](https://github.com/tconbeer/sqlfmt/issues/165) - thank you [@rcaddell](https://github.com/rcaddell) and [@rjay98](https://github.com/rjay98)!)
+-   jinja blocks are now dedented before line merging, instead of after. This results in small changes to formatted output in some cases where jinja blocks are used
+-   fixes an issue where jinja else and elif statements could cause unstable formatting. May impact whitespace for the tokens following `{% else %}` and `{% elif %}` statements
 
 ## [0.7.0] - 2022-04-24
 

src/sqlfmt/jinjafmt.py

@@ -2,7 +2,7 @@
 from dataclasses import dataclass, field
 from importlib import import_module
 from types import ModuleType
-from typing import Optional
+from typing import Optional, Tuple
 
 from sqlfmt.line import Line
 from sqlfmt.mode import Mode
@@ -21,15 +21,19 @@ def __init__(self) -> None:
         except ImportError:
             self.black = None
 
-    def format_string(self, source_string: str, max_length: int) -> str:
+    def format_string(self, source_string: str, max_length: int) -> Tuple[str, bool]:
         """
         Attempt to use black to format source_string to a line_length of max_length.
-        Return source_string if black isn't installed or it can't parse source_string
+        Return source_string if black isn't installed or it can't parse source_string.
+
+        Return a tuple of the formatted string and a boolean that indicates whether
+        black successfully ran on the string
         """
         formatted_string = source_string
+        is_blackened = False
 
         if not self.black:
-            return formatted_string
+            return formatted_string, is_blackened
 
         black_mode = self.black.Mode(line_length=max_length)
         try:
@@ -49,8 +53,12 @@ def format_string(self, source_string: str, max_length: int) -> str:
                 # there is other jinja syntax that isn't valid python,
                 # so if this still fails, just stop trying
                 pass
+            else:
+                is_blackened = True
+        else:
+            is_blackened = True
         finally:
-            return formatted_string
+            return formatted_string, is_blackened
 
 
 @dataclass
@@ -64,15 +72,19 @@ class JinjaTag:
     (opening_marker, verb, code, closing_marker) = ("{%-", "set", "my_var=4", "%}")
     """
 
+    source_string: str
     opening_marker: str
     verb: str
     code: str
     closing_marker: str
     depth: int
+    is_blackened: bool = False
 
     def __str__(self) -> str:
-        if self.is_indented_multiline_tag:
+        if self.is_indented_multiline_tag and self.is_blackened:
             return self._multiline_str()
+        elif self.is_indented_multiline_tag:
+            return self.source_string
         else:
             s = f"{self.opening_marker} {self.verb}{self.code} {self.closing_marker}"
             return s
@@ -123,7 +135,9 @@ def from_string(cls, source_string: str, depth: int) -> "JinjaTag":
         if verb and code:
             verb = f"{verb} "
 
-        return JinjaTag(opening_marker, verb, code, closing_marker, depth)
+        return JinjaTag(
+            source_string, opening_marker, verb, code, closing_marker, depth
+        )
 
     def max_code_length(self, max_length: int) -> int:
         """
@@ -174,15 +188,9 @@ def _format_jinja_node(self, node: Node, max_length: int) -> None:
             tag = JinjaTag.from_string(node.value, node.depth[0])
 
             if tag.code and self.use_black:
-                tag.code = self._format_python_string(
+                tag.code, tag.is_blackened = self.code_formatter.format_string(
                     tag.code,
                     max_length=tag.max_code_length(max_length),
                 )
 
-            if (not self.use_black) and tag.is_indented_multiline_tag:
-                return
-            else:
-                node.value = str(tag)
-
-    def _format_python_string(self, source_string: str, max_length: int) -> str:
-        return self.code_formatter.format_string(source_string, max_length)
+            node.value = str(tag)

src/sqlfmt_primer/primer.py

@@ -31,17 +31,17 @@ def get_projects() -> List[SQLProject]:
             name="gitlab",
             git_url="https://github.com/tconbeer/gitlab-analytics-sqlfmt.git",
             git_ref="f4168905006e604eaac284a9e08885f15e514fce",  # sqlfmt 0.8.0
-            expected_changed=19,
-            expected_unchanged=2187,
+            expected_changed=12,
+            expected_unchanged=2194,
             expected_errored=0,
             sub_directory=Path("transform/snowflake-dbt/models"),
         ),
         SQLProject(
             name="rittman",
             git_url="https://github.com/tconbeer/rittman_ra_data_warehouse.git",
             git_ref="00c7e4b8d93f8ca0460ad3d9b3f9ea48c213627b",  # sqlfmt 0.8.0
-            expected_changed=3,
-            expected_unchanged=277,
+            expected_changed=1,
+            expected_unchanged=279,
             expected_errored=4,  # true mismatching brackets
             sub_directory=Path("models"),
         ),
@@ -76,8 +76,8 @@ def get_projects() -> List[SQLProject]:
             name="dbt_utils",
             git_url="https://github.com/tconbeer/dbt-utils.git",
             git_ref="7c4d7f2a8dca533207b4e8a63a919f73eb0bb136",  # sqlfmt 0.8.0
-            expected_changed=7,
-            expected_unchanged=124,
+            expected_changed=3,
+            expected_unchanged=128,
             expected_errored=0,
             sub_directory=Path(""),
         ),

tests/data/unformatted/300_jinjafmt.sql

@@ -16,6 +16,16 @@
 {% do long_list.append(
     "another_long_name"
 )%}
+
+-- ensure jinja with python errors are not indented further when formatting
+{{
+    dbt_utils.star(
+        from=ref(model_name),
+        except=long_list,
+        relation_alias=model_name,
+        suffix="something_else"
+    )
+}}
 )))))__SQLFMT_OUTPUT__(((((
 {{
     config(
@@ -40,3 +50,13 @@
 ] %}
 
 {% do long_list.append("another_long_name") %}
+
+-- ensure jinja with python errors are not indented further when formatting
+{{
+    dbt_utils.star(
+        from=ref(model_name),
+        except=long_list,
+        relation_alias=model_name,
+        suffix="something_else"
+    )
+}}

tests/unit_tests/test_jinjafmt.py

@@ -57,7 +57,7 @@ def uninstall_black(monkeypatch: pytest.MonkeyPatch) -> None:
     ],
 )
 def test_jinja_tag_from_string(tag: str, result: Tuple[str, str, str, str]) -> None:
-    assert JinjaTag.from_string(tag, 0) == JinjaTag(*result, 0)
+    assert JinjaTag.from_string(tag, 0) == JinjaTag(tag, *result, 0)
 
 
 @pytest.mark.parametrize(
@@ -180,14 +180,14 @@ def test_black_wrapper_format_string_no_black(
     result = jinja_formatter.code_formatter.format_string(
         source_string=source_string, max_length=88
     )
-    assert result == source_string
+    assert result == (source_string, False)
 
 
 @pytest.mark.parametrize(
     "tag,expected",
     [
-        (JinjaTag("{{", "", "any code!", "}}", 0), 88 - 2 - 2 - 2),
-        (JinjaTag("{%-", "set", "_", "%}", 0), 88 - 3 - 3 - 2 - 2),
+        (JinjaTag("{{ any code! }}", "{{", "", "any code!", "}}", 0), 88 - 2 - 2 - 2),
+        (JinjaTag("{%- set _ %}", "{%-", "set", "_", "%}", 0), 88 - 3 - 3 - 2 - 2),
     ],
 )
 def test_max_code_length(tag: JinjaTag, expected: int) -> None: