Skip to content

Commit

Permalink
fix #365: don't merge multiline jinja unless operator
Browse files Browse the repository at this point in the history
  • Loading branch information
tconbeer committed Jan 24, 2023
1 parent 54b7a2c commit 68e3504
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 9 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,14 @@ All notable changes to this project will be documented in this file.

## [Unreleased]

### Formatting Changes + Bug Fixes

- sqlfmt no longer merges together lines containing multiline jinja blocks unless those lines start with an operator or comma ([#365](https://github.com/tconbeer/sqlfmt/issues/365) - thank you, [@gavlt](https://github.com/gavlt)!).

## [0.15.2] - 2023-01-23

### Features

- adds support for ARM-based platforms using Docker.

## [0.15.1] - 2023-01-20
Expand Down
5 changes: 4 additions & 1 deletion src/sqlfmt/analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,10 @@ def write_buffers_to_query(self, query: Query) -> None:
# indented too far -- they should be formatted as if they
# have no open_brackets
for line in reversed(self.line_buffer):
if line.closes_jinja_block_from_previous_line:
if (
line.is_standalone_jinja_statement
and line.closes_jinja_block_from_previous_line
):
for node in line.nodes:
node.open_brackets = []
else:
Expand Down
25 changes: 20 additions & 5 deletions src/sqlfmt/merger.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,22 @@ def _extract_components(
Given a list of lines, return 2 components:
1. list of all nodes in those lines, with only a single trailing newline
2. list of all comments in all of those lines
Raise CannotMergeException if lines contain nodes that cannot
be merged.
"""
nodes: List[Node] = []
comments: List[Comment] = []
final_newline: Optional[Node] = None
allow_multiline_jinja = True
has_multiline_jinja = False
for line in lines:
if has_multiline_jinja and not (
line.starts_with_operator or line.starts_with_comma
):
raise CannotMergeException(
"Can't merge lines containing multiline nodes"
)
# skip over newline nodes
content_nodes = [
cls._raise_unmergeable(node, allow_multiline_jinja)
Expand All @@ -73,17 +83,22 @@ def _extract_components(
if content_nodes:
final_newline = line.nodes[-1]
nodes.extend(content_nodes)
# we can merge lines containing multiline jinja nodes iff:
# 1. the multiline node is on the first line (allow_multiline
# is initialized to True)
# 2. the multiline node is on the second line and follows a
# standalone operator
# we can merge a line containing multiline jinja
# into a preceding line iff:
# the multiline node is on the second line and follows a
# standalone operator
if not (
allow_multiline_jinja
and len(content_nodes) == 1
and content_nodes[0].is_operator
):
allow_multiline_jinja = False
# we can merge a line into a preceding line that
# contains multiline jinja iff:
# the line starts with an operator or a comma
has_multiline_jinja = any(
[node.is_multiline_jinja for node in content_nodes]
)
comments.extend(line.comments)

if not nodes or not final_newline:
Expand Down
8 changes: 5 additions & 3 deletions tests/data/unformatted/201_basic_snapshot.sql
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@
{{
config(
target_database='analytics',
target_schema=target.schema + '_snapshots',
unique_key='id',
target_schema=target.schema+'_snapshots', unique_key='id',
strategy='timestamp',
updated_at='updated_at',
)
Expand All @@ -20,5 +19,8 @@ select * from {{ ref('stg_my_model') }}{% endsnapshot %}
strategy="timestamp",
updated_at="updated_at",
)
}} select * from {{ ref("stg_my_model") }}
}}

select *
from {{ ref("stg_my_model") }}
{% endsnapshot %}

0 comments on commit 68e3504

Please sign in to comment.