orgmode icon indicating copy to clipboard operation
orgmode copied to clipboard

`org_return` passes non-string to `nvim_feedkeys()`

Open broken-pen opened this issue 1 year ago • 6 comments

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: https://github.com/nvim-orgmode/orgmode/blob/0683da9a752d3023eea618b9efa6ad0948525235/lua/orgmode/org/mappings.lua#L574-L593

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

broken-pen avatar Aug 21 '24 22:08 broken-pen

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>?

kristijanhusak avatar Aug 22 '24 07:08 kristijanhusak

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().

broken-pen avatar Aug 22 '24 07:08 broken-pen

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 :sweat_smile:

broken-pen avatar Aug 22 '24 07:08 broken-pen

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.

kristijanhusak avatar Aug 22 '24 08:08 kristijanhusak

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.

broken-pen avatar Aug 22 '24 08:08 broken-pen

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.

seflue avatar Oct 19 '24 11:10 seflue