org_return passes non-string to nvim_feedkeys()

[troiganto]

Describe the bug

Under some conditions, pressing <CR> in insert mode results in the following message:

[orgmode] Invalid 'keys': Expected Lua string

The editor then hangs until ctrl-c is pressed, which will interrupt a call to vim.cmd[[redraw!]].

As far as I can tell, this is caused by these lines in the org_return action handler:

orgmode/lua/orgmode/org/mappings.lua

Lines 574 to 593 in 0683da9

	local old_mapping = vim.b.org_old_cr_mapping
	-- No other mapping for <CR>, just reproduce it.
	if not old_mapping or vim.tbl_isempty(old_mapping) then
	return vim.api.nvim_feedkeys(utils.esc('<CR>'), 'n', true)
	end
	local rhs = old_mapping.rhs
	local eval = old_mapping.expr > 0
	if old_mapping.callback then
	rhs = old_mapping.callback()
	eval = false
	end
	if eval then
	rhs = vim.api.nvim_eval(rhs)
	end
	return vim.api.nvim_feedkeys(rhs, 'n', true)

In cases where this bug occurs, b:org_old_cr_mapping has a field callback but no field rhs. The fallback then correctly calls callback(). However, it seems the return value is unconditionally passed to nvim_feedkeys(), even if eval is false and the return value is, say, nil.

This should be easily fixable and I wouldn't mind submitting a PR. I'd just like someone who knows the code to look over it and make sure my train of thought is correct.

Steps to reproduce

Not quite clear what is necessary. I think this is caused by orgmode setting up its imap <CR> ... after nvim-cmp has set up its imap <CR>. Lazy-loading makes this trigger reliably, but it should be possible to hit this with eager loading, too.

Expected behavior

No error message, org_return forwards <CR> to nvim-cmp, which then falls back to a regular line break.

Emacs functionality

No response

Minimal init.lua

N/A

Screenshots and recordings

No response

OS / Distro

Fedora 40

Neovim version/commit

0.10.1

Additional context

No response

[kristijanhusak]

It happened to me a few times also, but I wasn't able to reproduce it consistently to debug it.
So if callback return value is nil, and rhs is nil, does that mean that callback already did a feedkeys? I know that mappings in cmp does a feedkeys and suggest doing the same with custom ones, but I don't know if that's the one callback that is being triggered.

There are some autopairs plugins that also uses the callback, but they need to be eval-ed IIRC.

Would the fix for this just ignore doing anything if both callback result and rhs is nil, or we would still do a regular fallback to <CR>?

[troiganto]

I've checked the docs for nvim_set_keymap() and under {opts} it had to say this at the very end:

Returning nil from the Lua "callback" is equivalent to returning an empty string.

So I think the fix might be as simple as putting an or "" behind the call of callback().

[troiganto]

As for your question regarding feedkeys (almost forgot this one, sorry!), it looks like nvim-cmp does that in cmp.core.confirm: https://github.com/hrsh7th/nvim-cmp/blob/main/lua/cmp/core.lua#L372-L400

Though I can't say I completely comprehend this code, there's a lot going on in there 😅

[kristijanhusak]

I pushed a change that should do the check and return early. Pull the latest master and let me know if it happens again. If not, we can close this in a week or two.

[troiganto]

I've done the steps I did last time to reproduce it under my lazy-loading setup.

1. enter insert mode on an empty file and start completion to ensure nvim-cmp is loaded.
2. enter an org file to load orgmode.
3. Check let b:org_old_cr_mapping to verify that org has indeed overridden cmp. It should show a dict with key callback and no key rhs.
4. enter insert mode in the org file and hit <CR> a few times.

and it seems to work now! I'll report back if I notice any bug though.

[seflue]

I had the problem also often in the past and can confirm, that it didn't happen to me since you pushed your fix @kristijanhusak. I think, we can close this issue.