[vim/vim] [WIP] add ch_listen to listen socket (#3639)
mattn
This change add new function ch_listen() to listen socket channel. To know more description, please read my slide(P79-P81) of keynote speach on VimConf2018.
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/3639
Commit Summary
- support ch_listen
- update doc
- listen
- Merge branch 'master' into listen
File Changes
- M runtime/doc/eval.txt (14)
- M src/channel.c (234)
- M src/evalfunc.c (14)
- M src/proto/channel.pro (2)
- M src/structs.h (1)
- M src/testdir/test_channel.vim (26)
Patch Links:
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
Codecov
Codecov Report
Merging #3639 into master will increase coverage by
<.01%.
The diff coverage is63.02%.
@@ Coverage Diff @@ ## master #3639 +/- ## ========================================== + Coverage 77.37% 77.37% +<.01% ========================================== Files 99 99 Lines 139407 139526 +119 ========================================== + Hits 107862 107960 +98 - Misses 31545 31566 +21
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/evalfunc.c | 87.19% <100%> (+0.01%) |
⬆️ |
| src/channel.c | 82.29% <61.4%> (-0.93%) |
⬇️ |
| src/os_unix.c | 57.34% <0%> (-0.14%) |
⬇️ |
| src/term.c | 60.36% <0%> (-0.06%) |
⬇️ |
| src/syntax.c | 78.45% <0%> (+0.02%) |
⬆️ |
| src/screen.c | 79.24% <0%> (+0.06%) |
⬆️ |
| src/window.c | 83.55% <0%> (+0.06%) |
⬆️ |
| src/gui_gtk_x11.c | 48.5% <0%> (+0.39%) |
⬆️ |
| src/gui.c | 57.65% <0%> (+0.56%) |
⬆️ |
| ... and 1 more |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered by Codecov. Last update 3067a4d...dda0b89. Read the comment docs.
mattn
IMO, This is useful to communicate to Vim(s) with some protocols. For example, we will be possible to implement something without +clientserver feature.
依云
Would you add UNIX domain socket support? With TCP sockets you'd need a way to authenicate clients or trust all users on the machine (including all the daemon users for services). This is why I don't use plugins like deoplete.
mattn
Okay, I'll add it that specifying file path to the socket file with unix: prefix.
mattn
mattn
Now supported AF_UNIX. This should work on Windows 10 too.
mattn
@mattn pushed 50 commits.
- 18e4713 support blob
- 98240e3 support blob
- 089672a blob
- 188a2da blob
- 7d4fdd7 blob
- 44cd8d5 blob
- be06f9f blob
- 275e7f1 blob
- d244cb4 blob
- 433c5d4 blob
- a58f643 blob
- bc7ae2e blob
- ec4baa2 blob
- 8151328 blob
- a1d885e blob
- 36cfaad blob
- 7118dbb blob
- 540b799 blob
- bc0c71b blob
- 4072dd0 blob
- ad8cc38 blob
- 3403c43 blob
- 935653c blob
- 1f8f395 blob
- e0a321e blob
- 9ffe614 blob
- f2e95ab Merge branch 'master' into blob
- 3551a99 Merge branch 'master' into blob
- cfadbe7 fix index()
- d80385e fix blob
- f7bfed5 fix blob
- 4e22aca fix blob
- 42f0147 let blob[0:1]
- 31f9828 Merge branch 'master' into blob
- e6470d5 Merge branch 'blob' of bitbucket.org:mattn/vim into blob
- 3f6b845 Merge branch 'blob' of github.com:mattn/vim into blob
- 86ab621 fix blob
- 59c2285 add test
- 2f61611 fix Makefile
- 4ccd5f8 fix Make_mvc.mak
- d588615 Fix doc
- 0555010 Fix compilation warnings
- d7b5d60 I made the following modifications.
- a45c2e8 Merge pull request #1 from h-east/blob
- 03f65db support blob in if_python3
- ce7c111 add String#to_blob to return string 0zXXXX in if_ruby
- fe13680 Merge branch 'listen' of github.com:mattn/vim into listen
- bc86795 Merge branch 'master' into listen
- 557dc3a Merge master
- 9413706 Merge master
—
You are receiving this because you are subscribed to this thread.
Bram Moolenaar
This requires more documentation. A practical example is preferred.
Bram Moolenaar
Still awaiting a practical example.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Prabir Shrestha
Here is an example I can think of:
Vim is single threaded so even if we improve perf by vimscript9 or lua there will be certain heavy computation that will cause vim to pause for certain time. Being able to start a new vim process and communicate with sockets/pipes/stdin/stdout will help us. My previous ask on having remote lua plugins contains more details on a concrete example such as fuzzy matching. #2115.
natebosch/vim-lsc#291 Here was another example where dart language server was returning 27k items and vim was hanging while vscode had no issue.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Yegappan Lakshmanan
Another example use case for ch_listen() function:
Implementing a debugger for Vim scripts. We can run the script under debug in a
separate Vim instance where a server is running (using ch_listen). From the outside
Vim instance, we can connect to this server and run various commands (using the
JSON interface) to get the state of the script. Currently we need to use an external
server (written in Python or some other language) for this. Using ch_listen(), we can
natively do this in Vim without any external dependencies.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Ben Jackson
@yegappan did you see this? https://groups.google.com/forum/m/#!searchin/vim_dev/Vimspector$20/vim_dev/Xzb0a3bTg1U done without listen uses a debug adapter.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Bram Moolenaar
I can guess ch_listen() can make some things work that were not possible before. But this pull request is still missing an example. When someone wants to use ch_listen(), it must be clear how it works. Also to understand that it cannot do. A working example that someone can play with works best.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Yegappan Lakshmanan
@yegappan did you see this? https://groups.google.com/forum/m/#!searchin/vim_dev/Vimspector$20/vim_dev/Xzb0a3bTg1U done without listen uses a debug adapter.
vim-dev ML
> https://groups.google.com/forum/m/#!searchin/vim_dev/Vimspector$20/vim_dev/Xzb0a3bTg1U
> done without listen uses a debug adapter.
>
>
> I went through the Vimspector documentation. It looks like it supports
languages
with a debug adapter but doesn't support debugging Vimscript. I was
referring to
creating a debugger for Vimscript. Even with ch_listen(), we will need
support
for running the Vim debugger in the background while still being able to see
the Vim output screen. This will need some changes to the current Vim
debugger.
Regards,
Yegappan
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Ben Jackson
wasreferring to creating a debugger for Vimscript.
that's exactly what I've done, and what my RFC and demo are all about: https://github.com/puremourning/vim-debug-adapter and my vim debugger branch: https://github.com/puremourning/vim/tree/debugger
I think I might have copied a dud url - this is the RFC I sent: https://groups.google.com/d/msg/vim_dev/Xzb0a3bTg1U/m3WMcOu2BAAJ
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or unsubscribe.
Yegappan Lakshmanan
wasreferring to creating a debugger for Vimscript.
that's exactly what I've done, and what my RFC and demo are all about: https://github.com/puremourning/vim-debug-adapter and my vim debugger branch: https://github.com/puremourning/vim/tree/debugger
I think I might have copied a dud url - this is the RFC I sent: https://groups.google.com/d/msg/vim_dev/Xzb0a3bTg1U/m3WMcOu2BAAJ
vim-dev ML
>
> wasreferring to creating a debugger for Vimscript.
>
> that's exactly what I've done, and what my RFC and demo are all about:
> https://github.com/puremourning/vim-debug-adapter and my vim debugger
> branch: https://github.com/puremourning/vim/tree/debugger
>
> I think I might have copied a dud url - this is the RFC I sent:
> https://groups.google.com/d/msg/vim_dev/Xzb0a3bTg1U/m3WMcOu2BAAJ
>
>
>
This looks very interesting and useful. Hopefully this gets integrated into
Vim at some point.
Regards,
Yegappan
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Ben Jackson
Thanks. My goal is to get it all to a shippable/professional level "in 2020". I expect the first PRs in Q3. It's likely to be a lot of smaller PRs and some discussion with Bram/etc. so I wouldn't expect it super soon, but you're more than welcome to try the branch out now. It does mostly work.
—
You are receiving this because you commented.
Alisue
Any news? I'd love to make a plugin which shows a prompt/buffer to input text for commands in jobstart() through some environment variable (e.g. EDITOR or SSH_ASKPASS).
I can make it for Neovim while it has RPC but Vim. If there is ch_listen(), I can use it to handle the Vim from a process executed in jobstart().
Bram Moolenaar
This is still lacking an example.
The help is very brief. I guess "List to a channel" actually means "listen on a socket".
Does this work on all systems?
"sys/un.h" is a newly used include file, is this available everywhere or does it require a configure check?
The test lacks testing for the possible errors.
Ben Jackson
@puremourning commented on this pull request.
I was curious about how this would work, so I added some commentary. The API seems a little surprising to me, and it seems to introduce a new unix domain sockets support (which I think is welcome, but undocumented and untested).
> @@ -584,6 +584,17 @@ ch_info({handle}) *ch_info()*
Can also be used as a |method|: >
GetChannel()->ch_info()
+ch_listen({address} [, {options}]) *ch_listen()*
This seems to imply that there may only be one attached client on this socket. I would have expected there to be a sort of accept callback which is passed the channel which is created by the client socket event, returning true/false (maybe) as to whether or not to accept the connection.
Perhaps supporting multiple clients on the same port is just something that a text editor shouldn't do, but it doens't seem much of a stretch from 1 to N.
In src/channel.c:
> + {
+ is_unix = TRUE;
+ vim_memset((char *)&unixaddr, 0, sizeof(unixaddr));
+ unixaddr.sun_family = AF_UNIX;
+ STRNCPY(unixaddr.sun_path, channel->ch_hostname+5, sizeof(unixaddr.sun_path)-1);
+ }
+ else
+ {
+ /* Get the server internet address and put into addr structure */
+ /* fill in the socket address structure and connect to server */
+ vim_memset((char *)&server, 0, sizeof(server));
+ server.sin_family = AF_INET;
+ server.sin_port = htons(port);
+ if ((host = gethostbyname(channel->ch_hostname)) == NULL)
+ {
+ ch_error(channel, "in gethostbyname() in channel_open()");
does this error include anything that can be used to work out what went wrong?
What happens if we use something like ch_listen( '0.0.0.0:1234' ) ? How about IPV6 addresses ?
In src/channel.c:
> #endif
+ channel = add_channel();
+ if (channel == NULL)
+ {
+ ch_error(NULL, "Cannot allocate channel.");
+ return -1;
+ }
+
+ if (STRNCMP(channel->ch_hostname, "unix:", 5) == 0)
Is this unrelated to this PR? I mean, is this an undocumented way to use unix domain sockets?
I mean I'm all for supporting unix domain sockets for ch_open() and friends, but is that strictly part of this PR?
> @@ -2448,6 +2448,8 @@ ch_evalraw({handle}, {string} [, {options}])
ch_getbufnr({handle}, {what}) Number get buffer number for {handle}/{what}
ch_getjob({channel}) Job get the Job of {channel}
ch_info({handle}) String info about channel {handle}
+ch_listen({address} [, {options}])
+ Channel listen to a channel with {address}
I think this text could be improved, "Start listening for connections to {address}" perhaps.
> @@ -584,6 +584,17 @@ ch_info({handle}) *ch_info()*
Can also be used as a |method|: >
GetChannel()->ch_info()
+ch_listen({address} [, {options}]) *ch_listen()*
+ Listen to a channel with {address}. See |channel|.
+ Returns a Channel. Use |ch_status()| to check for failure.
+
+ {address} has the form "hostname:port", e.g.,
+ "localhost:8765" or ":8765".
+
+ If {options} is given it must be a |Dictionary|.
+ See |channel-open-options|.
do all options apply?
> @@ -584,6 +584,17 @@ ch_info({handle}) *ch_info()*
Can also be used as a |method|: >
GetChannel()->ch_info()
+ch_listen({address} [, {options}]) *ch_listen()*
+ Listen to a channel with {address}. See |channel|.
+ Returns a Channel. Use |ch_status()| to check for failure.
What happens if you write to or read from the channel before the client connects? For that matter, how is the user informed of the client connection?
Or is this API assuming that you'll wait synchronously for a client to connect to this port ? If so I think that's (probably) bad. I find it hard to imagine that all use cases for this would want to block until some client connects. I would have expected it to be more event driven, or at least.... that should be an option?
For simplicity sake, I can see that a synchronous version might be useful I suppose.
> @@ -584,6 +584,17 @@ ch_info({handle}) *ch_info()*
Can also be used as a |method|: >
GetChannel()->ch_info()
+ch_listen({address} [, {options}]) *ch_listen()*
+ Listen to a channel with {address}. See |channel|.
"Listening to a channel" isn't really what we're doing, we're listening on a port (ok, a socket), there result of which is a channel representing a connection to a client.
In src/channel.c:
> +
+ if (STRNCMP(channel->ch_hostname, "unix:", 5) == 0)
+ {
+ is_unix = TRUE;
+ vim_memset((char *)&unixaddr, 0, sizeof(unixaddr));
+ unixaddr.sun_family = AF_UNIX;
+ STRNCPY(unixaddr.sun_path, channel->ch_hostname+5, sizeof(unixaddr.sun_path)-1);
+ }
+ else
+ {
+ /* Get the server internet address and put into addr structure */
+ /* fill in the socket address structure and connect to server */
+ vim_memset((char *)&server, 0, sizeof(server));
+ server.sin_family = AF_INET;
+ server.sin_port = htons(port);
+ if ((host = gethostbyname(channel->ch_hostname)) == NULL)
at least on my system, man gethostbyname says
The getaddrinfo(3) and getnameinfo(3) functions are preferred over the gethostbyname(), gethostbyname2(), and gethostbyaddr() functions.
In src/channel.c:
> @@ -3569,6 +3877,48 @@ channel_read(channel_T *channel, ch_part_T part, char *func)
{
if (channel_wait(channel, fd, 0) != CW_READY)
break;
+ if (channel->ch_listen)
so is this the only way to accept from the socket? I.e. the user has to do something like:
let ch = ch_listen( address, #{ callback: function( 'HandleDataOnSocket' ) } ) " see time later... " this is a blocking call and we'll get data in the callback ? It will return after calling our callback ? call ch_read( ch )
I think we really need an example of how to use this API in practice because it's really not obvious how to use it (as-is) for any of the proposed use cases. Use of ch_read() as a proxy for accept() when the channel happens to be listening seems surprising to me.
In src/testdir/test_channel.vim:
> @@ -1908,6 +1908,32 @@ func Test_close_lambda()
call s:run_server('Ch_test_close_lambda')
endfunc
+let g:server_received_addr = ''
+let g:server_received_msg = ''
+func s:test_listen_accept(ch, addr)
+ let g:server_received_addr = a:addr
+ let g:server_received_msg = ch_readraw(a:ch)
+endfunction
+
+func Test_listen()
tests seem... optimistic.
the code seems to include a completely new mechanism for using unix: to use a domain socket, but no tests for it.
In src/testdir/test_channel.vim:
> @@ -1908,6 +1908,32 @@ func Test_close_lambda()
call s:run_server('Ch_test_close_lambda')
endfunc
+let g:server_received_addr = ''
+let g:server_received_msg = ''
+func s:test_listen_accept(ch, addr)
+ let g:server_received_addr = a:addr
+ let g:server_received_msg = ch_readraw(a:ch)
+endfunction
+
+func Test_listen()
+ call ch_log('Test_listen()')
+ let server = ch_listen('127.0.0.1:' . 12345, {"callback": function("s:test_listen_accept")})
+ if ch_status(server) == "fail"
+ call assert_report("Can't listen channel")
+ return
so you set the channel callback to something which calls ch_read, which is what calls accept ?
how does it receive subsequent messages ? how to you write to the channel ?
skanehira
I want to make a markdown preview plugin using ch_listen.
I think ch_listen brings a good experience because If we can use ch_listen, we don't need any dependencies to make some plugins.
Bram Moolenaar
Closed #3639.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
Bram Moolenaar
A lot of time has passed and there still is no example of how this wood be used.
Theoretically this would add a useful feature, but it's not clear if this is the right way to do this.
I'll close this now. Feel free to make a better version of this PR.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
Christian Brabandt
chrisbra left a comment (vim/vim#3639)People who would like this, can you please see if #19231 is working for your use case?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.