Clojure

Improve clojure.repl/dir-fn to work on namespace aliases in addition to canonical namespaces.

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: Release 1.7
  • Fix Version/s: Release 1.9
  • Component/s: None
  • Labels:
  • Patch:
    Code and Test
  • Approval:
    Ok

Description

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

After:

user=> (require '[clojure.string :as s])
nil
user=> (dir s)
blank?
capitalize
...etc

Patch: clj-1673-3.patch

Screened by: Alex Miller

  1. clj-1673-2.patch
    04/May/15 4:38 PM
    2 kB
    Alex Miller
  2. clj-1673-3.patch
    13/Oct/15 1:06 PM
    2 kB
    Jason Whitlark
  3. improve_dir.patch
    02/May/15 10:35 PM
    2 kB
    Jason Whitlark

Activity

Alex Miller made changes -
Field Original Value New Value
Labels enhancement repl
Alex Miller made changes -
Description I've always been frustrated that you couldn't use clojure.repl/dir with the aliases you define in your namespace. There doesn't seem to be any reason why you couldn't look for a symbol via ns-aliases.

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.

Patch is included, feel free to use it or change it as needed. I'm not sure as to the best way to test this.

FYI: I've signed the CA.

~Jason Whitlark
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.
Approval Triaged [ 10120 ]
Alex Miller made changes -
Priority Trivial [ 5 ] Minor [ 4 ]
Hide
Jason Whitlark added a comment -

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

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

Show
Jason Whitlark added a comment - Possible unit test, since clojure.string is aliased in the test file: (is (= (dir-fn 'clojure.string) (dir-fn 'str)))
Hide
Alex Miller added a comment -

Please add a test...

Show
Alex Miller added a comment - Please add a test...
Hide
Jason Whitlark added a comment -

Updated patch, tweaked dir-fn, added test.

Show
Jason Whitlark added a comment - Updated patch, tweaked dir-fn, added test.
Jason Whitlark made changes -
Attachment improve_dir.patch [ 14102 ]
Jason Whitlark made changes -
Attachment improve_dir.patch [ 13928 ]
Hide
Jason Whitlark added a comment -

Should be good to go. I removed the old patch.

Show
Jason Whitlark added a comment - Should be good to go. I removed the old patch.
Alex Miller made changes -
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.
Extend clojure.repl/dir to work with the aliases in the current namespace

After:
{code}
user=> (require '[clojure.string :as s])
nil
user=> (dir s)
blank?
capitalize
...etc
{code}

*Patch:* improve_dir.patch
*Screened by:* Alex Miller
Alex Miller made changes -
Labels repl ft repl
Jason Whitlark made changes -
Patch Code [ 10001 ] Code and Test [ 10002 ]
Hide
Alex Miller added a comment -

Tweaked patch just to remove errant blank line, otherwise same.

Show
Alex Miller added a comment - Tweaked patch just to remove errant blank line, otherwise same.
Alex Miller made changes -
Attachment clj-1673-2.patch [ 14124 ]
Description Extend clojure.repl/dir to work with the aliases in the current namespace

After:
{code}
user=> (require '[clojure.string :as s])
nil
user=> (dir s)
blank?
capitalize
...etc
{code}

*Patch:* improve_dir.patch
*Screened by:* Alex Miller
Extend clojure.repl/dir to work with the aliases in the current namespace

After:
{code}
user=> (require '[clojure.string :as s])
nil
user=> (dir s)
blank?
capitalize
...etc
{code}

*Patch:* clj-1673-2.patch
*Screened by:* Alex Miller
Rich Hickey made changes -
Fix Version/s Release 1.8 [ 10254 ]
Approval Triaged [ 10120 ] Vetted [ 10003 ]
Alex Miller made changes -
Approval Vetted [ 10003 ] Screened [ 10004 ]
Rich Hickey made changes -
Approval Screened [ 10004 ] Ok [ 10007 ]
Hide
Michael Blume added a comment -

Applied this patch directly to master, test failure:

[java] ERROR in (test-dir) (core.clj:4023)
     [java] expected: (= (dir-fn (quote clojure.string)) (dir-fn (quote str)))
     [java]   actual: java.lang.Exception: No namespace: str found
     [java]  at clojure.core$the_ns.invokeStatic (core.clj:4023)
     [java]     clojure.repl$dir_fn.invokeStatic (repl.clj:186)
     [java]     clojure.repl$dir_fn.invoke (repl.clj:-1)
     [java]     clojure.test_clojure.repl$fn__21967$fn__21976.invoke (repl.clj:28)
     [java]     clojure.core$with_redefs_fn.invokeStatic (core.clj:7211)
     [java]     clojure.core$with_redefs_fn.invoke (core.clj:-1)
     [java]     clojure.test_clojure.repl$fn__21967.invokeStatic (repl.clj:27)
     [java]     clojure.test_clojure.repl/fn (repl.clj:-1)
     [java]     clojure.test$test_var$fn__7962.invoke (test.clj:703)
     [java]     clojure.test$test_var.invokeStatic (test.clj:703)
     [java]     clojure.test$test_var.invoke (test.clj:-1)
     [java]     clojure.test$test_vars$fn__7984$fn__7989.invoke (test.clj:721)
     [java]     clojure.test$default_fixture.invokeStatic (test.clj:673)
     [java]     clojure.test$default_fixture.invoke (test.clj:-1)
     [java]     clojure.test$test_vars$fn__7984.invoke (test.clj:721)
     [java]     clojure.test$default_fixture.invokeStatic (test.clj:673)
     [java]     clojure.test$default_fixture.invoke (test.clj:-1)
     [java]     clojure.test$test_vars.invokeStatic (test.clj:717)
     [java]     clojure.test$test_all_vars.invokeStatic (test.clj:723)
     [java]     clojure.test$test_ns.invokeStatic (test.clj:744)
     [java]     clojure.test$test_ns.invoke (test.clj:-1)
     [java]     clojure.core$map$fn__4770.invoke (core.clj:2639)
     [java]     clojure.lang.LazySeq.sval (LazySeq.java:40)
     [java]     clojure.lang.LazySeq.seq (LazySeq.java:49)
     [java]     clojure.lang.Cons.next (Cons.java:39)
     [java]     clojure.lang.RT.next (RT.java:688)
     [java]     clojure.core$next__4327.invokeStatic (core.clj:64)
     [java]     clojure.core$reduce1.invokeStatic (core.clj:924)
     [java]     clojure.core$reduce1.invokeStatic (core.clj:914)
     [java]     clojure.core$merge_with.invokeStatic (core.clj:2943)
     [java]     clojure.core$merge_with.doInvoke (core.clj:-1)
     [java]     clojure.lang.RestFn.applyTo (RestFn.java:139)
     [java]     clojure.core$apply.invokeStatic (core.clj:647)
     [java]     clojure.test$run_tests.invokeStatic (test.clj:754)
     [java]     clojure.test$run_tests.doInvoke (test.clj:-1)
     [java]     clojure.lang.RestFn.applyTo (RestFn.java:137)
     [java]     clojure.core$apply.invokeStatic (core.clj:645)
     [java]     clojure.core$apply.invoke (core.clj:-1)
     [java]     user$eval29133.invokeStatic (run_test.clj:8)
     [java]     user$eval29133.invoke (run_test.clj:-1)
     [java]     clojure.lang.Compiler.eval (Compiler.java:6934)
     [java]     clojure.lang.Compiler.load (Compiler.java:7381)
     [java]     clojure.lang.Compiler.loadFile (Compiler.java:7319)
     [java]     clojure.main$load_script.invokeStatic (main.clj:275)
     [java]     clojure.main$script_opt.invokeStatic (main.clj:335)
     [java]     clojure.main$script_opt.invoke (main.clj:-1)
     [java]     clojure.main$main.invokeStatic (main.clj:421)
     [java]     clojure.main$main.doInvoke (main.clj:-1)
     [java]     clojure.lang.RestFn.invoke (RestFn.java:408)
     [java]     clojure.lang.Var.invoke (Var.java:379)
     [java]     clojure.lang.AFn.applyToHelper (AFn.java:154)
     [java]     clojure.lang.Var.applyTo (Var.java:700)
     [java]     clojure.main.main (main.java:37)
Show
Michael Blume added a comment - Applied this patch directly to master, test failure:
[java] ERROR in (test-dir) (core.clj:4023)
     [java] expected: (= (dir-fn (quote clojure.string)) (dir-fn (quote str)))
     [java]   actual: java.lang.Exception: No namespace: str found
     [java]  at clojure.core$the_ns.invokeStatic (core.clj:4023)
     [java]     clojure.repl$dir_fn.invokeStatic (repl.clj:186)
     [java]     clojure.repl$dir_fn.invoke (repl.clj:-1)
     [java]     clojure.test_clojure.repl$fn__21967$fn__21976.invoke (repl.clj:28)
     [java]     clojure.core$with_redefs_fn.invokeStatic (core.clj:7211)
     [java]     clojure.core$with_redefs_fn.invoke (core.clj:-1)
     [java]     clojure.test_clojure.repl$fn__21967.invokeStatic (repl.clj:27)
     [java]     clojure.test_clojure.repl/fn (repl.clj:-1)
     [java]     clojure.test$test_var$fn__7962.invoke (test.clj:703)
     [java]     clojure.test$test_var.invokeStatic (test.clj:703)
     [java]     clojure.test$test_var.invoke (test.clj:-1)
     [java]     clojure.test$test_vars$fn__7984$fn__7989.invoke (test.clj:721)
     [java]     clojure.test$default_fixture.invokeStatic (test.clj:673)
     [java]     clojure.test$default_fixture.invoke (test.clj:-1)
     [java]     clojure.test$test_vars$fn__7984.invoke (test.clj:721)
     [java]     clojure.test$default_fixture.invokeStatic (test.clj:673)
     [java]     clojure.test$default_fixture.invoke (test.clj:-1)
     [java]     clojure.test$test_vars.invokeStatic (test.clj:717)
     [java]     clojure.test$test_all_vars.invokeStatic (test.clj:723)
     [java]     clojure.test$test_ns.invokeStatic (test.clj:744)
     [java]     clojure.test$test_ns.invoke (test.clj:-1)
     [java]     clojure.core$map$fn__4770.invoke (core.clj:2639)
     [java]     clojure.lang.LazySeq.sval (LazySeq.java:40)
     [java]     clojure.lang.LazySeq.seq (LazySeq.java:49)
     [java]     clojure.lang.Cons.next (Cons.java:39)
     [java]     clojure.lang.RT.next (RT.java:688)
     [java]     clojure.core$next__4327.invokeStatic (core.clj:64)
     [java]     clojure.core$reduce1.invokeStatic (core.clj:924)
     [java]     clojure.core$reduce1.invokeStatic (core.clj:914)
     [java]     clojure.core$merge_with.invokeStatic (core.clj:2943)
     [java]     clojure.core$merge_with.doInvoke (core.clj:-1)
     [java]     clojure.lang.RestFn.applyTo (RestFn.java:139)
     [java]     clojure.core$apply.invokeStatic (core.clj:647)
     [java]     clojure.test$run_tests.invokeStatic (test.clj:754)
     [java]     clojure.test$run_tests.doInvoke (test.clj:-1)
     [java]     clojure.lang.RestFn.applyTo (RestFn.java:137)
     [java]     clojure.core$apply.invokeStatic (core.clj:645)
     [java]     clojure.core$apply.invoke (core.clj:-1)
     [java]     user$eval29133.invokeStatic (run_test.clj:8)
     [java]     user$eval29133.invoke (run_test.clj:-1)
     [java]     clojure.lang.Compiler.eval (Compiler.java:6934)
     [java]     clojure.lang.Compiler.load (Compiler.java:7381)
     [java]     clojure.lang.Compiler.loadFile (Compiler.java:7319)
     [java]     clojure.main$load_script.invokeStatic (main.clj:275)
     [java]     clojure.main$script_opt.invokeStatic (main.clj:335)
     [java]     clojure.main$script_opt.invoke (main.clj:-1)
     [java]     clojure.main$main.invokeStatic (main.clj:421)
     [java]     clojure.main$main.doInvoke (main.clj:-1)
     [java]     clojure.lang.RestFn.invoke (RestFn.java:408)
     [java]     clojure.lang.Var.invoke (Var.java:379)
     [java]     clojure.lang.AFn.applyToHelper (AFn.java:154)
     [java]     clojure.lang.Var.applyTo (Var.java:700)
     [java]     clojure.main.main (main.java:37)
Hide
Jason Whitlark added a comment - - edited

After poking at it for a while, I discovered that the test in question isn't being executed in clojure.test-clojure.repl namespace, but in user. So replacing the failing test with:
(is (= (the-ns 'clojure.test-clojure.repl) ns))
also fails.

Which is very surprising to me. Is this the expected behavior? I could fix the test by doing the following:

(binding [*ns* (the-ns 'clojure.test-clojure.repl)]
(is (= (dir-fn 'clojure.string) (dir-fn 'str))))

But I don't want to mask an error, if this behavior is unexpected. Guidance please.

Show
Jason Whitlark added a comment - - edited After poking at it for a while, I discovered that the test in question isn't being executed in clojure.test-clojure.repl namespace, but in user. So replacing the failing test with: (is (= (the-ns 'clojure.test-clojure.repl) ns)) also fails. Which is very surprising to me. Is this the expected behavior? I could fix the test by doing the following: (binding [*ns* (the-ns 'clojure.test-clojure.repl)] (is (= (dir-fn 'clojure.string) (dir-fn 'str)))) But I don't want to mask an error, if this behavior is unexpected. Guidance please.
Hide
Alex Miller added a comment -

I don't want this to get applied as is so moving to incomplete for now.

Show
Alex Miller added a comment - I don't want this to get applied as is so moving to incomplete for now.
Alex Miller made changes -
Approval Ok [ 10007 ] Incomplete [ 10006 ]
Hide
Alex Miller added a comment -

You know this could be due to direct linking - calls inside the clojure code to other clojure code are now direct linked (static) calls as of 1.8.0-alpha3 and can't be redef'ed.

Show
Alex Miller added a comment - You know this could be due to direct linking - calls inside the clojure code to other clojure code are now direct linked (static) calls as of 1.8.0-alpha3 and can't be redef'ed.
Hide
Alex Miller added a comment -

Yeah, if you turn direct linking off the test passes.

Show
Alex Miller added a comment - Yeah, if you turn direct linking off the test passes.
Hide
Jason Whitlark added a comment -

Fixed test to work with direct linking.

Show
Jason Whitlark added a comment - Fixed test to work with direct linking.
Jason Whitlark made changes -
Attachment clj-1673-3.patch [ 15215 ]
Alex Miller made changes -
Approval Incomplete [ 10006 ] Vetted [ 10003 ]
Alex Miller made changes -
Fix Version/s Release 1.9 [ 10750 ]
Fix Version/s Release 1.8 [ 10254 ]
Alex Miller made changes -
Description Extend clojure.repl/dir to work with the aliases in the current namespace

After:
{code}
user=> (require '[clojure.string :as s])
nil
user=> (dir s)
blank?
capitalize
...etc
{code}

*Patch:* clj-1673-2.patch
*Screened by:* Alex Miller
Extend clojure.repl/dir to work with the aliases in the current namespace

After:
{code}
user=> (require '[clojure.string :as s])
nil
user=> (dir s)
blank?
capitalize
...etc
{code}

*Patch:* clj-1673-3.patch

Hide
Jason Whitlark added a comment -

This is ready to go. Do I need to do anything to get it back in the queue?

Show
Jason Whitlark added a comment - This is ready to go. Do I need to do anything to get it back in the queue?
Hide
Alex Miller added a comment -

We've missed the window for 1.8 but it's in the queue for 1.9.

Show
Alex Miller added a comment - We've missed the window for 1.8 but it's in the queue for 1.9.
Alex Miller made changes -
Description Extend clojure.repl/dir to work with the aliases in the current namespace

After:
{code}
user=> (require '[clojure.string :as s])
nil
user=> (dir s)
blank?
capitalize
...etc
{code}

*Patch:* clj-1673-3.patch

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

After:
{code}
user=> (require '[clojure.string :as s])
nil
user=> (dir s)
blank?
capitalize
...etc
{code}

*Patch:* clj-1673-3.patch

*Screened by:* Alex Miller
Approval Vetted [ 10003 ] Screened [ 10004 ]
Rich Hickey made changes -
Approval Screened [ 10004 ] Ok [ 10007 ]
Stuart Halloway made changes -
Status Open [ 1 ] Closed [ 6 ]
Resolution Completed [ 1 ]

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: