Add a sqlfluff format CLI command #4473
Conversation
alanmcruickshank
requested review from
a team,
barrywhart,
dmateusp and
tunetheweb
and removed request for
a team and
dmateusp
Pull Request Test Coverage Report for Build 4353649815
💛 - Coveralls |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #4473 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 204 204
Lines 15484 15514 +30
=========================================
+ Hits 15484 15514 +30
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| @@ -695,154 +704,67 @@ def do_fixes(lnt, result, formatter=None, **kwargs): | |||
| return False # pragma: no cover | |||
|
|
|||
|
|
|||
| @cli.command(cls=DeprecatedOptionsCommand) | |||
There was a problem hiding this comment.
@alanmcruickshank: Can you suggest how to review this? It looks like a lot of changes starting here, but I'm guessing some of this was code reorg?
There was a problem hiding this comment.
Good point. I'll add a summary in the PR above.
src/sqlfluff/cli/commands.py
Outdated
| help="An optional suffix to add to fixed files.", | ||
| ) | ||
| @click.option( | ||
| "--FIX-EVEN-UNPARSABLE", |
There was a problem hiding this comment.
In the spirit of format being a simpler, safer command, I wonder if we should consider omitting this option, which is known to be unsafe.
src/sqlfluff/cli/commands.py
Outdated
| ), | ||
| ) | ||
| @click.option( | ||
| "--show-lint-violations", |
There was a problem hiding this comment.
Is this option relevant for format?
| @@ -48,6 +48,9 @@ class Rule_JJ01(BaseRule): | |||
| # sections. | |||
| crawl_behaviour = SegmentSeekerCrawler({"raw"}) | |||
| targets_templated = True | |||
| # NOTE: This rule does return fixes, but when is_fix_compatible | |||
| # is set, it spirals. | |||
| # is_fix_compatible = True | |||
There was a problem hiding this comment.
I've made a separate issue for this: #4474.
I left the comment there in case someone else tries to do the same as what I tried.
Ok this is a long time coming. This resolves #2436 and adds the
sqlfluff formatCLI command.It's basically a variant on
sqlfluff fix, but with a hardcoded set of rules. This has a nice upside that if someone is usingexclude_rules, they can still exclude rules - but they can't enable additional ones, so it remains fairly safe.As a first pass I've added the following rules to the command:
capitalisationlayoutambiguous.unionconvention.not_equalconvention.coalesceconvention.select_trailing_commaconvention.is_nulljinja.paddingstructure.distinctI haven't added extensive testing, because the majority of the machinery is the same as
sqlfluff fix. There should be enough here to get coverage.How to review (@barrywhart):
clickarguments into@lint_options. No changes to the functionality, just a reorg. Those options are--processesand--disable-progress-bar. That happens in the first commit in the PR afc6ec3fixcommand into two branches (and two methods), one for stdin fixing (_stdin_fix()) and one for file based fixing (_paths_fix()) - both are used infixandformat. No functionality changes - just refactor.cli_format()method to supportsqlfluff formatcli command. It's mostly a copy paste ofsqlfluff fixbut with several things removed or changed to simplify things. Specifically, it hard codes therulesargument and always sets theforceargument.