Add joyride.core namespace w/ *file*
#18
Conversation
|
I'll take a look tomorrow! |
src/main/joyride/sci.cljs
Outdated
| @@ -19,7 +19,8 @@ | |||
| (def !ctx (volatile! | |||
| (sci/init {:classes {'js goog/global | |||
| :allow :all} | |||
| :namespaces (:namespaces pconfig/config) | |||
| :namespaces (merge (:namespaces pconfig/config) | |||
There was a problem hiding this comment.
Minor thing but for performance reasons it's better to just use assoc here.
There was a problem hiding this comment.
Tried, but I can't figure out how to do this with assoc while keeping the merge order of joyride.core las...
There was a problem hiding this comment.
(assoc (:namespaces ...) 'joyride.core {...})
| @@ -21,7 +12,7 @@ | |||
| (aset editor "selection" original-selection) | |||
| (p/do! (eu/insert-text!+ "#_" editor insert-position)))) | |||
|
|
|||
| (when run-main? | |||
| (when j/*file* | |||
There was a problem hiding this comment.
Looks good to me. It didn't even occur to me that this would maybe already enough for the REPL! Except maybe in the :load-file nREPL operation where this variable would be bound:
There was a problem hiding this comment.
supplied path and filename info to set source file and line number metadata
So that means this is not the whole solution to #4 then... Since we should actually be doing that, right?
There was a problem hiding this comment.
This is a part of the solution of #4 so let's continue. The other part is checking of we are currently in the invoked script. E.g. (= *file* (joyride.core/invoked-script)) or so.
src/main/joyride/extension.cljs
Outdated
| @@ -4,6 +4,7 @@ | |||
| ["vscode" :as vscode] | |||
| [joyride.nrepl :as nrepl] | |||
| [joyride.sci :as sci] | |||
| [sci.core] | |||
There was a problem hiding this comment.
Maybe we can use:
[joyride.sci :as jsci]
[sci.core as sci]
?
|
@PEZ Left a few very minor comments but the overall approach looks good! |
|
@borkdude now it's ready for review again. Thanks in advance! 🙏 |
|
Now updated with all implemented |
Fixes: #5
It took an embarrassing amount of time for me to figure this out. 😄 But now I think I am starting to get more of it and am glad I didn't give up and tossed it back to you, @borkdude.
These are the minimal changes. I choose to start like this to make the change easier to give and receive feedback on.
I would like to do rename
joyride.scitojoyride.core, because it gets a bit of a mind fuck remembering whichsciis which.Also, I updated the example to use this new facility. And then we are now running into the same problem as Calva has with what is on
masterand what is published. The example won't really work with v0.0.2. In Calva we solve it by having adevbranch where we direct PRs and accumulate changes, We then merge to the default branch when we release. It works, but causes some friction.