Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Evaluate block does not work for top-most block #228

Open
hovsater opened this issue Dec 29, 2020 · 2 comments
Open

Evaluate block does not work for top-most block #228

hovsater opened this issue Dec 29, 2020 · 2 comments

Comments

@hovsater
Copy link

hovsater commented Dec 29, 2020

First of all, thanks for all work you've put into Chlorine, it's a wonderful package. ❤️

I'm not sure this is a bug or not. If it's not, feel free to close this issue. 🙂

I thought the difference between Chlorine: Evaluate Block and Chlorine: Evaluate Top Block is that Chlorine: Evaluate Block will evaluate the current form (relative to the cursor position) while Chlorine: Evaluate Top Block would evaluate the top-most form (relative to the cursor position).

(for [x (range 1 10)] (* x 2))  ; Cursor is immediately after `x`.

The above example would evaluate to (1 2 3 4 5 6 7 8 9) using Chlorine: Evaluate Block while it would evaluate to (2 4 6 8 10 12 14 16 18) using Chlorine: Evaluate Top Block. Of course, I could achieve the same thing by moving my cursor to the start of the line and run Chlorine: Evaluate Block, correctly giving me (2 4 6 8 10 12 14 16 18) as well.

This all make sense to me. Now here's a different example.

Thread$State

The above example would evaluate to java.lang.Thread$State using Chlorine: Evaluate Top Block. But Chlorine: Evaluate Block does not evaluate this example at all.

Is that a bug, or am I just misunderstanding how these features work?

@mauricioszabo
Copy link
Owner

It's not a bug, nor a misunderstanding - it's what we call a "leaking abstraction" 🤣

In the beginning, both "evaluate top block" and "evaluate block" did not evaluate anything - because it's not a "block", really, just the "current var". But then I changed the way I detect top-blocks so I could re-use the code on VSCode.... and then things became weird. The current approach does detect top-level variables as "top-blocks", and also sometimes detects more things (like @(:some state) will include the @, and #_(+ 1 2) will evaluate the block - this is intentional - but will not evaluate if there are multiple #_ before the top-block).

I could add support for Thread$State, but then I have to check for (:something other-var) to eval the "block", not the "current var". I'm working on a migration to Pathom to solve things for me more easily and then I'll be able to track these inconsistencies (by using Pathom I don't have to re-parse the contents multiple times to detect blocks/top-blocks/vars/namespaces/etc - on some big files like the clojure/core.clj this is a big performance hit).

I will probably try to get the current "block", if it it does not exist, it'll get the current "var". What do you think?

@hovsater
Copy link
Author

hovsater commented Dec 29, 2020

Thanks again for an awesome explanation about the intricacies of how Chlorine work under the hood. I think what you're suggesting makes sense. I don't think it's worth pursuing a solution to this until you've started the migration. While I haven't used Pathom myself, I can see the benefit of being able to express the parsing more fluently instead of having to do multiple passes over the result.

While I'm still learning Clojure myself (day 3 to be exact 😄), I'd be happy to contribute in financial terms if needed. Do you have any preferred medium for financial contributions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants