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

VS Code extension #366

Open
8 of 12 tasks
benfdking opened this issue Jun 10, 2024 · 15 comments · Fixed by #369
Open
8 of 12 tasks

VS Code extension #366

benfdking opened this issue Jun 10, 2024 · 15 comments · Fixed by #369
Assignees
Labels
enhancement New feature or request

Comments

@benfdking
Copy link
Collaborator

benfdking commented Jun 10, 2024

We should be able to package the whole thing in WASI. I was looking at this guide: https://code.visualstudio.com/blogs/2024/06/07/wasm-part2

  • CI System to run and check this
    • Build and lint
  • CI System that deploys this
  • I do think we should register a formatter which this doesn't yet accomplish
  • Document well how to run this locally
  • Open in a folder where there is a SQL file to test this.
  • Are we picking up the .sqlfluff config files in web version?
  • Add readme page to the extension
  • Reading the config can cause a panic (for example, if the dialect is not supported). Make this more stable.
  • apply fixes individually, similar to quick fixes from rust-analyzer
  • Local doesn't seem to be working on desktop
  • online when you run too many, the u32 runs out
@gvozdvmozgu
Copy link
Collaborator

WASI needs an extension from here, but it only works with VS Code Insiders. I think I'll try using regular WASM instead

@benfdking
Copy link
Collaborator Author

We've got a setup working for quary itself. It's pretty fiddly but you should be able to copy the setup.

@benfdking
Copy link
Collaborator Author

Are probably the two main building blocks.

We essentially read wasm into a string because for web extensions you need to deploy it as a single js file that gets loaded and then you can call the functions.

Sorry, should have mentioned that this should really work as a VSCode web extension.

@benfdking benfdking moved this to In progress in sqruff roadmap 🗺️ Jun 13, 2024
@benfdking benfdking added the enhancement New feature or request label Jun 13, 2024
@benfdking benfdking linked a pull request Jun 13, 2024 that will close this issue
@github-project-automation github-project-automation bot moved this from In progress to Done in sqruff roadmap 🗺️ Jun 16, 2024
@benfdking
Copy link
Collaborator Author

benfdking commented Jun 16, 2024

I think there are a few things we should improve, but in the meantime, we should merge this:

  • CI System to run and check this
    • Build and lint
  • CI System that deploys this
  • I do think we should register a formatter which this doesn't yet accomplish
  • Document well how to run this locally
  • Open in a folder where there is a SQL file to test this.
  • Are we picking up the .sqlfluff config files in web version?

Ignore this for authoritative list at the top.

@benfdking benfdking reopened this Jun 16, 2024
@benfdking benfdking moved this from Done to In progress in sqruff roadmap 🗺️ Jun 17, 2024
@benfdking
Copy link
Collaborator Author

In deployed version when running on mac locally, getting the following error:

image

@benfdking
Copy link
Collaborator Author

Here's the extension by the way https://marketplace.visualstudio.com/items?itemName=Quary.sqruff

@gvozdvmozgu
Copy link
Collaborator

I can't find an API for file system access in the browser. I've checked extensions that work in the browser, and they seem limited to user-opened files. It looks like there isn't a file system API available.

@benfdking
Copy link
Collaborator Author

If you look at

https://github.com/quarylabs/quary/blob/main/js/packages/quary-extension/src/web/servicesRustWasm.ts
https://github.com/quarylabs/quary/blob/main/rust/wasm-binding/src/rpc_proto_scaffolding.rs

we have created a "fileSystem" that is passed in through JS function calls? Do you think it would be possible to implement the same thing?

@gvozdvmozgu
Copy link
Collaborator

image
I tried using vscode.workspace.fs, but I get this error (maybe because we are in a WebWorker?). I'll try to find a workaround again

@gvozdvmozgu
Copy link
Collaborator

It seems that it's possible to send a request from lsp-worker to browser.ts.

lsp-worker.ts:

async function load_file(path: string): Promise<string> {
    return await connection.sendRequest("load_file", path);
}

browser.ts:

 cl.onRequest("load_file", async (param: string) => {
     let contents = await vscode.workspace.fs.readFile(Uri.parse(param, true));
     return new TextDecoder().decode(contents);
  });

I am not sure if this is correct and if I fully understand the nature of the error above, but I will settle for this option.

@benfdking
Copy link
Collaborator Author

Nice work @gvozdvmozgu! I've copied the list of "tasks" at the top. Anything else you can think of?

@gvozdvmozgu
Copy link
Collaborator

Reading the config can cause a panic (for example, if the dialect is not supported). It would be great to have the ability to apply fixes individually, similar to quick fixes from rust-analyzer.

@enricoschaaf
Copy link

is there any way to test the vscode extension? I was not able to set it up #933

@amoralesc
Copy link

amoralesc commented Nov 28, 2024

This feels like a good place to ask feature requests for the VSCode extension.

Could the extension support settings customization? The sqlfluff extension includes multiple settings that help adjust the behavior of the program. I think some of the most important include:

  • sqlfluff.config: Specify an additional configuration file that overrides the standard configuration files. This one is useful for not having to place the config file at the project root, but at a custom location, like under a .config directory.
  • sqlfluff.executablePath: Points to the sqlfluff executable. It's not immediately clear in sqruff's README if a sqruff executable is bundled with the extension or if one is used from the path. Some other popular tools in Rust like Ruff (Python linter/formatter) include both options (bundling one with the extension, but also allowing the user to bring one from the environment). I really like this approach because the user can always customize the version of the package when necessary, but can also use whatever comes with the extension for a quick fix.
  • sqlfluff.format.languages: The languages formatting is enabled for. Also has one for the linter (sqlfluff.linter.languages). The activation event for this extension is on SQL files. But the default SQL support on VSCode is usually not enough when using a dialect (like Postgres). sqlfluff actually has a custom activation event, I guess to support the languages programmatically. This setting would then be really useful when the SQL languages are changed programmatically. For example, .sql files are picked up as sql language by default. However, I actually like to set file associations for .sql files as postgres. If I have to set the default formatter for Postgres files as this extension, I would then do:
    // settings.json
    {
      // associate *.sql files to postgres
      "files.associations": {
        "*.sql": "postgres"
      },
      // set sqruff as the formatter for postgres files
      "[postgres]": {
        "editor.defaultFormatter": "Quary.sqruff",
        "editor.formatOnSave": true,
        "editor.codeActionsOnSave": {
          "source.fixAll": "explicit"
        }
      }
    }
    But I'm not sure this extension will actually understand the Postgres language.
  • sqlfluff.format.arguments: This is useful for setting extra arguments for the sqlfluff fix command. Include --force if running sqlfluff < 3.0.0. Also has one for the linter (sqlfluff.linter.arguments). The settings are good when the flags modify the behavior of the linter/formatter.
  • sqlfluff.linter.run: Determines if the linter runs on save, on type, or disabled. Some other linters that include this setting are ESLint and Ruff. Also has a similar one for the formatter to enable/disable it (sqlfluff.format.enabled).
  • Then there is a bunch of settings that basically simulate the use of the config file, without actually using one. Enabling rules, disabling rules, ignoring files. Cool, but the config file is better for this (Consistency on VSCode, other editors and on CI).

A custom README for the extension would be really useful too!

@amoralesc
Copy link

And sorry for the off topic comment, but this seems like a great tool! I love projects that improve over the big giants that aren't worrying about a modern, fast approach to developer UX. I think a parallel I see with sqruff to sqlfluff is the one happening at Python with ruff to black/flake (and hopefully uv to poetry).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: In progress
Development

Successfully merging a pull request may close this issue.

4 participants