search

quarylabs / sqruff

Fast SQL formatter/linter
https://playground.quary.dev/?secondary=Format
Apache License 2.0
352 stars 14 forks source link

Create a pre-commit-hook for sqruff #742

Open concur1 opened 1 month ago

concur1 commented 1 month ago

Pre commit hooks can be a useful development workflow that allow formatters/linters to run before a change is commited to git. There is a python pre-commit-hook utility that allows these to be configured with yaml. Here is an example of Ruff's pre-commit-hook: https://github.com/astral-sh/ruff-pre-commit

concur1 commented 1 month ago

Hi @benfdking and @gvozdvmozgu I have created a pre-commit hook here: https://github.com/concur1/sqruff-pre-commit

I mainly tried to just copy the ruff pre-commit hook, but based it on cargo/rust instead of python/pypi as I thought that is more appropriate here.

I was wondering if you would want to take ownership of this or if you would prefer to leave it with me?

It works (from my testing at least) but there is one small thing that someone who knows more about rust might be able to help with: I have been forced to include a dummy/useless src/main.rs file even though it does not get used. As far as I can tell cargo requires some src files to exist. Does anyone know if there is a cargo.toml config that can be used to allow building with no source files?

gvozdvmozgu commented 1 month ago

Why do we need to create a package?

benfdking commented 1 month ago

I presume we can just take the commit yaml file and merge that into this repo?

concur1 commented 1 month ago

Why do we need to create a package?

When people run pre-commit it will point to this repo and install sqruff for them based on the cargo.toml. I will give reasons why I think it needs to be a separate repo (for now) bellow.

I presume we can just take the commit yaml file and merge that into this repo?

I don't think that is possible with the current sqruff structure, we would need the root cargo.toml to create a sqruff binary, and ideally that would be all that it does. I will explain more below.

I couldn't find any simimlar rust examples. From reading the pre-commit documentation and asking in their discord it seems that:

  1. The pre-commit hook documentation specifies that the hook must exist in a repo with "a Cargo.toml file which produces at least one binary (example), whose name should match the entry definition for your hook.". - in this case it is the sqruff binary.
  2. Both the .pre-commit-hook.yaml and the cargo.toml must live in the repo root.
  3. Currently the cargo.toml that produces the sqruff binary is under crates/cli so it will not get picked up by pre-commit.
  4. There doesn't seem to be a way of passing a path to the desired cargo.toml (not found in the docs and I have asked in discord).