weavejester / hiccup

Fast library for rendering HTML in Clojure
http://weavejester.github.io/hiccup
Eclipse Public License 1.0
2.68k stars 174 forks source link

image should encode the url #74

Closed yogthos closed 11 years ago

yogthos commented 11 years ago

for example if you have an image link with a filename 12 - 2.jpg

this will work correctly

[:img {:src "/img/12 - 2.jpg"}]

but using image does not

(image "/img/12 - 2.jpg")

because to-url will throw an exception parsing the string

URI.java:2829   java.net.URI$Parser.fail
URI.java:3002   java.net.URI$Parser.checkChars
URI.java:3086   java.net.URI$Parser.parseHierarchical
URI.java:3044   java.net.URI$Parser.parse
URI.java:595    java.net.URI.<init>
util.clj:47 hiccup.util/eval1841[fn]
...
weavejester commented 11 years ago

I think throwing an exception in the case of a bad image source is the right approach. Silently fixing the mistakes of developers can only lead to bad things.

yogthos commented 11 years ago

I guess the way I was looking at it is that you have a valid file name relative to the system path, so url encoding is something that would be handled. Might be worth documenting in the API that src has to be url encoded, and not simply a valid path relative to the fs.

weavejester commented 11 years ago

It may be a valid file name, but it's not a valid URI, and the src attribute of an image tag takes the latter.

Perhaps an assertion on the image function might be useful, but I'm not sure it's really Hiccup's job to inform the user when they're writing bad HTML.

yogthos commented 11 years ago

Helpful errors are always good in my experience. I'd advocate for the assert myself, but it's your call obviously. :)