A reason for generating separate f_vim_... wrappers could be to behave more like other builtins with automatic conversion to an int if an int is expected and such

I think it's reasonable and even preferable that the API is a bit more rigid.

I would also suggest 0 being a special value always meaning curbuf/curwin

Can't think of a reason against.

I can't shake the idea that it might be useful to somehow namespace the API functions:

• a global v:api dict (I don't like this idea), e.g. call v:api.vim_get_current_buffer()
• nvim#vim_get_current_buffer
• a new "scope style" qualifier, e.g. n:vim_get_current_buffer()
• put the functions in v:, e.g. v:vim_get_current_buffer()

Maybe it's not useful though, and it's better that they are treated as first-class VimL functions. After all, VimL is itself an implicit namespace.

Arguably buffer nrs should be identified with buffer "handles", otherwise would be a disaster. (It seems reasonable a nvim session would only have <2^31 buffer creation events but I could be wrong...)

Is this the reason for replace int64 handle with tp_id, etc.?

Is this the reason for replace int64 handle with tp_id, etc.?

Not really, I replaced buf->handle with buf->b_fnum and tried to be consistent, but I guess it didn't fall out too well. Another way is to #define b_fnum handle and then when porting 1557 we could just use win->handle instead of w_id internally.

I can't shake the idea that it might be useful to somehow namespace the API functions

I don't have any super strong opinion really, but for me vim_/buffer_ etc is already a "namespace". Remember vim has deprecatedbuffer_number() in favor for bufnr() etc so it seems vim will stay away from these sort of names. Relatedly I thought of blacklisting/whitelisting functions if we don't wan't to expose everyone, to reduce namespace clutter. We could disallow vim_eval() for instance as it is better to be explicit with deepcopy(eval(...)) (is more efficient as well).

Remember vim has deprecatedbuffer_number() in favor for bufnr() etc so it seems vim will stay away from these sort of names

You're probably right, I'm overthinking it. The API methods will tend to be very verbose, which is unlikely to conflict with future vim functions. No extra namespace ceremony needed.

Relatedly I thought of blacklisting/whitelisting functions if we don't wan't to expose everyone, to reduce namespace clutter.

👍

@bfredl vim_eval is not needed if you want to evaluate expression. But it is a perfect candidate to test how evaluation result would be received on remote end (without actually setting this remote end up). Also deepcopy(eval(…)) and vim_eval(…) have way too much differences in the current state:

1. No funcrefs support.
2. No recursive containers support.
3. If one container is present twice (e.g. let l=[[]]|call add(l, l[0])) after vim_eval there will be two containers.

You're probably right, I'm overthinking it. The API methods will tend to be very verbose, which is unlikely to conflict with future vim functions. No extra namespace ceremony needed.

I would still go for api_*() “namespace”: there are much differences in how values are treated by regular and API functions, above is one of the examples. Such naming is also an explanation to “why there exists both eval() and vim_eval(), what’s the difference?”

I don't have any super strong opinion really, but for me vim_/buffer_ etc is already a "namespace". Remember vim has deprecatedbuffer_number() in favor for bufnr() etc so it seems vim will stay away from these sort of names. Relatedly I thought of blacklisting/whitelisting functions if we don't wan't to expose everyone, to reduce namespace clutter. We could disallow vim_eval() for instance as it is better to be explicit with deepcopy(eval(...)) (is more efficient as well).

Also remember that new Vim functions are named like json_decode() or ch_open(). Or win_getid(). Given that we are likely to merge win_getid() it is also likely that there will be a relevant call in API your variant will be confusing (win_getid() and window_get_id(), which one to use?). Existence of api_window_get_id() here does not look that confusing: one function is Neovim-specific API binding, another one is regular VimL function.

Also note that all variants listed after echo win<Tab> fit in one line and present me unique functions. What will this look like after adding a growing bunch of functions from api/window.c? Same for buf<Tab>.

And you completely forgot about tabpage: new Vim naming scheme is using underscores, but all existing tab* functions start with tabpage*, not tab* like win* or buf* like window- and buffer-related ones. So here naming conflict is a lot more likely.

Regarding function table: I would really suggest to turn this into a hash. I even started doing something like this, but stopped at problem “how to keep khash (or hashtab) reimplementation in lua consistent with it in C code”. Patch is still available: 821958a.

But it is a perfect candidate to test how evaluation result would be received on remote end.

Good point.

And you completely forgot about tabpage

Not completely, it is still tabpagenr where the api name would be of the form tabpage_get_number.

For me names like buffer_add_highlight just sound more natural than api_buffer_add_highlight or api_buffer_set_lines. The plugin will add highlights and lines to the buffer, exactly what the name says. Speaking of tabbing, changes are the plugin writer would try buf[fer]<tab> for these operations and not api_buf<tab> . That's also why I mentioned blacklisting, we don't need to import every name into the "global" namespace if we are afraid of cluttering. (to avoid complete duplicates) Also window_get_id() won't exist with this design either as the handle would be the id.

Regarding function table: I would really suggest to turn this into a hash. I even started doing something like this, but stopped at problem “how to keep khash (or hashtab) reimplementation in lua consistent with it in C code”.

I though of a hash too, but I thought instead of just initializing the hash with c code (then in principle we could modify msgpack_rpc_add_method_handler to do the part for API functions). I wonder if it really would bring significant time cost to the startup (given that we already build the rpc method table in this way).

@bfredl Currently all tabpage* functions have no underscore. But just as well all new functions have underscore like win_getid (older functions look like winheight()), so the point is that name conflict is a lot more likely in the future, not that it is possible right now.

@bfredl I do not think this will bring significant cost to the startup, but neither I think that increasing time and memory usage while there is such an easy optimization available is a good idea. Only problem there was “rewriting khash.h in lua” vs “making cross-compiling impossible” vs “adding gperf as a dependency”. Do not remember why I did not post this as a PR with this question: probably 1 or 3 would be accepted.

@ZyX-I Ok, I went with api_ prefix and I cherry-picked your functions hash branch. I wonder, can't we just require that on cross-compiling the host c compiler is available as well? (Mean, neovim is certainly not the first project that wants to support cross-compiling and also use host c as part of the build process)

Still a lot of expected warnings I hope to go though this weekend (mostly conversion and add dummy void * arguments) but should be working for basic testing.

I currently do not know how to do this with cmake. Using gperf seems to be easier, AFAIK it should be available everywhere we currently want to build (linux, windows and mac os).

Yes gperf looks what we want to use, but even then we might want to build it as a dependency when building in USE_BUNDLED mode as we do for other dependencies. It seems our third-party cmakelists already defines HOSTDEPS_C_COMPILER for this purpose.

Couldn't we commit the result of gperf, instead of requiring gperf as a dependency?

We could automate this in travis:

• when travis builds any PR, it runs gperf
• if the result is different than the current source, post it to gist and leave a comment on the PR
  • alternative: leave a comment with instructions for the developer to run gperf locally
• developer manually commits the result and updates the PR

We could, but it seems to just be more complicated for the developer, for no real benifit. We are already requiring (host) lua to be present for building nvim, and building it and running it automatically for the developer, why can't we do the same with gperf? Now, I could imagine shipping the gperf output as part of release tarballs, as the expectaction there is that no modification will be made to the code.

We are already requiring (host) lua to be present for building nvim, and building it and running it automatically for the developer, why can't we do the same with gperf?

I don't feel strongly about it, but gperf is a bit more obscure, and our build stack already gets plenty of complaints. Lua doesn't seem comparable: we will eventually ship lua to the end user, so it's not only a build-time dependency.

we will eventually ship lua to the end user,

As I suggested neither would end user nor distro people (the ones mostly complaining of our build system, I would presume) need to have gperf if we ship the result as part of the release tarballs, so gperf won't either be a pure bulid-time dependency. So the situation do seem comparable with lua generation to me.

It would only affect the developers situation, and having a small c utility built for you (after a bunch of other c dependencies) seems less burdensome to me than requesting the developer to copy some text from a gist for some reason or getting a weird error message suggesting the developer to build and run gperf themselves.

I guess my main point is, the developer does not need to care at all about most of our auto-generating, say, for internal prototypes and the option table, but if the developer happens to change api prototypes now the developer is suddenly forced to care a lot of our process. At least to me this inconsistency would seem a bit odd and hard to explain.

I made an attempt of building gperf only if it's needed. The generated hash could be shipped in a release tarball by
cp build/src/nvim/auto/funcs.generated.h src/auto/
which will turn off building gperf. Now the logic is quite ad-hoc (partially as "deps" and "nvim" are two completely separate cmake domains) but this could be extended to cache other generated source or I could imagine enabling shipping a completely static source distribution by cp -r build/src/nvim/auto/ src/auto/ (if we find a reason for it that is)

The feature itself is more or less "done", so marking RFC.
Some gaps:

• check if building gperf on mingw works
• building gpref on msvc: I have no idea what to do here, some windows expert might want to jump in (see third-party/cmake/BuildGperf.cmake)
• could use a bit more testing
• make api docs accessible from nvim. doxygen exports machine readable xml, we "just" need to convert it to something reasonably ft=help compatible. (but probably this should be a separate PR)

Marking as RDY, if no more comments of the code I will merge this, as there are a few PRs wanting to add API functions. (docs I'm not happy with, but as said that will be another PR with auto generating docs for methods, which also will make it easier to reason the surrounding docs)

Thanks for this, @bfredl