Skip to content
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

fix: clean-up neovim processes. closes #2038 #2395

Merged
merged 1 commit into from
Feb 22, 2018
Merged

fix: clean-up neovim processes. closes #2038 #2395

merged 1 commit into from
Feb 22, 2018

Conversation

jpoon
Copy link
Member

@jpoon jpoon commented Feb 20, 2018

What this PR does / why we need it: Properly kills neovim processes on exit. Refactoring of neovim to a class.

Which issue(s) this PR fixes #2038

Special notes for your reviewer:

} else {
await cmd.execute(vimState.editor, vimState);
}
} catch (e) {
if (e instanceof VimError) {
if (e.code === ErrorCode.E492 && configuration.enableNeovim) {
await Neovim.command(vimState, command);
await vimState.nvim.run(vimState, command);
Copy link
Member Author

@jpoon jpoon Feb 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Chillee, I don't quite understand this bit. Why are we retrying with neovim again?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we pass the command into our own command line command/parser, there's lots of cases in which the parser will error (since it doesn't support a lot of commands).

If the parser can't parse it, then we try sending it to the neovim parser (which may support the command).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So basically, the above code that detects if the command is neovim capable and passes it to Neovim only works for commands that we know are neovim capable. There's lots of commands that we don't implement, and the parser errors out in those cases.

Copy link
Member

@Chillee Chillee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It all makes sense to me. Seems like a pretty straightforward refactor to make it dispose properly.

@jpoon jpoon merged commit 8e73cab into master Feb 22, 2018
@jpoon jpoon deleted the neovim-clean-up branch February 22, 2018 15:20
@jplew
Copy link

jplew commented Feb 26, 2018

just want to say that v11.0 has restored neo-vim functionality for me. Neo-vim features were broken for me in 10.13 but this patch seems to have fixed it. Thanks!

This pull request was closed.
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.

3 participants