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

ns-form sent within cider-eval-defun-at-point seems incomplete #1026

Closed
friemen opened this issue Mar 17, 2015 · 13 comments
Closed

ns-form sent within cider-eval-defun-at-point seems incomplete #1026

friemen opened this issue Mar 17, 2015 · 13 comments

Comments

@friemen
Copy link

friemen commented Mar 17, 2015

ClojureScript project is https://github.com/friemen/zackzack

After starting a REPL using cider-jack-in I start the backend and the weasel+piggieback based browser-connected REPL.

Then I connect with the browser using http://localhost:8080/testindex.html

I load for example src/cljs/zackzack/demo/playground.cljs
I eval code using cider-eval-defun-at-point for example function playground-view.

I receive warnings as shown below and changed code is not loaded/compiled:

; CIDER 0.9.0snapshot (package: 20150316.2241) (Java 1.8.0_40, Clojure 1.6.0, nREPL 0.2.8)
zackzack.backend> (start!)
:started
zackzack.backend> (cljs-repl)
<< Started Weasel server on ws://0.0.0.0:9001 >>
Type `:cljs/quit` to stop the ClojureScript REPL
nil
WARNING: Use of undeclared Var zackzack.demo.playground/convert-text at line 23 /home/riemensc/Workspaces/Clojure/workspace/zackzack/src/cljs/zackzack/demo/playground.cljs
WARNING: Use of undeclared Var zackzack.demo.playground/view at line 19 /home/riemensc/Workspaces/Clojure/workspace/zackzack/src/cljs/zackzack/demo/playground.cljs
WARNING: Use of undeclared Var zackzack.demo.playground/textfield at line 20 /home/riemensc/Workspaces/Clojure/workspace/zackzack/src/cljs/zackzack/demo/playground.cljs
WARNING: Use of undeclared Var zackzack.demo.playground/textfield at line 21 /home/riemensc/Workspaces/Clojure/workspace/zackzack/src/cljs/zackzack/demo/playground.cljs
WARNING: Use of undeclared Var zackzack.demo.playground/button at line 22 /home/riemensc/Workspaces/Clojure/workspace/zackzack/src/cljs/zackzack/demo/playground.cljs

I added (message ns-form) in cider-interaction.el, function cider--dummy-file-contents.
In Messages buffer the line (ns zackzack.demo.playground) appears, but the complete ns-form in cljs file is:

(ns zackzack.demo.playground
  (:require [examine.constraints :as c]
            [examine.core :as e]
            [clojure.string :as string]
            [zackzack.specs :refer [action-link button checkbox column
                                    datepicker panel
                                    selectbox table textfield view]])
  (:require-macros [examine.macros :refer [defvalidator]]
                   [cljs.core.async.macros :refer [go]]))

I suspect the ns-form sent to compiler is incomplete, which causes the warnings.
Around end of November 2014 this worked flawlessly, so the problem appeared very recently.

Oh, and not to forget: I really appreciate all the excellent work that you have done for the Emacs/Clojure community!

@bbatsov
Copy link
Member

bbatsov commented Mar 17, 2015

The contents of *nrepl-messages* would helpful to sort this out.

@friemen
Copy link
Author

friemen commented Mar 17, 2015

Contents of *nrepl-messages* after cider-eval-defun-at-point:

(--->
  op  "load-file"
  session  "f547943f-05e6-4db2-9047-b1fc0ba9fd39"
  file  "(ns zackzack.demo.playground)\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n(defn playground-view\n  []\n  (view \"playground\"\n        :elements [(textfield \"firstname\" :label \"First name\")\n                   (textfield \"lastname\" :label \"Last name\")\n                   (button \"convert\")]\n        :actions {:convert convert-text}))\n"
  file-path  "/home/riemensc/Workspaces/Clojure/workspace/zackzack/src/cljs/zackzack/demo/playground.cljs"
  file-name  "playground.cljs"
  id  "13"
)
(<-
  err  "WARNING: Use of undeclared Var zackzack.demo.playground/view at line 19 /home/riemensc/Workspaces/Clojure/workspace/zackzack/src/cljs/zackzack/demo/playground.cljs\n"
  id  "13"
  session  "f547943f-05e6-4db2-9047-b1fc0ba9fd39"
)
(<-
  err  "WARNING: Use of undeclared Var zackzack.demo.playground/textfield at line 20 /home/riemensc/Workspaces/Clojure/workspace/zackzack/src/cljs/zackzack/demo/playground.cljs\n"
  id  "13"
  session  "f547943f-05e6-4db2-9047-b1fc0ba9fd39"
)
(<-
  err  "WARNING: Use of undeclared Var zackzack.demo.playground/textfield at line 21 /home/riemensc/Workspaces/Clojure/workspace/zackzack/src/cljs/zackzack/demo/playground.cljs\n"
  id  "13"
  session  "f547943f-05e6-4db2-9047-b1fc0ba9fd39"
)
(<-
  err  "WARNING: Use of undeclared Var zackzack.demo.playground/button at line 22 /home/riemensc/Workspaces/Clojure/workspace/zackzack/src/cljs/zackzack/demo/playground.cljs\n"
  id  "13"
  session  "f547943f-05e6-4db2-9047-b1fc0ba9fd39"
)
(<-
  err  "WARNING: Use of undeclared Var zackzack.demo.playground/convert-text at line 23 /home/riemensc/Workspaces/Clojure/workspace/zackzack/src/cljs/zackzack/demo/playground.cljs\n"
  id  "13"
  session  "f547943f-05e6-4db2-9047-b1fc0ba9fd39"
)
(<-
  id  "13"
  session  "f547943f-05e6-4db2-9047-b1fc0ba9fd39"
  value  "nil"
)
(<-
  id  "13"
  session  "f547943f-05e6-4db2-9047-b1fc0ba9fd39"
  status  ("done")
)

@cichli
Copy link
Member

cichli commented Mar 17, 2015

This might be nrepl/weasel#48. Will investigate this evening.

Do you still get the issue if you do (require 'zackzack.demo.playground) in the REPL before evaluating any forms with C-x C-e?

@friemen
Copy link
Author

friemen commented Mar 17, 2015

Yes, problem stays the same:

; CIDER 0.9.0snapshot (package: 20150316.2241) (Java 1.8.0_40, Clojure 1.6.0, nREPL 0.2.8)
zackzack.backend> (start!)
:started
zackzack.backend> (cljs-repl)
<< Started Weasel server on ws://0.0.0.0:9001 >>
Type `:cljs/quit` to stop the ClojureScript REPL
nil
cljs.user> 
zackzack.demo.playground> (require 'zackzack.demo.playground)
zackzack.demo.playground
WARNING: Use of undeclared Var zackzack.demo.playground/view at line 3 <cljs repl>
WARNING: Use of undeclared Var zackzack.demo.playground/textfield at line 4 <cljs repl>
WARNING: Use of undeclared Var zackzack.demo.playground/textfield at line 5 <cljs repl>
WARNING: Use of undeclared Var zackzack.demo.playground/button at line 6 <cljs repl>
WARNING: Use of undeclared Var zackzack.demo.playground/convert-text at line 7 <cljs repl>
#<function zackzack$demo$playground$playground_view(){
return zackzack.demo.playground.view.call(null,"playground",new cljs.core.Keyword(null,"elements","elements",657646735),new cljs.core.PersistentVector(null, 3, 5, cljs.core.PersistentVector.EMPTY_NODE, [zackzack.demo.playground.textfield.call(null,"firstname",new cljs.core.Keyword(null,"label","label",1718410804),"First name"),zackzack.demo.playground.textfield.call(null,"lastname",new cljs.core.Keyword(null,"label","label",1718410804),"Last name"),zackzack.demo.playground.button.call(null,"convert")], null),new cljs.core.Keyword(null,"actions","actions",-812656882),new cljs.core.PersistentArrayMap(null, 1, [new cljs.core.Keyword(null,"convert","convert",912478626),zackzack.demo.playground.convert_text], null));
}>

@cichli
Copy link
Member

cichli commented Mar 17, 2015

I managed to replicate with zackzack master and can confirm that modifying cider--dummy-file-contents to include the full, rather than truncated, ns form solves the issue at hand.

@bbatsov what's the reasoning behind the truncation here?

@cichli
Copy link
Member

cichli commented Mar 17, 2015

@swannodette quick question - feel free to ignore surrounding context of the CIDER issue - should repeated applications of ns forms be additive in ClojureScript REPLs, like in Clojure REPLs?

For example, in a normal Clojure REPL I can do:

user=> (ns my-test-ns.core (:require [clojure.string :as string]))
nil
my-test-ns.core=> (def a string/blank?)
#'my-test-ns.core/a
my-test-ns.core=> (ns my-test-ns.core)
nil
my-test-ns.core=> (def a string/blank?)
#'my-test-ns.core/a
my-test-ns.core=> 

But in a Node REPL (using quick start guide and rlwrap java -cp cljs.jar:src clojure.main node_repl.clj):

ClojureScript:cljs.user> (ns my-test-ns.core (:require [clojure.string :as string]))
true
ClojureScript:my-test-ns.core> (def a string/blank?)
#<function clojure$string$blank_QMARK_(s){
return goog.string.isEmptySafe(s);
}>
ClojureScript:my-test-ns.core> (ns my-test-ns.core)
nil
ClojureScript:my-test-ns.core> (def a string/blank?)
WARNING: No such namespace: string, could not locate string.cljs at line 1 <cljs repl>
WARNING: Use of undeclared Var string/blank? at line 1 <cljs repl>
repl:13
throw e__3919__auto__;
      ^
ReferenceError: string is not defined
    at repl:1:109
    at repl:9:3
    at repl:14:4
    at Object.exports.runInThisContext (vm.js:74:17)
    at Domain.<anonymous> ([stdin]:41:34)
    at Domain.run (domain.js:197:16)
    at Socket.<anonymous> ([stdin]:40:25)
    at Socket.emit (events.js:107:17)
    at readableAddChunk (_stream_readable.js:163:16)
    at Socket.Readable.push (_stream_readable.js:126:10)
ClojureScript:my-test-ns.core> 

@swannodette
Copy link

@cichli probably best to simulate the Clojure behavior if we can. Feel free to open a JIRA issue. Thanks.

@cichli
Copy link
Member

cichli commented Mar 17, 2015

@swannodette thanks for the quick response, logged as CLJS-1139.

@bbatsov
Copy link
Member

bbatsov commented Mar 18, 2015

@cichli Ah, another difference between Clojure and ClojureScript that I wasn't aware of. The current solution you proposed is sound.

@vspinu
Copy link
Contributor

vspinu commented Mar 18, 2015

@cichili what's the reasoning behind the truncation here?

Clojure script is very slow on evaluation of full ns. So we are loading full ns once and then assuming that (ns foo) is fast and evaluation is additive.

See the very long #830 for reasons and #956 for where the change was made.

@bbatsov
Copy link
Member

bbatsov commented Mar 18, 2015

Clojure script is very slow on evaluation of full ns. So we are loading full ns once and then assuming that (ns foo) is fast and evaluation is additive.

That's not the actual reason for the truncation - that's the reason for the caching. The reason for the truncation is that if the ns hasn't changed since it was cached we don't need the whole form.

@vspinu
Copy link
Contributor

vspinu commented Mar 18, 2015

That's not the actual reason for the truncation - that's the reason for the caching. The reason for the truncation is that if the ns hasn't changed since it was cached we don't need the whole form.

... and we don't use the whole form in order to speed up CS. As far as I recall there were no other reasons not to use the full form. Before we were perfectly happy with evaluating full ns every time.

cichli added a commit to cichli/cider that referenced this issue Mar 19, 2015
cichli added a commit to cichli/cider that referenced this issue Mar 19, 2015
cichli added a commit to cichli/cider that referenced this issue Mar 19, 2015
cichli added a commit to cichli/cider that referenced this issue Mar 19, 2015
@friemen
Copy link
Author

friemen commented Mar 20, 2015

Thanks!
cider-eval-defun-at-point works now as expected.

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

Successfully merging a pull request may close this issue.

5 participants