[Feature] Clarify the parameter of callback in source.resolve

[yioneko]

Relevant source

nvim-cmp/lua/cmp/source.lua

Lines 371 to 381 in 4c05626

	---Resolve CompletionItem
	---@param item lsp.CompletionItem
	---@param callback fun(item: lsp.CompletionItem)
	source.resolve = function(self, item, callback)
	if not self.source.resolve then
	return callback(item)
	end
	self.source:resolve(item, function(resolved_item)
	callback(resolved_item or item)
	end)
	end

The item parameter of callback could be nil, and cmp will fallback it to the original item.

nvim-cmp/lua/cmp/entry.lua

Lines 487 to 498 in 4c05626

	---Resolve completion item.
	---@param callback fun()
	entry.resolve = function(self, callback)
	if self.resolved_completion_item then
	return callback()
	end
	table.insert(self.resolved_callbacks, callback)
	if not self.resolving then
	self.resolving = true
	self.source:resolve(self.completion_item, function(completion_item)
	self.resolved_completion_item = misc.safe(completion_item) or self.completion_item

And in entry.resolve, the returned completion_item will be always truthy and self.resolved_completion_item will be set to that value either.

The problem

Whether source.resolve returns a truthy item or nil, cmp will always assume the item is resolved successfully and will never trigger resolution twice. Take cmp-nvim-lsp for example:

no-twice-resolve.mp4

If I quickly navigate menu down to let some resolution skipped (source here) and navigate back, the item resolution will not trigger again and the document still displays as the previous one's.

Possible solution?

1. Provide another callback function named cancel or reject to let the source decide which to call. Then cmp can use this information to determine whether resolution could be re-triggered.
2. Change the current behavior of cmp to not provide fallback of a nil item returned from source. Since self.resolved_completion_item is kept as nil, cmp will re-trigger source.resolve as is. But this might be a slight breaking change? I'm not sure about it.

[hrsh7th]

1. The LSP spec does not define cancel or error on completionItem/resolve
2. I can't understand the actual problem. Could you explain the problem? (completionItem/resolve can't be cancelled` is not valid explanation.)

[yioneko]

This is not relevant to LSP spec but a client side specific problem. I'll try to explain it in more details here.
entry.resolve allows the returned entry to be nil, but what does this nil value actually means? In the current implementation, cmp assumes it has been resolved and fallback to the original item.

But what if completionItem/resolve is cancelled by $/cancelRequest? In this case the returned item is also nil but it should be considered as not resolved yet, to allow the resolve to be re-triggered (navigate away then navigate back to it on the menu).

So here is just a semantic problem of a nil item returned from callback. Whether it means:

1. The source recognized it should not be resolved and request cmp to use the original value as resovled one.
2. The resolution is an uncoverable failure and cmp should fallback to the orginal value, and should never re-trigger the resolve.
3. The resolution is cancelled for perf reason but re-trigger by navigating back should be still possible.

Currently cmp assumes 1 and 2, but cannot handle 3.

[hrsh7th]

Oh. I didn't know the LSP spec supported cancellation from the server side.

Hmm... I don't mind removing the ability to treat a "null response" as success.

[yioneko]

I'm not sure if changing the fallback behavior will break some sources since it's now the source's responsibility to specify the resolve is success or failure. But I also think this is more reasonable because cmp should not make any assumption about the success status of sources at all.

[hrsh7th]

At least, currently nvim-cmp defines the typing that completionItem/resolve argument isn't null.