API for connecting a client to an existing server by name

[mudge]

It's not currently possible to create a Vimrunner::Server instance (and therefore Vimrunner::Client connected to it) that represents an existing, running Vim server.

This means that it is not possible to start a Vim server independently of Vimrunner and then control it remotely.

We could consider this out of scope but perhaps Vimrunner's remit is to provide an abstraction on top of Vim's remote functionality which should include such a method of interaction.

Perhaps something like:

$ vim --servername FOO

vim = Vimrunner.connect("FOO")
vim.insert("Hello world!")

[AndrewRadev]

This seems like a very good idea and it shouldn't be difficult to implement in the current architecture. I did a first attempt in the connecting-to-existing-server branch. I just added a second parameter, name, to the initializer. With this, the two use cases are:

server = Vimrunner::Server.new('vim')
server.insert('foo')
server.kill

system('vim --servername FOO')
server = Vimrunner::Server.new('vim', 'FOO')

if server.connected?
  server.insert('foo')
else
  puts "Something went wrong"
end

I'm not sure if a simple connected? is enough, though, since it will probably not return true right away, so it kind of depends on the use case. We could add a timeout and return false only after 5 seconds. Alternatively, we could expose wait_until_started (and maybe rename it to wait_until_connected for consistency), so that people can create a server and wait until it's good and ready. We could also make the timeout a parameter to allow more flexibility (maybe someone's Vim starts up slowly?). So, maybe something like this:

system('vim --servername FOO')
server = Vimrunner::Server.new('vim', 'FOO')
server.wait_until_connected(10)
server.insert('foo')

I'm also very much in favor of making a shorthand Vimrunner.connect, but I would rather just have it a shorthand, a one-liner, just as the other methods in that module.

Let me know what you think. Also, feel free to hack on this branch or make a new one if you have a completely different vision of the API.

[mudge]

This looks good; I'm a little uneasy about the mandatory first parameter being the Vim binary but perhaps that would go away as soon as we make a Vimrunner.connect perhaps like so:

module Vimrunner
  def self.connect(name)
    Server.new(Platform.vim, name).new_client
  end
end

After all, the use case I'm imagining is that the user starts their own server independently (as in your system example) and the use of the Platform.vim is a mere implementation detail.

Exposing wait_until_connected with a timeout sounds good too exactly for your use case of waiting for a responsive server.

[AndrewRadev]

Ah, hm, I hadn't realised the first parameter thing. In that case, I propose we just pass in an options hash and make both of them optional (because they are optional and completely independent, anyway):

Vimrunner::Server.new({
  :executable => 'vim',
  :name       => 'FOO',
})

# or even:

Vimrunner::Server.new(executable: 'vim', name: 'FOO')

It's true that Vimrunner.connect would solve the issue and keep backwards compatibility, but then, if we'll be breaking the API, we might as well do it now. It's not used by a whole lot of people, and the ones I know of just use Vimrunner.start anyway.

What do you think?

[mudge]

Hi Andrew,

I've pushed some commits with the following changes and additions:

• Change Vimrunner::Server.new to take a hash of options;
• Add Vimrunner::Server#connect as an analogue to Vimrunner::Server#start for existing servers: it will block until a server is connected and then return a Client;
• Add Vimrunner.connect as a convenience API for connecting to a server and then returning a client.

Please have a look and let me know what you think (and sorry for the delay).

[AndrewRadev]

Looks pretty good to me. I particularly like re-using connect within start, it's a very natural refactoring.

Could you also mention the connect functionality in the README, so people are aware of it?

(and sorry for the delay)

Don't worry about it, I haven't had the time to work on Vimrunner myself these last few days. I'll try to address the other issues soon.

[mudge]

Thanks for having a look; I've now added a mention of Vimrunner.connect and an example to the README.

[AndrewRadev]

Merged! I'm going to hold off on deploying a new version, since we have some more changes in the pipeline anyway.