Fork me on GitHub
#babashka
<
2022-05-15
>
borkdude13:05:29

If you wanna try:

$ bash <(curl ) --version 0.8.3-SNAPSHOT --dir .

$ ./bb schema.clj
(ns schema)

(require '[babashka.deps :as deps])
(deps/add-deps '{:deps {prismatic/schema {:mvn/version "1.2.1"}}})

(require '[schema.core :as s])

(def MySchema {:a [s/Int]})

(prn (s/validate MySchema {:a [1 2 3]}))

(try (s/validate MySchema {:a [:foo]})
     (catch Exception e
       (prn (:error (ex-data e)))))

ambrosebs04:05:27

Very nice! I'm going through the schema code to see if I can improve the bb support from there https://github.com/plumatic/schema/pull/440/files

borkdude08:05:52

Gonna look into that record equivalence issue now

borkdude15:05:17

@U055XFK8V Pushed a fix for record (non-)equivalence to babashka master now. So I think you can remove the workaround in schema

🎉 1
borkdude15:05:30

@U055XFK8V macOS binary should be installable now with the same invocation as earlier

borkdude15:05:15

I'm going to check out your branch, I lost my earlier work where I got the tests running :(

ambrosebs15:05:30

the defrecord fix works I think! currently investigating a locals clearing issue. something to do with the infinite-arity-fn-test test

ambrosebs15:05:00

note that my branch only runs schema.core-test atm

borkdude15:05:22

yeah, I've only tested that previously too

👍 1
ambrosebs15:05:34

should clojure.lang.MultiFn/addMethod work in bb?

borkdude15:05:15

I could expose that

borkdude15:05:21

not currently

ambrosebs15:05:53

I worked around it just fine fwiw

ambrosebs16:05:49

Here's a witness for the locals clearing (I think) https://github.com/plumatic/schema/pull/440/files#diff-fe966952e8b139238a859638669987516020fd5f9520243788d79a1987723e3dR257

./bb test  
ERROR in (infinite-arity-fn-test) (/Users/ambrose/Projects/schema-local-dev/bb-test-suite/test/cljc/schema/core_test.cljc:)
expected: (= 5 (f 4))
  actual: java.lang.AssertionError: Assert failed: input-checker-sym local cleared!!!
G__5086
 at java.lang.reflect.Constructor.newInstance (Constructor.java:490)
    sci.impl.Reflector.invokeConstructor (Reflector.java:310)
    sci.impl.interop$invoke_constructor.invokeStatic (interop.cljc:57)
    sci.impl.analyzer$analyze_new$reify__8934.eval (analyzer.cljc:880)
    sci.impl.analyzer$analyze_throw$reify__8890.eval (analyzer.cljc:777)
    sci.impl.analyzer$return_if$reify__8849.eval (analyzer.cljc:669)
    sci.impl.analyzer$return_do$reify__8163.eval (analyzer.cljc:136)
....
Reprod in CI: https://github.com/plumatic/schema/runs/6493118067?check_suite_focus=true#step:9:19

ambrosebs16:05:27

Oh, interesting I get this error on JVM: Error: Exception in thread "main" java.lang.RuntimeException: Conditional read not allowed, compiling:(schema/macros.clj:243:72) Looks like bb is more lenient?

borkdude16:05:30

Yeah, bb is more lenient :)

ambrosebs16:05:00

Settled on checking babashka.version 🙂 if I changed it to .cljc, cljs tries to compile it...

borkdude16:05:38

Yeah, that works

👍 1
borkdude19:05:51

@U055XFK8V Thanks for the issues!

👍 1
borkdude19:05:01

About the local clearing issue, I'm not what the problem there is

ambrosebs19:05:50

For now I used a workaround that pushes the locals into the fn.

borkdude19:05:34

I don't even understand what the issue is, do you have a tiny repro of that, or is it not important?

ambrosebs20:05:10

Not yet, tried several times. Basic issue seems to be local being cleared in code like (let [local (delay)] (fn [] ... @local)) but I can't find a minimal reprod.

ambrosebs20:05:27

In particular local is nil at @local and throws a NPE. Assuming it's related to locals clearing.

borkdude20:05:33

That would be very interesting to find

ambrosebs20:05:54

Yes, I'll keep trying.

borkdude20:05:58

Thanks, this is really good for improving SCI. About foo.bar` resolving to user/foo.bar, that's interesting. Apparently (def foo.bar 1) is allowed, but maybe just by accident.

borkdude20:05:36

Ah I see, so:

Symbols containing / or . are said to be 'qualified'.
This is why no further resolution takes place probably

👍 1
ambrosebs20:05:38

This might be the reprod:

./bb
Babashka v0.8.3-SNAPSHOT REPL.
Use :repl/quit or :repl/exit to quit the REPL.
Clojure rocks, Bash reaches.

user=> (defn minimal []
  (let [f (let [cleared 1]
            (fn ([a] (inc cleared))
              ([a & bs])))]
    (assert (= 5 (f 4)))))
#'user/minimal
user=> (minimal)
java.lang.NullPointerException [at <repl>:3:22]

ambrosebs20:05:21

I'll try and reduce some more. Related to multiple (+rest) arities it seems.

ambrosebs20:05:47

Smaller:

user=> ((let [cleared 1]
          (fn ([] (assert cleared))
              ([& bs]))))
java.lang.AssertionError: Assert failed: cleared [at <repl>:1:1]

ambrosebs20:05:03

Can't make smaller. I'll open a bb issue.

borkdude20:05:52

That's a great find! Thanks!

😄 1
borkdude20:05:48

Interestingly aliases can contain dots:

user=> (require '[clojure.set :as c.set])
nil
user=> `(c.set/union )
(clojure.set/union)

borkdude20:05:14

So it seems just "simple symbols" with dots should remain "unresolved"

ambrosebs20:05:41

Yes, I'm guessing the thinking is that it might be an unresolved reference to a class.

borkdude20:05:00

right, the class foo.bar (similar to foo.baz.Bar )

borkdude20:05:12

makes a ton of sense

borkdude21:05:55

OK, the reader issue is pushed to SCI and bb now. The rest I'll look into later this week

borkdude10:05:53

All issues fixed :)

borkdude10:05:15

It seems:

Ran 95 tests containing 506 assertions.
2 failures, 0 errors.
only 2 failures left, not sure what those are

borkdude10:05:04

It seems Testing schema.coerce-test are also passing :)

ambrosebs13:05:46

Nice thanks!

borkdude13:05:27

@U055XFK8V Exciting stuff. Let me know if there's anything left to do. If not, maybe we can launch a new bb soon which will be able to run schema!

borkdude13:05:47

I guess we could wait for the PR to be merged and a new version of schema to be released

ambrosebs14:05:03

😄 Yes, I'll work on the PR. I'm a maintainer on schema now, should be lower friction.

🎉 1
ambrosebs17:05:33

Another problem related to marker protocols: https://github.com/babashka/sci/issues/743

borkdude17:05:01

Thanks! Will fix now

ambrosebs17:05:34

maybe that last one isn't a bug. LMK

ambrosebs17:05:00

Yeah actually, it's unrelated to the "problem" I saw.

borkdude17:05:45

It's an edge case, records aren't really classes in SCI, since SCI cannot create new classes, so those things are implemented differently

ambrosebs17:05:45

Is there a way to get the original class from a fake record class symbol?

ambrosebs17:05:58

I mean, the original class name.

borkdude18:05:40

That symbol is the name

borkdude18:05:56

but these are a bit implementation-detail-ish, I don't know if I might change this in the future

borkdude18:05:40

yeah, so:

user=> (defrecord Foo [])
#'user/Foo
user=> Foo
user/Foo

ambrosebs18:05:52

I mean, something equivalent to .getName on a class.

borkdude18:05:15

yes, the record "var" is bound to a symbol, which is the name

borkdude18:05:50

user=> (str Foo)
"user/Foo"

ambrosebs18:05:03

Yes, but I want the "fake" JVM name.

borkdude18:05:23

That would be a string replace / to .

👍 1
borkdude18:05:32

user=> user.Foo
user/Foo

ambrosebs18:05:48

Hmm dashes=>underscores though?

borkdude18:05:56

yeah, munge

borkdude18:05:32

hmm, that's not something SCI does correctly at the moment

borkdude18:05:59

Are you using records with hyphens in them?

ambrosebs18:05:15

yeah schema.core_test.Foo

borkdude18:05:29

oh, so the ns name... lemme test

borkdude18:05:51

yeah, that works:

user-foo=> (defrecord Foo [])
#'user-foo/Foo
user-foo=> user_foo.Foo
user-foo/Foo

borkdude18:05:51

Maybe I should invent a new type for this which prints as the java name

ambrosebs18:05:36

can you make a magic type that responds to (.getName ^Class %) ?

borkdude18:05:10

I don't think that'll work... since then I would have to create something of type Class that can give a different name

borkdude18:05:10

of course we can special-case the interop, but that feels a bit hacky

borkdude18:05:51

Does this relate to the two remaining failing tests btw?

ambrosebs18:05:00

