Fork me on GitHub
#chlorine-clover
<
2020-08-06
>
Rowan Barnard01:08:25

Ah I see, yeah I am always using submit top-level form to REBL to evaluate my code as I go, not sure if this is the usual/recommended way for a normal workflow

Rowan Barnard02:08:16

I am not understanding why evaluate selection works and top-level form doesn't? I was reading in the book something about the difference between forms and expressions and what makes a valid form - does that have anything to do with it?

Rowan Barnard02:08:09

And thanks for looking into it 🙂

mauricio.szabo02:08:22

To explain why, I'll need to dive a little bit on internals of Chlorine 😆 Evaluating a selection is just capturing what's selected in the screen and sending to the REPL. It's quite easy code

mauricio.szabo02:08:14

To evaluate blocks, we need to parse parts of the code, to detect where a block starts and ends. I use a ClojureScript library to do it.

mauricio.szabo02:08:31

Top blocks are even worse, because the whole file must be parsed so it's possible to detect when the "top block" starts and ends.

Mikael Andersson14:08:43

I worked with PEZ (the author of Calva) so I know that he/they had some issues related to this, identifying blocks, and having to parse from top form. In the end, I believe Calva ended up with some combination of lexing and using heuristics to cut improve performance, and not get completely stuck if a form was wrong. So it might be worth looking at the code, or ask somebody involved in those parts of Calva.

mauricio.szabo15:08:41

The first time I saw the calva code that did it, it was using VSCode APIs to make this identification. Don't know if pez did change it in the newer versions...

Mikael Andersson15:08:29

I could be mistaken, but I'm almost certain it isn't done that way when I looked at the code a month or three ago. Unless of course I completely misunderstood either the question or the code 🙂 I could probably have another look this weekend, as I have a few changes I sort of promised @U0ETXRFEW I would make into a PR ages ago, so I could investigate and to the PR in one fell swoop, unless it gets too hot. My apt becomes a baking oven when its warm making all attempts at thinking entirely futile.

pez16:08:33

Hi there, @UQRPN1XGW, I miss working with you!

pez16:08:47

Not quite sure what is the context for this thread, but anyway, Indeed, top level indenting/formatting has been troublesome in Calva. Fortunately, Calva is good ar always keeping the document formatted, so this case is not too common. I don't think the actual identification is particularly vscode bound, but maybe I misunderstand something. I have a branch where I have made an npm module of the parser and token cursor used to traverse the forms. I was intending to make npm modules of the rest of the indenter as well, but I got stuck in resolving a dependency on VS Code... (Also, there are two indenters in Calva. 😃 One written in TypeScript and a cljs one. Though today the indenter part of the cljs one is no longer used. It is only used for formatting.)

pez16:08:05

Looking forward to that PR, @UQRPN1XGW. You might have to start over, because there has been a lot of work on Calva since you forked. Then again, you are a merge master so maybe you know how to untangle it.

mauricio.szabo16:08:18

@U0ETXRFEW we were talking about identifying forms, top-level forms, etc. Even this is not vscode-specific in any way? If so, maybe I can get some clues on how to work with unbalanced code, or code that have syntax errors 🙂

pez17:08:45

Roger that. Have a look at cloujure-lexer-test.ts and token-cursor-test.ts https://github.com/BetterThanTomorrow/calva/tree/master/src/extension-test/unit/cursor-doc The lexer has a junk token, that makes sure it doesn't crash on funny input. In the Token Cursor unbalance is not much handled. There are some attempts to help with it, but generally it treats it as ”unbalance in the Force: all bets are off”. It never gets stuck though. There are a lot of anti patterns in Calva, but this token cursor thing I can truly recommend. It should be noted that CIDER (or clojure mode, or whatever it is) also uses a token cursor. Much similar to Calva's, because Calva is distilled from CIDER, haha.

mauricio.szabo02:08:53

The problematic part is that when the code have an error, Chlorine needs to figure out how to behave. Currently, it ignores the wrong code and tries to keep parsing (sometimes getting the wrong result in the process, as it's happening with the issue you opened 😅)

seancorfield02:08:08

(and it's also worth mentioning that if you're using my eval extensions, the selected code to be evaluated is expected to be valid in a (let [value code] ...) expression, so it has to be a valid single form)

seancorfield02:08:42

In the built-in eval, I think selection can be multiple forms, @mauricio.szabo?

mauricio.szabo02:08:25

Yes, they'll be wrapped in an implicit do

seancorfield02:08:49

My extensions don't wrap it in do because the code is submit'd to REBL and you want it to be the actual code being evaluated rather than being wrapped in do, but in the new tap>-based logic, I could do it -- and just give up on the veracity of what is passed to REBL 🙂

seancorfield02:08:21

Even so, wrapping a selection of 123) (map inc is (do ..) isn't going to help you -- some selections just aren't going to be valid, no matter what you do 🙂

seancorfield02:08:17

@flyingpython So, was that enough detail? 🙂

Rowan Barnard02:08:45

Ah OK I get the general picture thanks!

seancorfield02:08:17

@mauricio.szabo did you see the ClojureScript site issue I tagged you in? Rewriting the broken ProtoREPL instructions to working Chlorine instructions...?

mauricio.szabo14:08:33

Yes, I saw! Thanks, I'll try to make a PR for it 🙂. The only issue is that currently Chlorine does not support Figwheel (that seems to be the most used tool for ClojureScript)

mauricio.szabo14:08:47

Not really, because it's not that configurable (and actually slower than the rewrite-cljs). For example: with rewrite-cljs I can ignore the #_ literals when parsing text, or I can keep or remove newlines, and it'll not change tags like '( 1 2) (edamame will try to expand quotes and ::symbols for example)

borkdude15:08:23

slower? have you done any benchmarks? should not be much slower than rewrite-clj(c)(s)

borkdude15:08:06

edamame has configuration for how you want to resolve these things

mauricio.szabo15:08:10

I believe it was an old version, to be honest 🙂

borkdude15:08:59

I haven't done any benchmarks myself, but last time I checked it was pretty much the same (since both solutions are based on tools.reader)

borkdude15:08:16

user=> (require '[edamame.core :as e])
nil
user=> (e/parse-string "#_(+ 1 2 3)")
nil

borkdude15:08:52

user=> (e/parse-string "'(+ 1 2 3)" {:quote (fn [x] x)})
(+ 1 2 3)

mauricio.szabo15:08:12

The thing is, what I do need on Chlorine is to parse a string, lazily, ignore syntax errors if possible, then re-construct the original parsed string without any changes (or with minimum changes). Rewrite-cljs does exactly this. Edamame / tools.reader / other tools do different things

mauricio.szabo15:08:26

For example, what I do want to do is to be able to: (parse "'(+ 1 2)") => "'(+ 1 2)" (parse "'(+\n1 2)") => "'(+\n1 2)" (parse "(+ 1 2))") => "(+ 1 2)" (parse "#_(+ 1 2)") => "(+ 1 2)"

mauricio.szabo15:08:41

And even (parse "(+ #_1 2)") => "(+ #_1 2)" I don't really need to convert things to EDN at this moment because I'm only interested in the structure of the code

borkdude15:08:42

True (rewrite-cljs is still based on tools.reader as a library though, like edamame, hence performance should be roughly the same). Yes, if you want to "rewrite" then obviously rewrite-clj(s)(c) is a better fit. Also when you want to parse and ignore errors. That's why I ended up with rewrite-clj in clj-kondo initially. Edamame has a different focus: you want to parse code to actually run it. So it should crash when there's something wrong

mauricio.szabo15:08:37

@U04V15CAJ exacly! That's the point I missed on the first versions of Chlorine 😄

borkdude15:08:07

you were never actually using edamame back then right? it didn't even exist

borkdude15:08:21

or, how long does chlorine go back in history? :)

mauricio.szabo15:08:51

No, I never published any version that used edamame, but I tried once to change the code when I was using tools.reader. The "detection of forms" changed a lot: 1st version used Atom API to get the blocks (it's faster than what I'm using now, but it had to go because it only works in Atom, and also because it did detect parenthesis inside string and comments as "valid code" 😱

borkdude15:08:21

I almost used rewrite-clj inside babashka, but I figured it would be faster if I would work with the sexprs directly

mauricio.szabo15:08:46

The 2nd version used tools.reader, but to be reliable it needed to parse the code multiple times to get around errors in source (not ideal, and never worked 100%)

borkdude15:08:59

yeah, rewrite-clj(s)(c) is perfect for that. you should maybe try at some point, since it's going to be the successor of both: https://github.com/lread/rewrite-cljc-playground

borkdude15:08:06

and it will be maintained under clj-commons

lread15:08:49

hey ho! 👋 yep, toiling away at rewrite-cljc as we speak!

mauricio.szabo15:08:33

great! As soon as these namespaces are supported (don't know if they already are) I'll migrate: https://github.com/mauricioszabo/repl-tooling/blob/master/src/repl_tooling/editor_helpers.cljs#L5-L10 🙂

borkdude15:08:09

should be there I think. I'm also using some of this in carve

borkdude15:08:02

you can try it as a drop-in library in a branch. the only thing you need to change is rewrite-clj -> rewrite-cljc.

lread15:08:24

Yeah that’s all available. The last hurdle I am working on is how to support namespaced keywords and namespaced maps. Note that zip.base and zip.move are considered internal, you can now access what you need from zip. The reader namespace is also considered internal, but I’ve not broken compatibility therein, so should be ok for you to use. And until I get a release up on clojars, you will be frustrated if you want to release your work on clojars.

lread15:08:37

I noticed you were using reader a while ago, and have a todo to look into your usage.

borkdude15:08:35

Is that still about this one? https://github.com/xsc/rewrite-clj/issues/54#issuecomment-493038733 I thought you already solved that a long time ago.

mauricio.szabo15:08:57

@UE21H2HHD well, every usage of rewrite-cljs should be contained into this single namespace I sent to you 🙂. There's also a test namespace to understand why I need all this "zip dance" 😄

lread16:08:39

@U04V15CAJ, there are shortcomings to my original solution when in comes to sexpring and also cljs so I am hammock ing on it. I’m going to be so bold to ping you for your feedback when I flesh out a plan.

borkdude16:08:42

I guess carve doesn't suffer from this, since it doesn't do much sexpr-ing.

borkdude16:08:48

oh it does, btw

borkdude16:08:05

but it's not critical

lread16:08:38

@mauricio.szabo I’ll have to take some time to study that code to understand what it is doing, looking forward to it.

lread16:08:34

@U04V15CAJ, yeah I think carve should be fine, with current sexpr shortcomings.