dundalek/closh

Some concerns around ~/.closhrc

Closed this issue · 5 comments

Issue #91 notified me of the fact that closhrc is evaluated in the namespace clojure.core. The error in #91 felt a bit counterintuitive (not user, but clojure.core as the namespace), but more importantly while trying to reproduce this I found more serious issues with this setup. The first two are just UX related, but the last one can give serious problems without ever knowing why.

This is what I found:

The good

~/.closhrc

(throw (ex-info "something wrong" {}))

You get an error that is easy to find and solve

Error while loading init file ~/.closhrc:
 #error {
 :cause something wrong
 :data {}
 :via
 [{:type clojure.lang.Compiler$CompilerException
   :message clojure.lang.ExceptionInfo: something wrong {}, compiling:(/Users/jeroen/.closhrc:1:1)
   :at [clojure.lang.Compiler load Compiler.java 7526]}

The bad

~/.closhr

(defn closh-prompt []
  (/ 1 0)

It is unclear (without a background) where this error is coming from

➜  closh git:(master) ✗ closh
ArithmeticException Divide by zero  clojure.lang.Numbers.divide (Numbers.java:163)
ArithmeticException Divide by zero  clojure.lang.Numbers.divide (Numbers.java:163)
...

Same for

~/.closhr

(defn closh-prompt []
  (throw (ex-info "foo" {})))

The error

➜  closh git:(master) ✗ closh
ExceptionInfo foo  clojure.core/ex-info (core.clj:4739)
ExceptionInfo foo  clojure.core/ex-info (core.clj:4739)
...

Proposed solution:

Wrap closh-prompt in a try/catch, print a warning (with mention of ~/.closhrc) and fallback to default ("$)

The ugly

~/.closhr

(defn get [m k]
  (println "bar"))

There is no error, but clojure.core/get has been overriden and who knows what will happen...

➜  closh git:(master) ✗ closh
bar
bar
bar
bar
bar
bar
bar
bar
bar
bar
bar
bar
bar
$

Proposed solution:

Not sure, as any namespace can override the clojure.core/get if :refer-exclude isn't used, see below. I'm not sure if there is a way to solve this other than completely seperating the classpath of closh from a user-code.

~/.closhr

(let [old-ns *ns*]
  (ns foo.bar)
  
  (println *ns*)
  
  (defn get [m k]
    (println "even from foo.bar"))
  
  ;; Going back
  (in-ns (symbol (str *ns*))))

Same as before

➜  ~ closh
#object[clojure.lang.Namespace 0x325f7fa9 foo.bar]
even from foo.bar
even from foo.bar
even from foo.bar
even from foo.bar
even from foo.bar
even from foo.bar
even from foo.bar
even from foo.bar
even from foo.bar
even from foo.bar
even from foo.bar
even from foo.bar
even from foo.bar
$

Agree with your proposed solution to wrap it in try/catch, I should be more considerate of catching the potential errors from now on.

For the ugly part it seems that just evaling things in user namespace should solve it. With the prompt fix it seems a symptom was fixed but the underlying problem remains. It is strange that switching the namespace explicitly does nothing.

Also if I put (println *ns*) into ~/.closhrc it prints #object[clojure.lang.Namespace 0x34451ed8 user]. So the user ns seems to be set, but is not taken into consideration. I am going to investigate more.

Ah yeah I think you are right that the user namespace would solve the issue:

➜  ~ lein repl
nREPL server started on port 51529 on host 127.0.0.1 - nrepl://127.0.0.1:51529
REPL-y 0.3.7, nREPL 0.2.12
Clojure 1.8.0
Java HotSpot(TM) 64-Bit Server VM 1.8.0_141-b15
    Docs: (doc function-name-here)
          (find-doc "part-of-name-here")
  Source: (source function-name-here)
 Javadoc: (javadoc java-object-or-class-here)
    Exit: Control+D or (exit) or (quit)
 Results: Stored in vars *1, *2, *3, an exception in *e

user=> (defn get [m k] 1)
WARNING: get already refers to: #'clojure.core/get in namespace: user, being replaced by: #'user/get
#'user/get
user=> (clojure.core/get {:a 2} :a)
2
user=> (get {:a 2} :a)
1

It would just be a local issue, nothing really unexpected

@jeroenvandijk This should be fixed in 8ec940a. Could you please verify on your machine?

A bit late (holidays), but I can confirm this works on my machine too! The new error message is helpful too. Thanks!

Cool, thanks 👍