Fork me on GitHub
#unrepl
<
2018-01-04
>
bozhidar18:01:36

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.

volrath20:01:38

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?

volrath20:01:10

actually, let me move this to the main thread...

dominicm18:01:57

Oh cool, would love to

pesterhazy19:01:49

@bozhidar very nice. Also love the name

pesterhazy19:01:53

background: (clojure.repl/doc a.b) throws an exception

richiardiandrea19:01:07

@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

pesterhazy19:01:18

Have you tried with a.b?

richiardiandrea19:01:36

oh now let me try

richiardiandrea19:01:35

@pesterhazy I get ClassNotFoundException of course 😉

richiardiandrea19:01:07

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

cgrand19:01:15

@pesterhazy doesn’t your need to have a failsafe doc comes from the fact that you automatically call doc on the current symbol?

pesterhazy19:01:05

Yeah I could check if it’s a valid symbol

cgrand19:01:48

@richiardiandrea try (doc java.lang.Object) for a different exception

pesterhazy19:01:51

Although it should work for nss too so...

pesterhazy19:01:16

Maybe best not to rely on doc

cgrand19:01:29

@pesterhazy can’t you just handle the error?

richiardiandrea19:01:46

@pesterhazy I think info is supposed to do something more than doc

pesterhazy19:01:08

Sure I’m doing that now, with the eval hack

richiardiandrea19:01:10

@cgrand yep I am just saying that in inf-clojure we handle the error client side

richiardiandrea19:01:28

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

bozhidar20:01:28

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.

volrath20:01:56

> 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?

bozhidar20:01:07

Not a big deal, of coruse, but a little annoying.

bozhidar20:01:47

@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. 🙂

bozhidar20:01:47

Obviously we don’t have to get it right from the start. 😄

volrath20:01:38

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

volrath20:01:37

so I agree with @richiardiandrea, clients should do their own "middle layer"

volrath20:01:00

another thing that I wonder is how should we proceed regarding cljs

bozhidar20:01:54

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.

dominicm20:01:24

Exposing exceptions (and ergo implementation) stops you from categorizing so well.

dominicm20:01:28

The more tightly we define the API, the simpler it is to use the library, as the inputs and outputs are defined.

bozhidar20:01:39

> 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.

dominicm20:01:29

We can always extend functions with new return values, and make it clear that functions may return new values in the future.

volrath20:01:26

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

richiardiandrea20:01:56

Yeah..if you think of cljs-lumo-planck then each and every of it throw different things...

dominicm20:01:35

@volrath clients should expose that information via the api

dominicm20:01:42

And give a PR

dominicm20:01:30

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.

cgrand21:01:40

But the ground truth is that we do have exceptions that vary through versions and implementations. We are not designing from scratch. As I understand you are saying "it’s error prone, let us do it for you", to me it’s bound to get you a leaky abstraction. Why not return both the category keyword (or whatever) and the original exception… either as a pair or as a n ex-info. My preference to the latter.

dominicm21:01:15

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.

dominicm21:01:16

I think expected to me means in terms of "user of the client" input.

dominicm21:01:51

I feel like I'm misunderstanding you and @volrath

volrath10:01:15

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

volrath10:01:26

I like the idea of throwing with ex-info

dominicm10:01:16

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.

cgrand11:01:02

@dominicm not sure I get what you mean by PR in this context

cgrand11:01:34

Pull Request?

dominicm11:01:16

@cgrand yeah. Nothing stops a user of the library exposing additional information as data.

cgrand11:01:12

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?

dominicm11:01:47

Yes and/or create a local fork.

cgrand11:01:11

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.

dominicm11:01:06

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.

volrath20:01:52

@dominicm not sure if I follow that last bit

dominicm20:01:26

@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.

volrath20:01:13

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.

dominicm21:01:52

Maybe I'm not understanding, why would it throw when a var doesn't exist? That's not an exceptional case. That's something you can indicate back to the client as data and give details about the failure.

dominicm21:01:42

Those details can be as minimal as required.

richiardiandrea21:01:41

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

richiardiandrea21:01:09

By serialize I mean maybe more normalize to a standard format