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
Simple API config path #2080
Merged
Merged
Simple API config path #2080
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
bfa8419
Add ability to use default and extra config files in simple API
jpy-git 2fbb019
Refined code and added unit test
jpy-git e385d9e
Merge branch 'main' into simple_api_config_paths
jpy-git 4281ac3
Fix docstrings
jpy-git 2d9a1f3
Merge branch 'simple_api_config_paths' of https://github.com/jpy-git/…
jpy-git 166d2b3
Update src/sqlfluff/api/simple.py
jpy-git e258668
Improve coverage
jpy-git File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
SELECT foo FROM {{ table_name }}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
[sqlfluff:templater:jinja:context] | ||
table_name=bar |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can hopefully remove those dialect/rules/exclude_rules arguments to the linter in the future as they should be part of the config. i.e. decouple linter and simple API 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would mean you'd have to have a config file to pass. Might be overkill for some simple options. For example I presume https://online.sqlfluff.com/ uses the simple API and just needs to specify the dialect.
But it is duplication which is annoying.
BTW what happens if you specify a dialect AND a config file? Which ones wins out? Or should that error and it should be either/or but not both?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tunetheweb you can still pass a dialect to the Simple API (and the dialect would overwrite that in the config as you'd expect)?
The Simple API itself is unchanged from a user perspective:
I'm specifically talking about
Linter
in the comment which doesn't need those arguments 😄There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as in we don't need to be able to do
Linter(dialect="snowflake")
since the simple api was the only thing using that. Everything else doesLinter(config=cfg)
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah Gothca.
Yes In agree explicit dialect or rule should override config, so that's good, but I wonder if we need to expose this in the docs though so others know that's the way it's implemented?