Skip to content

Conversation

saep
Copy link
Contributor

@saep saep commented Jul 17, 2015

This is an extension of #2979 that provides an API function for calling dict functions.
Note that this branch is based on those changes since I could reuse that code completely for the implementation of the current state.

@ZyX-I proposed this:

Or, maybe a better idea: strip off self handling from vim_call_function completely (restore the original functionality), but additionally add vim_call_dict_function(Object self, Boolean internal, String functionName, Array args, Error *err) which will do the following:

  1. Convert self to a dictionary: it should either be a Dictionary or a String object, in first case it is simply converted dictionary, in the last case it is expression passed to eval() which must evaluate to a dictionary.
  2. If internal is true then check whether functionName is a key in self that refers to a function reference. If yes, replace function name with value of this reference. If no, raise an error. If internal is false then functionName is passed to call_func untouched.
  3. Call call_func with arguments and self dictionary.

Intended usage: same as vim_call_func, but for constructs like g:SyntasticLoclist.current().filter(…): g:SyntasticLoclist.current() goes into self as a string, arguments of .filter() may be different and go into args, rationale is the same: first string is a simple static string that does not change, arguments in this case do not require double serializing (from dictionary expected by .filter() to string then string to messagepack).

Additional rationale (behind the internal argument): this make it possible to call anonymous functions without knowing their internal name (i.e. number), thus anonymous functions can be truly anonymous like in extended-funcref branch (they are not stored in a global functions dictionary under any name there). (Or in VimL-to-lua translator, if I remember correctly.)

Here is a list of things that still have to be discussed, tested or implemented (inclusive or):

  • Calling with a string to be evaluated to a dict and internal set to false
  • Calling with a string to be evaluated to a dict and internal set to true
  • Calling with a msgpack-rpc dict and internal set to false
  • Calling with a msgpack-rpc dict and internal set to true

@marvim marvim added the WIP label Jul 17, 2015
@ZyX-I
Copy link
Contributor

ZyX-I commented Jul 17, 2015

msgpack-rpc dict + internal=true is not possible, isn’t it? AFAIR function references cannot be passed through the API.

@saep saep changed the title [WIP] Add vim_call_dict_function to API [RFC] Add vim_call_dict_function to API Jul 19, 2015
@marvim marvim added RFC and removed WIP labels Jul 19, 2015
@bfredl
Copy link
Member

bfredl commented Jul 19, 2015

Please rebase on master, now #2979 is merged.

saep added 5 commits July 25, 2015 14:27
This change is made to reuse code for vim_call_dict_function.
…o statement

Some code paths were able to free the dict, even if it came from the self
argument. This would cause segfault if the self object is freed.
@justinmk justinmk added the needs:discussion issue needs attention from an expert, or PR proposes significant changes to architecture or API label Mar 2, 2017
@justinmk
Copy link
Member

justinmk commented May 1, 2018

@saep Can you rebase this?

@justinmk justinmk removed the needs:discussion issue needs attention from an expert, or PR proposes significant changes to architecture or API label May 1, 2018
@saep
Copy link
Contributor Author

saep commented May 1, 2018

So, I tried to rebase and the result can be found here: saep@3a05715

Mysteriously, this PR isn't updated with the changes I made because github doesn't know where this PR originated from. I can make another PR and reference it here, but the state isn't ready yet.

First I tried to build neovim via the nix installation instructions which do not seem to work at all (cmakeConfigurePhase is not available this way)

$ cd path/to/neovim
$ nix-shell '<nixpkgs>' -A neovim
$ cmakeConfigurePhase
build $ make

Then I tried adjusting the nix expression from nixpkgs, but I wasn't able to run any tests. After googling for half an hour, I gave up and created a debian VM to build and test my changes. This seemed to work initially, but when I tried to debug the failing test cases via GDB=1 TEST_FILE=test/functional/api/vim_spec.lua TEST_FILTER=call_dict_function make functionaltest, I managed to get these errors:

[  ERROR   ] test/functional/api/vim_spec.lua @ 21: api before_each
...im/.deps/usr/share/lua/5.1/nvim/child_process_stream.lua:28: ENOENT: no such file or directory

stack traceback:                 
        ...im/.deps/usr/share/lua/5.1/nvim/child_process_stream.lua:28: in function 'spawn'                                                                                       test/functional/helpers.lua:285: in function 'spawn'                                                                                                                      test/functional/helpers.lua:355: in function <test/functional/helpers.lua:319> 

I'm not motivated to continue anymore. Feel free to use my changes in any way you like and finish the PR.

justinmk added a commit to justinmk/neovim that referenced this pull request May 2, 2018
@justinmk
Copy link
Member

justinmk commented May 2, 2018

@saep Thanks for trying. I merged it in #8353 , it seems to work.

@teto Any tips on building Nvim with nix? Is there any pending work (nix PRs or whatever, say in an upcoming release) that you are aware of that will improve the situation?

@justinmk justinmk closed this May 2, 2018
justinmk pushed a commit to justinmk/neovim that referenced this pull request May 2, 2018
justinmk pushed a commit to justinmk/neovim that referenced this pull request May 6, 2018
@teto
Copy link
Member

teto commented May 7, 2018

As Nix rebuilds the package when the package building instructions changes, it can become a problem. Previously nix would rebuild neovim when you changed the runtime configuration (python/ruby support etc) so I pushed a change to distinguish between the 2 in the traditional nix style:

  • have a package (neovim) that just generates a shell script that wraps up the runtime configuration
  • the package neovim-unwrapped contains the C binary

I updated the wiki to account for the changes https://github.com/neovim/neovim/wiki/Building-Neovim

I've sent a PR to nixos to revamp the lua support a few months ago. You can run the functional tests with it but as it's a huge PR, it takes time to merge. It's possible to hack LUA_PATH to point at the .deps directory too but there is no out of the box support for running the tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants