Fork me on GitHub
#cljs-dev
<
2018-12-01
>
mfikes02:12:58

The "improvements to exception messages and printing" work has progressed to the point where you can see the light at the end of the tunnel. It is probably just a matter of use and banging out rough edges now. See https://gist.github.com/mfikes/cca647f962f38edb599596f8e7bffc43

🎉 4
dnolen16:12:03

that’s great!

mfikes16:12:44

In the end, it might help if tools.reader had some mild updates (with respect to what is put in its exception message strings), but it appears we can pull this off without strictly needing a change in tools.reader.

bronsa16:12:07

I'll take any patches you throw my way FWIW

bronsa16:12:50

reasonable ones, that is :)

dnolen16:12:04

since Clojure 1.10 is around the corner I think we should focus our efforts on fixing up spec and aligning changes

mfikes16:12:51

Cool. the only thing I saw so far @bronsa is for example that the reader error for 08 includes line and column number info in the message string. Perhaps we can get things working in ClojureScript and we can then determine if tools.reader changes to messages might improve the experience, without breaking anything or making a hard dependency.

đź‘Ť 4
dnolen16:12:51

anything else should be de-prioritized for now

mfikes16:12:24

Yeah, @dnolen it looks like you may be using Blocker for that, right?

mfikes18:12:54

FWIW, unit tests on master are currently broken. I may take a look at them later on today. https://github.com/mfikes/clojurescript/commits/master

borkdude18:12:45

@dnolen damn, I already thought I missed this case, but since the tests worked I wasn’t sure about that one: https://github.com/clojure/clojurescript/commit/ad07bbd3ed5c9108e156983ce4a1d289a0f768dc

mfikes20:12:43

Cool, 1-line patch in CLJS-2997; pushing it through CI, etc.

borkdude20:12:38

it seems before the recent patch in that area by me and David, that case wasn’t spec-checked either: https://github.com/clojure/clojurescript/blob/6353a9b381144d6d0caa621322af9587922e7c07/src/main/cljs/cljs/spec/test/alpha.cljs#L127 that’s why I was unsure about it..

borkdude21:12:10

if there are all these special cases for arities, in what case will the “normal” ret fn body be called?

mfikes21:12:32

When it is a function that looks like (fn [& args]) (the pure-variadic? case)

borkdude21:12:19

and when it’s not pure-variadic but has a variadic case (so one or more fixed + varargs arguments) it ends up here: https://github.com/clojure/clojurescript/blob/6353a9b381144d6d0caa621322af9587922e7c07/src/main/cljs/cljs/spec/test/alpha.cljs#L127

borkdude21:12:50

and that case was never spec-checked

borkdude21:12:06

that was changed by David on November 30. What does your change entail?

mfikes21:12:05

The patch is in CLJS-2997

borkdude21:12:54

yes, but what does it do?

mfikes21:12:13

I would call it "unrolling" the args

mfikes21:12:49

If you have a max fixed arity of say, 3, args will come in as a sequence like [1 2 3 [4 5 6]]

mfikes21:12:32

In terms of a concrete example, (fn [a b c & rest])

mfikes21:12:02

So, we want to conform the args as they were passed [1 2 3 4 5 6]

borkdude21:12:23

ah. does this problem also occur in Clojure proper?

mfikes21:12:52

I don't know the internals of Clojure function dispatch.

borkdude21:12:01

probably not, but it might be good to check

borkdude21:12:20

I encountered this problem in CLJ spec which probably also affects CLJS spec: https://dev.clojure.org/jira/browse/CLJ-2443 But it’s still being triaged.