search

microsoft / language-server-protocol

Defines a common protocol for language servers.
https://microsoft.github.io/language-server-protocol/
Creative Commons Attribution 4.0 International
11.22k stars 798 forks source link

provide a way to ask client to show modal message box #1337

Open heejaechang opened 3 years ago

heejaechang commented 3 years ago

currently, ShowXXXMessage API from RemoteWindow doesn't let server ask client to show modal message box and it is completely up to client.

It would be nice if one can ask client to show modal window instead of leaving it to client.

the usage case for us is we need to decide whether we should proceed with some expansive refactoring after user action, so we want to ask the user but prevent the user from changing code before answer.

rwols commented 3 years ago

Is this a duplicate of https://github.com/microsoft/language-server-protocol/issues/1164?

heejaechang commented 3 years ago

@rwols that would be superset of what I am asking. since the issue is asking much more than what I am asking. probably requiring a lot more discussion. ShowXXXMessage already exist and having MessageOption like what vscode offer seems much smaller, incremental change that doesn't have big impact.

dbaeumer commented 3 years ago

I personally think it is a bad idea to let servers open modal dialogs. The reason is that modal dialogs should only be shown in close relation to a user UI interaction. Modal dialogs that pop up without that close relation ship cause lots of confusion (I once did this in ESLint and got very bad feedback for it :-)).

So I would recommend that you in the extension code intercept the rename and might do some custom handling and show the modal dialog there. Would that work for you?

heejaechang commented 3 years ago

@dbaeumer ya, custom LSP and command is what I am doing for now, but that basically requires adding this special handling for all clients that we want to support this feature. that was the part I would like to remove in future. hence the asking.

dbaeumer commented 3 years ago

I see your point but allowing servers to open a modal dialogs is really something I would like to avoid.

DanTup commented 3 years ago

I don't know if it'd work for you, but Dart uses the document version number to confirm the file was not modified while waiting for a response from refactor confirmations:

https://github.com/dart-lang/sdk/blob/3aa464af26a4467d3977150108c1a9c7b0b1a53a/pkg/analysis_server/lib/src/lsp/handlers/handler_rename.dart#L171-L204

I think this would work better than a modal dialog, as the user may have already modified the file in between the refactor request being sent to the server, and the servers request for a modal being processed by the client (and while the server can delay processing that edit, the refactor would still compute edits based on contents that are different to the editor has, so they'd either be rejected anyway, or potentially corrupt the users code).

heejaechang commented 3 years ago

our rename touches more than one file in the workspace (open or closed) so simple client version check doesn't work, but feels like checking might be better than modal window if there is a way.

DanTup commented 3 years ago

If you really want to invalidate the rename on any file edit, perhaps you could keep a single global "workspace version" that you just increase on each textDocument/change (and perhaps open if the content may vary from what's on disk) and then grab that before/after the prompt to see if anything was changed.

Of course, neither that or a modal prompt would prevent files being modified on-disk outside of the editor during that period.

michaelmesser commented 2 years ago

I personally think it is a bad idea to let servers open modal dialogs. The reason is that modal dialogs should only be shown in close relation to a user UI interaction. Modal dialogs that pop up without that close relation ship cause lots of confusion (I once did this in ESLint and got very bad feedback for it :-)).

Just because modal dialogs are the wrong solution to some problems, doesn't need they aren't the right solution to other problems.

So I would recommend that you in the extension code intercept the rename and might do some custom handling and show the modal dialog there. Would that work for you?

Isn't the point of LSP to not need custom code in extensions?

dbaeumer commented 2 years ago

Isn't the point of LSP to not need custom code in extensions?

LSP is about standardizing programming language messages and not UI. I would like to leave the UI part to the client.