It relates to schema.core-test/record-test which I just uncommented

ambrosebs18:05:29

It's just a printing problem I think.

ambrosebs18:05:05

(not (= (record schema.core_test.Foo {:x Any, (optional-key :y) Int})
        (record schema.core-test/Foo {:x Any, (optional-key :y) Int})))

ambrosebs18:05:22

testing s/explain

ambrosebs18:05:30

Why is the var #'Foo needed here? Could we just return a symbol 'schema.core_test.Foo from defrecord and class references like Foo?

borkdude18:05:24

The var is necessary so I can store things on it, like the print-method

borkdude18:05:51

but the symbol stored in the var could be more like the class

borkdude18:05:10

and then it will likely print the same

👍 1
ambrosebs18:05:16

Is defrecord returning #'Foo instead of 'Foo an oversight? I was hoping for a general pattern like "record classes in bb are like clj, except sometimes they are symbols". Here it's a var.

borkdude18:05:30

the return value we could change

ambrosebs18:05:18

cool. I think returning a symbol naming the class is a good solution.

borkdude18:05:44

ok we'll do that

borkdude18:05:44

still figuring out the satisfies one

👍 1
ambrosebs18:05:13

Might be worth testing how symbol resolution works on a record that's imported from another ns.

borkdude18:05:58

That should already work correctly

ambrosebs19:05:07

Tried it, syntax quoting it doesn't work, resolves in the current ns on an imported record.

ambrosebs19:05:28

bar=> (import user.Foo)
user/Foo
bar=> `Foo
bar/Foo

borkdude19:05:40

Ah you mean in syntax quote

borkdude19:05:52

ok, another thing to fix

😄 1
borkdude19:05:13

can you also post an issue about that?

borkdude19:05:34

ok, the satisfies? change is up now

🎉 1
borkdude19:05:02

Pushed this change now:

(defrecord Foo [])
user.Foo

❤️ 1
ambrosebs19:05:58

satisfies? change works!

ambrosebs19:05:46

will (= Foo 'user.Foo) ?

borkdude19:05:59

Double-checked:

$ clojure -M:babashka/dev -e "(defrecord Foo []) (= Foo 'user.Foo)"
true

ambrosebs19:05:08

woop! should be just those 2 tests left. no idea what they are about yet.

ambrosebs19:05:43

nice, that fixed one test. another popped up due to Explainer => schema.core-test/Explainer`

borkdude19:05:43

Not sure what you mean with "another ...", a new problem or a problem solved?

ambrosebs19:05:22

good question...

ambrosebs19:05:57

how can I download the previous snapshot?

ambrosebs19:05:20

I deleted the bb.old LOL

ambrosebs19:05:42

The test fancy-explain-test started failing. I think it's because the left side of the (is = uses class resolution for the record class name, and the right side uses syntax quote. So was "working" before you "broke" one side. Just a coincidence, if you'd have fixed both problems at once, it would have passed.

ambrosebs19:05:53

confirmed, fixed! 2 failures left.

borkdude19:05:31

ok, 745 in progress, going to take a small break now

😁 1
ambrosebs20:05:39

there's a problem with the pprint changes (just uncommented the test), getting a SOE. investigating..

borkdude20:05:33

@U055XFK8V Will continue tomorrow, I'm out of dev budget for today :)

borkdude20:05:45

If you manage to make a repro, I'll look at it tomorrow

ambrosebs20:05:35

@borkdude thanks! have a great one

borkdude09:05:17

@U055XFK8V All solved now, I think

ambrosebs14:05:06

Just tried it, SOE is fixed but:

FAIL in (pprint-test) (/Users/ambrose/Projects/schema-local-dev/bb-test-suite/test/cljc/schema/core_test.cljc:)
expected: (= "(maybe Int)" (str/trim (with-out-str (pprint/pprint (s/maybe s/Int)))))
  actual: (not (= "(maybe Int)" "{:schema\n {:p?\n  #object[clojure.core$integer_QMARK_ 0x2104eb01 \"clojure.core$integer_QMARK_@2104eb01\"],\n  :pred-name integer?}}"))

ambrosebs14:05:32

I'll look into it later

borkdude14:05:50

So somehow the print-method isn't correctly registered... wonder where that's defined

borkdude14:05:05

or rather, simple-dispatch

ambrosebs15:05:19

It's an indirect extension, via a protocol interface

borkdude15:05:15

that's fine

borkdude15:05:54

So this example works:

(defrecord Foo [x])

(require '[clojure.pprint :refer [simple-dispatch]])

(defn print-awesome [_foo]
  (println "<<<<foo>>>>"))

(defmethod simple-dispatch Foo [o]
  (print-awesome o))

(clojure.pprint/pprint (->Foo 1))

borkdude15:05:42

This works too:

(defrecord Foo [x])

(require '[clojure.pprint :as pprint])

(clojure.core/defmethod pprint/simple-dispatch Foo [_]
  (pprint/write-out "<<<<foo>>>>"))

(clojure.pprint/pprint (->Foo 1))

ambrosebs15:05:34

./bb
Babashka v0.8.3-SNAPSHOT REPL.
Use :repl/quit or :repl/exit to quit the REPL.
Clojure rocks, Bash reaches.

user=> (defprotocol P)
#'user/P
user=> (defrecord R [])
user.R
user=> (defmethod clojure.pprint/simple-dispatch user.P [_] (println "PRETTY"))
#object[clojure.lang.MultiFn 0x6c3bc635 "clojure.lang.MultiFn@6c3bc635"]
user=> (clojure.pprint/pprint (R.))
{}
nil

borkdude15:05:35

Right! I was just trying:

(ns foo)
(defrecord Foo [x])

(ns bar
  (:require [clojure.pprint :as pprint]))

(clojure.core/defmethod pprint/simple-dispatch foo.Foo [_]
  (pprint/write-out "<<<<foo>>>>"))

(clojure.pprint/pprint (foo.Foo. 1))
but that did work

borkdude15:05:48

This still works for me:

(ns user
  (:require [clojure.pprint :as pprint]))

(defrecord Foo [x])

(clojure.core/defmethod pprint/simple-dispatch user.Foo [_]
  (pprint/write-out "<<<<foo>>>>"))

(clojure.pprint/pprint (->Foo 1))

borkdude15:05:21

I'll try yours

ambrosebs15:05:43

doh, I forgot to extend the protocol 🙂

ambrosebs15:05:51

multitasking sorry

ambrosebs15:05:20

./bb
Babashka v0.8.3-SNAPSHOT REPL.
Use :repl/quit or :repl/exit to quit the REPL.
Clojure rocks, Bash reaches.

user=> (defprotocol P)
#'user/P
user=> (defrecord R [] P)
user.R
user=>  (defmethod clojure.pprint/simple-dispatch user.P [_] (println "PRETTY"))
#object[clojure.lang.MultiFn 0x6c3bc635 "clojure.lang.MultiFn@6c3bc635"]
user=> (clojure.pprint/pprint (R.))
{}
nil

ambrosebs15:05:11

clj                                                                                                                               
Clojure 1.11.1
user=> (require 'clojure.pprint)
nil
user=> (defprotocol P)
P
user=> (defrecord R [] P)
user.R
user=> (defmethod clojure.pprint/simple-dispatch user.P [_] (println "PRETTY"))
#object[clojure.lang.MultiFn 0x7cedfa63 "clojure.lang.MultiFn@7cedfa63"]
user=> (clojure.pprint/pprint (R.))
Execution error (IllegalArgumentException) at user/eval171 (REPL:1).
Multiple methods in multimethod 'simple-dispatch' match dispatch value: class user.R -> interface user.P and interface clojure.lang.IPersistentMap, and neither is preferred

borkdude15:05:19

Right, so :

(ns user
  (:require [clojure.pprint :as pprint]))

(defprotocol Schema)
(defrecord ASchema []
  Schema)

(defrecord Foo [x])

(defmethod clojure.pprint/simple-dispatch user.Schema [_] (println "PRETTY"))

(prefer-method clojure.pprint/simple-dispatch user.Schema clojure.lang.IRecord)
(prefer-method clojure.pprint/simple-dispatch user.Schema java.util.Map)
(prefer-method clojure.pprint/simple-dispatch user.Schema clojure.lang.IPersistentMap)

(clojure.pprint/pprint (ASchema.))

👍 1
borkdude15:05:36

Oh, so a simple-dispatch on a protocol... damn, I only supported it for a record now I think ;)

ambrosebs15:05:17

the same logic is used for print-method, is there anything special in sci to handle that? https://github.com/plumatic/schema/blob/1116d1c68347b980ca03a7d4458213b4e13323ae/src/cljc/schema/core.cljc#L123-L124

borkdude15:05:31

yes, protocols aren't really types in SCI, because again, SCI cannot really create new JVM types

borkdude15:05:43

so it's all custom

borkdude15:05:13

but I can probably apply the same trick as I did with records to protocols

👏 1
ambrosebs15:05:38

I added another test, this branch isn't being hit either https://github.com/plumatic/schema/pull/440/commits/e79497a4dc32171129b189572d1088292ae18270

FAIL in (print-method-test) (/Users/ambrose/Projects/schema-local-dev/bb-test-suite/test/cljc/schema/core_test.cljc:)
expected: (= "(maybe Int)" (str/trim (with-out-str (println (s/maybe s/Int)))))
  actual: (not (= "(maybe Int)" "#schema.core.Maybe{:schema #schema.core.Predicate{:p? #object[clojure.core$integer_QMARK_ 0x2104eb01 clojure.core$integer_QMARK_@2104eb01], :pred-name integer?}}"))

borkdude15:05:12

This is going to take me some more time, maybe later this weekend

👍 1
ambrosebs16:05:07

Assuming bb uses the same multimethod impl as clojure

borkdude16:05:07

Yeah, it's a little bit more complicated than that

borkdude16:05:26

Currently the SCI context in bb uses the global print-method / simple-dispatch

borkdude16:05:39

and those global methods use the global core's isa?

borkdude16:05:21

I'll have to think a little bit more about this, my brain hurts ;)

😁 1
borkdude14:05:21

Why doesn't this print "P"?

(ns dude
  (:require [clojure.pprint :as pprint :refer [simple-dispatch]]))

(defprotocol P)

(defrecord R [] P)

(defmethod simple-dispatch dude.P [_]
  (println "P"))

(prefer-method simple-dispatch dude.P clojure.lang.IRecord)
(prefer-method simple-dispatch dude.P java.util.Map)
(prefer-method simple-dispatch dude.P clojure.lang.IPersistentMap)
(prefer-method simple-dispatch dude.P java.lang.Long)

(extend-protocol P
  java.lang.Long
  (explain [_] "yo"))

(pprint/pprint 1)

borkdude14:05:43

I think it's a bad idea to change the pprint-ing of types you don't own anyway. It seems schema has to attach the pprint-method to something in order to trigger explain. And that something seems to be records.... right?

borkdude14:05:07

So would it be sufficient if (for now) if SCI made simple-dispatch + protocol work only for records that implement that protocol?

borkdude14:05:45

I guess a workaround for the current version would be to do:

(defmethod simple-dispatch Maybe [x] (pprint/write-out (explain x)))
which would be a bit tedious but it would work and you could probably remove the prefer-methods

borkdude14:05:48

E.g. this works as is in clojure and bb (master) without prefer-methods:

(require '[clojure.pprint :as pprint :refer [simple-dispatch]])

(defprotocol Schema
  (explain [_]))

(defrecord Maybe []
  Schema
  (explain [_]
    "M"))

(defmethod simple-dispatch Maybe [x] (pprint/write-out (explain x)))

(pprint/pprint (->Maybe))

ambrosebs14:05:47

> Why doesn't this print "P"? I believe it's because isa? uses Class/isAssignableFrom, not clojure.core/extends?.

ambrosebs14:05:09

> So would it be sufficient if (for now) if SCI made simple-dispatch + protocol work only for records that implement that protocol? That's a good start yes. I can manually add that code in schema.

borkdude14:05:14

You would only have to add code as of now, not if SCI would support the "good start", then it would work as is, I think?

ambrosebs14:05:00

Doh I misread your proposal twice xD

ambrosebs14:05:47

Your proposal sounds perfect, I can't think of any other cases that are important tbh?

borkdude14:05:54

So to recap: 1. schema could make it work with bb master by adding the explicit record print-methods 2. SCI could potentially fix protocol printing for records, then you would not have to change anything

👍 1
borkdude14:05:59

I will try 2 but not sure how easy it is, but we'd still have 1 as a fallback and it would not be incorrect to add it

ambrosebs15:05:32

I'm going to try 1), I think this approach is needed for CLJS support for these two multimethods (currently the extensions are #?(:clj ..).

ambrosebs15:05:00

I can then just delete the :bb branch if/when support works in sci.

borkdude15:05:23

but 1 already works in bb (master)

borkdude15:05:52

so then 2 becomes less important (for schema)

ambrosebs15:05:53

yep exactly. Only important for libs that provide new schemas. Probably can revisit this if we find another library that needs this pattern.

borkdude15:05:10

Right, so in CLJS:

(require '[clojure.pprint :as pprint])

(defprotocol Dude
  (explain [_]))

(extend-protocol IPrintWithWriter
  Dude
  (-pr-writer
    [_this w _]
    (.write w "hello")))

(defrecord DudeR []
  Dude)

(pprint/pprint (->DudeR))
you would still have to provide the specific implementation for IPrintWithWriter on the record, each time, right?

ambrosebs15:05:55

yes I don't think this shortcut exists in cljs

borkdude15:05:32

ok, lucky for SCI then ;)

😁 1
ambrosebs15:05:48

looks like print-method doesn't exist in cljs and cljs.pprint/simple-dispatch is basically a closed system..

ambrosebs15:05:23

so it'll be sci-specific for now, but potentially useful for cljs if those restrictions are lifted.

borkdude15:05:50

What about extending IPrintWithWriter on each record, or does schema already do that?

borkdude15:05:16

that's AFAIK how you make pretty-printing work in CLJs

borkdude15:05:36

No sorry, prn I mean

borkdude15:05:09

amirite that Schema does not do the pretty printing stuff for CLJS right now?

borkdude16:05:28

I guess so:

cljs.user=> (cljs.pprint/pprint (s/maybe s/Int))
{:schema {:p? #object[cljs$core$integer_QMARK_], :pred-name integer?}}
At least the bb version won't be behind CLJS :)

borkdude16:05:14

I guess people could just use:

(str (s/explain (s/maybe s/Int)))
"(maybe Int)"
if they wanted the pretty one

ambrosebs16:05:26

yeah, no cljs printing support at all except explain. thanks, I'll use those two approaches!

ambrosebs16:05:09

for pprint: cljs.pprint/simple-dispatch uses a fixed dispatch table, I don't think the same approach works as clojure.pprint/simple-dispatch. Can't see any handy alternatives.

borkdude16:05:54

Maybe summon the #clojurescript people? I also don't know...

ambrosebs16:05:02

oh, it defaults to pr-str. That's probably fine.

ambrosebs17:05:43

the good news is that the bb support is almost done, but I can't figure out why IPrintWithWriter doesn't work in cljs:

./bb test
....
FAIL in (simple-validated-defn-test) (/Users/ambrose/Projects/schema-local-dev/bb-test-suite/test/cljc/schema/core_test.cljc:)
expected: (= +simple-validated-defn-schema+ (s/fn-schema simple-validated-defn))
  actual: (not (= (=> (both Str (pred odd-str?)) (both (pred odd?) long)) (=> (=> (maybe (=> Any Any)) (protocol s/Schema)) [(=> (maybe (=> Any Any)) (protocol s/Schema))])))

FAIL in (simple-primitive-validated-defn-test) (/Users/ambrose/Projects/schema-local-dev/bb-test-suite/test/cljc/schema/core_test.cljc:)
expected: (= +primitive-validated-defn-schema+ (s/fn-schema primitive-validated-defn))
  actual: (not (= (=> long (both (pred odd?) long)) (=> (=> (maybe (=> Any Any)) (protocol s/Schema)) [(=> (maybe (=> Any Any)) (protocol s/Schema))])))
....
Ran 113 tests containing 587 assertions.
2 failures, 0 errors.

ambrosebs17:05:18

notice the pretty output 🙂

borkdude17:05:02

@U055XFK8V Looking good. But now what, still failing tests for bb? ;)

borkdude17:05:19

Everytime I fix something, there are still 2 failing tests, bwhaha

ambrosebs21:05:55

LOL yes, the 2 remaining tests looked a real pain to debug without pretty printing, so they've been failing this whole time.

ambrosebs22:05:21

we've been making progress, I kept uncommenting tests and we've been knocking them over 🙂

ambrosebs22:05:53

Ok I think the problem is that schema holds a global registry of Class => schema to lookup defn schemas. I assume all defn's in bb are the same class and every defn just clobbers each other. This is probably the most fragile part of schema IMO.

borkdude22:05:20

yeah, each same-arity function is re-using the same JVM function object

👍 1
ambrosebs22:05:35

I think I can just use the cljs approach in bb, which is to associate fn identity with a schema instead of its class.

ambrosebs22:05:37

./bb test

Testing schema.core-test

Testing schema.macros-test

Testing schema.coerce-test

Testing schema.experimental.abstract-map-test

Testing schema.test-test

Testing schema.utils-test

Ran 113 tests containing 587 assertions.
0 failures, 0 errors.

borkdude22:05:50

wwwwwww00000t!!!!!

borkdude22:05:21

you too!!!

😄 1
ambrosebs22:05:29

now I just need to figure out the cljs pprinting then I'll get this merged

borkdude22:05:20

So, how reliant is schema on record "classes" being a symbol etc in bb? I'll be back tomorrow, need to sleep now

ambrosebs22:05:21

it may be impossible actually, the map implementation takes over. I'll just comment it out.

ambrosebs22:05:13

do a grep for :bb in my branch, you'll see some class? calls broadened to (some-fn class? symbol?). it's not too bad.

ambrosebs22:05:52

the class is mostly kept around for printing purposes.

borkdude09:05:49

I wonder if we could/should make SCI/bb user code more resilient against this implementation choice. What if we want to move the record representation from a Symbol to a deftype later on for example

borkdude09:05:58

That's the only thing I worry about with this PR

borkdude09:05:09

I guess a more cross-platform way would be to do:

(-> (str klazz) (str/split #" ") last)
to get the class name for both Java classes and for SCI records

borkdude09:05:24

and we could promise that as a stable API...

borkdude09:05:44

(not completely satisfying either maybe)

borkdude10:05:06

I guess SCI could offer an API to interrogate this, like:

sci.core/record-class?
or:
babashka.core/record-type?
and
babashka.core/type-name
which either gives the class name or the record name as a string?

ambrosebs15:05:33

@borkdude yes that sounds much better than symbol? checks.

borkdude16:05:24

OK, so maybe sci itself could expose such things in all SCI environments

borkdude16:05:33

then it would also work for nbb, etc

borkdude19:05:16

I'm trying to implement type-name now...

(defn type-name [x]
  (if (and (symbol? x)
           (-> x meta :sci.impl/record))
    (str x)
    #?(:clj (class x))
    #?(:cljs (.-name x))))

borkdude19:05:26

but how do I do this in CLJS properly, I wonder

borkdude19:05:15

maybe nil should be interpreted as: don't know, you're on your own. it seems getting the name from (defrecord Foo []) is impossible in CLJS right now

borkdude19:05:08

Btw, this looks suspicious:

#?(:clj java.util.regex.Pattern :cljs nil) 'Regex

borkdude19:05:55

Perhaps record-type-name is a better API name and then I'll only give back a string for records and nil for other things

borkdude19:05:36

user=> (sci/eval-string "(defrecord Foo []) (sci.runtime/record-type? Foo)")
true
user=> (sci/eval-string "(defrecord Foo []) (sci.runtime/record-type-name Foo)")
"user.Foo"

borkdude19:05:45

Pushed that to master now (sci + bb)

borkdude20:05:31

Let me know if that works for you

ambrosebs22:05:21

> but how do I do this in CLJS properly, I wonder It looks like schema just gives up in this case and returns the entire prototype https://github.com/plumatic/schema/blob/1116d1c68347b980ca03a7d4458213b4e13323ae/src/cljc/schema/core.cljc#L199

ambrosebs23:05:09

> Let me know if that works for you it feels like this abstraction could be a bit cleaner. it seems too tied to records, I think I want an abstraction over 1. "is this a fake class/prototype created due to limitations of sci" and 2. "what is the name of this class/prototype, if any?". could we reify these metadata symbols as records themselves? like provide a sci.runtime.Type record that responds to a type-name fn that returns a nilable string. That way if you decide to fake classes for other things (definterface, defprotocol, deftype etc), you can build on top of Type without requiring changes in schema. and anywhere you normally do (extend-protocol ... Class) you can do (extend-protocol ... sci.runtime.Type) also, without clashing with existing (extend-protocol ... Symbol).

borkdude10:05:39

Let me know if that sounds good

pinkfrog14:05:49

Even if I add

;; see 
;; pprint does not order between IPersistentMap and IDeref
(prefer-method pprint/simple-dispatch clojure.lang.IPersistentMap clojure.lang.IDeref)
I still get
: Multiple methods in multimethod 'simple-dispatch' match dispatch value: class babashka.process.Process -> interface clojure.lang.IPersistentMap and interface clojure.lang.IDeref, and neither is preferred task.util 
when running process

borkdude14:05:01

@i There is now a new namespace for this. Add (require '[babashka.process.pprint]) and it should solve the problem

pinkfrog14:05:39

The docs says either works.

borkdude14:05:01

Then the docs need updating

borkdude14:05:18

PR welcome, let's remove the second suggestion

pinkfrog14:05:57

Is it confirmed or my personal false test?

borkdude15:05:14

I think the first suggestion works more reliably

pinkfrog15:05:22

Yet, I don’t understand why a prefer is added but still the warning occurs.

uwo16:05:28

Hello babashka channel 👋 I'm trying to write a small tool that takes user input twice. The second time I try to read from stdin fails because the stream is closed. I think I'm missing something obvious ^

uwo16:05:44

ah right. slurp is using with-open. guessing I'll have to create my own reader

borkdude16:05:32

you may just do :in :inherit I think

borkdude16:05:43

then you will not use the *in* from Clojure yet

borkdude16:05:31

E.g.

$ bb -e '(babashka.tasks/shell "fzf") (read-line)'
wotbrw.clj
1
"1"

uwo16:05:39

Thanks, that's perfect! Out of curiosity what am I'm doing wrong below. task below waits for fzf input, but then does not wait for read-line input:

;; Invoked with `find * -type f | bb some-task`
  some-task {:requires ([babashka.process :as p])
             :task
             (let [fzf (p/process ["fzf" "-m" "--print-query"]
                                  {:in :inherit
                                   :err :inherit
                                   :out :string})
                   proc (:proc fzf)
                   pick (:out (deref fzf))
                   _ (prn "pick result" pick)
                   _ (do (print "input: ") (flush)) 
                   read-input (read-line)]
               (prn "read-line-input: " read-input))}

borkdude16:05:48

you need to deref the process

borkdude16:05:53

so it will block

uwo16:05:43

the fzf process is being deref'd, I believe

borkdude16:05:46

I'll be back tonight

uwo16:05:00

metal Thanks again!

borkdude18:05:05

ok, so what's up

borkdude19:05:23

So this works:

{:tasks {some-task {:requires ([babashka.process :as p]
                               [babashka.fs :as fs]
                               [clojure.string :as str])
                    :task
                    (let [fzf (p/process ["fzf" "-m" "--print-query"]
                                         {:in (str/join "\n"
                                                        (map str (remove fs/directory? (fs/list-dir ""))))
                                          :err :inherit
                                          :out :string})
                          proc (:proc fzf)
                          pick (:out (deref fzf))
                          _ (prn "pick result" pick)
                          _ (do (print "input: ") (flush)) 
                          read-input (read-line)]
                      )}}}

👀 1
borkdude19:05:48

I think you could do the input argument to fzf via a command line args rather than via stdin

borkdude19:05:05

You can get command line args with *command-line-args*

uwo19:05:45

I need to allow the user to use the fuzzy finding interface of fzf ... where they type and see how the list filters. (as opposed to passing a string before hand via command-line-args

borkdude19:05:25

yes. you can do that like in the above?

uwo19:05:57

yes, absolutely, just tried it! thanks so much for your time!

mukundzare19:05:35

How do I use the medley dependency on a machine which has no internet connectivity? I have transferred the medley jar manually to my project/deps folder on machine annd used : {:paths ["src"] :deps {medley/medley {:local/root "deps/medley-1.4.0.jar"} But still when I run bb -m foo.bar, I get the error: Error building classpath. Failed to read artifact descriptor for org.clojure.jar:1.11.0 . Note that I also previously manually transferred clojure tools v1.11.0 to the machine as without it, there was another error message for clojure tools being missing.

borkdude19:05:08

@mukundzare This is because you're still going through the tools deps machinery to fabricate a classpath which probably tries to pull in clojure 1.11 somehow. I think for no internet connection it's best to make an uberjar and use the --classpath uberjar.jar option

borkdude19:05:28

You can make such an uberjar with bb uberjar uberjar.jar on a machine which does have internet

mukundzare19:05:58

Alright, I will try that, thanks!

borkdude19:05:19

or you can manually do: --classpath src:deps/medley-1.4.0.jar

borkdude19:05:34

You can also set BABASHKA_CLASSPATH=...

mukundzare19:05:08

I always get confused by a few of these conventions, but I will try and sort through them

mukundzare19:05:41

Like how does --classpath differ from :paths and :deps in the bb.edn

mukundzare19:05:10

And why is there an additional BABASHKA_CLASSPATH variable on top of it?

borkdude19:05:26

:paths and :deps are similar to how you use them in deps.edn . When you invoke Clojure, it will calculate the classpath from them, which, if you haven't done that before, usually needs the internet.

borkdude19:05:22

Those options were introduced a while after we already had --classpath and BABASHKA_CLASSPATH: before you had to manually construct the classpath using $(clojure -Spath)

borkdude19:05:50

The environment variable allows you to invoke bb as you would do it locally as when you still had internet and plays nice with things like Dockerfiles.

borkdude19:05:13

All of this is also documented in https://book.babashka.org/

mukundzare19:05:35

Now it's quite clear to me! Thanks a lot.

mukundzare19:05:22

I refer the babashka book multiple times a week but I sometimes get quite stumped by a few of these concepts even after reading and re-reading them, it's great to have your guidance in this slack channel @borkdude 😀

borkdude19:05:50

@mukundzare You're welcome. I did not mean to say RTFM, I'm happy to shed some extra light on this :)

mukundzare19:05:14

I like your humility. I might be posting here more often.👍