Add support for lsp_code_actions_on_format#2747
Add support for lsp_code_actions_on_format#2747mj026 wants to merge 8 commits intosublimelsp:mainfrom
lsp_code_actions_on_format#2747Conversation
Add a reusable CodeActionTaskBase class which can be used for both CodeActionOnSaveTask and CodeActionsOnFormatTask
So this can be used for the LspFormatDocumentCommand
rchl
left a comment
There was a problem hiding this comment.
LSP.sublime-settings and sublime-package.json should also be updated.
plugin/code_actions.py
Outdated
| The amount of time the task is allowed to run is defined by user-controlled setting. If the task | ||
| runs longer, the native save will be triggered before waiting for results. | ||
| """ | ||
| CODE_ACTIONS = "lsp_code_actions_on_save" |
There was a problem hiding this comment.
I would probably name it just SETTING_NAME, but I guess it isn't too important.
There was a problem hiding this comment.
I would be fine with that too.
It has to be a specifically structured setting so I was thinking of more specific name but it's not easy.
There was a problem hiding this comment.
I’ll change it into SETTING_NAME, feels like the most obvious name for it.
|
Thank you for reviewing so fast! Will work on your suggestions the following day(s) and update the PR accordingly. |
- Make "on_tasks_completed" a "public" hook - Add a overridable "on_before_tasks" hook - Make SaveTasksRunner a hidden implementation detail - Share the "run" method for both LspSaveCommand and LspFormatDocumentCommand - Pass **kwargs instead of kwargs to the "on_tasks_completed" hook so we can unpack specific keyword arguments easily (for example, the "select" parameter)
|
Hi @rchl, I implemented all your suggestions: I like it now how Let me know if you need anything else for this PR to go through. |
| }, | ||
| "markdownDescription": "A dictionary of code action identifiers that should be triggered on save.\n\nCode action identifiers are not officially standardized so refer to specific server's documentation on what is supported but `source.fixAll` is commonly used to apply fix-on-save code actions.\n\nThis option is also supported in syntax-specific settings and/or in the `\"settings\"` section of project files. Settings from all those places will be merged and more specific (syntax and project) settings will override less specific (from LSP or Sublime settings).\n\nOnly \"source.*\" actions are supported." | ||
| }, | ||
| "lsp_code_actions_on_format": { |
There was a problem hiding this comment.
Needs to be referenced here also:
Lines 474 to 478 in 57388ca
| def on_before_tasks(self) -> None: | ||
| if self._save_tasks_runner: | ||
| self._save_tasks_runner.cancel() | ||
| sublime.set_timeout_async(self._trigger_on_pre_save_async) | ||
| self._save_tasks_runner = SaveTasksRunner(self, self._tasks, partial(self._on_tasks_completed, kwargs)) | ||
|
|
||
| def on_tasks_completed(self, **kwargs: dict[str, Any]) -> None: | ||
| self._save_tasks_runner = None |
There was a problem hiding this comment.
I'm thinking that the implementation contract could be made even more obvious by having abstract methods in the base that would have to be implemented (and wouldn't have to call super() which can be error-prone).
I can push the changes later to avoid back and forth.
There was a problem hiding this comment.
I get your point. I’ll see what I can think of to make this a bit more robust.
| r("log_debug", False) | ||
| r("log_max_size", 8 * 1024) | ||
| r("lsp_code_actions_on_save", {}) | ||
| r("lsp_code_actions_on_format", {}) |
There was a problem hiding this comment.
These seem to be ordered alphabetically (also for the variable name above).
f806ca0 to
90081a4
Compare
This PR allows the execution of (user defined) code actions before formatting when the
LSP: Format Filecommand is triggered by using a new settinglsp_code_actions_on_format.Implemented as described in issue #2701.