Fork me on GitHub
#cljs-dev
<
2017-01-10
>
juhoteperi00:01:13

What do parse-require-spec and parse-import-spec do with deps?

juhoteperi00:01:29

They use (swap! deps conj ...) so with the change it is possible that they add duplicates to deps vector

juhoteperi00:01:01

Ah, but the patch takes care of that by using distinct

ewen10:01:37

clojurescript iterators do not behave the same when there is no element left, some return nil, others throw an exception. Is it intended ? should they all throw an exception like clojure's ?

dnolen13:01:03

@ewen no, there are two kinds of iterators

dnolen13:01:23

ES6 iterators work according to JS rules

dnolen13:01:00

if there’s some inconsistency with non-ES6 iterators, then sure, that can be fixed

tmulvaney13:01:58

I notice PersistentVector and Subvec implement IAssociative but don't have a -contains-key? method. Is this intentional?

ewen13:01:35

I think the way print-namespace-maps is set when starting the REPL is wrong. We are setting print-namespace-maps as a side effect of binding the init option. The "evaluate-form" call should be wrapped in a function instead: https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/repl.cljc#L826. Patch needed ?

dnolen14:01:17

but you should cross-reference what was done in Clojure here

dnolen14:01:56

@ewen it's better to give some context, not just the problem. Why does it matter?

ewen14:01:54

@dnolen It matters because the evaluation of the javascript code is done outside the read-eval-print loop! Thus exceptions won't be catched

ewen14:01:37

I would be surprised this works at all

dnolen14:01:02

@ewen what exceptions?

dnolen14:01:09

it’s just a simple side effect

dnolen14:01:46

I can’t say whether it does or doesn’t work since I didn’t try it myself yet - but I’m assuming the patch author tested it as they wanted the feature

dnolen14:01:14

@ewen you’re missing what I’m saying

dnolen14:01:20

why doesn’t this matter?

dnolen14:01:28

what exception will that form ever throw?

ewen14:01:03

there is a typo in the code I linked, its written (let [init (evaluate-form ...)]) instead of (let [init #(evaluate-form)])

ewen14:01:44

the side effect happens when we bind "init" ! not when (init) is called

dnolen14:01:05

@ewen I would test first that it doesn’t work

dnolen14:01:46

I suspect it does, because that side-effect is trivial

dnolen14:01:45

and all REPL environments needs to load cljs.core before init anyway

dnolen14:01:20

@ewen to clarify, I’m not suggesting the code couldn’t be cleaner - but if it works it’s really not a priority

ewen14:01:54

@dnolen ok i will try it too and come back to you

ewen16:01:46

@dnolen it tried it and it works with the raw cljs repl

ewen16:01:02

the reason this code causes me troubles is that i am developing a clj/cljs repl: https://github.com/EwenG/replique

ewen16:01:13

When evaluating a form in a replique repl, if the javascript environment is not connected, instead of blocking the thread, the replique repl displays a message to the user, telling him that the javascript environment is not connected.

ewen16:01:27

It is not possible anymore with the way the repl binds print-namespace-maps. They are two reasons to this:

ewen16:01:41

- the code that binds print-namespace-maps cannot be overridden by the init parameter

ewen16:01:47

- the code that binds print-namespace-maps is called outside the read-eval-print loop, so instead of returning a :js-eval-error or :js-eval-exception and continuing the loop, it throws an exception and breaks out of the repl function

dnolen16:01:09

@ewen your issue seem to be concerned about a much wider scope than the original problem

dnolen16:01:20

in anycase all of the REPL code assumes you can eval immediately - there’s no real consideration for other cases

dnolen16:01:55

so your init is just one example of this

dnolen16:01:36

also note changing how REPLs work is not really up for consideration

dnolen16:01:51

at this point any effects to downstream tooling is undesirable

ewen17:01:04

ok, I will build upon a custom repl function then. I was hoping I could avoid that

dnolen17:01:05

@ewen the REPL namespace is meant to be reusable if for some reason you can’t use the main REPL entry point

dnolen17:01:51

reusability of the main entry is just not as important as not breaking tooling

dnolen17:01:23

making it easier for a few REPL developers just less important goal than not affecting tooling for thousands of users

ewen18:01:07

@dnolen yes i fully agree, and the behaviour of the repl did change with this commit: the repl now blocks the thread even when passed an empty :init param. The change I was suggesting would result in having a behaviour that better match to the previous one, which would decrease the possibility to break existing tooling

dnolen18:01:17

@ewen this is unlikely - all REPLs I’m aware of block on construction of REPL environment anyway

ewen18:01:27

replique does not

ewen18:01:23

but i agree it will probably not affect most of existing repls