Clojurians Log v2

Okay so here it is more split out into it's own namespaces mink.mainhttps://github.com/johnmn3/mink One issue I'm still trying to work through is finding a way to pass some options like :prompt through from the cljs.cli interface. It's easy from the cljs.repl/repl* entry point, which is fine for socket-repl/prepl. But for the main entry I'd like to lean into cljs.cli as much as possible

huh that might be a gap - I don't think I really thought much about custom prompts.

I'll just use repl*

hrm I'm still confused - https://github.com/vouch-opensource/krell/blob/master/src/krell/repl.clj#L271

fast initial prompt is a "root" level option far as I now - same as prompt - but maybe I'm missing something that you're saying about why this doesn't work

Well, my theory is that because repl/fast-initial-prompt? does not exist in this map: https://github.com/clojure/clojurescript/blob/9696bac5bd47abbc8741631989448db48ee017db/src/main/clojure/cljs/repl.cljc#L1078 It's not getting overwritten by that map, since that map comes after the other two in the merge-with (though the nil work in that merge-with maybe is preventing that, not sure), but because some keys like :prompt are in that third map, keys in the prior maps are maybe getting clobbered. I guess the merge with strategy is supposed to prevent that particular clobbering?

Yeah, maybe I was wrong about that, with the merge strategy there. I'll fire up a cljs branch and test that path

Oh wait, maybe not. Cause :prompt becomes repl-prompt in the opening binding form of the defn for repl*, in the :or part

So it's not nil, so repl-prompt always wins

oh wow, looking at krell, I didn't realize -re could take a namespaced foreign repl-env

Yeah I should have studied krell more. Lots of good stuff in there

hrm I don't follow, repl-prompt only wins if it isn't passed in. But can pass it in via IReplEnvOptions

(again I could be misreading, but I'm feeling like it should work just fine and has been accounted for)

Aren't the repl env opts coming in via repl*'s first arg, repl-env? Or do they come in via the second opts map? I thought they were coming in via repl-env, which gets parsed by -repl-options and then is merged with the :prompt that came from opts. Are you saying the IReplEnvOptions get passed in via the right (second) opts param to repl*?

they have to be, otherwise my point about fast initial prompt wouldn't work

in Krell

Oh okay. I misread it

IReplEnvOptions has a docstring about this

(which is cool, cause I didn't remember if we had been smart enough to do that 🙂

it was designed specifically for the case where you need to declare the options for the REPL

well, fast-initial-prompt get's pulled off renv here and put on to opts as the second arg to repl*?

https://github.com/clojure/clojurescript/blob/9696bac5bd47abbc8741631989448db48ee017db/src/main/clojure/cljs/cli.clj#L327

Well, I gotta trace it with the codebase running

But I thought I traced it before

But I'll chase it down tomorrow

I think when I made that work I wasn't careful enough - there's no reason to limit it to fast initial prompt

yeah I think is just a gap - https://clojure.atlassian.net/browse/CLJS-3423

hrm did you look at IReplEnvOptions ?

(not saying that's obvious at all, just asking whether you did)

@dnolen I ended up going with an api that https://github.com/johnmn3/mink/blob/06340632fa99051c298c360a9d2b83ca072165ec/src/mink/main.clj#L229. Oh, hmm, I may have messed up with that but I thought it wasn't working for my use case. I'll try again with that though. With my current impl I'm just taking all options. I just need to figure out which ones to split into compiler opts vs repl opts vs other opts and passing them down the correct path. Starting to lean on cljsc/known-opts for the compiler opts, But if there's an obvious way to split them up using existing filters, I thought I'd come here and ping you if anything comes to mind. WRT IReplEnvOptions... Oh, cli/main takes it as the first arg. Hmm. I'm not remembering why that didn't work... I'll check it

So I was passing in the new repl-env to cli/main here: https://github.com/johnmn3/mink/commit/5cd63e3afa3632df2bdd26c6390464e7677a08d7#diff-2d7325406c2778f685f58aed0d88dcfffa1966fec83e8f804fdd360d9d4f5d58R792

Maybe I couldn't figure out how to allow user inputs to flow down into my specific repl-env 🤔 I'll dig in and try to figure out what the specific issue was

IIRC, things like :prompt were not taking effect, when fed in from the IReplEnvOptions, but that could have just been user error

So reverting back to here and adding a :prompt fn right here: https://github.com/johnmn3/mink/commit/399d97f1ee341013cb3847720d7dae3964b005dc#diff-9f72dc7c40300c6abc33e2482a28f65590fa016720a5bb5d26a6996a43f58d6cR807 (in mink.main/repl-env*) I'm not seeing that the prompt fn is getting picked up and used. Prompt should be mink=> but it's still cljs.user=>

Maybe it needs go under a particular key in the repl-env?

wait...

Yeah, no mas. In both:

repl/IReplEnvOptions
  (-repl-options ...

(merge (BrowserEnv.)
    {:host host
     ...

:prompt (fn [& args] (println :args args) (print "mink> "))

Looking in cljs.repl, it looks like :prompt is brought in via the opts (second param) and not the repl-env (first param):

(defn repl*
  [repl-env {:keys [init inits need-prompt quit-prompt prompt flush read eval print caught reader
                    print-no-newline source-map-inline wrap repl-requires ::fast-initial-prompt?

Again I think I could probably just add "-ro" "{:prompt blah ... to the cli/main fn call. Just seems hacky that way. And getting the whitespace to work going from clj to cli and back was brittle - never actually got it working, but I'm sure it's doable.

And I'd have to not clobber a user provided -ro so that's complicated

cli repl opts probably should over-ride default repl-env opts. But in this case I believe even nils are overriding the repl-env for some of these keys

Which isn't the end of the world - I can use repl* and that certainly aligns behavior more-so with socket repl/prepl I guess. So I'm not sweating it

cool! if you could put together a patch that would be great - happy to see this fixed up.

Okay, I've been working on getting mink more aligned with working with web workers, which was my original impetus to mess with it in the first place. So I want to make sure my own requirements sussed out before requesting any changes. I should have a patch in this week.

Hmm, okay. I do think it's right that cli provided --repl-opts should override default repl opts... Even when a custom repl-env provides different repl opts. And people might be depending on that precedence. Seems like it might be a probably-non-breaking-change if opts continues to take precedence over repl-env, unless 1) repl-env populates one of those options and 2) user provided opts are nil anyway for those options.

So like, only when a custom repl-env doesn't provide a :prompt and the user doesn't provide a :prompt in their -ro opts or whatever else, then :prompt should default to repl-prompt

And that seems pretty minimal of a change

I mean users supplying prompt via command line wasn’t a design goal - so I’m not super concerned. But yeah we could take some care to not break the small number of folks doing that

Here's two takes on a possible patch. One is probably too naively simple: https://github.com/johnmn3/clojurescript/compare/master...johnmn3-small-code-change And this second one is probably more surgical, behavior-wise, but it's pretty messy and could probably be refactored into something more sensible: https://github.com/johnmn3/clojurescript/compare/master...johnmn3-bigger-change Gonna hammock those for a bit and look at it again later. Maybe a cond-> might clean it up a bit

I read the cli code - I'm wondering what the problem is? We call repl* w/ the "compiler options" could't you just add the bit you need?

Add flag strings? like (conj (vec args) "-w" "src")?

Or use repl-opt?

Behaviors will be easier to sync up between all three repls by using repl* directly.

There's a good amount of logic in default-main and default-compile from the cli namespace I'm missing out on though

There's probably some way to do it with bindings

I vendored in the cljs.cli namespace at one point but it got complicated

is it required to use the name in your namespace? I don't think that it is a good idea to have 3-party libraries using cljs.repl.* namespace if this name is required, i think that the cljs main could be extended, allowing any namespace.

I thought it was required. Maybe it isn't? Do you know if it can already be done outside of cljs.repl.*?

Or would that extension point need to be added to clojurescript?

I mean, for sure, it could be built like clj -M -m my-repl-ws.main blah blah. I just like that you don't have to learn anything new (ideally) other than the new -re ws. The rest can just follow the normal clojure guides and docs. Not all features/flags pass through correctly right now but that's the goal.

Hmm, do you think I'm violating some copyright just by using cljs.* in a project 🤔 I hadn't thought of that

I view this as a "framework solution" - like where you can provide the system things in places where the system can pick them up. I wouldn't be opposed to clj -M -m cljs.main -re ... though, if y'all want to make it less frameworky

like, "if -re param has a dot in it, assume it's a foreign repl env and the whole namespace needs to be read out"

Not sure how that would work for the socket repl / prepl situation though, which I'm getting for free here too

I suppose the client side ns can be whatever I want, so I don't have to use the clojure/cljs names there

So it might be a little less jarring for users not expecting to come from a foreign lib

Oh, the socket repl / prepl takes the whole namespace, so it can already use foreign repl bits

It just occurred to me that I'd be squatting on a potential future namespace of the same name. I figured the different org coordinate would be sufficient to disambiguate within a given project. But when required by other libs, those libs might break (potentially?) if they want to move to org.clojure/cljs.repl.ws, if both libs don't have the same behavior. So I'll back out of that

Unless y'all are sure you never want to impl that namespace in the future and you don't mind if others sit on that namespace. Or I can put a PR together to allow namespaces for foreign repl env impls and then there won't be name clashes. Or just back out of it and only provide the socket repl/prepl. No big deal to me either way

for starting up REPLs, that is how it works. It would probably be better to have your own specific some-ns.main like other REPLs do, then no conflicts.

This is pretty minimal wrt. dependencies, but that's also why I dragged my feet on this mostly - adding a dependency even if it's only one library is really annoying.

Yeah I agree nobody is really asking for this. It's a thing I need every once in a while

Cool I'll do a main

Gonna call it https://en.m.wikipedia.org/wiki/Stoat :)

A stoat piggiebacking on a weasel piggiebacking on a woodpecker

hehe