liquidz / vim-iced

Clojure Interactive Development Environment for Vim8/Neovim
https://liquidz.github.io/vim-iced/
MIT License
516 stars 35 forks source link

Add support for evaluation in context like CIDER #434

Closed liquidz closed 1 year ago

liquidz commented 2 years ago

cf. https://github.com/clojure-emacs/cider/issues/2113 cc @sheluchin

liquidz commented 2 years ago

@sheluchin I tried to add <Plug>(iced_eval_in_context) in feature/eval-in-context branch.

For example, you can define mappings as follows.

nmap <Leader>ece <Plug>(iced_eval_in_context)<Plug>(sexp_outer_list)

Could you try?

sheluchin commented 2 years ago

@liquidz Thank you for trying this.

It looks like it doesn't quite work in this version. In the simplest form:

(identity x)

produces this error:

(clojure.core/let [42] (identity x))
=> CompilerException
Syntax error macroexpanding clojure.core/let at (iced_test.clj:28:1).
[42] - failed: even-number-of-forms? at: [:bindings] spec: :clojure.core.specs.alpha/bindings

It looks like the iced#operation#eval_in_context function gets the entire evaluation context but doesn't do anything to figure out the individual bindings, so it misses the x part.

It should also be able to handle multiple bindings somehow.

liquidz commented 2 years ago

@sheluchin Thanks for your confirmation! The current behavior is same as CIDER as far as I've actually tried in Emacs.

You should input x 42 (let-style) as context like this post in clojurian slack. https://clojurians.slack.com/archives/C053AK3F9/p1661180273024399?thread_ts=1661168165.432639&cid=C053AK3F9 image image (1)

It looks like the iced#operation#eval_in_context function gets the entire evaluation context but doesn't do anything to figure out the individual bindings, so it misses the x part.

I think the approach to figure out bindings automatically is not appropriate because of the order of bindings.

For example, if we pass 1 2 3 as context for the form (+ a b c), we would expect a = 1, b = 2, c = 3. But, it is tired to specify the appropriate values in the appropriate order when the form is large or has many bindings.

In comparison, it would be better to simply input a 1 b 2 c 3 like clojure.core/let, so that we don't have to be aware of the binding order. And even if the form is changed as (* x (+ a b c)), we can evaluate it by simply adding x 4, as in a 1 b 2 c 3 x 4.

It should also be able to handle multiple bindings somehow.

As described above, we can handle multiple bindings like a 1 b 2 c 3 with let-style.

sheluchin commented 2 years ago

Ah, my mistake! Yes, as you describe - providing the full binding form - it does work as expected and is a nice workflow tool. I like the idea of not having to make edits to the file when testing stuff quickly in the REPL.

It's nice that you can scroll through the history with up and down arrows too :) Do you think it would be difficult to add an FZF interface for the history?

Thanks again, @liquidz. It's good stuff.

liquidz commented 2 years ago

Do you think it would be difficult to add an FZF interface for the history?

Cool. :history i command shows the history of input, so I think it can be supported. But it may be needed to define another mapping. I'll try.

sheluchin commented 2 years ago

I think I found a small bug in the current version. It looks like if you start entering bindings and then press escape, it still tries to evaluate it, rather than canceling.

liquidz commented 2 years ago

@sheluchin Nice catch! Thanks. I've fixed in in feature/eval-in-context branch.

liquidz commented 2 years ago

@sheluchin

Do you think it would be difficult to add an FZF interface for the history?

I've added <Plug>(iced_eval_in_selected_context) in feature/eval-in-context branch for trial. (https://github.com/liquidz/vim-iced/commit/d3c5f0313c18e842f468343f9830d29f65d422de)

E.g.

nmap <Leader>esce <Plug>(iced_eval_in_selected_context)<Plug>(sexp_outer_list)

With this mapping, we can evaluate codes with the context selected by selector. cf. https://liquidz.github.io/vim-iced/vim-iced.html#g%3Aiced%23selector%23search_order

But we cannot switch between <Plug>(iced_eval_in_context) and <Plug>(iced_eval_in_selected_context). So it is difficult to realize the case where a user has filtered by selector, but still wants to enter a different context. It might be possible to create an original input buffer, but I don't think it would be worth the effort.

Thus, the approach providing different mappings does not seems to be a good, as it is tired to use different mappings for inputting and selecting. It may be smarter to use <C-e>, vim's buit-in function, if we want to select from the history while entering context.

How do you think?

sheluchin commented 2 years ago

I've fixed in in feature/eval-in-context branch.

Thanks! Works.

But we cannot switch between (iced_eval_in_context) and (iced_eval_in_selected_context).

I see what you mean. It is awkward to make a selection and then modify it in some way. I don't think it's very important as <Plug>(iced_eval_in_context) + <C-f> already makes search + edit fairly easy. Adding fuzzy search on top would be a nice QOL improvement, but it's not a big deal.

However, just so you're aware, FZF does have an option for dealing with this kind of situation. You can see it with the :History: and :History/ commands from fzf.vim. It looks like this:

image

Detail.

#iced#selector has to be able to handle the other selectors as well, so I see how that could make it more complicated.

In any case, this is a nice new feature :) Thanks again, @liquidz.

liquidz commented 2 years ago

@sheluchin Thanks for your confirmation!

However, just so you're aware, FZF does have an option for dealing with this kind of situation. You can see it with the :History: and :History/ commands from fzf.vim. It looks like this:

Ah I see, it is editable after selection.

I don't think it's very important as (iced_eval_in_context) + already makes search + edit fairly easy. Adding fuzzy search on top would be a nice QOL improvement, but it's not a big deal.

OK. I'll delete <Plug>(iced_eval_in_selected_context) for now, and will merge to dev branch. If we find a good way to achieve a method like fzf.vim for supported selectors, I'll consider adding it again.

liquidz commented 1 year ago

Just released v3.11.3086