A small first step https://github.com/clojure-emacs/orchard š
Iāve granted commit access to @volrath, @pesterhazy and @dominicm. Iāll starting moving some logic out of cider-nrepl
over the weekend (hopefully), but Iād appreciate it if you join the effort to make something cross-tool compatible.
Oh cool, would love to
@bozhidar very nice. Also love the name
one simple thing to contribute could be a fail-safe doc
command: https://github.com/Unrepl/unravel/blob/0b7fe0e95912915d3144c79026778463702e3163/src/unravel/loop.cljs#L129
background: (clojure.repl/doc a.b)
throws an exception
@pesterhazy that's weird
@pesterhazy it must some problem related to something else, here is how inf-clojure
does it: https://github.com/clojure-emacs/inf-clojure/blob/master/inf-clojure.el#L711
Have you tried with a.b?
oh now let me try
@pesterhazy I get ClassNotFoundException
of course š
I mean is, should doc
be failsafe? Or should be the client's responsibility to catch and show the error? In inf-clojure
the latter has been chosen
@pesterhazy doesnāt your need to have a failsafe doc comes from the fact that you automatically call doc on the current symbol?
Yeah I could check if itās a valid symbol
@richiardiandrea try (doc java.lang.Object)
for a different exception
Although it should work for nss too so...
Maybe best not to rely on doc
@pesterhazy canāt you just handle the error?
@pesterhazy I think info
is supposed to do something more than doc
Sure Iām doing that now, with the eval hack
@cgrand yep I am just saying that in inf-clojure
we handle the error client side
but Paulus raised a good point, should this "middle layer" handle error handling and to which extent? Imho it shouldn't because different clients might need/want to do things differently
this is awesome! I was, in fact, wondering what to do next, and orchard
was the top choice... I have lots of free time this month except for next week, so it's just a matter of coordinating with you guys so that I can get to work here.
@bozhidar is this the type of files that we're expecting to have in orchard? https://github.com/Unrepl/spiral/blob/master/tools/src/spiral/tools/completion.clj -- basically same as cider-nrepl but replacing middlewares for plain functions? what other considerations do we have to be aware of while migrating?
actually, let me move this to the main thread...
Iām guessing itād be nice if there was a function that returned some more meaningful result when somethingās not found. cider-nrepl
for instances has to catch all exceptions in the middleware otherwise sync requests would time out.
> A small first step https://github.com/clojure-emacs/orchard š
this is awesome! I was, in fact, wondering what to do next, and orchard
was the top choice... I have lots of free time this month except for next week, so it's just a matter of coordinating with you guys so that I can get to work here.
@bozhidar is this the type of files that we're expecting to have in orchard? https://github.com/Unrepl/spiral/blob/master/tools/src/spiral/tools/completion.clj -- basically same as cider-nrepl but replacing middlewares for plain functions? what other considerations do we have to be aware of while migrating?
Not a big deal, of coruse, but a little annoying.
@volrath Yeah, exactly. Well itād be nice if when migrating things you also replace what was extracted with references to orchard
.
> basically same as cider-nrepl but replacing middlewares for plain functions?
Yes.
> what other considerations do we have to be aware of while migrating?
Just the standard ones - good names, some documentation and tests, reasonable APIs. š
Obviously we donāt have to get it right from the start. š
regarding exceptions and the doc
thing.. I think this library should not shadow exceptions and clients should handled them on their own. In general, i think of orchard
as just another library that can be extended by the client's own tooling. cider needs to do this by adding the nrepl wrappers, and I'm pretty sure spiral will need to have it's own clj code in some cases as well. I guess the key is to make it as generic as possible
so I agree with @richiardiandrea, clients should do their own "middle layer"
another thing that I wonder is how should we proceed regarding cljs
Iām fine either way. My point was that if you had a function returning :orchard/symbol-not-found
or something like this, itās more or less the same level of abstraction, but a bit easier to handle in the tooling code.
Exposing exceptions (and ergo implementation) stops you from categorizing so well.
The more tightly we define the API, the simpler it is to use the library, as the inputs and outputs are defined.
> another thing that I wonder is how should we proceed regarding cljs
cljs-tooling
is tiny and was written before the era of cljc
, so I guess we should integrate into orchard
.
I agree.
We can always extend functions with new return values, and make it clear that functions may return new values in the future.
my fear is that exceptions provide meaningful information that can be used by clients in some cases, so shadowing them becomes tricky. In any case, a client could have its own clj code to catch exceptions and return an value that's easier to handle by the client's code
Yeah..if you think of cljs-lumo-planck then each and every of it throw different things...
@volrath clients should expose that information via the api
And give a PR
I'd consider poking at those exceptions use of an "internal API", not something that's defined or guaranteed to continue working into the future.
so, I tried doc
in lumo and it returns nil
when the var doesn't exist, instead of throwing like clojure.repl/doc
-- so now I think I'm closer to @dominicm's opinion on having the API handle certain errors. again, my fear is that I think this is a slippery slope towards having an inconsistent and, as @cgrand said, leaky abstraction. I mean inconsistent in terms of having to carefully decide whether an error is worth shadowing, since shadowing might mean loss of information
I like the idea of throwing with ex-info
Loss of information isn't an argument I understand. If there's useful information to expose, it can be exposed (via a PR). Ex-info has a nice middle ground if you set the cause trace, as people can choose to poke at internals. The problem is if your contract marks a particular exception, you have no chance at changing the function, nor at general portability.
@dominicm not sure I get what you mean by PR in this context
Pull Request?
@cgrand yeah. Nothing stops a user of the library exposing additional information as data.
Iām still not sure that I follow. Letās say you have orchardās doc which returns errors as data. Comes a user of orchard who is not satisfied by the data returned and needs more. You are suggesting that s/he should create a PR to have her/his change merged?
Yes and/or create a local fork.
Not lossing information allows the user to ship his own workaround right now and without having to fork. This doesnāt preclude him to at the same time discuss a change to the upstream lib. To me it has the following benefits: ā¢ no fork (not having to understand the code and no extra build/deploy, not having to maintain the fork if the change never makes it upstream) ā¢ ship right now without waiting on upstream patch ā¢ (corollary of the previous point) less pressure on the maintainer of the lib, time to think about a proper fix or even to say no.
I like those benefits, to the degree that they don't prevent improvements. My concern is about the workaround becoming part of general use. I actually think ex-info provides a nice middle ground in this. The ex-info exception is the API, the cause stack is the workaround. I want to ensure that orchard maintainers never feel compelled to prevent a change of code for the good, because other users might be relying on workarounds. Where those workarounds rely on implementation-specific exceptions.
@dominicm not sure if I follow that last bit
@volrath exceptions are hard to make part of the API. They tend to shift. I'd think that any client using a random exception thrown by a function will break in a future version of the library. I tend to prefer that a function will use data, and clearly indicate what may go wrong. This gives more flexibility for the function to change, without breaking clients.
so you're saying that if a client needs an exception info/stacktrace, it should ask for it "post-mortem"? If so, I think this would be a pretty big commitment for a library targetting any type of clojure tooling. Another approach could be have the client ask to shadow certain well known possible errors (like doc
when the var doesn't exist), so for example, the orchard/doc
function could take an optional parameter indicating not to throw on this particular error.
In boot, as @dominicm might know, we serialize to string the stack traces so that it can be shared between tasks (from boot-cljs
). That can be an option too
By serialize I mean maybe more normalize to a standard format
Those details can be as minimal as required.
My argument is more about expected cases, e.g. Non-existing var is something we expect can happen. I'm not sure it's a good idea to have clients build on exceptions which may not exist later. It gives friction to change. I guess I'm suggesting that the API should include some "error" cases which are expected (by some definition), and give a straightforward data description for each case.