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.

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

sql :: missing OPTIONS for sqlparse #218

Open
ed9w2in6 opened this issue Nov 28, 2022 · 7 comments
Open

sql :: missing OPTIONS for sqlparse #218

ed9w2in6 opened this issue Nov 28, 2022 · 7 comments

Comments

@ed9w2in6
Copy link

Currently the formatter for sqlformat goes like this:

(format-all--buffer-easy executable "--encoding" ienc "-")

Which is incorrect:
Output of sqlformat --help

usage: sqlformat  [OPTIONS] FILE, ...

Format FILE according to OPTIONS. Use "-" as FILE to read from stdin.

positional arguments:
  filename

optional arguments:
  -h, --help            show this help message and exit
  -o FILE, --outfile FILE
                        write output to FILE (defaults to stdout)
  --version             show program's version number and exit

Formatting Options:
  -k CHOICE, --keywords CHOICE
                        change case of keywords, CHOICE is one of "upper",
                        "lower", "capitalize"
  -i CHOICE, --identifiers CHOICE
                        change case of identifiers, CHOICE is one of "upper",
                        "lower", "capitalize"
  -l LANG, --language LANG
                        output a snippet in programming language LANG, choices
                        are "python", "php"
  --strip-comments      remove comments
  -r, --reindent        reindent statements
  --indent_width INDENT_WIDTH
                        indentation width (defaults to 2 spaces)
  --indent_after_first  indent after first line of statement (e.g. SELECT)
  --indent_columns      indent all columns by indent_width instead of keyword
                        length
  -a, --reindent_aligned
                        reindent statements to aligned format
  -s, --use_space_around_operators
                        place spaces around mathematical operators
  --wrap_after WRAP_AFTER
                        Column after which lists should be wrapped
  --comma_first COMMA_FIRST
                        Insert linebreak before comma (default False)
  --encoding ENCODING   Specify the input encoding (default utf-8)

Currently I am using this hack:

(format-all--buffer-easy executable "-k" "upper" "-r" "--encoding" ienc "-")

Which will convert upcase keywords and reindent.
I am not sure about the standard of this project say if customisation is required, etc.

I can write a pull request if not one wants to write one.

@lassik
Copy link
Owner

lassik commented Nov 28, 2022

Do you mean that the user args should come before the "-"?

@ed9w2in6
Copy link
Author

No, it is incorrect because sqlformat does not do formatting when no options are supplied.

@ed9w2in6
Copy link
Author

This is more explicit:

(format-all--buffer-easy executable "--keywords=upper" "--identifiers=lower" "--indent_width=4" "--reindent" "--encoding" ienc "-")

I just saw this: https://github.com/sqlfluff/sqlfluff, which can do more auto styling and it comes with default styling configs but it is really a linter.

@lassik
Copy link
Owner

lassik commented Dec 1, 2022

No, it is incorrect because sqlformat does not do formatting when no options are supplied.

Does it just return the old SQL unmodified? I so, that's a feature of sqlparse, not a bug.

@lassik
Copy link
Owner

lassik commented Dec 1, 2022

I'd be happy to accept a PR for sqlfluff if someone sends one.

Here is the usage: https://docs.sqlfluff.com/en/stable/cli.html

(format-all--buffer-easy executable "fix" "--nocolor" "-") should be a good start.

@ed9w2in6
Copy link
Author

ed9w2in6 commented Dec 1, 2022

No, it is incorrect because sqlformat does not do formatting when no options are supplied.

Does it just return the old SQL unmodified? I so, that's a feature of sqlparse, not a bug.

Yes but then isn't that against the users expectation of running the format-all command? Since sqlformat only accepts configs from CLI, the only way to make it do some form of formatting is by editing the elisp code. Or is there a way to config format-all that I do not know?

I am happy with my hack at the moment but I will look into sqlfluff when I have time. sqlfluff might fit better into this project since:

  1. it has a default config, and
  2. it can be configured with configuration files.

@lassik
Copy link
Owner

lassik commented Dec 1, 2022

Yes but then isn't that against the users expectation of running the format-all command?

It probably is surprising.

The format-all policy is that counter-intuitive behavior should be fixed upstream in the formatter itself. If editor plug-ins like format-all set their own defaults for each formatter, then different plugins will inevitably set different defaults and users are going to be confused about where the settings are coming from.

If the policy is that all defaults are set in the upstream formatter, that's simpler for both users and maintainers of the formatter and the plug-ins.

Since sqlformat only accepts configs from CLI, the only way to make it do some form of formatting is by editing the elisp code. Or is there a way to config format-all that I do not know?

There is. Users can place the command line flags in the format-all-formatters variable, which can be set via a hook function and/or a project-specific .dir-locals.el file.

For example:

(add-hook 'sql-mode-hook
          (lambda ()
            (setq-local format-all-formatters
                        '(("SQL" (sqlformat
                                  "--keywords=upper"
                                  "--identifiers=lower"
                                  "--indent_width=4"
                                  "--reindent"))))))

I am happy with my hack at the moment but I will look into sqlfluff when I have time.

Thanks! Any help increasing formatter coverage is appreciated.

sqlfluff might fit better into this project since:

We try to be un-opinionated and add all formatters, even ones like sqlformat that are counter-intuitive :)

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

No branches or pull requests

2 participants