<< Back to previous view

[CLJS-1008] Browser repl doesn't find upstream dependencies. Created: 05/Feb/15  Updated: 07/Feb/15  Resolved: 07/Feb/15

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Brian Jenkins Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: repl
Environment:

MacOS 10.9.5
Running a repl through cider & emacs.

project.clj attached.


Attachments: Text File fix-upstream-deps-from-repl.patch     File project.clj    
Patch: Code

 Description   

I found that when running piggieback from the repl like this:

```clojure
(defn piggieback-repl []
(cemerick.piggieback/cljs-repl :repl-env (cljs.repl.browser/repl-env :port 9000)) )
```

get-upstream-deps doesn't find anything.

This prevents me from being able to use piggieback with Om's new cljsjs dependency on React.

Comment on get-upstream-deps says

```
Should be run in the main thread. If
not, pass (java.lang.ClassLoader/getSystemClassLoader) to use the
system classloader.
```

I guess the repl isn't the main thread?



 Comments   
Comment by David Nolen [ 07/Feb/15 10:05 PM ]

fix https://github.com/clojure/clojurescript/commit/f311ebd121bf2385efcfba6f7bfb07251cf812f1





[CLJS-1006] Implicit dependency of clojure.browser.repl on cljs.repl Created: 05/Feb/15  Updated: 05/Feb/15  Resolved: 05/Feb/15

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Murphy McMahon Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: repl

Attachments: Text File 1006.patch    

 Description   

The clojure.browser.repl/connect function monkey patches goog.require with :optimizations :none. The substituted version uses goog.basePath-relative URLs to retrieve assets, one of which is cljs.repl (the repl-connection callback returns "goog.require('cljs.repl')"). Therefore clojure.browser.repl has an implicit dependency on cljs.repl, which will not be able to be located if the :output-dir of application compilation (eg cljsbuild) and that passed to cljs.repl/repl* are not one in the same.



 Comments   
Comment by David Nolen [ 05/Feb/15 7:13 AM ]

fixed https://github.com/clojure/clojurescript/commit/576fd1c70933fc91180c769cb639c262b7c79071





[CLJS-1005] Browser REPL creates "out" directory no matter what Created: 05/Feb/15  Updated: 05/Feb/15  Resolved: 05/Feb/15

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Murphy McMahon Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: repl

Attachments: Text File 1005.patch    

 Description   

Steps to reproduce:

$ lein new mies test-brepl
$ cd test-brepl
$ sed -i 's/:output-dir "out"/:output-dir "not-out"/' scripts/brepl.clj
$ ./scripts/brepl # <Ctrl-c> <Ctrl-c>
$ ls -la

Expected: Only "not-out" and ".repl-0.0-XXXX" directories should be created for compiled javascript.

Actual: "out" directory is also created.



 Comments   
Comment by David Nolen [ 05/Feb/15 7:13 AM ]

fixed https://github.com/clojure/clojurescript/commit/4bd3e6563d36871b1c5e603c35fc7bb4a9b68eb2





[CLJS-887] cljs.repl.browser does not serve css by default Created: 17/Nov/14  Updated: 12/Feb/15  Resolved: 12/Feb/15

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Christopher Shea Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: repl
Environment:

*


Attachments: File cljs-browser-repl-serve-css.diff    
Patch: Code

 Description   

A browser repl's server serves js, cljs, map, and html files by default, but not css.



 Comments   
Comment by John Chijioke [ 10/Dec/14 2:05 AM ]

What is css needed for from the repl?

Comment by Christopher Shea [ 10/Dec/14 9:13 AM ]

cljs.repl.browser/send-static can already send css with the appropriate MIME type, so this just seems like a minor oversight.

If you follow the Using the browser as an Evaluation Environment section of the ClojureScript wiki and have a css file linked from your html page, it will not be served, which can make it more difficult to work on your project (you won't see the effects of changing an element's class, e.g.).

As a workaround, I've been doing this when setting up my repl:

(server/dispatch-on :get
  (fn [{:keys [path]} _ _] (.endsWith path ".css"))
  browser/send-static)

It's not that my interactions with the repl require the css, it's the browser that's connected to the repl that does.

Comment by David Nolen [ 12/Feb/15 10:41 AM ]

fixed https://github.com/clojure/clojurescript/commit/43d0bb1e99aa2f23f1e7cf5004d347b6e6a70ff0





[CLJS-749] Changing ClojureScript versions may break browser REPL Created: 14/Jan/14  Updated: 02/Dec/14  Resolved: 02/Dec/14

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Osbert Feng Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: repl

Attachments: Text File cljs-749.patch    
Patch: Code

 Description   

By default in a project using ClojureScript, the .repl/ directory is used to store compiled CLJS for the browser REPL. This compilation is skipped if the directory already exists (src/clj/cljs/repl/browser.clj:205:create-client-js-file). However, it really should be re-compiled if the version of ClojureScript is upgraded/changed or else the browser REPL may fail with some very difficult to interpret error messages. At the moment, it seems people simply know to delete .repl/ when changing ClojureScript versions (https://groups.google.com/forum/#!topic/clojure/C-H4gSWIUwc) but this can be extremely tough on new users when they upgrade ClojureScript for the first time.

We could append clojurescript-version to the directory name. Unfortunate that it generates a new directory each time a new version of CLJS/BrowserREPL combo is used, but shoudl not occur too often and makes it very explicit that :working-dir should be tied to CLJS version. Also developers utilizing ClojureScript though lein checkouts will still have to delete .repl/ since clojurescript-verion is only set by script/build.

See attached patch.

NOTE: I do not have a CA agreement on file, but one is in the mail.

NOTE: Sorry if this is bad form, but as a preceding commit, in cases where clojurescript-version is unbound I changed (clojurescript-version) to return "" instead of ". This is so that when clojurescript-version is unbound .repl/ will be used instead of .repl-./ Let me know if this should be considered as a separate issue.

Alternatively, we could remove the exists check entirely, and instead recompile client.js every time (repl-env) is called at the cost of slowing down browser REPL startup.



 Comments   
Comment by David Nolen [ 02/Dec/14 5:48 AM ]

Seems like a fine fix but this patch needs to be rebased to master. Thanks.

Comment by Osbert Feng [ 02/Dec/14 12:42 PM ]

Please see the updated patch. Thanks!

Comment by David Nolen [ 02/Dec/14 12:48 PM ]

Fixed https://github.com/clojure/clojurescript/commit/50cc86ff3c4c39181a198a4f9be788c149eaae00
https://github.com/clojure/clojurescript/commit/85773301cf12541a053890643d1d943a6ed361de
https://github.com/clojure/clojurescript/commit/d25aea69697cca2ef5fa8b9a6f7e4fd089685ace

Comment by David Nolen [ 02/Dec/14 12:49 PM ]

In the future squashed patches are preferred. Thanks for your contribution!





[CLJ-1673] Improve clojure.repl/dir-fn to work on namespace aliases in addition to canonical namespaces. Created: 11/Mar/15  Updated: 11/Mar/15

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.7
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Jason Whitlark Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: repl

Attachments: Text File improve_dir.patch    
Patch: Code
Approval: Triaged

 Description   

Extend clojure.repl/dir to work with the aliases in the current namespace

Patch: improve_dir.patch

Question: This does introduce a dependency on *ns*, but only for looking up aliases. If no alias is found, behavior is the same as the current implementation.



 Comments   
Comment by Jason Whitlark [ 11/Mar/15 4:00 PM ]

Possible unit test, since clojure.string is aliased in the test file:

(is (= (dir-fn 'clojure.string) (dir-fn 'str)))





[CLJ-1671] Clojure stream socket repl Created: 09/Mar/15  Updated: 26/Mar/15

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.7
Fix Version/s: Release 1.7

Type: Enhancement Priority: Major
Reporter: Alex Miller Assignee: Stuart Halloway
Resolution: Unresolved Votes: 0
Labels: repl

Attachments: Text File clj-1671-2.patch     Text File clj-1671-3.patch     Text File clj-1671-4.patch    
Patch: Code
Approval: Incomplete

 Description   

Programs often want to provide REPLs to users in contexts when a) network communication is desired, b) capturing stdio is difficult, or c) when more than one REPL session is desired. In addition, tools that want to support REPLs and simultaneous conversations with the host are difficult with a single stdio REPL as currently provided by Clojure.

The goal is to provide a simple streaming socket repl as part of Clojure. The socket repl should support both program-to-program communication (by exchanging data) and human-readable printing (which mirrors current behavior). The socket repl server will be started only when supplying a port to clojure.main or by explicitly starting it by calling the provided function.

Each socket connection will be given its own repl context and unique starting user namespace (user, user1, user2, etc) with separate repl stack and proper bindings. On session termination, the namespace will be removed. *in* and *out* will be bound to incoming and outgoing streams. Tools can communicate with the runtime while also providing a user repl by opening two connections to the same server.

There are two known cases where the repl interprets non-readable objects and prints them for human consumption in a non-readable way: objects with no specific print-method and when handling Throwables. Both cases (Object and Throwable) now have a print-method implementation to return a tagged-literal representation. By default the socket repl will print exceptions with the print-method data form. Optionally, the user can set a custom repl exception printer. A function that provides the current human readable exception printing will be provided.

Problems:

  1. Socket server to accept connections and serve a repl to a client
    • Runtime configuration via data
  2. Client repl sessions should be independent
    • Separate user namespace
    • Separate bindings
    • Namespace removed on client shutdown
    • Communicate solely via data for program-to-program communication
  3. Stdio has both out and err streams but socket has only single out
  4. Repls should be nestable
    • Repl within a repl binds streams appropriately
    • Means of control (exit)

Features:

  1. Printing as data
    • Of object without print-method: as #object
    • Of Throwable: as #error
  2. Start socket server from command line
    • Configure: host, port, whether to prompt, error printing
  3. Start socket server programmatically
    • Configure: host, port, whether to prompt, error printing, whether to bind err to out
    • Control: close returned socket server to stop listening
  4. Start stdio repl from command line
    • Configure: whether to prompt, error printing
  5. On socket client accept
    • Create new user namespace
    • Bind error printer according to server config
  6. In socket client repl
    • Bind new error printer function
  7. On socket client disconnect
    • Remove user namespace
    • Close socket

Impl notes: This adds a new -s option to clojure.main that will start a socket server listening on a given host:port. Each client is given a new userN namespace (starting from user1). It binds *in*, *out*, and *err*. Each client connection consumes a daemon thread named "Socket REPL Client N" (matching the user namespace). On client disconnect, the user namespace is removed and thread will die.

There are two system properties that can be used to control whether the prompt and the default error printer:

  • clojure.repl.socket.prompt (default=false) - whether to print prompts
  • clojure.repl.socket.err-printer (default=clojure.main/err->map) - function to format exceptions

The existing stdio repl behaves the same as before, but it's behavior can be influenced by two new similar system properties:

  • clojure.repl.stdio.prompt (default=true)
  • clojure.repl.socket.err-printer (default=clojure.main/err-print)

You can also start a socket repl server programmatically (shown here with all kwarg options - pick the ones you need):

(def ss 
  (clojure.main/socket-repl-server 
    :host "localhost"                   ;; default=<loopback>, accepts InetAddress or String
    :port 5555                          ;; default=0 (ephemeral)
    :use-prompt true                    ;; default=false
    :bind-err true                      ;; default=true, to bind \*err* to \*out*
    :err-printer clojure.main/err->map  ;; default=clojure.main/err->map, print Throwable to \*out*
    ))

The server socket is returned. Closing it will stop listening on the port (existing client connections will still be alive).

If you want to test with the server port, telnet makes a great client:

;; term 1:
$ java -cp target/classes -Dclojure.repl.socket.prompt=true clojure.main -s 127.0.0.1:5555

;; term 2:
$ telnet 127.0.0.1 5555
Trying 127.0.0.1...
Connected to localhost.
Escape character is '^]'.
user1=> (+ 1 1)
2
user1=> (/ 1 0)
#error {:cause "Divide by zero",
 :via
 [{:type java.lang.ArithmeticException,
   :message "Divide by zero",
   :at [clojure.lang.Numbers divide "Numbers.java" 158]}],
 :trace
 [[clojure.lang.Numbers divide "Numbers.java" 158]
  [clojure.lang.Numbers divide "Numbers.java" 3808]
  [user1$eval1 invoke "NO_SOURCE_FILE" 1]
  [clojure.lang.Compiler eval "Compiler.java" 6784]
  [clojure.lang.Compiler eval "Compiler.java" 6747]
  [clojure.core$eval invoke "core.clj" 3078]
  [clojure.main$repl$read_eval_print__8287$fn__8290 invoke "main.clj" 265]
  [clojure.main$repl$read_eval_print__8287 invoke "main.clj" 265]
  [clojure.main$repl$fn__8296 invoke "main.clj" 283]
  [clojure.main$repl doInvoke "main.clj" 283]
  [clojure.lang.RestFn invoke "RestFn.java" 619]
  [clojure.main$socket_repl_server$fn__8342$fn__8344 invoke "main.clj" 450]
  [clojure.lang.AFn run "AFn.java" 22]
  [java.lang.Thread run "Thread.java" 724]]}
user1=> (println "hello")
hello
nil

A dynamic var clojure.main/*err-printer* is provided to customize printing of exceptions. It's bound by :err-printer if invoked programmatically or the system property if started from the command line, but it can be dynamically rebound during the session if desired:

user1=> (set! clojure.main/*err-printer* clojure.main/err-print)
#object[clojure.main$err_print 0x317bee01 "clojure.main$err_print@317bee01"]
user1=> (/ 1 0)
ArithmeticException Divide by zero  clojure.lang.Numbers.divide (Numbers.java:158)

Patch: clj-1671-4.patch



 Comments   
Comment by Timothy Baldridge [ 09/Mar/15 5:50 PM ]

Could we perhaps keep this as a contrib library? This ticket simply states "The goal is to provide a simple streaming socket repl as part of Clojure." What is the rationale for the "part of Clojure" bit?

Comment by Alex Miller [ 09/Mar/15 7:33 PM ]

We want this to be available as a Clojure.main option. It's all additive - why wouldn't you want it in the box?

Comment by Timothy Baldridge [ 09/Mar/15 10:19 PM ]

It never has really been too clear to me why some features are included in core, while others are kept in contrib. I understand that some are simply for historical reasons, but aside from that there doesn't seem to be too much of a philosophy behind it.

However it should be noted that since patches to clojure are much more guarded it's sometimes nice to have certain features in contrib, that way they can evolve with more rapidity than the one release a year that clojure has been going through.

But aside from those issues, I've found that breaking functionality into modules forces the core of a system to become more configurable. Perhaps I would like to use this repl socket feature, but pipe the data over a different communication protocol, or through a different serializer. If this feature were to be coded as a contrib library it would expose extension points that others could use to add additional functionality.

So I guess, all that to say, I'd prefer a tool I can compose rather than a pre-built solution.

Comment by Rich Hickey [ 10/Mar/15 6:25 AM ]

Please move discussions on the merits of the idea to the dev list. Comments should be about the work of resolving the ticket, approach taken by the patch, quality/perf issues etc.

Comment by Colin Jones [ 11/Mar/15 1:33 PM ]

I see that context (a) of the rationale is that network communication is desired, which sounds to me like users of this feature may want to communicate across hosts (whether in VMs or otherwise). Is that the case?

If so, it seems like specifying the address to bind to (e.g. "0.0.0.0", "::", "127.0.0.1", etc.) may become important as well as the existing port option. This way, someone who wants to communicate across hosts (or conversely, lock down access to local-only) can make that decision.

Comment by Alex Miller [ 11/Mar/15 2:07 PM ]

Colin - agreed. There are many ways to potentially customize what's in there so we need to figure out what's worth doing, both in the function and via the command line.

I think address is clearly worth having via the function and possibly in the command line too.

Comment by Kevin Downey [ 11/Mar/15 5:49 PM ]

I find the exception printing behavior really odd. for a machine you want an exception as data, but you also want some indication of if the data is an error or not, for a human you wanted a pretty printed stacktrace. making the socket repl default to printing errors this way seems to optimize for neither.

Comment by Rich Hickey [ 12/Mar/15 12:29 PM ]

Did you miss the #error tag? That indicates the data is an error. It is likely we will pprint the error data, making it not bad for both purposes

Comment by Alex Miller [ 13/Mar/15 11:29 AM ]

New -4 patch changes:

  • clojure.core/throwable-as-map now public and named clojure.core/Throwable->map
  • catch and ignore SocketException without printing in socket server repl (for client disconnect)
  • functions to print as message and as data are now: clojure.main/err-print and clojure.main/err->map. All defaults and docs updated.
Comment by David Nolen [ 18/Mar/15 12:44 PM ]

Is there any reason to not allow supplying :eval in addition to :use-prompt? In the case of projects like ClojureCLR + Unity eval generally must happen on the main thread. With :eval as something which can be configured, REPL sessions can queue forms to be eval'ed with the needed context (current ns etc.) to the main thread.

Comment by Kevin Downey [ 20/Mar/15 2:12 PM ]

I did see the #error tag, but throwables print with that tag regardless of if they are actually thrown or if they are just the value returned from a function. Admittedly returning exceptions as values is not something generally done, but the jvm does distinguish between a return value and a thrown exception. Having a repl that doesn't distinguish between the two strikes me as an odd design. The repl you get from clojure.main currently prints the message from a thrown uncaught throwable, and on master prints with #error if you have a throwable value, so it distinguishes between an uncaught thrown throwable and a throwable value. That obviously isn't great for tooling because you don't get a good data representation in the uncaught case.

It looks like the most recent patch does pretty print uncaught throwables, which is helpful for humans to distinguish between a returned value and an uncaught throwable.

Comment by Kevin Downey [ 25/Mar/15 1:10 PM ]

alex: saying this is all additive, when it has driven changes to how things are printed, using the global print-method, rings false to me

Comment by Sam Ritchie [ 25/Mar/15 1:15 PM ]

This seems like a pretty big last minute addition for 1.7. What's the rationale for adding it here vs deferring to 1.8, or trying it out as a contrib first?

Comment by Alex Miller [ 25/Mar/15 2:13 PM ]

Kevin: changing the fallthrough printing for things that are unreadable to be readable should be useful regardless of the socket repl. It shouldn't be a change for existing programs (unless they're relying on the toString of objects without print formats).

Sam: Rich wants it in the box as a substrate for tools.

Comment by Alex Miller [ 26/Mar/15 10:03 AM ]

Marking incomplete, pending at least the repl exit question.





[CLJ-1389] Re-loading a namespace ignores metadata specified for the namespace Created: 20/Mar/14  Updated: 20/Mar/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Howard Lewis Ship Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: metadata, namespace, repl


 Description   

Using the REPL I added some metadata to a namespace and reloaded it.

(ns io.aviso.rook-test5)

to

(ns io.aviso.rook-test5
  "A testing namespace"
  {:inherted   :namespace
   :overridden :namespace})

But requesting the meta data yields nil:

(-> 'io.aviso.rook-test5 find-ns meta)
=> nil

I have tested a few variations, such as putting the metadata on the symbol instead of providing an attribute map. In all cases, the metadata from before the load persists.

Using remove-ns before re-loading the namespace does the right thing ... the metadata shows up as expected.






[CLJ-1358] doc macro does not expand special cases properly Created: 17/Feb/14  Updated: 17/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.6
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Chad Taylor Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: repl

Attachments: Text File CLJ-1358.patch    
Patch: Code and Test

 Description   

The doc macro supports three special cases, mapping & to fn, catch to try, and finally to try. However, the macro does not currently expand these cases - it executes them like a function instead. This is evident if you use the following at a REPL:

user> (macroexpand '(doc try))   ;; ok
((var clojure.repl/print-doc) ((var clojure.repl/special-doc) (quote try)))

user> (macroexpand '(doc catch)) ;; broken
;; -- unexpectedly prints try doc -- ;;
nil

user> (= (with-out-str (doc catch)) (with-out-str (doc try))) ;; broken, expect true
;; -- unexpectedly prints try doc -- ;;
false

Workaround: Call doc with the symbol to which the special case is mapped, fn or try.

Cause: Incorrect quoting when handling special cases in doc macro

Solution: Update special case quoting approach to match the other cases.

Patch: CLJ-1358.patch



 Comments   
Comment by Chad Taylor [ 17/Feb/14 10:41 PM ]

Adding a patch with code and test.





[CLJ-1247] Document the availability/usage of *e, *1, *2, ... in REPL Created: 24/Aug/13  Updated: 29/Aug/13  Resolved: 24/Aug/13

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Jakub Holy Assignee: Alex Miller
Resolution: Completed Votes: 0
Labels: documentation, repl


 Description   

For new users of Clojure, it is very hard to find out that evaluation results in any Clojure REPL are bound to *1 - *3 and the latest exception to *e. Since it is a pretty useful feature, it should be documented at a visible place. Where that place is, I am not sure.

One possibility would be to add it to the docstring of clojure.main/repl and make http://clojure.org/repl_and_main link to it (i.e. in the "Launching a REPL" section, we could add something like "Read the <link to=somewhere>docstring of clojure.main/repl</link> to learn about options and available vars. See also utility functions/macros in the clojure.repl namespace."

Note: This was originally reported under nREPL in NREPL-43.



 Comments   
Comment by Alex Miller [ 24/Aug/13 2:52 PM ]

I updated the http://clojure.org/repl_and_main page to include much of this info.

Comment by Jakub Holy [ 29/Aug/13 3:16 AM ]

Lovely, thanks!





[CLJ-1191] Improve apropos to show some indication of namespace of symbols found Created: 04/Apr/13  Updated: 29/Aug/14  Resolved: 29/Aug/14

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5, Release 1.6
Fix Version/s: Release 1.7

Type: Enhancement Priority: Minor
Reporter: Andy Fingerhut Assignee: Unassigned
Resolution: Completed Votes: 1
Labels: repl

Attachments: Text File clj-1191-patch-v1.txt     Text File clj-1191-patch-v2.txt     Text File clj-1191-v3.patch    
Patch: Code and Test
Approval: Ok

 Description   

apropos does find all symbols in all namespaces that match the argument, but the return value gives no clue as to which namespace the found symbols are in. It can even return multiple occurrences of the same symbol, which only gives a clue that the symbol exists in more than one namespace, but not which ones. For example:

user=> (apropos "replace")
(postwalk-replace prewalk-replace replace re-quote-replacement replace replace-first)

user=> (apropos 'macro)
(macroexpand-all macroexpand macroexpand-1 defmacro)

It would be nice if the returned symbols could indicate the namespace.

With the screened patch clj-1191-v3.patch applied the output for the examples above becomes:

user=> (apropos "replace")
(clojure.core/replace clojure.string/re-quote-replacement clojure.string/replace clojure.string/replace-first clojure.walk/postwalk-replace clojure.walk/prewalk-replace)

user=> (apropos 'macro)
(clojure.core/defmacro clojure.core/macroexpand clojure.core/macroexpand-1 clojure.walk/macroexpand-all)

Patch: clj-1191-v3.patch

Screened by: Alex Miller



 Comments   
Comment by Andy Fingerhut [ 04/Apr/13 8:25 PM ]

Path clj-1191-patch-v1.txt enhances apropos to put a namespace/ qualifier before every symbol found that is not in the current namespace ns. It also finds the shortest namespace alias if there is more than one. Examples of output with patch:

user=> (apropos "replace")
(replace clojure.string/re-quote-replacement clojure.string/replace clojure.string/replace-first clojure.walk/postwalk-replace clojure.walk/prewalk-replace)

user=> (require '[clojure.string :as str])
nil
user=> (apropos "replace")
(replace clojure.walk/postwalk-replace clojure.walk/prewalk-replace str/re-quote-replacement str/replace str/replace-first)

user=> (in-ns 'clojure.string)
#<Namespace clojure.string>
clojure.string=> (clojure.repl/apropos "replace")
(re-quote-replacement replace replace-by replace-first replace-first-by replace-first-char replace-first-str clojure.core/replace clojure.walk/postwalk-replace clojure.walk/prewalk-replace)

Comment by Colin Jones [ 05/Apr/13 1:34 PM ]

+1

apropos as it already stands is quite helpful for already-referred vars, but not for vars that are only in other nses.

This update includes the information someone would need to further investigate the output.

Comment by Alex Miller [ 20/Aug/14 11:22 AM ]

If you have "use"d many namespaces (which is not uncommon at the repl), this updated apropos still doesn't help you understand where a particular function is coming from (as the ns will be omitted). It's cool that this patch is "unresolving" and finding the shortest-alias etc but I think it's actually doing too much. In my opinion, simply providing the full namespace for all matches would actually be more useful (and easier).

Comment by Andy Fingerhut [ 20/Aug/14 12:27 PM ]

Patch clj-1191-patch-v2.txt dated Aug 20 2014 modifies apropos so that every symbol returned has a full namespace qualifier, even if it is in clojure.core. Before this patch:

user=> (apropos "replace")
(prewalk-replace postwalk-replace replace replace-first re-quote-replacement replace)

user=> (apropos 'macro)
(macroexpand-all macroexpand macroexpand-1 defmacro)

After this patch:

user=> (apropos "replace")
(clojure.core/replace clojure.string/re-quote-replacement clojure.string/replace clojure.string/replace-first clojure.walk/postwalk-replace clojure.walk/prewalk-replace)

user=> (apropos 'macro)
(clojure.core/defmacro clojure.core/macroexpand clojure.core/macroexpand-1 clojure.walk/macroexpand-all)
Comment by Alex Miller [ 20/Aug/14 1:34 PM ]

Some comments on the code itself:

1) I don't think we should do anything special for ns - there are plenty of ways to search your current ns. I think it unnecessarily adds a lot of complexity without enough value.
2) Rather than finding vars and work back to syms, I think this should instead retain the ns context as it walks the ns-publics keys so that you can easily reassemble a fully-qualified symbol name.
3) Why do you need the set at the end? Seems like symbols should already be unique at this point?

Comment by Andy Fingerhut [ 20/Aug/14 5:02 PM ]

Patch clj-1191-patch-v3.txt attempts to address Alex Miller's comments on the v2 patch.

Perhaps the diff will get down to a 1-line change

Comment by Andy Fingerhut [ 20/Aug/14 5:05 PM ]

Patch clj-1191-v3.patch is identical to clj-1191-patch-v3.txt mentioned in the previous comment, but conforms to the requested .patch or .diff file name ending.





[CLJ-1176] clojure.repl/source fails when *read-eval* bound to :unknown Created: 06/Mar/13  Updated: 31/Jan/14  Resolved: 31/Jan/14

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: Release 1.6

Type: Defect Priority: Minor
Reporter: Tim McCormack Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: repl

Attachments: Text File 0001-CLJ-1176-Bind-read-eval-true-in-clojure.repl-source-.patch     Text File clj-1176-source-read-eval-2.patch     Text File clj-1176-source-read-eval-3.patch    
Patch: Code and Test
Approval: Ok

 Description   

clojure.repl/source is broken in Clojure 1.5.1 when *read-eval* is bound to :unknown, since source-fn reads without binding.

user> (alter-var-root #'*read-eval* (constantly :unknown))
:unknown
user> (source drop-last)
RuntimeException Reading disallowed - *read-eval* bound to :unknown  clojure.lang.Util.runtimeException (Util.java:219)

Approach: Throw explicit error stating the cause in this case.

Patch: clj-1176-source-read-eval-3.patch

Screened by: Stuart Sierra



 Comments   
Comment by Tim McCormack [ 06/Mar/13 4:04 PM ]

The attached patch just binds *read-eval* to true inside source-fn.

Comment by Stuart Halloway [ 29/Mar/13 6:24 AM ]

Note: Allowing this implies that you trust the data on your classpath. If there are reasons somebody might not, we should reject this patch and people will have to be explicit when calling source.

Comment by Tim McCormack [ 29/Mar/13 6:37 AM ]

Ugh, that's a fair point when it comes to sandboxing. I'll check with the owners of clojurebot and lazybot.

Comment by Tim McCormack [ 04/May/13 10:40 PM ]

I haven't come up with any scenarios where this is problematic, and I haven't heard anything back from the bot owners. As for sandboxing, clojure.repl can easily be excluded.

Comment by Gabriel Horner [ 24/May/13 9:42 AM ]

Looks good

Comment by Alex Miller [ 18/Aug/13 2:55 PM ]

Would like to screen this one again. I think it has open questions and is worth a discussion somewhere.

Comment by Alex Miller [ 11/Oct/13 4:47 PM ]

To me, this seems like we would be opening a security hole and a cleverly concocted resource could exploit it.

Other alternatives:
1) do nothing (user can always bind read-eval around a call to source if they want to do this safely)
2) add a source-unsafe or other wrapper function that did this
3) change source-fn to use edn/read instead? This may introduce some cases where source using non-edn features could not be read. I'd be ok with that.

Comment by Andy Fingerhut [ 16/Oct/13 8:24 PM ]

Maybe this is well known to everyone already, but in case not, doing a require or use on a namespace containing the following function on a Unix-like system to create and/or update the last modification time of the file bartleby.txt. If you remove that file, and then do (source badfn) while read-eval is bound to true, you can see that it will do the shell command again. Obviously much more dangerous side effects could be performed instead of that.

(ns bad.fn)

(defn badfn [x]
  (let [y [#=(clojure.java.shell/sh "touch" "bartleby.txt")]]
    x))

Avoiding that behavior in source-fn, yet still showing the source code, would require a different implementation of read other than clojure.core/read and clojure.edn/read.

Comment by Alex Miller [ 17/Oct/13 8:21 AM ]

Based on comments on the mailing list, most people are not concerned about this from a security point of view. I'm going to let this one through and Rich can decide further.

Comment by Rich Hickey [ 25/Oct/13 7:20 AM ]

If you haven't set read-eval and you need to read-eval, then you'd better set it, right? We're not going to do that for you. The only patch that will be accepted for this is one that generates a better error message.

Comment by Alex Miller [ 29/Dec/13 10:47 PM ]

Updated with new patch that detects and throws an error if calling source with read-eval is false.

Comment by Stuart Sierra [ 10/Jan/14 4:37 PM ]

Screened OK.

Note that this only changes the error message when read-eval is bound to false, not when it is bound to :unknown.

Comment by Stuart Sierra [ 10/Jan/14 4:44 PM ]

Changed my mind: resetting to 'incomplete'.

This patch doesn't fix the situation in the original report. To improve the error message, it should handle the :unknown case.

If *read-eval* is false, then source still works as long as the source form doesn't contain #=

Comment by Alex Miller [ 14/Jan/14 9:01 AM ]

Stuart - totally good catch. Things did not work how I thought they worked! I have updated the patch.

Comment by Stuart Sierra [ 17/Jan/14 9:44 AM ]

Screened ✔





[CLJ-1167] repl value of *file* is "NO_SOURCE_PATH", of *source-path* is "NO_SOURCE_FILE" Created: 19/Feb/13  Updated: 03/Sep/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Brian Marick Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: repl


 Comments   
Comment by Brian Marick [ 19/Feb/13 4:22 PM ]

Forgot to mention: I think it's intended to be the other way around, given the names.





[CLJ-1088] repl/source could support protocol functions Created: 21/Oct/12  Updated: 31/Jan/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.5
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Chouser Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: repl

Attachments: Text File 0001-Add-support-for-protocol-fns-to-repl-source.-CLJ-1088.patch    
Patch: Code

 Description   
user=> (source clojure.core.protocols/coll-reduce)
Source not found

But since the protocol fn's var's metadata points to the protocol var, and the protocol var knows the file and line where it was defined, it would be trivial to improve 'source' to look like this:

user=> (source clojure.core.protocols/coll-reduce)
(defprotocol CollReduce
  "Protocol for collection types that can implement reduce faster than
  first/next recursion. Called by clojure.core/reduce. Baseline
  implementation defined in terms of Iterable."
  (coll-reduce [coll f] [coll f val]))


 Comments   
Comment by Chouser [ 21/Oct/12 10:00 AM ]

Add one-line patch to clojure.repl/source so that it will find the protocol definition for a given protocol function.

Comment by Andy Fingerhut [ 31/Jan/14 6:25 PM ]

Patch 0001-Add-support-for-protocol-fns-to-repl-source.-CLJ-1088.patch no longer applies cleanly as of commits made to Clojure master on Jan 31 2014, probably due to the patch for CLJ-1176. I have not investigated how easy or difficult it would be to update.





[CLJ-1081] REPL binding not working that works with with-bindings Created: 30/Sep/12  Updated: 05/Feb/14

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.4
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Steven Devijver Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: repl


 Description   

This works as expected:

java -jar clojure-1.4.0.jar -e "(do (require 'clojure.repl) (.setDynamic #'clojure.repl/print-doc) (with-bindings {#'clojure.repl/print-doc str} (eval '(clojure.repl/doc println))))"

Output:

"{:ns #<Namespace clojure.core>, :name println, :arglists ([& more]), :added \"1.0\", :static true, :doc \"Same as print followed by (newline)\", :line 3325, :file \"clojure/core.clj\"}"

But the same thing does not work in the REPL:

java -jar clojure-1.4.0.jar -e "(do (require 'clojure.repl) (.setDynamic #'clojure.repl/print-doc) (clojure.main/repl :init (fn [] {#'clojure.repl/print-doc str}))))"

Output for Output of {{(doc println)}}:

user=> (doc println)
-------------------------
clojure.core/println
([& more])
Same as print followed by (newline)
nil
user=>




 Comments   
Comment by Steven Devijver [ 01/Oct/12 5:51 AM ]

Found a work-around:

java -jar clojure-1.4.0.jar -e "(do (require 'clojure.repl) (.setDynamic #'clojure.repl/print-doc) (with-bindings {#'clojure.repl/print-doc str} (clojure.main/repl)))))"

I'm still not sure whether the method above using :init should or should not work.





[CLJ-304] clojure.repl/source does not work with deftype Created: 20/Apr/10  Updated: 19/Sep/14

Status: In Progress
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Assembla Importer Assignee: Unassigned
Resolution: Unresolved Votes: 7
Labels: repl


 Description   

clojure.repl/source does not work on a deftype

user> (deftype Foo [a b])
user.Foo
user> (source Foo)
Source not found

Cause: deftype creates a class but not a var so no file/line info is attached anywhere.

Approach:

Patch:

Screened by:



 Comments   
Comment by Assembla Importer [ 24/Aug/10 4:38 PM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/304

Comment by Assembla Importer [ 24/Aug/10 4:38 PM ]

chouser@n01se.net said: That's a great question. get-source just needs a file name and line number.

If IMeta were a protocol, it could be extended to Class. That implementation could look for a "well-known" static field, perhaps? __clojure_meta or something? Then deftype would just have to populate that field, and get-source would be all set.

Does that plan have any merit? Is there a better place to store a file name and line number?

Comment by Assembla Importer [ 24/Aug/10 4:38 PM ]

stu said: Seems like a reasonable idea, but this is going to get back-burnered for now, unless there is a dire use case we have missed.

Comment by Gary Trakhman [ 19/Feb/14 10:31 AM ]

I could use this for cider's file/line jump-around mechanism as well.

With records, I can work around it by deriving and finding the corresponding constructor var, but it's a bit nasty.

Comment by Bozhidar Batsov [ 03/Mar/14 6:37 AM ]

I'd also love to see this fixed.

Comment by Andy Fingerhut [ 03/Mar/14 8:33 AM ]

Bozhidar, voting on a ticket (clicking the Vote link in the right of the page when viewing the ticket) can help push it upwards on listings of tickets by # of votes.

Comment by Bozhidar Batsov [ 19/Sep/14 1:17 PM ]

Andy, thanks for the pointer. They should have made this button much bigger, I hadn't noticed it all until now.





Generated at Mon Mar 30 15:11:14 CDT 2015 using JIRA 4.4#649-r158309.