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

[Feature request] Support format selection #94

Closed
rubensa opened this issue Apr 14, 2023 · 20 comments
Closed

[Feature request] Support format selection #94

rubensa opened this issue Apr 14, 2023 · 20 comments
Assignees

Comments

@rubensa
Copy link
Contributor

rubensa commented Apr 14, 2023

Add support to format selected text in any opened file.

NOTE: In my specific use case, I'd like to format some sql changes and some select queries for views present in a Liquibase changelog in YAML format using the snowflake dialect.

@RobertOstermann
Copy link
Contributor

Do you know if this is currently possible using the sqlfluff command line tool? If so I will look into this further, but if it is not possible using the command line tool then you should create an issue on the sqlfluff repository.

@voney
Copy link

voney commented Jun 15, 2023

Do you know if this is currently possible using the sqlfluff command line tool? If so I will look into this further, but if it is not possible using the command line tool then you should create an issue on the sqlfluff repository.

This is possible, you'd use - as the path for the file and pass it in via stdin.

@RobertOstermann
Copy link
Contributor

@voney Thanks for the info, that was helpful in completing this issue.

@rubensa Format Selection should be available in v2.3.5. It currently does not work if you have the executeInTerminal setting enabled, but otherwise should work.

@rubensa
Copy link
Contributor Author

rubensa commented Jul 15, 2023

Great! Thanks @RobertOstermann

@rubensa rubensa closed this as completed Jul 15, 2023
@RobertOstermann
Copy link
Contributor

@rubensa just noticed that you wanted to be able to do this in any open file, currently it works like format document and is only available for sql files. I will look into adding formatting for any file, or maybe add a setting that lets you choose the files it can format.

@RobertOstermann
Copy link
Contributor

@rubensa I have release v2.3.6 which adds the settings sqlfluff.format.languages and sqlfluff.linter.languages, you can set which languages this extension works with using those two settings.

@rubensa
Copy link
Contributor Author

rubensa commented Jul 15, 2023

Thanks @RobertOstermann
I'll try to check it when I have some time.

@voney
Copy link

voney commented Jul 16, 2023

@RobertOstermann great news, glad I could help. :)

@rubensa
Copy link
Contributor Author

rubensa commented Jul 25, 2023

Hi @RobertOstermann

Just checked the format selection option.

In settings I set:

        "sqlfluff.format.languages": [
          "sql",
          "snowflake-sql",
          "yaml"
        ],

then opened a .yml file, selected a piece of SQL inside the YAML file and I was able to right click and select Format Selection [Ctrl+K Ctrl+F].

The selection was formatted as expected!!

Only a suggestion... Should it be possible to ad an option to keep original indentation?

I mean, in my case, the SQL is under a YAML tag like:

- changeSet:
    id: createView-example
    author: liquibase-docs
    changes:
    - createView:
        catalogName: cat
        encoding: UTF-8
        fullDefinition: false
        path: A String
        relativeToChangelogFile: true
        remarks:  A String
        replaceIfExists:  false
        schemaName:  public
        selectQuery:
          select id, name from person where id > 10
        viewName:  v_person

When I select the select id, name from person where id > 10 line and format it I get:

- changeSet:
    id: createView-example
    author: liquibase-docs
    changes:
    - createView:
        catalogName: cat
        encoding: UTF-8
        fullDefinition: false
        path: A String
        relativeToChangelogFile: true
        remarks:  A String
        replaceIfExists:  false
        schemaName:  public
        selectQuery:
select
  id,
  name
from person where id > 10
        viewName:  v_person

Should It bee possible to get this instead (applying the original indentation to each resulting formatted SQL line):

- changeSet:
    id: createView-example
    author: liquibase-docs
    changes:
    - createView:
        catalogName: cat
        encoding: UTF-8
        fullDefinition: false
        path: A String
        relativeToChangelogFile: true
        remarks:  A String
        replaceIfExists:  false
        schemaName:  public
        selectQuery:
          select
            id,
            name
          from person where id > 10
        viewName:  v_person

@RobertOstermann
Copy link
Contributor

@rubensa Yeah that makes sense, not sure if it would be difficult or not, but I'll look into it.

@rubensa
Copy link
Contributor Author

rubensa commented Jul 26, 2023

Also on this... Should It be possible to allow only the Format Selection [Ctrl+K Ctrl+F] to be associated with vscode-sqlfluff?

