'mini.files' + LSP

[echasnovski]

Right now, 'mini.files' has events triggered after successful file system action is performed. It seems to me like the best design to allow any user-specific actions which should follow file system manipulation. In particular, LSP actions via DidCreateFiles/DidRenameFiles/DidDeleteFiles.

If anybody has ideas/snippets/results of how this can be implements, lets discuss them here.

Relevant discussions: #377 (in particular, this comment and this comment), #378.

[echasnovski]

Answer to this comment:

you already list all planned actions for the user to confirm, which may be wrong? So what's wrong with having that programmatically. Just like the other problems with unexpected fs actions mentioned above, the user can just synchronize more often right?

Those are indeed only planned actions which 'mini.files' will try to do.

More frequent synchronization will not solve all problems: moving directory inside itslef is not possible, manipulating files/directories inside directory without write permissions, etc.

As it is, the functionality of lsp refactoring is completely impossible, with the pre action callback it becomes possible but with some edge cases that you can just let the user deal with in the callback that they are writing. All that is needed on mini.files side is the callback with the information, a couple of lines of code. I don't think it's a concern/responsibility of the plugin that the user could make mistakes with the callback. This is always a problem with file explorers and their operations, but atleast in this case you don't have to think about it yourself just let the user worry about it.

I don't really understand, is DidCreateFiles/DidRenameFiles/DidDeleteFiles not enough for Lsp refactoring?

[IndianBoy42]

No, the did*Files notifications are just for notifying the server that some fs operation was done. It is a notification not a request, so there is no response to the client. It is only if the server/needs or wants it to update some internal state itself.

the will*Files methods are where LSPs return workspaceEdits, in the same way as code actions or variable rename requests, that the client should apply before doing the fs operation, according to the specification. Thats the spec, so all servers will do their computations and return edits assuming none of the fs operations are done yet.

Those are indeed only planned actions which 'mini.files' will try to do.

Right, so whats the difference with the user seeing that and assuming they will be done, versus the callback (user written code) seeing that and assuming they will be done? If there is a mistake there is a mistake, letting the user be in control of it (that is including the users code). The user can just undo any edits. Perhaps an on failure event might be useful if we want to rollback the workspace edits, but I don't know if any other editor does that.

[IndianBoy42]

Als, we should focus on when/whether file creation, deletion or renaming can fail. Moving files seems harder to detect possible failures, but that is not a lsp request actually. (regardless if the user wants to do things based on the files moving then its their responsibility and I think thats fine for a plugin that wants to stay as a minimal and extensible core)

[echasnovski]

No, the did*Files notifications are just for notifying the server that some fs operation was done. It is a notification not a request, so there is no response to the client. It is only if the server/needs or wants it to update some internal state itself.

the will*Files methods are where LSPs return workspaceEdits, in the same way as code actions or variable rename requests, that the client should apply before doing the fs operation, according to the specification. Thats the spec, so all servers will do their computations and return edits assuming none of the fs operations are done yet.

That's... a very special design choice. I can't think of any its benefit compared to reverse design (did*Files perform actions, will*Files is a notification that something will/might happen).

Those are indeed only planned actions which 'mini.files' will try to do.

Right, so whats the difference with the user seeing that and assuming they will be done, versus the callback (user written code) seeing that and assuming they will be done? If there is a mistake there is a mistake, letting the user be in control of it (that is including the users code). The user can just undo any edits. Perhaps an on failure event might be useful if we want to rollback the workspace edits, but I don't know if any other editor does that.

The difference is that with user interaction they can see right away what was actually done. In the callback there won't be such a feedback and will result essentially result into workspace edits on an action which wasn't actually done. And those edits can be hard to track, as far as I understand.

Als, we should focus on when/whether file creation, deletion or renaming can fail. Moving files seems harder to detect possible failures, but that is not a lsp request actually. (regardless if the user wants to do things based on the files moving then its their responsibility and I think thats fine for a plugin that wants to stay as a minimal and extensible core)

