tpope / vim-fireplace

fireplace.vim: Clojure REPL support
https://www.vim.org/scripts/script.php?script_id=4978
1.74k stars 139 forks source link

Add support for spec middleware (form, example) #335

Closed christoph-frick closed 5 years ago

christoph-frick commented 5 years ago

This is a first attempt in supporting the features from the spec middleware.

One problem so far is the cursor jumping to the beginning of the file after cs[fe].

So far i tested with:

(ns vim-fireplace-spec.core
  (:require [clojure.spec.alpha :as s]))

(alias 'self *ns*)

(s/def ::id int?)
(s/def ::name string?)
(s/def ::user (s/keys :req [::id ::name]))

#_(
   ::id
   ::self/id
   :vim-fireplace-spec.core/id
   ::name
   ::self/name
   :vim-fireplace-spec.core/name
   ::user
   ::self/user
   :vim-fireplace-spec.core/user
   )

#_ (->> (s/registry) (keys) (map str))
tpope commented 5 years ago

I'm out of the loop, can you link to the documentation of what this spec middleware is exactly?

The cursor jump is because functions in VimL return 0 by default, and :exe 0 is the same as :0 which jumps to the first line. return '' will fix it.

christoph-frick commented 5 years ago

Relevant links:

I have no clue how that looks in emacs.

Thanks for the hint about the jump. I'll fix that.

christoph-frick commented 5 years ago

I have force-pushed the fix with the return "".

christoph-frick commented 5 years ago

I have addressed all your points

tpope commented 5 years ago

I'm afraid I saved the hardest part for last: error handling. We need to handle the case the op isn't there. This is the first time we don't have a fallback for if CIDER isn't installed, which is fine, but I need to think a bit on how to proceed.

We also need error handling for if the op is there but fails. What are the failure cases, and how can we handle them?

christoph-frick commented 5 years ago

I'll list all i thought about so far:

As for the fallback: no cider or old cider: we can do the same as orchard does. We could Eval some "try what spec is there" with the given keyword.

The only problem I see with this (and it's one with orchard too): the search for the "right" spec lib just favours the order of existence (try alpha1, then spec0 - soon to be started with alpha2). Right now this is not a huge problem, because most likely folks will use alpha1. But once alpha2 hits, the code bases are spec1 and spec2 "is there" and maybe incompatible and in the worst case scenario messes things up if loaded in parallel. So that could mean, a switch per project is needed to tell the tooling, which spec version is in use or have the tooling do more work in finding the right one.

tpope commented 5 years ago

The current "op is not there" behavior is silent failure too. We need separate errors for both not connected and connected but no op. This is the thing I will think on. (If you want to provide an eval callback, great, but I'm inclined to think it's not worth bothering with.)

The only other case I care about is "not known". The others seem fine as is.

christoph-frick commented 5 years ago

I have addressed above three points and tested them (still works, unknown op gives an proper error, no repl gives an proper error).

tpope commented 5 years ago

Thanks for following through!

christoph-frick commented 5 years ago

Cheers