lsp_save_all or a way to enable lsp_code_actions_on_save for save_all

[fserb]

I usually have { "keys": ["super+s"], "command": "save_all" }, but I can't seem to find a way to make lsp_code_actions_on_save to run on save_all.

I can think of two solutions for this:

1. LSP could support a lsp_save__all command that would run LSP (when it makes sense) on all unsaved files.

2. LSP could work on regular save commands (no special lsp_save) that would hook to ST4 save events.

If there's another alternative that would also trigger it, please let me know.

[rchl]

Supporting something like lsp_save_all should be doable. Though with many unsaved files it could be a bit slow since it would have to sequentially trigger save for each file, waiting for save to complete before proceeding to the next file.

2\. LSP could work on regular save commands (no special `lsp_save`) that would hook to ST4 save events.

It's not possible with the current ST API.

[jwortmann]

It's not possible with the current ST API.

There are the following methods of sublime_plugin.ViewEventListener (and the same methods for EventListener):

• on_pre_save()
• on_pre_save_async()
• on_post_save()
• on_post_save_async()

I haven't followed the "code actions on save" PRs, so I don't know why lsp_save is necessary. Maybe you could summarize it, to save me from the effort to search through the code?

Without knowing the background of lsp_save, I would also advocate the option 2. from above, if somehow possible, due to the annoying bug #1791 of the lsp_save command (why is this issue closed when the bug still exists??).

[rchl]

on_post* happen after save so are obviously not suitable (unless we want to save the file twice which would likely be problematic for some).

on_pre_save is synchronous so it would result in a very bad UX - it would make the app freeze for a short time.

on_pre_save_async triggers asynchronously but there is no way to make the actual save operation wait for the asynchronous handler to finish. So this would also result in a double-save in the best case and in the worst with a recursive loop unless we would add some ugly globals.

So the custom command is needed so that we can perform our tasks asynchronously and then trigger the native save.

[rchl]

Feel free to thumbs-up sublimehq/sublime_text#3273 and maybe the ST API for that will be added some day.

[jwortmann]

Thanks, I understand. I guess the on_post* methods would also result in an infinite loop unless additional logic to prevent that was added. And as you already wrote in that issue, on_pre_save_async seems indeed absolutely useless to me, since it is basically the same as on_post_save_async. They should fix on_pre_save_async to wait with the save until the function has finished (wouldn't really be backward compatible, so I doubt it will happen like that, but existing packages which use it could switch to on_post_save_async in theory).

[fserb]

Yeah, the on_pre_save_async situation seems just silly.

That said, I think having a blocking lsp_save_all meanwhile would be really nice. :)

[rwols]

The problem with on_pre_save is that we'd end up using a condition variable to wait on some mutex while doing work in the async thread. But if you do that then calling any ST API function in the async thread results in a deadlock.

[rwols]

An ideal approach would be for SublimeHQ to acknowledge that code actions and formatting are concepts that are missing in Sublime Text, and write appropriate provider API endpoints. Then ST can implement the formatting/code-actions-on-save functionality, while plugins only have to respond to the provider endpoint.

[fserb]

I tried my hand at this. I ended up with a something like:

class LspSaveAllCommand(sublime_plugin.WindowCommand):
    def run(self) -> None:
        for sheet in self.window.sheets():
            sheet.view().run_command("lsp_save", None)

which mostly works, except that the focus/active view is all over the place once this is called.

Maybe some tasks of LSP assume that the current view is on focus or force it? I couldn't figure out which one yet.

I also considered saving the active_view() and restoring the focus once all lsp_saves are done. But that's extra hard, since it's all async. One option I tried was to register an EventListener for on_post_save. The problem there is that was not trivial to keep track of which view to return to (and it feels overall awful). :)

[rchl]

I don't see the view switching issue. Though if you try to save a view that is not saved on disk then that will trigger a save dialog and switch the active view. Maybe something like this would be better so that it only saves files that are modified and only if they are on disk:

class LspSaveAllCommand(sublime_plugin.WindowCommand):
    def run(self) -> None:
        for sheet in self.window.sheets():
            view = sheet.view()
            if view and view.is_dirty() and view.file_name():
                view.run_command("lsp_save", None)

[fserb]

If you edit file A, don't save, switch to file B then do lsp_save_all, file A comes back to the top. Also, if you have multiple columns, it always go back to the first one.

[fserb]

I have 3 tasks running on save:WillSaveWaitTask, FormattingTask, CodeActionOnSaveTask).

If I disable the 3rd one, the focus doesn't switch. (I'm using the gopls server)

[fserb]

Bear with me while I learn the code and try to figure this out. :)

What seems to be happening so far:
FormattingTask correctly formats everything properly (it doesn't add imports, but it does fix syntax) and if I just run it, it doesn't force the focus back.

Even when I disable FormattingTask, CodeActionOnSaveTask still formats everything, but when it runs it brings the focus to the view.

[fserb]

Ok. I think I found the problem.
CodeAction calls core.edit eventually for the changes. And at some point apply_workspace_edit will be called to apply the parsed edits.

apply_workspace_edit calls open.open_file which in turn calls sublime.Window.open_file, and according to Sublime's API: "Opens the named file, and returns the corresponding view. If the file is already opened, it will be brought to the front".

[fserb]

Aaaannnnnddd I found a fix. Sending the PR.