lua: autocmds take 2

[tjdevries]

Restarting my attempt of #12378

Plan is:

1. Add a bunch more functional tests for autocmds
2. Add nvim_* api functions to access and test autocmds
3. THEN we'll start the refactor again of do_cmdline and/or create a new function that can be used by autocmds.
4. When we do this, we can change the return type of the LineGetter to something that could return a union of items (command, LuaRef, vim funcref). And then we execute it there

Once we do that, it's very easy to extend the nvim_* api functions to accept lua refs (as a new key) and use the refactoring that we did.

I'll be moving code over as I implement this from the original PR.

Status:

• May 28, 2021: I've implemented most of step 1 & 2. Probably will add more weird edge cases as I think of them.

[RustemB]

[tjdevries]

Haha, this is my next big core feature I will work on 😁 it is a fun one to tackle.

[bfredl]

I still need to find how rpc api is handled to see if adding that to exsisting api functions will be a breaking change :P

It is not a breaking change.

[tjdevries]

hi, may I ask if there is any update on this PR?

I am working on it when I have time :) In general, it is considered bad form to do these kinds of posts in a PR that has commits somewhat recently.

[bfredl]

@tjdevries What you think about getting this merged for 0.7? I remember you said you "hated" some things in the present implementation, but does those affect observable behavior? I'd say as long as the behavior is reasonable (including error handling and such), we can get this merged early in the 0.7 cycle, and re-iterate on the implementation, including possible small changes of behavior if needed.

[clason]

Just noting this down here for the sake of completeness: Does this PR cover User autocommands and doautocmd? Would be nice (in a future PR) to have an API for that, which ideally adds support for custom event structures.

[tjdevries]

@bfredl let me spend some time this week and next on it, and then ask for your review. If you like it and feel OK w/ the hack that I have in there, I feel confident it could work pretty well.

[bfredl]

@tjdevries I rebased to resolve some surface level conflicts.

[kevinhwang91]

IMO, command field in nvim_create_autocmd should be deleted, callback should be the last position argument, which can easy to convert to await.

[mvllow]

Would it make sense to merge command and callback? Perhaps command could accept a string or lua function?

[clason]

No, it would not make sense -- these have to be different for technical reasons (same as for the nvim_set_keymap() API). If people want the extra convenience, they can create a Lua API on top of it (similarly as with vim.keymap.set). At the moment, the goal is to make sure that the underlying low-level API is sound and bug-free; convenience comes after that.

Also, drive-by bikeshedding after a PR is merged is a) poor form and b) not going to lead to very much due to the lack of visibility.

[hkupty]

Not sure if this is the intended behavior, but for an autocmd where I set a description, it is duplicated in the command, which together with not having the event key in this response, makes it hard to figure to which event that autocommand is bound to:

Note that this was match only by group, which in this case does make sense not to show up in the response, but if I search only for TabLeave event, I can never figure out for which plugin/group that autocmd maps to:

[lewis6991]

Please don't comment on this PR with user specific issues or questions. Either raise an issue or reach out on matrix.

Thanks

[tjdevries]

Note, for anyone who was using this immediately after merge, there was a problem with our merge into master, which was resolved in this PR:

#17551

Note, that there are changes to the API functions here, so you will need to change how you were calling the API