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] remote: add node host #7458

Merged
merged 3 commits into from Nov 11, 2017

Conversation

Projects
None yet
6 participants
@billyvg
Contributor

billyvg commented Oct 29, 2017

This adds support for node.js as a remote host, also adds health check.

@billyvg billyvg changed the title from remote: add node host to (WIP) remote: add node host Oct 29, 2017

let s:stderr = {}
let s:job_opts = {'rpc': v:true}

function! s:job_opts.on_stderr(chan_id, data, event)

This comment has been minimized.

@bfredl
endif
call health#report_info('Host: '. host)

let latest_npm_cmd = has('win32') ? 'cmd /c npm info neovim --json' : 'npm info neovim --json'

This comment has been minimized.

@justinmk

justinmk Oct 31, 2017

Member

should not need a separate command for Windows, I believe the ruby check does it that way because of the different quoting. (Though it may possibly be unnecessary there as well, since system([...]) is called with a list.)

This comment has been minimized.

@janlazo

janlazo Oct 31, 2017

Contributor

Safe to use same command in Windows. Nevermind. npm.cmd calls npm node in the same directory. npm in Windows is a shell script.

This comment has been minimized.

@justinmk

justinmk Oct 31, 2017

Member

Ah, ok.

This comment has been minimized.

@justinmk

justinmk Oct 31, 2017

Member

It's the same case for ruby, I think I recall.

This comment has been minimized.

@billyvg

billyvg Nov 1, 2017

Contributor

(This used ruby's provider script as a template)

@bfredl bfredl changed the title from (WIP) remote: add node host to [WIP] remote: add node host Oct 31, 2017

@marvim marvim added the WIP label Oct 31, 2017

@justinmk

This comment has been minimized.

Member

justinmk commented Nov 9, 2017

@billyvg still WIP?

@billyvg

This comment has been minimized.

Contributor

billyvg commented Nov 10, 2017

@justinmk Nope, I think it's good.

@justinmk justinmk changed the title from [WIP] remote: add node host to [RFC] remote: add node host Nov 11, 2017

@justinmk justinmk merged commit b6a603f into neovim:master Nov 11, 2017

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
QuickBuild Build pr-7458 finished with status SUCCESSFUL
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@justinmk justinmk removed the WIP label Nov 11, 2017

@watsoncj watsoncj referenced this pull request Nov 14, 2017

Open

update README #41

return
endif

if !executable('node') || !executable('npm') || !executable('yarn')

This comment has been minimized.

@watsoncj

watsoncj Nov 14, 2017

why is yarn a requirement, @billyvg ?

This comment has been minimized.

@billyvg

billyvg Nov 14, 2017

Contributor

Oops, it shouldn't be, it seems I didn't pull in neovim/node-host@40c370a

justinmk added a commit to justinmk/neovim that referenced this pull request Nov 18, 2017

NVIM v0.2.2
FEATURES:
b6a603f neovim#7458 node.js remote-plugin support
f5d4da0 :checkhealth : validate 'runtimepath' (neovim#7526)

FIXES:
e6beb60 :terminal : fix crash on resize (neovim#7547)
07931ed tui: 'guicursor': use DECSCUSR for xterm-likes (neovim#7576)
f185c73 neovim#7561 'os_open: UV_EINVAL on NULL filename'
e8af34d win: provider: Detect(): return *.cmd path (neovim#7577)
eacd788 :checkhealth : fix check for npm and yarn (neovim#7569)
a43a573 health.vim: normalize slashes for script path (neovim#7525)
69e3308 cmake: install runtime/rgb.txt
d0b05e3 runtime: Fix syntax error in `runtime/syntax/tex.vim` (neovim#7518)
55d8967 tutor: some fixes (neovim#7510)

CHANGES:
9837a9c remove legacy alias to `v:count` (neovim#7407)
c5f001a runtime: revert netrw update (neovim#7557)
67e4529 defaults: scrollback=10000 (neovim#7556)
881f9e4 process_close(): uv_unref() detached processes (neovim#7539)

justinmk added a commit to justinmk/neovim that referenced this pull request Nov 18, 2017

NVIM v0.2.2
FEATURES:
a6de144 'viewoptions': add "curdir" flag neovim#7447
b6a603f node.js remote-plugin support neovim#7458
f5d4da0 :checkhealth : validate 'runtimepath' neovim#7526

FIXES:
e6beb60 :terminal : fix crash on resize neovim#7547
f19e5d6 work around gnome-terminal memory leak neovim#7573
07931ed 'guicursor': use DECSCUSR for xterm-likes neovim#7576
f185c73 'os_open: UV_EINVAL on NULL filename' neovim#7561
e8af34d win: provider: Detect(): return *.cmd path neovim#7577
eacd788 :checkhealth : fix check for npm and yarn neovim#7569
a43a573 health.vim: normalize slashes for script path neovim#7525
69e3308 cmake: install runtime/rgb.txt
d0b05e3 runtime: syntax error in `runtime/syntax/tex.vim` neovim#7518
55d8967 tutor: some fixes neovim#7510

CHANGES:
9837a9c remove legacy alias to `v:count` neovim#7407
c5f001a runtime: revert netrw update neovim#7557
67e4529 defaults: scrollback=10000 neovim#7556
881f9e4 process_close(): uv_unref() detached processes neovim#7539
@janlazo

This comment has been minimized.

Contributor

janlazo commented Nov 28, 2017

I can't find any functional tests for node provider in https://github.com/neovim/neovim/tree/master/test/functional/provider.

@justinmk

This comment has been minimized.

Member

justinmk commented Nov 28, 2017

@janlazo There aren't any. Would be nice to have a couple.

@janlazo

This comment has been minimized.

Contributor

janlazo commented Nov 30, 2017

Any docs to refer to? There's no VimL sugar for node provider so I can't think of a test case for node_spec.lua

@janlazo

This comment has been minimized.

Contributor

janlazo commented Nov 30, 2017

Is it impossible to write a one-liner test for node host through VimL?
The example in https://github.com/neovim/node-client#api-work-in-progress uses decorators but I don't have babel (neovim/node-client#43).

@justinmk

This comment has been minimized.

Member

justinmk commented Nov 30, 2017

I don't know. Maybe @billyvg can comment.

@billyvg

This comment has been minimized.

Contributor

billyvg commented Nov 30, 2017

@janlazo you can try the example mentioned here: neovim/node-client#43

@billyvg billyvg deleted the billyvg:feat/add-node-host branch Nov 30, 2017

@janlazo

This comment has been minimized.

Contributor

janlazo commented Nov 30, 2017

I tried that with some changes (run echo "Hello", not vsplit) but I couldn't register the command to rplugin.vim.
I placed the new file in %LOCALAPPDATA%\nvim\rplugin\node\hello.js (Windows user). rplugin.vim registers the file but it has no functions/commands attached to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment