ClojureScript

port clojure.data

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code

Description

clojure.data ports somewhat nicely to ClojureScript. The only wrinkle is that clojure.data extends its protocols to the java.util.Map/Set/List interfaces, which doesn't work in ClojureScript. The naive solution is to extend the protocols to every relevant type. I've done this in the attached patch 0001-CLJS-378-port-clojure.data.patch (16 Sep 2012).

Activity

Hide
Tom Jack added a comment -

This patch (differences-from-clojure.patch) shows the changes made to the Clojure version.

Show
Tom Jack added a comment - This patch (differences-from-clojure.patch) shows the changes made to the Clojure version.
Hide
David Nolen added a comment - - edited

Nice. But I think is probably best submitted as a patch to clojure.data not ClojureScript. Code duplication aside, it's simple for a library to provide both Clojure / ClojureScript functionality - this is done in core.logic. To address the code duplication issue - it's a good reason to get behind Feature Expressions - http://dev.clojure.org/display/design/Feature+Expressions

Show
David Nolen added a comment - - edited Nice. But I think is probably best submitted as a patch to clojure.data not ClojureScript. Code duplication aside, it's simple for a library to provide both Clojure / ClojureScript functionality - this is done in core.logic. To address the code duplication issue - it's a good reason to get behind Feature Expressions - http://dev.clojure.org/display/design/Feature+Expressions
Hide
Tom Jack added a comment -

How would that work given that clojure.data is in Clojure itself? Cf. clojure.{set,string,walk,zip,core.reducers}.

Show
Tom Jack added a comment - How would that work given that clojure.data is in Clojure itself? Cf. clojure.{set,string,walk,zip,core.reducers}.
Hide
David Nolen added a comment -

Oops sorry! Was completely confused Yes this looks good.

Show
David Nolen added a comment - Oops sorry! Was completely confused Yes this looks good.
Hide
David Nolen added a comment -

Thinking a bit more about this patch I wonder if it wouldn't be better to handle things in the default case by using an explicit conditional for the protocol partitions? The patch would be significantly smaller as well allowing users implementing the core protocols to get the desired behavior for free. While I have some reservation about a closed dispatch over the core protocols as far as I can tell the current list of core protocols is pretty well defined and unlikely to change.

Show
David Nolen added a comment - Thinking a bit more about this patch I wonder if it wouldn't be better to handle things in the default case by using an explicit conditional for the protocol partitions? The patch would be significantly smaller as well allowing users implementing the core protocols to get the desired behavior for free. While I have some reservation about a closed dispatch over the core protocols as far as I can tell the current list of core protocols is pretty well defined and unlikely to change.
Hide
Tom Jack added a comment -

Like this? (0002-CLJS-378-port-clojure.data.patch, 19 Sep 2012)

Show
Tom Jack added a comment - Like this? (0002-CLJS-378-port-clojure.data.patch, 19 Sep 2012)
Hide
David Nolen added a comment -

This is looking good! Can we get some tests to go along with it? Thanks much.

Show
David Nolen added a comment - This is looking good! Can we get some tests to go along with it? Thanks much.
Hide
Tom Jack added a comment -

Now (0003-CLJS-378-port-clojure.data.patch, 19/Sep/12) with tests.

Show
Tom Jack added a comment - Now (0003-CLJS-378-port-clojure.data.patch, 19/Sep/12) with tests.

People

  • Assignee:
    Unassigned
    Reporter:
    Tom Jack
Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: