ripienaar / graphite-graph-dsl

A small DSL to describe graphite graphs
http://www.devco.net/
Apache License 2.0
167 stars 43 forks source link

Uninitialized constant URI::REGEXP::UNSAFE #19

Closed jrust closed 12 years ago

jrust commented 12 years ago

Since pull request #16 it no longer works in ruby 1.9.3 because the URI::REGEXP::UNSAFE constant is only available in 1.8.6. This is the offending line: https://github.com/ripienaar/graphite-graph-dsl/blob/master/lib/graphite_graph.rb#L342

However, I'm not sure how to fix it because it's not clear to me what it's doing or why that was added.

ripienaar commented 12 years ago

OK, yah not entirely sure myself, have commented on the original pull hopefullyt he author can help

ripienaar commented 12 years ago

Have pushed 0.0.5 that uses CGI.escape instead of URI.encode and it seems good now

jrust commented 12 years ago

Unfortunately that led to too much being escaped. I end up with image URLs like: /render/?title%3DBilling%26vtitle%3DBilling which result in No Data images. It seems like just the value parts of the key/value pairs should be escaped. This got all my graphs working again:

url_str = url_parts.map { |pair| k,v = pair.split('='); "#{k}=#{CGI.escape(v)}" }.join("&")
properties[:placeholders].each { |k,v| url_str.gsub!("%{#{k}}", v.to_s) } if properties[:placeholders].is_a?(Hash)
url_str
ripienaar commented 12 years ago

Ah yeah indeed, you would just escape the values.

Don't code before coffee :)

Will do another release soon

ripienaar commented 12 years ago

OK, I did 0.0.6, lets see how that one goes :)

jrust commented 12 years ago

Yup, worked great after bumping gdash up to the 0.0.6. Thanks.