Fork me on GitHub
#sci
<
2020-08-24
>
mauricio.szabo01:08:09

Hi, @borkdude. Just a single question: I'm using SCI on ClojureScript, and want to pass ClojureScript records to, and from SCI. So, I saw the :readers option. But if I send an options like {'example.Error ex/map->Error}, when I evaluate the code #example.Error{:type "SomeError, :message "A Message"} it returns a PersistentArrayMap, not the record

borkdude06:08:50

@mauricio.szabo Do you want to read records from source code strings?

borkdude06:08:40

I can reproduce the problem:

cljs.user=> (type (sci/eval-string "#example.Error{:type \"SomeError\", :message \"A Message\"}" {:readers {'example.Error map->Error}}))
cljs.core/PersistentArrayMap

borkdude12:08:17

@mauricio.szabo Do you need a new mvn/version or do you use it as a git dep?

mauricio.szabo12:08:35

A mvn/version is better, because I'm using on Shadow-CLJS 😄. Thanks a lot!

mauricio.szabo13:08:33

(BTW, that was FAST! 😄)

borkdude13:08:33

@mauricio.szabo ok: borkdude/sci {:mvn/version "0.1.1-alpha.7"}

borkdude13:08:46

Btw, since yesterday night the CLJS tests for sci run really sloooow on my machine, no idea why

borkdude13:08:07

on CI I don't really observe this difference

borkdude13:08:35

I'm also observing this with 0.1.0 which was released quite a while ago. I guess my holiday laptop (Macbook Air) just doesn't cut it anymore, but the JVM tests are still super fast.

mauricio.szabo14:08:27

@borkdude well, everything seems normal here on my machine with SCI tests... maybe some node/macOS update?

kwrooijen20:08:34

Hey, I'm trying to use clojure.walk/macroexpand-all inside of sci but it's failing with the classic Caused by: .FileNotFoundException: Could not locate clojure/spec/alpha__init.class, clojure/spec/alpha.clj or clojure/spec/alpha.cljc on classpath. Basically I'm running this:

(sci.core/eval-string
   "(macroexpand-all '(-> 1 println))"
   {:bindings {'macroexpand-all clojure.walk/macroexpand-all}})
;;=> (println 1)

borkdude20:08:20

Don't do that :)

borkdude20:08:23

Sci already has macroexpand. The macroexpand-all should call that macroexpand, not clojure's own macroexpand.

kwrooijen20:08:04

So I need to re-implement macroexpand-all in sci?

borkdude20:08:14

Yes, or contribute it to sci

kwrooijen20:08:05

All right, for the short-term would evaluating the implementation inside of sci be enough?

borkdude20:08:11

I think so yes

kwrooijen20:08:12

I'd love to contribute though 🙂

borkdude20:08:56

You can contribute by making an issue first :)

kwrooijen20:08:57

All right, well I actually already tried that. But I didn't get the result I expected:

(sci.core/eval-string "
(defn walk [inner outer form]
  (cond
   (list? form) (outer (apply list (map inner form)))
   (map-entry? form)
   (outer [(inner (key form)) (inner (val form))])
   (seq? form) (outer (doall (map inner form)))
   (record? form)
     (outer (reduce (fn [r x] (conj r (inner x))) form form))
   (coll? form) (outer (into (empty form) (map inner form)))
   :else (outer form)))

(defn prewalk [f form]
  (walk (partial prewalk f) identity (f form)))

(defn macroexpand-all [form]
  (prewalk (fn [x] (if (seq? x) (macroexpand x) x)) form))

 (macroexpand-all '(-> 1 println))")
;;=> (-> 1 println) ;; <<<<<<<<<<< INCORRECT
When I evaluate these functions, and run it locally, it does work

borkdude20:08:35

ok, let me try

borkdude20:08:09

oh I see. yes, well, that has a different reason

borkdude20:08:37

The -> is built in kind of as a primitive in sci, it was one of the first macros I implemented, when user-defined macros weren't possible yet

borkdude20:08:46

so it's not really expanded like other macros

kwrooijen20:08:26

Oh! I see. ->> does work

kwrooijen20:08:29

very interesting

borkdude20:08:55

I have trouble finding where exactly the implementation of that "special" macro is

borkdude20:08:45

ah, there it is, in analyzer.cljc:

;; TODO: implement as normal macro in namespaces.cljc
                -> (expand-> ctx (rest expr))
                ;; TODO: implement as normal macro in namespaces.cljc
                as-> (expand-as-> ctx expr)
                

borkdude21:08:11

it's historical, not necessarily the way it has to be now

kwrooijen21:08:05

Maybe I can cook something up then

borkdude21:08:52

I think we already have tests for -> so if they keep working, you don't have to add new ones I think

kwrooijen21:08:08

Ah ok, I'll remove those (and update my branch)

borkdude21:08:23

or you can separate them out from the bug chunk of core tests and move them to their own deftest, which is cleaner I think

kwrooijen21:08:55

I see, I'll move them then

borkdude21:08:23

You can also add a test for macroexpanding it

kwrooijen21:08:42

Done, simple tests but they show what's expected

borkdude21:08:41

I just merged a huge PR (having to do with the new stack trace support in babashka), so there's a conflict now. Can you solve it? Can wait till tomorrow, since I'm calling it a day

borkdude21:08:34

Btw, one more thing: the namespaces-test namespace is intended for tests related to ns etc

borkdude21:08:51

So you can still stick your -> tests in core_tests.cljc but in a separate deftest

kwrooijen21:08:16

All right, I'll fix that tomorrow 🙂

kwrooijen21:08:38

Or well might as well do it now lol

kwrooijen21:08:12

Anyway, good night! Calling it a day as well 👋