Skip to content

Conversation

@nettoyoussef
Copy link

@nettoyoussef nettoyoussef commented Nov 5, 2024

This pull request adds support for SQLFluff.
There are some problems though. The only way I found to format files with a custom configuration template (which is probably what most users working on production databases would like) was:

(add-hook 'sql-mode-hook
  (lambda ()
    (define-format-all-formatter sqlfluff
      (:executable "sqlfluff")
      (:install "pip install sqlfluff")
      (:languages "SQL")
      (:features)
      (:format (format-all--buffer-easy executable 
                 "fix"
                 "--config=/path/to/config.ctg"
                 "--nocolor" "-")
      )
    )
  )
)

@lassik
Copy link
Owner

lassik commented Nov 5, 2024

Thank you. Looks good to me.

The only thing is that --dialect=postgres and --config=/path/to/config.ctg should not go into the default options passed by format-all. Users can add their own command line arguments like this:

(add-hook 'sql-mode-hook
          (lambda ()
            (setq format-all-formatters
                  '(("SQL" (sqlfluff "--dialect=postgres" "--config=/path/to/config.ctg"))))))

@nettoyoussef
Copy link
Author

nettoyoussef commented Nov 5, 2024

I am glad to contribute, I've been a user for several years by now :)

The code above doesn't seem to work for me. I tried running format-all-buffer with (setq format-all-debug t) and:

(add-hook 'sql-mode-hook
  (lambda ()
    (setq format-all-formatters
      '(("SQL" (sqlfluff "fix" "--config=/path/to/config.ctg" "--ignore=parsing,templating" "--nocolor" "-")))
    )
  )
)

Which gave the output:

Format-All: Formatting query_temp.sql as SQL using sqlfluff fix --config\=/path/to/config.ctg --ignore\=parsing\,templating --nocolor -

condition-case: Symbol’s function definition is void: nil

Also, is there a way to use the linter output even when there is an error?
SQLFluff has a kind of output called unfixable, which is just the linter saying that it could not implement all rules, but it still provides better output than before.
I tried a hack with the code below, redirecting the stderr to /dev/null, but it didn't work:

(add-hook 'sql-mode-hook
  (lambda ()
    (define-format-all-formatter sqlfluff
      (:executable "sqlfluff")
      (:install "pip install sqlfluff")
      (:languages "SQL")
      (:features)
      (:format (format-all--buffer-easy executable 
                 "fix"
                 "--config=/path/to/config.ctg"
                 "--ignore=parsing,templating"
                 "--nocolor" 
                 "-"
                 "2>/dev/null" ;; avoid error checking, a bit risky
                )
      )
)))

Regarding --dialect=postgres, while I agree that the implementation should not be opinionated, the linter doesn't run without a dialect, so if we don't include it we are basically forcing users to implement a correction/configuration.

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 this pull request may close these issues.

2 participants