Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only merge multiline jinja with operators #365

Closed
tconbeer opened this issue Jan 19, 2023 Discussed in #351 · 0 comments · Fixed by #374
Closed

Only merge multiline jinja with operators #365

tconbeer opened this issue Jan 19, 2023 Discussed in #351 · 0 comments · Fixed by #374
Labels
enhancement New feature or request style for issues that change the sqlfmt style
Milestone

Comments

@tconbeer
Copy link
Owner

Let's fix this. Only merge multi-line jinja with operators (incl. as, comma, etc.).

Discussed in #351

Originally posted by gavlt January 9, 2023
A small select statement will be "optimistically" (or maybe "aggressively") merged onto closing Jinja brackets.

My use case here are dbt models against a Redshift database when we want to select a whole table from one schema into another and/or apply a (potentially new) partitioning which is managed by a dbt config block.

The resulting output feels a little compact to me. While sqlfmt doesn't necessarily share the complete Black philosophy, this style feels counter to readability and minimising git diffs (i.e. if in future I want to drop the config block it will impact the select statement also).

In #249 (comment) regarding preserving blank lines (like black) you made the following comment about this style/behaviour/request being popular, which I haven't been able to find any further reference to:

In other words, we'll allow 2 blank lines in between statements (e.g., between a {{ config }} block and select, which seems popular)

To Reproduce

{{
    config(
        dist="a_col",
        sort="another_col",
    )
}}

select * from {{ ref("another_model") }}

Actual behavior

{{
    config(
        dist="a_col",
        sort="another_col",
    )
}} select * from {{ ref("another_model") }}

Expected behavior
A blank line is preserved, or inserted, or the closing brackets are retained on the preceding line?

Additional detail
I see this behaviour is explained in the LineMerger:

# 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

Is this change in style planned and/or on the roadmap or in flight, perhaps as part of #249 (which is intended for v0.15?)

@tconbeer tconbeer added enhancement New feature or request style for issues that change the sqlfmt style labels Jan 19, 2023
tconbeer added a commit that referenced this issue Jan 24, 2023
* fix #365: don't merge multiline jinja unless operator

* chore: bump primer refs

* chore: update dbt_utils primer stats
@tconbeer tconbeer added this to the v0.16.0 milestone Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request style for issues that change the sqlfmt style
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant