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

Friendlier behavior for borked namespaces? #161

Closed
trptcolin opened this issue Jun 13, 2014 · 10 comments
Closed

Friendlier behavior for borked namespaces? #161

trptcolin opened this issue Jun 13, 2014 · 10 comments

Comments

@trptcolin
Copy link
Contributor

Currently, if you open a file with compilation errors in the namespace (like a broken macro), things like :Doc map, cpp to evaluate expressions, etc. don't work.

The case I have in mind is the Clojure Koans, which are currently implemented to throw when the ns is required. Not a good idea to have top-level side brokenness for prod apps, of course, but given one, it'd be cool to be able to use fireplace anyway.

Doing the workaround you mention in #157 (adding (in-ns 'user) to the top) gets it working, but it'd be nice if we could avoid that. I get that if the ns invocation fails, we can't know what context we're in, but here the ns invocation works fine - it's stuff lower down the file that fails. But I guess since s:repl.preload uses require, it's considered to either work or not - no in between.

So that motivation aside, I don't have a great understanding of how s:repl.preload works, whether this is possible, or even if it's a good idea. It'd just be really convenient to be able to do stuff within a broken file.

@daveray
Copy link
Contributor

daveray commented Jun 13, 2014

I've had similar issues. It's hacky, but it would be nice if there was a way to tell fireplace two things:

  • "just trust me, foo.bar is the namespace of the current buffer"
  • "please use exactly this nrepl connection for this buffer"
    I've especially wished for the second option when using checkout dependencies. Rather than just evaluating code with the existing, top-level nrepl connection, vim-fireplace insists on trying to start a new repl for me.

@tpope
Copy link
Owner

tpope commented Jun 13, 2014

I'd say the solution is to tweak the logic to be dependent not on a successful require but on the existence of the namespace after the attempt. How can we do that (being careful not to step on ClojureScript's toes)?

@tpope
Copy link
Owner

tpope commented Jun 13, 2014

tpope/vim-salve#3 discusses the checkouts use case and provides a workaround.

@daveray
Copy link
Contributor

daveray commented Jun 13, 2014

@tpope perfect. g:leiningen_no_auto_repl is really the default behavior I want anyway. thanks!

@trptcolin
Copy link
Contributor Author

I guess the tricky thing about require is that it removes the ns if it wasn't already loaded :frown:. Horrible, horrible hack workaround (most assuredly not cljs-friendly): (#'clojure.core/load-one "koans.01-equalities" true true).

@trptcolin
Copy link
Contributor Author

Just to clarify what I mean: trptcolin@a797eec

If it weren't for cljs, I'd go ahead and do a PR of that, because despite the awfulness of touching a private fn, it gets me exactly what I wanted: it shows an error message, but AFAICT allows fireplace to work after that. Feel free to use that code if you want - at best I'm sure there are vimscript formatting/style issues, just wanted to post it as a strawman.

Not sure how cljs behaves when part of an ns is broken, but maybe there's a way we could fork behavior, and at worst fall back to the existing behavior for cljs? I see a few places checking expand('%:e') ==# 'cljs', so maybe that's the way to go? (I'm actually not sure whether this issue is applicable in cljs; haven't really used cljs in awhile.)

@tpope
Copy link
Owner

tpope commented Jun 20, 2014

Since we're in the context of a repl object a cleaner conditional would be self.user_ns() ==# 'user', but yeah, that's the right idea. Does load-one short circuit if the namespace already exists? We should avoid re-requiring if at all possible.

A cleaner solution might be an nREPL middleware, but this seems woefully specific to Vim, so maybe not worth it.

@tpope
Copy link
Owner

tpope commented Jun 27, 2014

Pushed a potential solution to ignore errors if the ns is created. I need people to try the preload branch and report back. I plan to cut a release next week and it would be nice to include this.

I tried on clojure-koans but the repl spews an error on startup.

@trptcolin
Copy link
Contributor Author

Yeah this works awesome for my issue. The repl errors in clojure-koans are a separate issue, quick-fixed on master & I'm looking into a real fix separately, maybe a lein thing.

@trptcolin
Copy link
Contributor Author

Ah OK, sorry for the noise, but to follow up, the koans error I was seeing was another annoying symptom of technomancy/leiningen#878.

Anyway to reiterate, the preload branch works awesome for me, using that happily now. Thanks!

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

3 participants