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.

Sign up for GitHub

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

Jump to bottom

fix: sqlfluff to allow config file other than .sqlfluff #4562

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

fix: sqlfluff to allow config file other than .sqlfluff #4562

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
91afcac
fix: sqlfluff to allow config file other than .sqlfluff
keatmin Jul 25, 2023
affd602
docs: update sqlfluff to add new variable
keatmin Jul 25, 2023
d356e21
docs(ale-sql): fix tags alignment
keatmin Aug 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion ale_linters/sql/sqlfluff.vim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ let g:ale_sql_sqlfluff_executable =
let g:ale_sql_sqlfluff_options =
\ get(g:, 'ale_sql_sqlfluff_options', '')

let g:ale_sql_sqlfluff_config_file =
\ get(g:, 'ale_sql_sqlfluff_config_file', '.sqlfluff')

function! ale_linters#sql#sqlfluff#Executable(buffer) abort
return ale#Var(a:buffer, 'sql_sqlfluff_executable')
endfunction
Expand All @@ -19,7 +22,7 @@ function! ale_linters#sql#sqlfluff#Command(buffer) abort
\ ale#Escape(l:executable)
\ . ' lint'

let l:config_file = ale#path#FindNearestFile(a:buffer, '.sqlfluff')
let l:config_file = ale#path#FindNearestFile(a:buffer, ale#Var(a:buffer, 'sql_sqlfluff_config_file'))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of manually specifying the location of the configuration file or adding an option for which file to use, I wonder if we can edit the linter command by prefixing the command with a work directory so sqlfluff automatically determines which configuration file to load. The project specifies some behaviour for searching for configuration files. https://docs.sqlfluff.com/en/2.1.4/configuration.html#nesting

Copy link
Author

@keatmin keatmin Aug 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@w0rp tbh that would be much preferred.

My original idea was to replace %t with %s in the linter and fixer and let the tool determines which configuration to load. The reason why it didn't load was because the linter/fixer was pointed to %t and it couldn't find any config files. Alternatively using cwd could probably help ?

I just recently migrated to ale from null-ls and not the author of the original code, so I wasn't sure if replacing the command with %s for both linter and fixer will have unintended consequences so I tried to make the code to be as backward compatible as possible.

Appreciate the guidance 🙏🏽 !

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might be able to get it to work by adding 'cwd': '%s:h' to the linter config. You can look at the rstcheck.vim file for an example.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might be able to get it to work by adding 'cwd': '%s:h' to the linter config. You can look at the rstcheck.vim file for an example.

I've tried adding this, but doesn't really help in my case; e.g., my directory structure would be something like

repo  # Project working directory
├── pyproject.toml  # The config I want applied
└── queries
    └── query.sql  # The file I'm editing

So, if I start vim from the project root (in this case, repo), open queries/query.sql, then cwd for the linter is set to repo/queires instead of repo, and sqlfluff fails to find pyproject.toml; i.e., the lint command is

['bash', '-c', 'cd ''/repo/queries'' && ''sqlfluff'' lint ...]

instead of the desired

['bash', '-c', 'cd ''/repo'' && ''sqlfluff'' lint ...]

If I run sqlfluff from vim directly (e.g., :!sqlfluff lint %) it does find the correct config; so it looks like vim "knows" the correct cwd, but I couldn't find any format strings in ale-command-format-strings that would give me that path.


if !empty(l:config_file)
let l:cmd .= ' --config ' . ale#Escape(l:config_file)
Expand Down
6 changes: 4 additions & 2 deletions autoload/ale/fixers/sqlfluff.vim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ call ale#Set('sql_sqlfluff_executable', 'sqlfluff')

function! ale#fixers#sqlfluff#Fix(buffer) abort
let l:executable = ale#Var(a:buffer, 'sql_sqlfluff_executable')
let l:options = ale#Var(a:buffer, 'sql_sqlfluff_options')

let l:cmd =
\ ale#Escape(l:executable)
\ . ' fix --force'
\ . ' fix --force '
\ . l:options

let l:config_file = ale#path#FindNearestFile(a:buffer, '.sqlfluff')
let l:config_file = ale#path#FindNearestFile(a:buffer, ale#Var(a:buffer, 'sql_sqlfluff_config_file'))

if !empty(l:config_file)
let l:cmd .= ' --config ' . ale#Escape(l:config_file)
Expand Down
7 changes: 7 additions & 0 deletions doc/ale-sql.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ g:ale_sql_sqlfluff_options *g:ale_sql_sqlfluff_options*

This variable can be set to pass additional options to the sqlfluff linter.

g:ale_sql_sqlfluff_config_file *g:ale_sql_sqlfluff_config_file*
*b:ale_sql_sqlfluff_config_file*
Type: |String|
Default: `'.sqlfluff'`

This variable can be set to pass config file type to sqlfluff.
Either pyproject.toml, setup.cfg, tox.ini, or pep8.ini.

===============================================================================

Expand Down