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

Format on save configuration bug #15

Closed
ShaneMazur opened this issue Jan 23, 2025 · 8 comments · Fixed by #16
Closed

Format on save configuration bug #15

ShaneMazur opened this issue Jan 23, 2025 · 8 comments · Fixed by #16

Comments

@ShaneMazur
Copy link

ShaneMazur commented Jan 23, 2025

Describe the bug
Formatting on save in vscode doesn't respect line length provided in pyproject.toml

vscode settings.json

    "files.associations": {
        "*.sql": "jinja-sql",
        "*.md": "jinja-md",
        "*.env": "properties"
    },
    "[jinja-sql]": {
        "editor.formatOnSave": true,
        "editor.defaultFormatter": "yassun7010.vscode-shandy-sqlfmt"
    },

pyproject.toml

# SQLFmt
[tool.sqlfmt]
line_length = 120
check = false
dialect = "polyglot"
exclude = [".venv/**", "dbt_packages/**", "target/**"]

Expected behavior
Same behaviour as when running sqlfmt from cli

Actual behavior
Uses default line length of 88 despite line length defined as 120 in toml config

Additional context
What is the output of sqlfmt --version?
sqlfmt, version 0.24.0

What is the output of pip list (or pipx list if you installed using pipx)?

Package                   Version
------------------------- ---------------
about-time                3.1.1
agate                     1.9.1
alive-progress            2.3.1
annotated-types           0.7.0
attrs                     24.3.0
azure-core                1.32.0
azure-storage-blob        12.24.0
babel                     2.16.0
backoff                   2.2.1
beautifulsoup4            4.12.3
black                     24.10.0
boto3                     1.36.2
botocore                  1.36.2
cachetools                5.5.0
certifi                   2024.12.14
cffi                      1.17.1
chardet                   5.2.0
charset-normalizer        3.4.1
click                     8.1.8
colorama                  0.4.6
cryptography              44.0.0
daff                      1.3.46
databricks-sdk            0.17.0
databricks-sql-connector  3.7.1
dbt-adapters              1.13.1
dbt-common                1.14.0
dbt-core                  1.9.1
dbt-databricks            1.9.2
dbt-extractor             0.5.1
dbt-semantic-interfaces   0.7.4
dbt-spark                 1.9.0
deepdiff                  7.0.1
diff_cover                9.2.1
elementary-data           0.16.1
et_xmlfile                2.0.0
google-api-core           2.24.0
google-auth               2.37.0
google-cloud-core         2.4.1
google-cloud-storage      2.19.0
google-crc32c             1.6.0
google-resumable-media    2.7.2
googleapis-common-protos  1.66.0
grapheme                  0.6.0
idna                      3.10
importlib-metadata        6.11.0
iniconfig                 2.0.0
isodate                   0.6.1
jaraco.classes            3.4.0
jaraco.context            6.0.1
jaraco.functools          4.1.0
Jinja2                    3.1.5
jinja2-simple-tags        0.6.1
jmespath                  1.0.1
jsonschema                4.23.0
jsonschema-specifications 2024.10.1
keyring                   25.6.0
leather                   0.4.0
lz4                       4.3.3
Markdown                  3.7
MarkupSafe                3.0.2
mashumaro                 3.14
monotonic                 1.6
more-itertools            10.6.0
msgpack                   1.1.0
mypy-extensions           1.0.0
networkx                  3.4.2
numpy                     2.2.2
oauthlib                  3.2.2
openpyxl                  3.1.5
ordered-set               4.1.0
packaging                 24.2
pandas                    2.2.3
parsedatetime             2.6
pathspec                  0.12.1
pip                       24.3.1
platformdirs              4.3.6
pluggy                    1.5.0
posthog                   2.5.0
proto-plus                1.25.0
protobuf                  5.29.3
pyarrow                   19.0.0
pyasn1                    0.6.1
pyasn1_modules            0.4.1
pycparser                 2.22
pydantic                  2.10.5
pydantic_core             2.27.2
pyfiglet                  0.8.post1
Pygments                  2.19.1
pymsteams                 0.2.5
pytest                    8.3.4
python-dateutil           2.9.0.post0
python-slugify            8.0.4
pytimeparse               1.1.8
pytz                      2024.2
PyYAML                    6.0.2
ratelimit                 2.2.1
referencing               0.36.1
regex                     2024.11.6
requests                  2.32.3
rpds-py                   0.22.3
rsa                       4.9
ruamel.yaml               0.18.10
s3transfer                0.11.1
shandy-sqlfmt             0.24.0
six                       1.17.0
slack_sdk                 3.34.0
snowplow-tracker          1.0.4
soupsieve                 2.6
sqlfluff                  3.3.0
sqlfluff-templater-dbt    3.3.0
sqlparams                 6.1.0
sqlparse                  0.5.3
tabulate                  0.9.0
tblib                     3.0.0
text-unidecode            1.3
thrift                    0.20.0
tqdm                      4.67.1
types-requests            2.32.0.20241016
typing_extensions         4.12.2
tzdata                    2024.2
urllib3                   2.3.0
zipp                      3.21.0

Cross posted here

@yassun7010
Copy link
Owner

yassun7010 commented Jan 24, 2025

Hi ShaneMazur

Thanks for the Issue!
The problem is that I didn't expect users to use the configuration file.

I need to investigate what configuration usage is in sqlfmt.
I will check over the weekend.

@tconbeer
Copy link

Config discovery is dependent on the current working directory; if for some reason that isn't the project directory, you may need to add your own config for users to point to their config files.

@ShaneMazur
Copy link
Author

@tconbeer @yassun7010

To confirm, the pyproject.toml is at the root of my project (see screenshots) and it does contain configuration for sqlfmt to use line length of 120 which is respected when running sqlfmt via terminal. The 2 screenshots below show how sqlfmt applies on save when I do and then do not have the following block in settings.json.

    "shandy-sqlfmt.args": [
        "-l 120"
    ],

With hardcoded line length in settings.json

Image

Without hardcoded line length in settings.json

Image

@yassun7010
Copy link
Owner

@tconbeer @ShaneMazur

Hi, I have investigated this problem.

The cause is that the VSCode extension performs formatting on temporary files.

There are cases where the formatter is executed on an incomplete SQL file on the editor, and this implementation is to ensure that the file data on the editor is not lost.

Image

@tconbeer

Is it possible to add one of the following options on the shandy-sqlfmt side?

  • Add SQL text formatting mode.
  • Add config file path option.

@tconbeer
Copy link

tconbeer commented Feb 2, 2025

sqlfmt will process stdin, so you can pipe the file through that if you want. Or you can use the Python API: format_string.

I can add a config path option.

@yassun7010
Copy link
Owner

yassun7010 commented Feb 3, 2025

sqlfmt will process stdin, so you can pipe the file through that if you want. Or you can use the Python API: format_string.

Sorry. I missed the document.
I will first try to do using stdin!

@ShaneMazur
Copy link
Author

@yassun7010 I think a bug was introduced in this fix. Seems that you are now adding additional newlines at the end of formatted files on save. When the extension formats the code below on save it adds 2 new lines (3 lines total):

select 'testing some stuff on save' as test

Testing with v1.11 of your extension this was not the case 👀 Only 1 newline is added

This means that using sqlfmt from terminal produces different formatting than sqlfmt on save using your extension

See screenshots for additional details:

  • Formatting on latest version

Image

Image

Image

  • Formatting on version 1.11

Image

Image

Image

@yassun7010
Copy link
Owner

yassun7010 commented Feb 4, 2025

@ShaneMazur

Added process for removing the last line break, and release v0.1.13.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants