Skip to content
This repository has been archived by the owner on Jun 4, 2022. It is now read-only.

Port planck's eval implementation #177

Merged
merged 4 commits into from
Jun 6, 2017
Merged

Port planck's eval implementation #177

merged 4 commits into from
Jun 6, 2017

Conversation

moxaj
Copy link
Contributor

@moxaj moxaj commented Jun 5, 2017

See #146

Question: should the new lumo.repl/eval be exposed anywhere else?

;; emit function values into JavaScript as numeric
;; references that are looked up.

(defonce ^:private fn-index (atom 0))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these don't need to be atoms. can you switch them to volatiles?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great ideas are always worth stealing. planck-repl/planck@0f29414

(defn- clear-fns!
"Clears saved functions."
[]
(reset! fn-refs {}))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vreset! once they're volaties

(defn- put-fn
"Saves a function, returning a numeric representation."
[f]
(let [n (swap! fn-index inc)]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vswap!

@anmonteiro
Copy link
Owner

This looks great! Left a few inline comments and I'll merge once you address them.

Thanks!

@moxaj
Copy link
Contributor Author

moxaj commented Jun 6, 2017

Sorry about the messy history! The windows specific commit should go in another branch.

@anmonteiro anmonteiro merged commit 1f306a7 into anmonteiro:master Jun 6, 2017
@anmonteiro
Copy link
Owner

thanks!

anmonteiro added a commit that referenced this pull request Jun 6, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants