clj-commons / camel-snake-kebab

A Clojure[Script] library for word case conversions
https://clj-commons.org/camel-snake-kebab/
Eclipse Public License 1.0
475 stars 49 forks source link

Add clojurescript support/build #11

Closed bbbates closed 10 years ago

bbbates commented 10 years ago

I've been using camel-snake-kebab for quite some time now, and recently had the need to use it for a clojurescript project I was working on. I was all set to fork it and create a standalone project (which I am still happy to do if need-be), but I thought it would be nice and convenient to have support for both Clj and Cljs in the one dependency. How hard could it be? As it turns out, a bit harder than I initially had planned for. I'll walk you through the changes I've had to do to get camel-snake-kebab to build for both clojure and clojurescript with (almost) the same codebase. There's quite a few, including a solution of sorts to Issue #6 (splitting the camel-snake-kebab ns).

project.clj

camel-snake-kebab -> camel-snake-kebab.core + camel-snake-kebab.macros + camel-snake-kebab.case-convert The reason I had to split this into multiple namespaces stems from the fact that namespace interning is not supported in clojurescript. So, to generate the conversion functions I needed to create a macro (gen-conversion-fns), which meant I needed at least one extra namespace to move this into. The macro needed to reference the case-convert function, so I had to shift that a third namespace to avoid a circular dependency.

camel-snake-kebab.case-convert Unfortunately, lookbehinds are not supported in javascript's regular expression interpreter, so therefore they can't be used in Cljs. This means I had to absolutely mangle the nice and elegant expression that was, to the monster in the changes below. Certainly, using cljx we could use different functions for splitting strings for clj and cljs, which would mean the old way of splitting words could still be used for clj, but I wanted to try and minimise the differences between the two.

qerub commented 10 years ago

Thank you, this looks great! I definitely want to include ClojureScript support. I want to do a review before merging, but given that I'm a bit busy these days it might have to wait until next week.

bbbates commented 10 years ago

No problem. You'll notice it's not quite as neat and elegant as it was - there are a few compromises that I had to make to get it to work.

qerub commented 10 years ago

[…] including a solution of sorts to Issue #6 (splitting the camel-snake-kebab ns).

It's still not necessary to rename camel-snake-kebab to camel-snake-kebab.core, but it might be a good occasion to do it.

I've upped the version of clojure required, as the clojurescript compiler requires a much more recent one than 1.4.0

Would it be acceptable to stick with 1.5.1 for now?

Unfortunately, lookbehinds are not supported in javascript's regular expression interpreter, so therefore they can't be used in Cljs. This means I had to absolutely mangle the nice and elegant expression that was, to the monster in the changes below.

I'm thinking that this might be a good occasion to get rid of the regexes completely and work on the characters directly, like this: https://github.com/qerub/camel-snake-kebab/commit/a44a6a6126dad9dc7385fa6702deeb91336ce2d6

One problem with this is that JavaScript doesn't have any good Character/{isWhitespace, isLowerCase, isUpperCase} functions and they are non-trivial wrt. Unicode.

I wonder if it would be acceptable to only transform case transitions between ASCII characters. What do you think?

Certainly, using cljx we could use different functions for splitting strings for clj and cljs, which would mean the old way of splitting words could still be used for clj, but I wanted to try and minimise the differences between the two.

I agree with this reasoning.

qerub commented 10 years ago

I wonder if it would be acceptable to only transform case transitions between ASCII characters. What do you think?

In that case, we can do something like this instead:

(defn ^:private classify-char [c]
  (case c
    (\- \_ \space \tab \newline \o013 \formfeed \return) :whitespace
    (\a \b \c \d \e \f \g \h \i \j \k \l \m \n \o \p \q \r \s \t \u \v \w \x \y) :lower
    (\A \B \C \D \E \F \G \H \I \J \K \L \M \N \O \P \Q \R \S \T \U \V \W \X \Y) :upper
    :other))
bbbates commented 10 years ago

It's still not necessary to rename camel-snake-kebab to camel-snake-kebab.core, but it might be a good occasion to do it. Agreed. Certainly if to allow for clojurescript, though, it IS required. Perhaps for backwards compatability, we could keep the core namespace as camel-snake-kebab, but I would imagine you would probably want to include this in a "major" release and warn about api changes, so, yeah, probably would be a good chance to include a change.

Would it be acceptable to stick with 1.5.1 for now? Sure - I just chose the latest, but everything still works in 1.5.1. I'll push up a commit for that.

I wonder if it would be acceptable to only transform case transitions between ASCII characters. What do you think? If you want to remove the regexes, I don't think we'd have much of a choice thanks to the restrictions by JS you mentioned. The classify-char function is probably a good start (gotta take numbers into account, too...).

qerub commented 10 years ago

It's still not necessary to rename camel-snake-kebab to camel-snake-kebab.core, but it might be a good occasion to do it.

Agreed. Certainly if to allow for clojurescript, though, it IS required.

What are you basing this on? One-part namespaces seem to work in ClojureScript too.

The classify-char function is probably a good start (gotta take numbers into account, too...).

Right… The test suite has some blind spots when it comes to this and I'm not certain what the expected behavior is either. This is what it currently does and what it probably should keep doing:

(->kebab-case "Adler32") ; => "adler-32"
(->kebab-case "Inet4Address") ; => "inet-4-address"
(->kebab-case "Arc2D") ; => "arc-2-d"
bbbates commented 10 years ago

What are you basing this on? One-part namespaces seem to work in ClojureScript too.

Sorry, should've been clearer. We need a separate (clojure only) namespace for the macro at a minimum. Didn't mean one-part namespaces don't work...

qerub commented 10 years ago

I experimented with using core.match for split and the result was pretty good. I ported your changes to this new base: https://github.com/qerub/camel-snake-kebab/commits/wip-0.2.0

I still have to:

qerub commented 10 years ago

OK, I decided against using core.match since the generated code was very bloated. I've done some further tweaks today and pushed everything to master. I've also pushed 0.2.0-SNAPSHOT to Clojars and would be delighted if you could try it.