I mean... I have both redhat.vscode-yaml and dorzey.vscode-sqlfluff extensions installed.
Once I add yaml to sqlfluff.format.languages if I do a Format Document VSCode asks me to associate the YAML files with one of the formatters.
If I associate it with YAML I can format the full YAML document but not the selection.
If I associate it with sqlfluff I can format the selection but not the full document.

@RobertOstermann
Copy link
Contributor

I am not sure if that is possible in VSCode. Do you know if there is a separate setting for the default format on selection?

@rubensa
Copy link
Contributor Author

rubensa commented Jul 26, 2023

Looks like no.

But I think It should be possible to add a specific vscode-sqlfluff option (like Format with sqlfluff) when you right click on the selection. Isn't it?

@RobertOstermann
Copy link
Contributor

Ah yes, I forgot about that. It would be possible to add and option to the context menu. I would have to look into that a bit more as well before committing to it though. I want to make sure I can put it behind a setting and default it to false, I think in most cases people would not want the extra context menu items showing up, the context menu already gets crowded as is.

@RobertOstermann
Copy link
Contributor

RobertOstermann commented Jul 27, 2023

@rubensa v2.4.0 has updates which should address your concerns.

The sqlfluff.format.languages now allows for objects alongside strings. These objects have additional settings for whitespace preservation and context menu items. You can also use the SQLFluff Format Selection command now, so if you wanted to add a key binding you can. Let me know if these settings work as expected.

"sqlfluff.format.languages": [
    {
      "language": "markdown" ,
      "contextMenuFormatOptions": false,
      "preserveLeadingWhitespace": true
    }
  ],

@rubensa
Copy link
Contributor Author

rubensa commented Jul 27, 2023

Thanks @RobertOstermann

I just checked but looks like there are some problems.

I have this in settings:

  "sqlfluff.format.languages": [
    {
      "language": "yaml",
      "contextMenuFormatOptions": true,
      "preserveLeadingWhitespace": true
    }
  ]

If I select the following (like in previous example):
Screenshot from 2023-07-27 07-40-59

right click and select SQLFluff Format Selection the result is like:
Screenshot from 2023-07-27 07-43-00

This is the OUTPUT:

------------------------------------------------------------

Range (Lines 14 to 15) Format triggered for /workspaces/sample/test.yml

------------------------------------------------------------

Reading from stdin, not file, input may be dirty/partial

--------------------Executing Command--------------------

/opt/conda/envs/dev/bin/sqlfluff fix --force --dialect snowflake --exclude-rules RF01 -

------------------------------------------------------------

Received close event, code 1 signal null
Raw stdout output:

------------------------------------------------------------

            select id, name from person where id > 10
          viewName: v_person

------------------------------------------------------------

Raw stderr output:

------------------------------------------------------------

  [1 templating/parsing errors found]


------------------------------------------------------------

Looks like two problems here.

  • First, the range selection: Looks like uses only line number, no positions (so if you select the full line and the cursor is as the beginning of next line -no characters selected on that line-, next line is taken as part of the selection)
  • Second, despite there are errors, and the text is not formatted the extensions adds leading spaces

@RobertOstermann
Copy link
Contributor

@rubensa ok, I will get those fixed hopefully this weekend.

For the first problem, I changed the range to be full lines to ensure I was getting the whitespace correctly on the first line. I probably will change that have the start of the range always be at the beginning of the line and the end of the range be wherever the cursor is. Would that work?

For the second I think I will just need to check if there are already leading spaces and if so do not add the whitespace. It might be a bit more complicated though, I'm not sure what edge cases there could be.

@rubensa
Copy link
Contributor Author

rubensa commented Jul 27, 2023

Again, thanks @RobertOstermann

For the first, I think that you should check if the last line contains any character selection and only take it into account if that's the case.

For the second, I think that if there are errors (so it is not formatted) just keep the original (do not modify it adding leading spaces, it's just doing nothing -but showing the error message-).

@RobertOstermann
Copy link
Contributor

@rubensa Made some fixes in v2.4.1, I didn't test them too much so hopefully they work. Let me know if they cause you any problems

@rubensa
Copy link
Contributor Author

rubensa commented Jul 28, 2023

Just tested and is working like a charm.

Again, thanks @RobertOstermann

@rubensa rubensa closed this as completed Jul 28, 2023
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

3 participants