I think actually trying to use present post-events to make will*Files requests is best here. I don't think it should fail miserably, at least. For example, I've looked at rust-lang/rust-analyzer#7009 and it seems to compute workspace edits only based on request information.

[IndianBoy42]

The difference is that with user interaction they can see right away what was actually done. In the callback there won't be such a feedback and will result essentially result into workspace edits on an action which wasn't actually done. And those edits can be hard to track, as far as I understand.

Looking at every other lsp client implementation I can find this isn't a concern anyone else seems to worry about. Apart from some basic sanity checks to the fs operation.

That's... a very special design choice. I can't think of any its benefit compared to reverse design (didFiles perform actions, willFiles is a notification that something will/might happen).

Having willRenameFiles return the edit allows the editor to preview the refactoring changes before making any changes. All the did* notifications are just to keep the server in sync with the filesystem state (since the protocol is designed to work even if the server doesn't have direct access to the client filesystem, although these notifications are actually not necessary in most cases)

I think actually trying to use present post-events to make will*Files requests is best here. I don't think it should fail miserably, at least.

I'd obviously rather follow the spec rather than pray that everything makes sense when done out of order. That will be even more invisible and hard to detect as a failure. Also since they are autocmds and asynchronous then applying workspaceEdits from here seems like a bad idea too. Again not sure about any of this because its far out of how things are 'supposed' to be done and the concerns are all in edge cases so it would be extremely hard to test. Going with the way that every other editor does it is the simplest way.

The difference is that with user interaction they can see right away what was actually done. In the callback there won't be such a feedback and will result essentially result into workspace edits on an action which wasn't actually done. And those edits can be hard to track, as far as I understand.

I still think letting users have the freedom to customize (and make mistakes) is valuable. Obviously we can never predict what will happen when it comes to fs operations, but version control, undo history and backups exist. None of this is fatal or so precious that we have to protect the user from themselves. I understand if its a big coding/maintenance burden that not every customization point can be added, but in this case just one callback covers the major use case.

If the pre operation callbacks are early enough (pre confirmation) then the callback can pop up some floating windows with previews of the edits.

[echasnovski]

Looking at every other lsp client implementation I can find this isn't a concern anyone else seems to worry about. Apart from some basic sanity checks to the fs operation.

This is exactly my concern, because they seem to rely on the fact that will*Files are emitted prior to actions which are assumed to succeed. 'mini.files' won't give such guarantees.

Having willRenameFiles return the edit allows the editor to preview the refactoring changes before making any changes.

I see. This does make some sense, albeit slightly confusing: why this can't be done in did*Files? I understand that this is the specification right now and more confused towards its design decisions.

I still think letting users have the freedom to customize (and make mistakes) is valuable. Obviously we can never predict what will happen when it comes to fs operations, but version control, undo history and backups exist. None of this is fatal or so precious that we have to protect the user from themselves. I understand if its a big coding/maintenance burden that not every customization point can be added, but in this case just one callback covers the major use case.

If the pre operation callbacks are early enough (pre confirmation) then the callback can pop up some floating windows with previews of the edits.

The only possible design I consider here is to trigger event prior executing all actions, meaning after confirmation.

---

Let's take this into more grounded conversation.

I won't implement this LSP functionality. If you want to try doing it, tell me what you might need from code and I'll help you making proper change to 'mini.files' source. Once you implement something you are happy with, we'll potentially iterate about the best way to merge those changes into 'mini.files' and celebrate the new functionality.

Right now I understand that single event with all future attempted file system actions should be enough, right? If so, add the following line as the first command of H.fs_actions_apply():

  H.trigger_event('MiniFilesActionPre', { action = 'pre', fs_actions = fs_actions })

This will trigger event prior to attempting other actions. fs_actions is a table with field for every future action. create and delete are arrays of paths, others - arrays of tables with from and to fields.

Does this sound good?