New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] Core service providers with python and clipboard support #895

Merged
merged 16 commits into from Jul 17, 2014

Conversation

Projects
None yet
@tarruda
Copy link
Member

tarruda commented Jun 26, 2014

This PR will:

  • Provide a basic infrastructure for implementing core services through the msgpack API
  • Implement all python-related functions and commands for vim, adding some support to python plugins(pre 7.4).
  • Add clipboard support using the same mechanism

It seems the basic features of Gundo, UltiSnips, Omnisharp and YouCompleteMe are ok, but YouCompleteMe is very slow(its possible that some neovim-specific tweaking will be required).

To test python plugins, my private python client branch is required(it has a hardcoded log configuration to /tmp/script-host.log). YouCompleteMe users should probably test for !has('neovim') before loading the plugin.

I also plan to add options for customizing the command used to start the providers(to use PyPy for example), but that will go in another PR.

In the future, the provider module will be used to externalize or override more core services, such as code completion(projects like YouCompleteMe will have first class support in Neovim) or the spell checker.

CC @SirVer @Valloric @Shougo @ZyX-I for feedback

@mikaelj

This comment has been minimized.

Copy link

mikaelj commented Jun 27, 2014

My previous worries about the timeout were unfounded -- CtrlP on this
branch takes about as long time as on Vim 7.4, and the freeze I thought was
happening /could/ be related to Powerline which doesn't update the status
line properly while recursing through directories. Since Powerline isn't on
by default in Neovim, I the status line is indeed updated and there is no
freeze. This enables me (and probably others) to start using Neovim
fulltime.

Thanks, and exciting to see about the new extension points!

On Thu, Jun 26, 2014 at 11:53 PM, Thiago de Arruda <notifications@github.com

wrote:

This PR will:

  • Provide a basic infrastructure for implementing core services
    through the msgpack API
  • Implement all python-related functions and commands for vim, adding
    some support to python plugins(pre 7.4).
  • Add clipboard support using the same mechanism

It seems the basic features of Gundo, UltiSnips, Omnisharp and
YouCompleteMe are ok, but YouCompleteMe is very slow(its possible that some
neovim-specific tweaking will be required).

To test python plugins, my private python client branch
https://github.com/tarruda/python-client/tree/scripting-support is
required(it has a hardcoded log configuration to /tmp/script-host.log).
YouCompleteMe users should probably test for !has('neovim') before
loading the plugin.

I also plan to add options for customizing the command used to start the
providers(to use PyPy for example), but that will go in another PR.

In the future, the provider module will be used to externalize or override
more core services, such as code completion(projects like YouCompleteMe
will have first class support in Neovim) or the spell checker.

CC @SirVer https://github.com/SirVer @Valloric
https://github.com/Valloric @Shougo https://github.com/Shougo @ZyX-I

https://github.com/ZyX-I for feedback

You can merge this Pull Request by running

git pull https://github.com/tarruda/neovim service-providers

Or view, comment on, or merge it at:

#895
Commit Summary

  • wstream: document default value for 'maxmem'
  • wstream: Pass WBuffer refcount as a constructor parameter
  • api: Refactor write_msg to use separate out/err buffers
  • channel: Refactor channel_from_job to return the channel id
  • channel: Implement channel_exists function
  • job: No longer free the job data. It's now done by the exit callback
  • channel: Bugfixes and refactor
  • provider: New module used to expose extension points for core
    services
  • provider: Add support functions for calling external interpreters
  • provider: Add support for python commands/functions

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#895.

@aktau

This comment has been minimized.

Copy link
Member

aktau commented Jun 27, 2014

It seems the basic features of Gundo, UltiSnips, Omnisharp and YouCompleteMe are ok, but YouCompleteMe is very slow(its possible that some neovim-specific tweaking will be required).

