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 blocking messages thread #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mluts
Copy link

@mluts mluts commented Apr 23, 2018

Hi, thank you for this repo, i'm glad that there is something like that in the wild.

While i was working on my plugin for vim i encountered a problem with messages thread. When i'm in the middle of accepting rpc request from nvim i can't do requests back to nvim because it will block messages thread forever.

So far these changes worked for me.

...And i think those handlers deserve their own functions

@mluts mluts force-pushed the handle-messages-asynchronously branch from fb26840 to 35be51c Compare April 23, 2018 08:09
@mluts mluts force-pushed the handle-messages-asynchronously branch from 35be51c to 11b832e Compare April 23, 2018 08:10
@jebberjeb
Copy link
Member

Thanks @mluts, will take a look shortly.

mdiin added a commit to mdiin/clojure-socketrepl.nvim that referenced this pull request May 15, 2018
I debated two different approaches:

1. Inject a namespace which loads needed dependencies and uses
`clojure.repl/apropos` as a fallback
2. Use a custom classloader with the classpath of the repl project and
any dependencies

Ultimately I decided against option 2 because it has a lot more upkeep
with regards to dynamic evaluation in the repl; e.g. user evals a new
defn, that won't be picked up (I think?). The other point is that I know
too little about how classloaders work to feel comfortable using a
custom one.

Omnicomplete function requires the plugin to be able to return data to
the client. This was not possible with the async implementation of
`run-command`, so I renamed that to `run-command-async` and added
`run-command` as a sync alternative.

NOTE: This requires clojure-vim/neovim-client#4
mdiin added a commit to mdiin/clojure-socketrepl.nvim that referenced this pull request May 15, 2018
I debated two different approaches:

1. Inject a namespace which loads needed dependencies and uses
`clojure.repl/apropos` as a fallback
2. Use a custom classloader with the classpath of the repl project and
any dependencies

Ultimately I decided against option 2 because it has a lot more upkeep
with regards to dynamic evaluation in the repl; e.g. user evals a new
defn, that won't be picked up (I think?). The other point is that I know
too little about how classloaders work to feel comfortable using a
custom one.

Omnicomplete function requires the plugin to be able to return data to
the client. This was not possible with the async implementation of
`run-command`, so I renamed that to `run-command-async` and added
`run-command` as a sync alternative.

NOTE: This requires clojure-vim/neovim-client#4
mdiin added a commit to mdiin/clojure-socketrepl.nvim that referenced this pull request May 16, 2018
I debated two different approaches:

1. Inject a namespace which loads needed dependencies and uses
`clojure.repl/apropos` as a fallback
2. Use a custom classloader with the classpath of the repl project and
any dependencies

Ultimately I decided against option 2 because it has a lot more upkeep
with regards to dynamic evaluation in the repl; e.g. user evals a new
defn, that won't be picked up (I think?). The other point is that I know
too little about how classloaders work to feel comfortable using a
custom one.

Omnicomplete function requires the plugin to be able to return data to
the client. This was not possible with the async implementation of
`run-command`, so I renamed that to `run-command-async` and added
`run-command` as a sync alternative.

NOTE: This requires clojure-vim/neovim-client#4
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.

2 participants