clarification: what response to ShowMessageRequest in edge cases?

[ljw1004]

Q1. If the LSP server issues window/showMessageRequest with the actions = ['hello'], should the client show a normal Close button (in whatever way is standard for the OS, e.g. a cross to the top right) in addition to the Hello button? If the user clicks this Close glyph, then what should be sent as the result?

Q2. If the client shouldn't show a normal Close button by default, then is there any way for the LSP server to request one to be shown?

Q3. If the LSP server issues window/showMessageRequest with the actions? field set to empty, what is the result? The documentation says the result is "the selected MessageActionItem". Should the client send back an error message as soon as a request is received with empty actions field? (If so, then why is the actions field marked as optional?)

Q4. The spec says that the response to window/showMessageRequest should be "the selected MessageActionItem". If the server happened to send a MessageActionItem which contained additional fields beyond just the specified title, is the client required to parrot those extra fields back to the server too?

[dbaeumer]

@ljw1004 agree that this needs better speced. Here is what I think we should do:

• the message dialog should show a close button.
• if the close button is pressed we should return null
• we should allow to mark a MessageActionItem as the close affordance. In this case the dialog shouldn't show a close button.

What do you think ?

[ljw1004]

I looked at what VSCode currently implements. It has some weird behavior:

• VSCode fails to respect cancellation: if the LSP server sends a window/showMessageRequest to pop up a dialog, and later sends a $/cancelRequest notification to cancel that request, then I expect VSCode to close the dialog. But VSCode doesn't do anything; it just leaves the dialog up. (The relevant method in vscode-languageserver-node/client doesn't pass on a cancellation token to VSCode).
• VSCode is just weird when two dialogs have the same message text, as illustrated by this sequence of events

1. LSP sends window/showMessageRequest with message text "ALPHA" -- VSCode displays it.
2. LSP sends window/showMessageRequest with message text "BETA" -- VSCode displays it too, so now there are two dialogs up.
3. LSP sends window/showMessageRequest with message text "ALPHA" -- VSCode actually hides the dialog from step [1] but without responding to it yet, and displays the dialog from step [3].
4. User clicks a button on the dialog from step [3] -- VSCode first sends the correct MessageActionItem for the dialog in step [3], and then it sends null for the dialog in step [1].

What should have happened is that VSCode should have responded null to dialog#1 at the moment it hid it to display dialog#3. It should not have delayed that null until after dialog#3 had been responded to.

Why is this important? My LSP server takes a minute to get ready, so I want it to display a dialog saying "Language server is busy [Close]", and then the LSP server itself will close (i.e. cancel) the dialog once it's done. I initially had it display this message just to the console, but users overwhelmingly preferred to see a toast notification. But if VSCode can't handle cancellation, then this won't work.

Here's an illustration of the weird VSCode behavior. The repro code is below.

REPRO CODE: implement a language server per the VSCode tutorial https://code.visualstudio.com/docs/extensions/example-language-server

Then replace its onCompletion handler with the following.

let cts = new CancellationTokenSource();

// This handler provides the initial list of the completion items.
connection.onCompletion((textDocumentPosition: TextDocumentPositionParams): CompletionItem[] | ResponseError<void> => {
	connection.console.log('char ' + String(textDocumentPosition.position.character));
	let actions = [{ title: "alpha", context: 15 }, { title: "beta", context: 12 }];
	if (textDocumentPosition.position.character === 4) {
		let params = { type: MessageType.Warning, message: "greetX", actions };
		let req = connection.sendRequest('window/showMessageRequest', params);
		connection.console.log('LSP sent request4 to VSCode: "greetX"');
		req.then(resp => connection.console.log('result4: ' + JSON.stringify(resp)));
	}
	else if (textDocumentPosition.position.character === 6) {
		let params = { type: MessageType.Warning, message: "greetY", actions };
		let req = connection.sendRequest('window/showMessageRequest', params);
		connection.console.log('LSP sent request6 to VSCode: "greetY"');
		req.then(resp => connection.console.log('result6: ' + JSON.stringify(resp)));
	}
	else if (textDocumentPosition.position.character === 8) {
		let params = { type: MessageType.Warning, message: "greetX", actions };
		let req = connection.sendRequest('window/showMessageRequest', params);
		connection.console.log('LSP sent request8 to VSCode: "greetX"');
		req.then(resp => connection.console.log('result8: ' + JSON.stringify(resp)));
	}
	else if (textDocumentPosition.position.character === 10) {
		cts.cancel();
		connection.console.log('LSP cancelled outstanding showMessageRequests');
	}
	return [
		{
			label: 'TypeScript',
			kind: CompletionItemKind.Text,
			data: 1
		},
		{
			label: 'JavaScript',
			kind: CompletionItemKind.Text,
			data: 2
		}
	]
});

[ljw1004]

For your other comments, @dbaeumer,

• VSCode always shows a close button. I think your clarification "client should always show a close button" is fine.
• If the close button is pressed then VSCode returns null. I think your clarification "client return null if close button is pressed" is fine.
• If any other button is pressed, then VSCode returns in the exact same object that you passed to it, including any fields you supplied in it that AREN'T part of the MessageActionItem type. This is perfect and necessary. For instance one of my action items can be {title:"Go!", context:42} and it it will send back the correct context number when I click Go. This fact is not clearly explained in the current LSP spec. (The spec does say it will return "the selected MessageItem" but doesn't allow for you to send more information than the MessageItem).

Your final clarification I wasn't sure how to read. Let's be sure we're on the same page. You suggested "we should allow to mark a MessageActionItem as the close affordance. In this case the dialog shouldn't show a close button"

What I think this probably means:

1. The client will always have a Close affordance. This might appear as an "X" icon in the titlebar (e.g. Windows), or as a red circle in the titlebar (e.g. Mac), or as a "Close" button that looks similar to the other buttons (e.g. VSCode today, since it pops up a dialog within Electron that lacks any other Close affordance).
2. The client is at liberty to display multiple equivalent Close affordances, e.g. a "Close" button in addition to the normal "X", or (in traditional Windows dialog style) a "Cancel" button in addition to the normal "X" in addition to the normal system-menu.
3. MessageActionItem should have an optional isClose?:boolean member. It is an error for the LSP server to send more than one MessageActionItem with isClose set to true in a given showMessageRequest.
4. If the user uses the Close affordance, that will respond with the one MessageActionItem that had isClose==true. Or, if there were no such MessagActionItems, then it will respond with null.

I think parts [3,4] aren't necessary and shouldn't be included in LSP. Instead, I think the LSP server should always recognize null as a valid response and should deal with it accordingly.

[dbaeumer]

@ljw1004 very nice analysis

@bpasero any comments on the strange behavior we have in terms of #230 (comment)

[bpasero]

Please isolate a reproducible case if something is wrong with message service with regards to showing the same message multiple times and file a bug. Thanks.

[rchl]

@dbaeumer I've noticed that in vscode-eslint you are now sending MessageActionItem with an id property in addition to title and expecting to get back id on choosing that action.

As explained in this issue, the MessageActionItem is specced as only containing the title property so it's not clear what to send back to the server.

Could this part of the issue be specified better at least?