Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doesn't recognize identifiers with slashes #62

Closed
trevor opened this issue Nov 8, 2013 · 7 comments
Closed

doesn't recognize identifiers with slashes #62

trevor opened this issue Nov 8, 2013 · 7 comments

Comments

@trevor
Copy link

trevor commented Nov 8, 2013

id's or classes that have a / character aren't processed. (This is usually caused in clojure by accidentally using name as a generic keyword-to-string function.)

Note this is a legal character in html5: http://www.w3.org/html/wg/drafts/html/master/dom.html#the-id-attribute

I'm assuming not all characters can easily be supported, but perhaps this is an easy fix.

A solution would be something a function like (subs (str :abc) 1), but there may be something better.

@cpetzold
Copy link
Contributor

Thanks for the report :)

@rm-hull
Copy link
Contributor

rm-hull commented Jan 29, 2014

@cpetzold - the 1a8445f fix you put in on Monday, changing name to string-or-keyword breaks things badly when I use the node macro, even though all the tests pass.

I'm getting stack traces like:

clojure.lang.ExceptionInfo: failed compiling file:
-----8<-----
blah blah, snipped
-----8<----- 
Caused by: java.lang.ClassCastException: clojure.lang.Symbol cannot be cast to java.lang.CharSequence
                core.clj:4386 clojure.core/re-matcher
                core.clj:4438 clojure.core/re-find
               macros.clj:122 dommy.macros/parse-keyword
               macros.clj:134 dommy.macros/compile-compound
               ...

Aliasing

(def string-or-keyword name)

On my local install, and it works, ... why the change?

@trevor
Copy link
Author

trevor commented Jan 29, 2014

@rm-hull I can't speak to the stack trace issue, but as for "why the change?" see this ticket. name isn't a suitable general function for stringification.

@rm-hull
Copy link
Contributor

rm-hull commented Jan 29, 2014

Hmmm... Well this solves it, I suppose:

(defn string-or-keyword [s]
  (if (keyword? s)
    (-> (str s) (subs 1))
    (str s))) 

by wrapping the s when it's not a keyword (it is a symbol, after all); I guess it should be called string-or-symbol-or-keyword or something to that effect!

@trevor
Copy link
Author

trevor commented Jan 29, 2014

@rm-hull I think that would work.

It's a shame this isn't standardized in core somewhere to be idiomatic.

@rm-hull
Copy link
Contributor

rm-hull commented Jan 30, 2014

Quick update on this: I didn't really fully consider the consequence of the exception message: Caused by: java.lang.ClassCastException: clojure.lang.Symbol cannot be cast to java.lang.CharSequence uintil just now...

Hunting through the code there was a single snippet as follows:

[div#id ...]

i.e. it was missing the starting colon, and was being treated as a symbol - this had worked previously when the code was using name as that would happily consume symbols. With the code change in 1a8445f this no longer worked.

Clearly my code was wrong, but is it worth fixing this up as per my previous comment? Happy to raise a PR for it.

@cpetzold
Copy link
Contributor

Ah funny, didn't realize the template macros worked with symbol tags. I think it's a bit dangerous for this to work, because switching from the macro to run-time template fns would break in this case.

Maybe an assertion that verifies that the tag is a keyword? I'm unsure about allowing string tags, as we may want to treat string vectors as text node groupings instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants