Improved command map

[kirs]

As I wrote in capistrano/capistrano#639, we need more ways to configure command map behaviour.

This PR adds prefixes feature.

[kirs]

@leehambley after your review, I'm going to make new versions of rvm/rbenv/bundler integrations using this code.

[leehambley]

@kirs looks good, the only thing I would have done differently is to specify whether add is a prefix, or suffix. (i.e does add push or unshift onto the list).

Given that this is mostly an internal API it doesn't matter if we just call it add, but perhaps we could also use push and/or unshift or something to be clear that sometimes you might combine them (i.e unshift rvm support to make sure it's ALWAYS first), and push for things such as bundler that always go right before the command?

I'd be happy to merge this as it is, and mark the API as private, and re-visit if you think we could benefit from push and unshift when you begin working on the integrations for the ruby switchers, and bundler, etc?

[kirs]

Do you mean the API like this?

SSHKit.config.command_map.prefix[:rake].push("bundle exec")
SSHKit.config.command_map.prefix[:rake].unshift("rbenv exec")

?

[leehambley]

Do you mean the API like this?

Yes, exactly, so that capistrano-{rvm,rbenv,chruby} can be included before, or after capistrano-bundler without problems, and they speficy really whether they are prefixes or suffixes.

I'm kind of afraid of load ordering issues, and of unclean terms like "add". But that change looks like exactly what I had in mind.

I assume it's possible to do something like prefixing everything in the command map, as we do in some places now, like pushing a general suffix, not just for rake?

(then maybe the map could be empty, and we could push /usr/bin/env as a prefix? )

[kirs]

There is a priority option that works like unshift now: SSHKit.config.command_map.add_prefix(:rake, "/usr/local/rbenv/bin exec", priority: :first)

But I'll rework it anyway with the syntax I wrote in the previous comment.

I don't have a clear idea how to prefix everything yet.

[kirs]

@leehambley updated

[kirs]

@leehambley what do you think about a new release of SSHKit with this API?

This will give us an opportunity to release new rubygems.org versions of capistrano-* plugins.

[leehambley]

I'd like to do it all for 3.1
On 17 Nov 2013 14:43, "Kir Shatrov" notifications@github.com wrote:

@leehambley https://github.com/leehambley what do you think about a new
release of SSHKit with this API?

This will give us an opportunity to release new rubygems.org versions of
capistrano-* plugins.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/45#issuecomment-28648629
.

[kirs]

I've prepared few changes to try prefixes:

capistrano/rbenv#14
capistrano/rvm#22
capistrano/bundler#16

[leehambley]

Awesome, thanks - let's try and get all of this out on Friday, will you
have time?

Lee Hambley

http://lee.hambley.name/
+49 (0) 170 298 5667

On 19 November 2013 08:50, Kir Shatrov notifications@github.com wrote:

I've prepared few changes to try prefixes:

capistrano/rbenv#14 capistrano/rbenv#14
capistrano/rvm#22 capistrano/rvm#22
capistrano/bundler#16 capistrano/bundler#16

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/45#issuecomment-28771641
.

[kirs]

Hopefully 😄