Colour me very interested! Any idea what the slowness is from (msgpack+TCP overhead)? Have you done profiling? What neovim-specific tweaking are you talking about? (I don't think this is extremely workable by the way, neovim should be more drop-in replace even in the beginning).

I suppose that this python-client is an evolution of the python script you used to communicate with neovim earlier, except that now it also accepts things from neovim?

@tarruda

This comment has been minimized.

Copy link
Member

tarruda commented Jun 27, 2014

Thanks, and exciting to see about the new extension points!

@mikaelj glad to see it working for your plugins, some things still need to be fixed(such as keyboard interrupt) but in time I'm sure we will get near 100% compatibility with vim

Colour me very interested! Any idea what the slowness is from (msgpack+TCP overhead)? Have you done profiling? What neovim-specific tweaking are you talking about?

I didn't investigate what is the problem, but if I were to guess, I'd say it's because YCM modifies the update_time option to poll for responses from its own server(lots of small timers could be expensive to libuv). But this is just speculation, I need to run it through a profiler to get to the real cause.

A neovim-specific tweak would be to simply call it from the response thread(the python client is thread-safe) , or better yet: Make the YCM server connect to Neovim socket and send updates through API directly, without passing through the http request/reponse pipeline.

Calling the messagepack API in a loop may also cause bottlenecks. While it should be fast enough for most use cases, it's still a lot slower than the python-vim bridge where all the communication happens in the same process and without any serialization/deserialzation overhead.

The :pydo implementation is good example of how the API should be used: Take a bunch of data, do some processing then return the result(try reducing the batch size from 5000 to 1 and see the performance impact)

I suppose that this python-client is an evolution of the python script you used to communicate with neovim earlier, except that now it also accepts things from neovim?

Yes the python client accepts messages from Neovim, but on my branch it also has the option of running a script host to emulate the environment provided by python-vim.

@Shougo

This comment has been minimized.

Copy link
Contributor

Shougo commented Jun 27, 2014

Thanks. I will test it later.

In the future, the provider module will be used to externalize or override more core services, such as code completion(projects like YouCompleteMe will have first class support in Neovim) or the spell checker.

As code completion author, neovim should implement asynchronous completion feature.
Current Vim, does not support it. So, it is killer feature.

@Shougo

This comment has been minimized.

Copy link
Contributor

Shougo commented Jun 27, 2014

@tarruda I tested your branch and installed python-client.
But python interface is not enabled.
Can you tell me the way to enable it?

@tarruda

This comment has been minimized.

Copy link
Member

tarruda commented Jun 27, 2014

@tarruda I tested your branch and installed python-client.
But python interface is not enabled.
Can you tell me the way to enable it?

Are you using this branch of python client? Try this:

pip uninstall neovim
pip install git+https://github.com/tarruda/python-client.git@plugin-host

If you already installed, show the output of tail -f /tmp/script-host.log

@Shougo

This comment has been minimized.

Copy link
Contributor

Shougo commented Jun 27, 2014

OK. But it crashes...

% NEOVIM_LISTEN_ADDRESS=/tmp/neovim nvim
*** buffer overflow detected ***: nvim terminated
======= Backtrace: =========
/lib/i386-linux-gnu/libc.so.6(+0x6998e)[0x4015598e]
/lib/i386-linux-gnu/libc.so.6(__fortify_fail+0x6b)[0x401e837b]
/lib/i386-linux-gnu/libc.so.6(+0xfb20a)[0x401e720a]
/lib/i386-linux-gnu/libc.so.6(__strcpy_chk+0x37)[0x401e66e7]
nvim(eval_init+0x5f)[0x80896ea]
nvim(main+0x77)[0x8067f5c]
/lib/i386-linux-gnu/libc.so.6(__libc_start_main+0xf3)[0x40105a83]
nvim[0x8069bae]
======= Memory map: ========
08048000-081ce000 r-xp 00000000 08:01 529467     /usr/local/bin/nvim
081ce000-081cf000 r--p 00186000 08:01 529467     /usr/local/bin/nvim
081cf000-081db000 rw-p 00187000 08:01 529467     /usr/local/bin/nvim
081db000-081e6000 rw-p 00000000 00:00 0 
099fe000-09a1f000 rw-p 00000000 00:00 0          [heap]
40000000-40020000 r-xp 00000000 08:01 673897     /lib/i386-linux-gnu/ld-2.19.so
40020000-40021000 r--p 0001f000 08:01 673897     /lib/i386-linux-gnu/ld-2.19.so
40021000-40022000 rw-p 00020000 08:01 673897     /lib/i386-linux-gnu/ld-2.19.so
40022000-40023000 r-xp 00000000 00:00 0          [vdso]
40023000-40026000 rw-p 00000000 00:00 0 
40026000-40042000 r-xp 00000000 08:01 655423     /lib/i386-linux-gnu/libgcc_s.so.1
40042000-40043000 rw-p 0001b000 08:01 655423     /lib/i386-linux-gnu/libgcc_s.so.1
40046000-40064000 r-xp 00000000 08:01 656935     /lib/i386-linux-gnu/libtinfo.so.5.9
40064000-40065000 ---p 0001e000 08:01 656935     /lib/i386-linux-gnu/libtinfo.so.5.9
40065000-40067000 r--p 0001e000 08:01 656935     /lib/i386-linux-gnu/libtinfo.so.5.9
40067000-40068000 rw-p 00020000 08:01 656935     /lib/i386-linux-gnu/libtinfo.so.5.9
40068000-40087000 r-xp 00000000 08:01 546113     /usr/local/lib/libuv.so.11
40087000-40088000 r--p 0001e000 08:01 546113     /usr/local/lib/libuv.so.11
40088000-40089000 rw-p 0001f000 08:01 546113     /usr/local/lib/libuv.so.11
40089000-4008a000 rw-p 00000000 00:00 0 
4008a000-400ce000 r-xp 00000000 08:01 673892     /lib/i386-linux-gnu/libm-2.19.so
400ce000-400cf000 r--p 00043000 08:01 673892     /lib/i386-linux-gnu/libm-2.19.so
400cf000-400d0000 rw-p 00044000 08:01 673892     /lib/i386-linux-gnu/libm-2.19.so
400d0000-400e8000 r-xp 00000000 08:01 673887     /lib/i386-linux-gnu/libpthread-2.19.so
400e8000-400e9000 r--p 00017000 08:01 673887     /lib/i386-linux-gnu/libpthread-2.19.so
400e9000-400ea000 rw-p 00018000 08:01 673887     /lib/i386-linux-gnu/libpthread-2.19.so
400ea000-400ec000 rw-p 00000000 00:00 0 
400ec000-40295000 r-xp 00000000 08:01 660358     /lib/i386-linux-gnu/libc-2.19.so
40295000-40297000 r--p 001a9000 08:01 660358     /lib/i386-linux-gnu/libc-2.19.so
40297000-40298000 rw-p 001ab000 08:01 660358     /lib/i386-linux-gnu/libc-2.19.so
40298000-4029b000 rw-p 00000000 00:00 0 
4029b000-402a2000 r-xp 00000000 08:01 673890     /lib/i386-linux-gnu/librt-2.19.so
402a2000-402a3000 r--p 00006000 08:01 673890     /lib/i386-linux-gnu/librt-2.19.so
402a3000-402a4000 rw-p 00007000 08:01 673890     /lib/i386-linux-gnu/librt-2.19.so
402a4000-402a7000 r-xp 00000000 08:01 673913     /lib/i386-linux-gnu/libdl-2.19.so
402a7000-402a8000 r--p 00002000 08:01 673913     /lib/i386-linux-gnu/libdl-2.19.so
402a8000-402a9000 rw-p 00003000 08:01 673913     /lib/i386-linux-gnu/libdl-2.19.so
402a9000-402ab000 rw-p 00000000 00:00 0 
bfd43000-bfd64000 rw-p 00000000 00:00 0          [stack]
[1]    30060 abort (core dumped)  NEOVIM_LISTEN_ADDRESS=/tmp/neovim nvim
@aktau

This comment has been minimized.

Copy link
Member

aktau commented Jun 27, 2014

OK. But it crashes...

If I may hazard a guess: you compiled neovim with the -DFORTIFY_SOURCE=2 switch. Neither vim nor neovim work with this due to some ill-advised struct hacks. How did you compile install neovim? There's already an issue for this #223. If you want to protect against overflows, DFORTIFY_SOURCE=1 is advised.

@tarruda

This comment has been minimized.

Copy link
Member

tarruda commented Jun 27, 2014

If I may hazard a guess: you compiled neovim with the -DFORTIFY_SOURCE=2 switch. Neither vim nor neovim work with this due to some ill-advised struct hacks. How did you compile install neovim? There's already an issue for this #223. If you want to protect against overflows, DFORTIFY_SOURCE=1 is advised.

Actually I got some crashes too, I think this is due to one of my last modifications(I'm currently doing some refactor)

@SirVer

This comment has been minimized.

Copy link

SirVer commented Jun 27, 2014

Interesting. I will try to run UltiSnips testsuite - which uses screen to send keystrokes through a terminal, a very real-world way of testing a text editor feature - through this branch of neovim and the python api and see how it holds up. Might not be able to do it before next week though. I'll report back (here?).

@Shougo

This comment has been minimized.

Copy link
Contributor

Shougo commented Jun 27, 2014

I updated it. But it crashes again...

@tarruda

This comment has been minimized.

Copy link
Member

tarruda commented Jun 27, 2014

I updated it. But it crashes again...

@Shougo can you run it with valgrind and put the log here? Eg:

valgrind --log-file=err.log ./build/bin/nvim
@Shougo

This comment has been minimized.

Copy link
Contributor

Shougo commented Jun 27, 2014

==12589== Memcheck, a memory error detector
==12589== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==12589== Using Valgrind-3.10.0.SVN and LibVEX; rerun with -h for copyright info
==12589== Command: ./build/bin/nvim
==12589== Parent PID: 29974
==12589== 
**12589** *** strcpy_chk: buffer overflow detected ***: program terminated
==12589==    at 0x402CDE7: ??? (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==12589==    by 0x4030F15: __strcpy_chk (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==12589==    by 0x808D475: eval_init (in /home/shougo/src/neovim-python/build/bin/nvim)
==12589==    by 0x806BCDA: main (in /home/shougo/src/neovim-python/build/bin/nvim)
==12589== 
==12589== HEAP SUMMARY:
==12589==     in use at exit: 96 bytes in 6 blocks
==12589==   total heap usage: 6 allocs, 0 frees, 96 bytes allocated
==12589== 
==12589== LEAK SUMMARY:
==12589==    definitely lost: 0 bytes in 0 blocks
==12589==    indirectly lost: 0 bytes in 0 blocks
==12589==      possibly lost: 0 bytes in 0 blocks
==12589==    still reachable: 96 bytes in 6 blocks
==12589==         suppressed: 0 bytes in 0 blocks
==12589== Rerun with --leak-check=full to see details of leaked memory
==12589== 
==12589== For counts of detected and suppressed errors, rerun with: -v
==12589== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
@tarruda

This comment has been minimized.

Copy link
Member

tarruda commented Jun 27, 2014

@Shougo as suggested by @aktau you probably compiled using -DFORTIFY_SOURCE=2, which for now isn't supported by Neovim. Could you try again?

@tarruda

This comment has been minimized.

Copy link
Member

tarruda commented Jun 27, 2014

Now it is possible to use the '+' and '*' registers in this branch. Instructions:

  • Install python-client from this branch: pip install git+https://github.com/tarruda/python-client.git@plugin-host
  • Install the 'xerox' python module: pip install xerox
  • Create this file under $HOME/.vim/pythonx (update: the filename should start with 'nvim_')
  • Optionally set the new unnamedclip option to use the clipboard with the unnamed registers
@Shougo

This comment has been minimized.

Copy link
Contributor

Shougo commented Jun 27, 2014

I used rm -rf build/ ; make clean && make CMAKE_BUILD_TYPE=MinSizeRel.
OK. I will try normal make.

@Shougo

This comment has been minimized.

Copy link
Contributor

Shougo commented Jun 27, 2014

The crash is dissapearred. But,

% NEOVIM_LISTEN_ADDRESS=/tmp/neovim nvim
Failed to start server: invalid argument

If I use vinarise plugin which uses python interface, the error messages are displayed.

The python provider exited prematurely and had to be restarted
Before returning from a RPC call, channel 14 was closed by the client
@Shougo

This comment has been minimized.

Copy link
Contributor

Shougo commented Jun 27, 2014

Now it is possible to use the '+' and '*' registers in this branch. Instructions:

I tested it, but

Vim(let):A clipboard provider is not available
@tarruda

This comment has been minimized.

Copy link
Member

tarruda commented Jun 27, 2014

@Shougo not all plugins are working yet, you can disable some with has('neovim'). Also make sure you updated the python client. The branch is now 'plugin-host', not 'scripting-support'

@Shougo

This comment has been minimized.

Copy link
Contributor

Shougo commented Jun 27, 2014

@Shougo not all plugins are working yet, you can disable some with has('neovim'). Also make sure you updated the python client. The branch is now 'plugin-host', not 'scripting-support'

OK. But your python-client does not work. why?

@tarruda

This comment has been minimized.

Copy link
Member

tarruda commented Jun 27, 2014

@Shougo I will investigate, but its working for me and the build passed on travis

@tarruda

This comment has been minimized.

Copy link
Member

tarruda commented Jun 28, 2014

@Shougo I have improved debugging on the provider module, you can verify error messages sent by the python-client with tail -f ~/.nvimlog(Any exceptions thrown before the python-client had a chance to startup)

Could you paste the output of ~/.nvimlog here?

@tarruda

This comment has been minimized.

Copy link
Member

tarruda commented Jun 28, 2014

@aktau It turns out that most performance issues I had with YCM were due to logging in the python-client. Once disabled it got much more acceptable.

@tarruda tarruda changed the title [WIP/RFC] Core service providers with python and clipboard support [RFC] Core service providers with python and clipboard support Jun 28, 2014

@aktau

This comment has been minimized.

Copy link
Member

aktau commented Jun 28, 2014

It turns out that most performance issues I had with YCM were due to logging in the python-client. Once disabled it got much more acceptable.

I had a small suspicion it might be something like that, but I didn't know how much you were actually logging. It was a lot, I presume?

@tarruda

This comment has been minimized.

Copy link
Member

tarruda commented Jun 28, 2014

I had a small suspicion it might be something like that, but I didn't know how much you were actually logging. It was a lot, I presume?

It's not extreme logging but it can significantly slow down plugins that call python on every keypress

@mikaelj

This comment has been minimized.

Copy link

mikaelj commented Jun 28, 2014

Out of pure curiousity, what strategies have you envisioned at this point
(if any) to optimize such use cases?

On Sat, Jun 28, 2014 at 11:40 PM, Thiago de Arruda <notifications@github.com

wrote:

I had a small suspicion it might be something like that, but I didn't know
how much you were actually logging. It was a lot, I presume?

It's not extreme logging but it can significantly slow down plugins that
call python on every keypress


Reply to this email directly or view it on GitHub
#895 (comment).

@tarruda

This comment has been minimized.

Copy link
Member

tarruda commented Jun 28, 2014

Out of pure curiousity, what strategies have you envisioned at this point
(if any) to optimize such use cases?

Other than increasing the log level to a higher value than logging.DEBUG, I didn't devise a strategy. I already expected that calling python through the msgpack-rpc API would be slower than with python-vim.

In other words: Not every plugin that runs fast on Vim will also run fast on Neovim. But with some tweaking, these plugins might even work better on Neovim(especially plugins that need to do a lot of background processing, like YCM)

tarruda added some commits Jun 26, 2014

provider: Add support for python commands/functions
This uses the provider/scripting infrastructure to reintroduce python support
through the msgpack-rpc API.

A new 'initpython' option was added, and it must be set to a command that will
bootstrap the python provider the first time it's needed.
provider: Add support for clipboard registers.
This reimplements the '+'/'*' clipboard registers(both are aliases to the same
register, no dedicated storage for the X11 selection) on top of the provider
infrastructure.

This adds two new 'unnamedclip' option, has the same effect of setting
'clipboard' to 'unnamed/unnamedplus' in vim

The 'clipboard' option was not reused because all values(except 'unnamedplus')
seem to be useless for Neovim, and the code to parse the option was relatively
big. The option remains for vim compatibility but it's silently ignored.
api: make buffer_{get,set}_slice automatically assume `include_end`
This is for compatibility with python-vim interface: When passing an end index
with a value higher than the last index, assume the `include_end` flag
getchar: fix infinite loop due to pending events
The `inchar` function could enter an infinite loop if there are events pending
to be processed when an interrupt is received.
api/events/msgpack: Insert log statements to improve debugging
Also changed the default log level to INFO so developers won't end up with big
log files without asking explicitly(DLOG statements were placed in really "hot"
code)
events: Refactor how event deferral is handled
- Remove all *_set_defer methods and the 'defer' flag from rstream/jobs
- Added {signal,rstream,job}_event_source functions. Each return a pointer that
  represent the event source for the object in question(For signals, a static
  pointer is returned)
- Added a 'source' field to the Event struct, which is set to the appropriate
  value by the code that created the event.
- Added a 'sources' parameter to `event_poll`. It should point to a
  NULL-terminated array of event sources that will be used to decide which
  events should be processed immediately
- Added a 'source_override' parameter to `rstream_new`. This was required to use
  jobs as event sources of RStream instances(When "focusing" on a job, for
  example).
- Extracted `process_from` static function from `event_process`.
- Remove 'defer' parameter from `event_process`, which now operates only on
  deferred events.
- Refactor `channel_send_call` to use the new lock mechanism

What changed in a single sentence: Code that calls `event_poll` have to specify
which event sources should NOT be deferred. This change was necessary for a
number of reasons:

- To fix a bug where due to race conditions, a client request
  could end in the deferred queue in the middle of a `channel_send_call`
  invocation, resulting in a deadlock since the client process would never
  receive a response, and channel_send_call would never return because
  the client would still be waiting for the response.
- To handle "event locking" correctly in recursive `channel_send_call`
  invocations when the frames are waiting for responses from different
  clients. Not much of an issue now since there's only a python client, but
  could break things later.
- To simplify the process of implementing synchronous functions that depend on
  asynchronous events.
api tests: set 'initpython' in BeforeEachTest
This is required to run some tests of the python client

@tarruda tarruda merged commit 4dc642a into neovim:master Jul 17, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

tarruda added a commit that referenced this pull request Jul 17, 2014

@hkupty

This comment has been minimized.

Copy link

hkupty commented Jul 17, 2014

@tarruda I wasn't having problems with unnamedClip, I just couldn't understand clearly how things work from reading the docs.. But now it's ok.
Great work!

@tarruda

This comment has been minimized.

Copy link
Member

tarruda commented Jul 17, 2014

@tarruda I wasn't having problems with unnamedClip, I just couldn't understand clearly how things work from reading the docs.. But now it's ok.
Great work!

I have improved the documentation a bit, now it contains a brief explanation of what is happening under the hood

@hkupty

This comment has been minimized.

Copy link

hkupty commented Jul 18, 2014

👍

@mikaelj

This comment has been minimized.

Copy link

mikaelj commented Jul 18, 2014

For now only six extension points are exposed:

  • clipboard_get
  • clipboard_set
  • python_execute
  • python_execute_file
  • python_do_range
  • python_eval

In the future, do you envision the language part of
_{execute,execute_file,do_range,eval} being possible to add by
users? As I read your current design, each new language needs explicit
support in neovim. One of the goals of neovim is to make it more dynamic
with regard to scripting languages.

What if you could define, through neovim confguration, a new language? This
language would then support 0-4 of the extensions points defined for the
"language interface". set languages+=python:/path/to/python/binary or
something.

  • Micke
@aktau

This comment has been minimized.

Copy link
Member

aktau commented Jul 18, 2014

@mikaelj as far as I understand it, these extension point are for legacy support. To provide vimscripts with the ability to call into python.

When coding directly in python, it's not needed to use these extension points. Neovim doesn't need to know that it's python communicating with neovim, all it sees are msgpack messages coming in.

clipboard_{get,set} are a bit different, as they're extension points for well-known coprocesses.

@tarruda

This comment has been minimized.

Copy link
Member

tarruda commented Jul 18, 2014

_{execute,execute_file,do_range,eval} being possible to add by
users? As I read your current design, each new language needs explicit
support in neovim. One of the goals of neovim is to make it more dynamic
with regard to scripting languages.

What if you could define, through neovim confguration, a new language? This
language would then support 0-4 of the extensions points defined for the
"language interface". set languages+=python:/path/to/python/binary or
something.

  • Micke

That is a nice idea, and it would already be somewhat possible if channel_from_job is exposed to vimscript:

" vimrc
let g:python_channel = channel_from_job('python', ['-c', 'import neovim; neovim.start_host()'])
command! -nargs=? Py call channel_send_call(g:python_channel, "python_execute", <q-args>)
command! -nargs=? PyFile call channel_send_call(g:python_channel, "python_execute_file", <q-args>)
command! -nargs=? PyDo call channel_send_call(g:python_channel, "python_do_range", <q-args>)
function! PyEval(str)
  return channel_send_call(g:python_channel, "python_eval", str)
endfunction

A problem with the above is that it would be incompatible with vim plugins since those use the builtin functions/commands, and it's not possible for the user to fine commands/functions that start with lowercase letters.

I'm not sure why vimscript has this limitation, perhaps @ZyX-I can explain

@mikaelj as far as I understand it, these extension point are for legacy support. To provide vimscripts with the ability to call into python.

When coding directly in python, it's not needed to use these extension points. Neovim doesn't need to know that it's python communicating with neovim, all it sees are msgpack messages coming in.

clipboard_{get,set} are a bit different, as they're extension points for well-known coprocesses.

True, the provider mechanism is for extending the C core with co-processes, but if we could lift the vimscript lowercase limitation, it would be immediately possible to move any knowledge about language-specific commands/functions to vimscript while maintaining compatibility with python-vim plugins

@mikaelj

This comment has been minimized.

Copy link

mikaelj commented Jul 18, 2014

On Fri, Jul 18, 2014 at 11:09 AM, Thiago de Arruda <notifications@github.com

wrote:

_{execute,execute_file,do_range,eval} being possible to add by
users? As I read your current design, each new language needs explicit
support in neovim. One of the goals of neovim is to make it more dynamic
with regard to scripting languages.

What if you could define, through neovim confguration, a new language? This
language would then support 0-4 of the extensions points defined for the
"language interface". set languages+=python:/path/to/python/binary or
something.

  • Micke

    That is a nice idea, and it would already be somewhat possible if
    channel_from_job is exposed to vimscript:

" vimrclet g:python_channel = channel_from_job('python', ['-c', 'import neovim; neovim.start_host()'])
command! -nargs=? Py call channel_send_call(g:python_channel, "python_execute", )
command! -nargs=? PyFile call channel_send_call(g:python_channel, "python_execute_file", )
command! -nargs=? PyDo call channel_send_call(g:python_channel, "python_do_range", )function! PyEval(str)
return channel_send_call(g:python_channel, "python_eval", str)endfunction

Yes, exactly, but perhaps with "python_execute" renamed to simply
"execute", for the generic "execute code in this language"-hook. Same for
execute_file/do_range/eval.

A problem with the above is that it would be incompatible with vim plugins
since those use the builtin functions/commands, and it's not possible for
the user to fine commands/functions that start with lowercase letters.

As for Python, I realize there needs to be a special case, but what of any
other language a user might want to add to his/hers neovim? Say I want to
interface with my Lisp of choice. I could just do this:

" vimrc
let g:lisp_channel = channel_from_job('sbcl', ['--eval', '(ql:quickload
"neovim")', '--eval', '(neovim:start-host)'])
command! -nargs=? Lisp call channel_send_call(g:lisp_channel, "execute",
)
command! -nargs=? LispFile call channel_send_call(g:lisp_channel,
"execute_file", )
command! -nargs=? LispDo call channel_send_call(g:lisp_channel, "do_range",
)
function! LispEval(str)
return channel_send_call(g:lisp_channel, "eval", str)
endfunction

Any reason to have the points prefixed with _? Maybe I don't
understand the mechanism correctly. This is how I would like the API to
look like.

-- Micke

@tarruda

This comment has been minimized.

Copy link
Member

tarruda commented Jul 18, 2014

Any reason to have the points prefixed with _? Maybe I don't
understand the mechanism correctly. This is how I would like the API to
look like.

For calling directly with channel_send_call through vimscript, there's no need. This is more of a namespace for generalizing the boilerplate in C. @8a091e7f5 is the commit that implements four generic functions that may be used by any language-specific command, and @486c8e37 builds on top of that to add python support.

I also want to reuse the provider mechanism for implementing language-specific hooks without giving Neovim the knowledge of every language at compile-time, for example:

  • python_complete
  • ruby_complete
  • perl_complete

Using language-specific namespacing we can make Neovim construct the full name by concatenating the filetype(of the current buffer) with the method name. We also don't assume that there must be exactly one channel per language. It would be useful for example, to let a single channel implement multiple language-specific completion functions(such as when connecting to a language-agnostic completion engine, like YouCompleteMe)

@mikaelj

This comment has been minimized.

Copy link

mikaelj commented Jul 18, 2014

On Fri, Jul 18, 2014 at 11:38 AM, Thiago de Arruda <notifications@github.com

wrote:

Any reason to have the points prefixed with _? Maybe I don't

understand the mechanism correctly. This is how I would like the API to
look like.

For calling directly with channel_send_call through vimscript, there's
no need. This is more of a namespace for generalizing the boilerplate in C.
@8a091e7 8a091e7 is the
commit that implements four generic functions that may be used by any
language-specific command, and @486c8e3
486c8e3 builds on top of that
to add python support.

I also want to reuse the provider mechanism for implementing
language-specific hooks without giving Neovim the knowledge of every
language at compile-time, for example:

  • python_complete
  • ruby_complete
  • perl_complete

Using language-specific namespacing we can make Neovim construct the full
name by concatenating the filetype(of the current buffer) with the method
name. We also don't assume that there must be exactly one channel per
language. It would be useful for example, to let a single channel implement
multiple language-specific completion functions(such as when connecting to
a language-agnostic completion engine, like YouCompleteMe)

Makes good sense. Cool, thanks for the explanation!

  • M
@aktau

This comment has been minimized.

Copy link
Member

aktau commented on src/nvim/ops.c in fba1d3b Jul 18, 2014

This looks like it has no side effects (has), but I assume it starts a provider on demand, no?

This comment has been minimized.

Copy link
Member

aktau replied Jul 18, 2014

Whoops, ignore me, I was an indentation level off.

@aktau

This comment has been minimized.

Copy link
Member

aktau commented on src/nvim/ops.c in fba1d3b Jul 18, 2014

Near as I can see, this allocates the strings into y_regs through xstrdup.

Later, strings are duplicated again in cstr_to_string. I don't see the corresponding free for these lines anywhere.

Or did you mean to use cstr_as_string? Or am I seeing badly... if this code is being tested the sanitizer must surely have picked up on this.

This comment has been minimized.

Copy link
Member

tarruda replied Jul 18, 2014

It will be freed later by free_register, but it's not being tested.

Good call though, I will add a test later with a dummy clipboard provider

This comment has been minimized.

Copy link
Member

aktau replied Jul 18, 2014

Alright, I'd feel safer 👍 ;)

@dbarnett

This comment has been minimized.

Copy link
Contributor

dbarnett commented Jul 18, 2014

I notice this doesn't change has('python'). Are you planning to do that later when python support is more stable?

@dbarnett

This comment has been minimized.

Copy link
Contributor

dbarnett commented Jul 18, 2014

Oh, I see. There's a :python command but no python providers yet?

@fwalch

This comment has been minimized.

Copy link
Member

fwalch commented Jul 18, 2014

@dbarnett If you follow the steps in the documentation, you'll get has('python').

Python scripting is also experimental and disabled by default.

@ZyX-I

This comment has been minimized.

Copy link
Contributor

ZyX-I commented Jul 18, 2014

A problem with the above is that it would be incompatible with vim plugins since those use the builtin functions/commands, and it's not possible for the user to fine commands/functions that start with lowercase letters.

I'm not sure why vimscript has this limitation, perhaps @ZyX-I can explain

This simplifies code a lot for a number of reasons:

  1. Built-in commands are defined completely in C and thus are special-cased. Or actually the opposite: user commands are special-cased.

  2. This kind of behaviour omits issues does not let one override built-in commands without a possibility for wars about whether something should (not) overrule something else. If you allow user-defined commands to have the same name as user ones you cannot possibly keep backwards compatibility and add any new commands at the same time: if e.g. some plugin defined :sign and then it was added to Vim then there is a problem: you either allow plugin-defined :sign (automatically breaking all plugins using this feature in case they are installed with plugin that define :sign) or break this plugin.

    In a project like Vim you cannot afford such things without proper namespacing. And Vim does not even have namespaces for functions (s:/<SID> functions are just usual global functions with a weird name "\<SNR>".sid."_".original_name).

There is also one thing: when translator will reach the point where it is possible to do optimizations knowing that :let is built-in :let and string(s) is using built-in string() will simplify optimizations. It would be good if there was ability to have user-defined strdisplaywidth() in older versions of Vim, but there is no way you can prevent the programmers from misusing this feature and have conflicts like “I used function tty() from plugin termlib, but after update it stopped working because it is now tty(client_id)”.

Thus you either have namespaces and make it impossible to use a huge number of optimizations or have a problem with backwards compatibility and a requirement to choose new function name very carefully to not disable widely used functions from some popular plugins. Since I would actually like to see VimL plugins buried and rewritten in better languages (VimL is still good for use from Ex mode) I would not add any features that make it significantly better for plugin developers. It would be a very hard work to make it better and still keep compatibility, convincing developers to use other languages is better.

Including “better” for Neovim lock-in: one can add to Python/Lua/Ruby/Perl interfaces and C core such feature that developers will not want to use Vim much easier then add namespaces to VimL: they are generally satisfied with extension language features (which are most hard to add) and will be very happy to see simpler features like sane interface to various things.

@bfredl

This comment has been minimized.

Copy link
Member

bfredl commented Jul 18, 2014

It looks like it wouldn't be hard to extend this to let the provider know the name of the register used (*, +, or unnamed), guess this already is the plan? (i e to access both Clipboard and Primary in X)

The clipboard provider will also make YankRing and similar plugins much simpler to implement, I think (right now YR needs to remap every register-affecting operator separately)

@justinmk justinmk referenced this pull request Jul 31, 2014

Merged

[RFC] August Newsletter #58

0 of 4 tasks complete
ADD(lines, STRING_OBJ(cstr_to_string((char *)reg->y_array[i])));
}

Object result = provider_call("clipboard_set", ARRAY_OBJ(lines));

This comment has been minimized.

@oni-link

oni-link Sep 5, 2014

Contributor

At this point feature "clipboard" may be missing (because the check in L5264 was not called: name=='*').

Trying to copy or paste a line with"*yy and "*p may fail.

(see also get_clipboard)

This comment has been minimized.

@tarruda
@elmart

This comment has been minimized.

Copy link
Member

elmart commented Jan 23, 2015

I've seen at coverity site that this PR should have fixed 68484 and 68485, but it didn't.
Could you have a look when you have time to, given you have perfect knowledge of that code?
Thanks.

dwb pushed a commit to dwb/neovim that referenced this pull request Feb 21, 2017

Fix neomake#utils#WideMessage for multibyte strings (neovim#895)
`strpart` handles bytes, which might cut strings like the following in
the middle of multibyte characters, resulting in a longer string (and
causing a hit-ENTER prompt):

> typography.symbols.ellipsis '...' is an approximation, use the ellipsis symbol '…'.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment