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

Not able to send a custom "organize imports" command due to missing template variable #2515

Closed
Mister7F opened this issue Sep 21, 2024 · 3 comments · Fixed by #2516
Closed

Comments

@Mister7F
Copy link

I would be cool be be able to sort import with a shortcut

    {
        "keys": ["primary+b"],
        "command": "lsp_execute",
        "args": {
          "command_name": "ruff.applyOrganizeImports",
          "command_args": [{ "uri": "file:///plugin/execute_command.py", "version": 123 }],
          "session_name": "LSP-ruff"
        }
    },

It kinda work like that (see discussion on sublime text discord), but it's buggy

It can be something like

  {
    "keys": [
      "primary+b"
    ],
    "command": "lsp_organize_imports",
  },

(or even be chained with lsp_format_document)

That example is for ruff, not sure if it can be generic for all LSP

@jwortmann
Copy link
Member

jwortmann commented Sep 21, 2024

The command name (ruff.applyOrganizeImports) and the command args for the workspace/executeCommand request are not standardized, i.e. they are server specific. Therefore I don't think this would be possible to implement in a generic way. (And there is probably only a limited amount of language servers which provide an "organize imports" or similar code action.)

I don't use Discord so I don't know what was discussed there, but from your code block it seems like for Ruff you need to provide the document URI and the version as command args. You probably don't want to hardcode those in your key binding; for the URI you can use "$file_uri" as a variable instead and it will be resolved dynamically. See https://lsp.sublimetext.io/commands/#execute-server-commands for an example and for more available variables of lsp_execute.

Still missing here would be a variable for the "version". But I guess it should be easy to add that as a new variable "$file_version". Is this what you need?

Edit: Actually I see now that the command args is a single entry with an object/dict. So this would be a VersionedTextDocumentIdentifier, and we could add a new varible for that one.

@rchl
Copy link
Member

rchl commented Sep 21, 2024

I've said on Discord that in this case the variables wouldn't work as this is an object and we don't do "deep" processing of dicts like that. But your observation that it's just a versioned document identifier will actually make that work, without having to "deep" process variables.

(separate discussion but I'm not sure if chaining would work since we would likely trigger formatting before receiving response for the previous request)

@rchl rchl changed the title lsp_organize_imports command Not able to send a custom "organize imports" command due to missing template variable Sep 21, 2024
@jwortmann
Copy link
Member

I made a PR to add it as a new variable, which should probably be sufficient here.

For the chaining I think we would need something like a new custom lsp_chain command, which queues the individual commands together and only runs the next command after the previous one has finished its response handler (if it is a LSP command that sends a request). Then the command arguments could be resolved dynamically with the updated document version. And something similar would probably also be needed for the multiple code actions on save to resolve #2510. But I haven't contemplated yet how exactly it could be implemented.

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

Successfully merging a pull request may close this issue.

3 participants