Can you combine the two PRs? Any other K_ codes that might need to be handled?

Sure thing!

I imagine pretty much all the K_S_* codes will need to be handled, but they should be a pretty straightforward fix. I'll add them to the PR too.

I wonder if ctrl-space (#3101) could be fixed in this way as well.
Here's a report about ctrl-left/right also: #5024

I'll look into it. Should be doable. Any key combination that vim can recognize should be able to be passed into the terminal fairly easily this way.

ctrl-left/right seem pretty plug and play for this fix. ctrl+space might be a little trickier, but I think I can figure something out for it.

@justinmk do you know of any good way to test to make sure these keys are working correctly? so far, I've just been finding programs/commands that expect them and using those, but that's not scaling very well, haha.
calling up vim from the embedded terminal with a special vimrc works pretty well for testing
showkey works better

As of right now, the shifted function keys only work "in theory". It seems like nvim isn't able to recognize the codes for them consistently. If there are any platforms that nvim knows how to handle the shifted function keys on, or if someone were to implement that in nvim, the shifted function keys should just work.

If you'd rather me take the shifted function key logic out, just let me know.

@rjmill Ctrl_AT should correspond to ctrl-space. (^<space> is sent to terminal as ^@).

If you'd rather me take the shifted function key logic out, just let me know.

The logic looks fine to me.

Ctrl_AT should correspond to ctrl-space. (^@ is sent to terminal as ^).

The problem with ctrl-space and ctrl+@ is that there doesn't seem to be a VTermKey enum value that corresponds to K_ZERO (which is what both key combos are by the time they get to the terminal logic.)

I've got some ideas on how to handle that, but they're all either dirty hacks or more significant/risky/clunky refactors that should probably get their own separate PR. It sounds like upstream changes aren't much of an option either.

@leonerd can you say whether vterm has a ctrl-space key or some other way to send that key?

Ctrl-space and ctrl-@ work now!

Side note, ctrl-2 was also broken, but is now "fixed." Not sure why, but gnome-terminal and urxvt both interpret ctrl-2 the same as ctrl-space and ctrl-@ . So if anyone had been complaining about ctrl-2 outputting weird characters, that's taken care of now.

Nice! Need to think about why that (much appreciated) hack is needed.

We should add some test coverage to this PR (could just plop it in a new describe() block in tui_spec.lua. Use thelpers.feed_data() to send the keys.). That way, we can move forward with this, and if some indirectly related problem is found later, we can fix it while being sure we didn't regress the progress made here.

Need to think about why that (much appreciated) hack is needed.

It's because there's already an ASCII representation for the character (0), but vim converts it to its own code, just like it does for all the other special keys. But since it's ASCII, vterm(quite sensibly) doesn't have a special enum for it. Hence why it needs to be handled differently.

It might be feasible to convert it to ASCII earlier in the key handling process in keymap.c, but I imagine it will break some stuff, given that 0 is falsey in C. Lots of room for strange behavior there.

It's because there's already an ASCII representation for the character (0), but vim converts it to its own code, just like it does for all the other special keys. But since it's ASCII, vterm(quite sensibly) doesn't have a special enum for it. Hence why it needs to be handled differently.

Sure, but I wondered why ASCII 0 was special, and ctrl-a (ASCII 1) is not. In gdb ctrl-a reaches terminal_send_key() as c=1. However, the answer is in this comment:

*  NUL cannot be in the input string, therefore it is replaced by
*  K_SPECIAL   KS_ZERO>KE_FILLER
*/
#define KS_ZERO                 255

It looks like this block in get_special_key_name is a more robust way to extract the modifiers from a K_S_ special key int. That block could be factored out to a function, then then modifiers there could be checked to set equivalent vterm modifiers. Then I think KEY2TERMCAP1(c) - 48 could be passed to VTERM_KEY_FUNCTION(...).

It's less readable, for little gain I suppose. Though it could fill in whatever remaining modifier+special keys we're missing support for.

I've got your suggestion mostly working, but there are still edge cases that aren't handled properly (including Ctrl_AT). I think it's a much cleaner solution than those switch statements, but it looks like we're still going to have to resort to hacks to get this one working fully.

For what it's worth, if we go with the switch statements, any special keys/combinations we miss will be pretty trivial to tack on as we find them. Whereas if we miss stuff with the cleaner solution, it will most likely be an ordeal to support any keys we miss.

I'll keep searching for a clean solution, if only to satisfy my curiosity, but I think there's a good chance that any practical solution to this is going to involve some kind of hack for the edge cases.

but there are still edge cases that aren't handled properly (including Ctrl_AT)

ASCII 0 is not in the table, so yes it will be a special case. But anything else that's not in the table is just not supported by Vim 's input model which maps these keys to single integers. Right?

Anything else that comes through would not be mapped to a K_* key I believe. If there's any hope of supporting those missing combos, we depend on the global modifier flags.

Note: also see if shift+tab can be handled.

This fix will handle shift+tab. (Shift+tab is actually what originally brought this issue to my attention, haha.)

I'll try to get this wrapped up over the next week.

Great!

After dusting this PR off, it looks like special keys are working as expecting in the terminal with the current implementation by putting it all into the big switch statement. I have a local branch where I implemented your suggestion to extract the block in get_special_key_name to its own function and use that to handle special key codes.

It looks like I left off trying to get the branch with the extracted function to the same point of functionality as the branch with the switch statement. It seems that it mostly just doesn't handle ASCII 0. I remember having concerns that there might be other edge cases besides ASCII 0, but I don't remember why, nor do I see why there would be other edge cases besides ASCII 0. As long as those concerns don't turn out to be valid, I should be able to merge those changes into this PR tonight.

@justinmk I think we were planning on reusing that block of code to help ensure that we didn't miss any special keys with the switch statement.

I agree that we should put the unmodify_key branch in its own PR (if we use it at all.) Putting everything into the switch statement feels a bit less elegant, but it's a lot more readable and straightforward to fix if we ever need to revisit it.

I'll go ahead and roll this back to the commit before the unmodify_key one.

@justinmk What else do I need to do to wrap up this PR?

Some tests would be nice, but it's probably too complicated. I've added this to 0.2 milestone so it will get a look soonish.

Thanks! I'd be happy to write some tests, but I'm not really sure where to start testing something like this. Is it something I can just figure out by poking around in the test suite?

I've recently made extensive use of the embedded terminal.
This is probably the most anticipated PR, personally.