thoughtbot / cocaine

A small library for doing (command) lines.
https://robots.thoughtbot.com
Other
785 stars 55 forks source link

Incorrect replacement of command options with same prefix #55

Closed sshaw closed 11 years ago

sshaw commented 11 years ago

While working around this I came across the following:

line  = Cocaine::CommandLine.new("echo", ":file1 :file11")
puts line.run(:file1 => "one", :file11 => "eleven") # echo 'one' 'one'1

After a wee bit of head scratching and source browsing I realized the problem --and the solution: wrap my interpolations with :{}. I didn't see this mentioned in the docs so my concern is that it's an undocumented feature and may be removed at some point. Please clarify.

Also why not just throw a word boundary at the end of the regex? I can submit a patch if needed.

jyurek commented 11 years ago

Hmm, you're right, it's not mentioned. It should be; this isn't something I intend on it going away. A word boundary would work as well. Thanks for the heads up.

jyurek commented 11 years ago

I fixed this in 93078c4e703683fa37aa52656c3f7a65c6d09917, but since there's a slight open question, I'm not going to push this in a gem right away. Thanks for reporting!

sshaw commented 11 years ago

Too pedantic, possibly, but with a word boundary the problem can still exist:

# with boundary
line  = Cocaine::CommandLine.new("echo", ":file1 :file1-1")
puts line.run("file1" => "one", "file1-1" => "eleven") # echo 'one' 'one'1

I think space would be better as it's more in line with the shell's param parsing rules. Leads to a kind of ugly regex:

command_string.gsub(/:\{?#{key}(?=\s|\Z)\}?/) { shell_quote(value) }

But maybe that's when the caller should just break out the brackets...