<< Back to previous view

[UNIFY-8] Handle sets correctly Created: 21/May/14  Updated: 21/May/14

Status: Open
Project: core.unify
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Pascal Germroth Assignee: Fogus
Resolution: Unresolved Votes: 0
Labels: None
Environment:

unify 0.5.5



 Description   

Currently unify allows using sets as expressions and just uses them as sequences, which, depending on the order of items, causes the unification to fail or succeed:

(unify #{ '[aa a] '[bb b] } ; =seq=> [bb b] [aa a]
       #{ '[?a a] '[?b b] } ; =seq=> [?b b] [?a a]
) ; => {?a aa, ?b bb}
(unify #{  '[a a]  '[b b] } ; =seq=> [a a] [b b]
       #{ '[?a a] '[?b b] } ; =seq=> [?b b] [?a a]
) ; => nil, expected {?a a, ?b b}

Unify should either handle sets (not sure if the algorithm allows for that easily) or throw an IllegalArgumentException when passed a set, but not silently seq it and behave unpredictably like that.






[UNIFY-7] Dead code in occurs? function Created: 28/Jul/13  Updated: 28/Jul/13

Status: Open
Project: core.unify
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Greg Chapman Assignee: Fogus
Resolution: Unresolved Votes: 0
Labels: None


 Description   

In the cond in the occurs? function, this condition:

(zip/end? z) false

appears twice. Unless I very much misunderstand, the second clause will never succeed.

Also, I'm wondering if it makes sense (in core.unify) for composite? to be true for strings. It looks like this means occurs? will futilely check each character of a string looking for variables.






[UNIFY-6] Create tests for k&v map unification Created: 25/May/12  Updated: 25/May/12

Status: Open
Project: core.unify
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Major
Reporter: Fogus Assignee: Fogus
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Substitution in map keys or values should both work. I doubt there are tests around this.






[UNIFY-5] core.unify locks Midje down to Clojure 1.3 Created: 26/Apr/12  Updated: 25/May/12  Resolved: 25/May/12

Status: Resolved
Project: core.unify
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Major
Reporter: Brian Marick Assignee: Fogus
Resolution: Completed Votes: 0
Labels: None


 Description   

According to `lein2 deps :tree` core.unify 0.5.2 is the (a?) reason that Midje can't use Clojure 1.4.



 Comments   
Comment by Fogus [ 26/Apr/12 10:23 AM ]

I will put together a point release for the next version today or tomorrow (latest). Thank you.

Comment by Fogus [ 25/May/12 10:01 PM ]

Tomorrow came and went. I apologize. The 0.5.3 version has been deployed and should be available on Maven Central in the next few hours.





[UNIFY-4] Vectors of different length incorrectly unify Created: 08/Feb/12  Updated: 25/May/12  Resolved: 25/May/12

Status: Resolved
Project: core.unify
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Thomas Winant Assignee: Fogus
Resolution: Completed Votes: 0
Labels: unify


 Description   
(unify '[1 ?x] '[1])
=> {?x nil}

I expect this unification to fail.



 Comments   
Comment by Fogus [ 26/Apr/12 11:48 AM ]

Which version are you using?

Comment by Thomas Winant [ 26/Apr/12 1:43 PM ]

Version 5.2

Comment by Fogus [ 25/May/12 10:00 PM ]

This is not a perfect solution, but it works as a stopgap as this will be handled in the next 0.1.0 version. For now the 0.5.3 version has been deployed and should be available on Maven Central in the next few hours.





[UNIFY-3] Enhance documentation Created: 03/Feb/12  Updated: 03/Feb/12

Status: Open
Project: core.unify
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Major
Reporter: Fogus Assignee: Fogus
Resolution: Unresolved Votes: 0
Labels: docs


 Description   

The current unify docs are spartan and "just the facts". It would be useful to have a set of docs that:

  • explain unification
  • explain the library use cases
  • show a simple example use





[UNIFY-2] Remove reflection warnings Created: 05/Jan/12  Updated: 05/Jan/12  Resolved: 05/Jan/12

Status: Resolved
Project: core.unify
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Fogus Assignee: Fogus
Resolution: Completed Votes: 0
Labels: performance, reflection, unify

Approval: Ok

 Description   
Reflection warning, clojure/core/unify.clj:30 - reference to field getClass can't be resolved.
Reflection warning, clojure/core/unify.clj:30 - reference to field isArray can't be resolved.

These reflective calls occur frequently enough that they should be resolved.



 Comments   
Comment by Fogus [ 05/Jan/12 7:37 AM ]

Fixed in 6b6d1130bf857439d1863f6fc574a7a6541b84b8.





[UNIFY-1] Cut 0.5.2 release jar Created: 14/Dec/11  Updated: 09/Jan/12  Resolved: 09/Jan/12

Status: Resolved
Project: core.unify
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Major
Reporter: Fogus Assignee: Fogus
Resolution: Completed Votes: 0
Labels: None


 Description   

There are a couple of bug fixes that should make it into a +0.0.1 release.






[TTRACE-9] Use aprint when displaying value Created: 29/Oct/14  Updated: 31/Oct/14

Status: Open
Project: tools.trace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Jérémie Grodziski Assignee: Luc Préfontaine
Resolution: Unresolved Votes: 0
Labels: enhancement, print

Attachments: File use-aprint.diff    
Patch: Code and Test

 Description   

Problem: when trace-ns is used, very long displays in the output stream is very hard to read when the fn calls are important and with voluminous data structure
Solution: Use Aprint library when printing value in the output string, now get syntax hightlighting and better layout of data structure
Tradeoffs: add a new dependency to tools.trace
Tests: yes, modify the cleanup fn to also clean the color information added by aprint, so 100% tests passed



 Comments   
Comment by Andy Fingerhut [ 30/Oct/14 7:36 AM ]

Jeremie, I am not certain, but I believe it is a policy of Clojure contrib libraries not to depend on libraries other than those in Clojure or Clojure contrib.

Perhaps if new arities were added to some existing tools.trace functions or macros, sprint or any other developer-desired function could be provided as arguments to control how printing occurs?

Comment by Jérémie Grodziski [ 30/Oct/14 8:17 AM ]

Hi Andy,

Thanks for your comment, yes I thought about that and known it would be the greatest concern...
I'll have a look at 2 possibilities:

  • the fastest: add new arg to tools.trace fns and macros for providing a filter that can add the colors and layout
  • the better: see if inlining the aprint code is worth considering, or better add the syntax hightlighting feature in clojure.pprint even though pprint is not used in tools.trace (and along with some neat library like Pretty that could be added to clojure.repl)

In the end, adding aprint (both for color and layout) made a HUGE difference for me in usability...

Comment by Alex Miller [ 30/Oct/14 10:18 AM ]

Andy, that is not a policy afaik, merely a preference to minimize them as much as possible. There are other contribs that have external dependencies. It's up to Luc as the contrib lead whether he thinks this is ok here.

Comment by Luc Préfontaine [ 30/Oct/14 10:59 AM ]

I've looked into this. 'Inlining aprint' in tools.trace would be my first choice.
However we need to get Vlad Bokov to fill a CA (I do not see him in the list of contributors)
and to grant us the rights to include part of is source code in tools.trace.

Being a regular user of this tool, I find the arity avenue painful. It happens that we use this in
integrated test mode to nail issues. Then the output goes to a file were tty ansi commands
are not welcomed.

I think we need a mode state to enable/disable this feature. In the repl you would need a single
statement to get the colored output whatever trace fns you are calling. That can be part of your
user profile.

Eventually this could migrate to core.pprint but that will require some time before it ends up in a clojure release.

Meanwhile we can keep this isolated in tools.trace so it can be moved later elsewhere.

Comments ?

Comment by Luc Préfontaine [ 30/Oct/14 12:50 PM ]

I sent an email to Vlad and the url to this ticket.
Will have a discussion off line first and then move along with the plan if we can.

Comment by Vlad Bokov [ 31/Oct/14 12:27 AM ]

Hi Jérémie, Alex and Luc,

I'm the author of aprint and really happy about you like my lib
Here are my thoughts about it:

1) I really like the idea to push it into tools.trace
2) Currently all the clojure contrib-libs have same patch acceptance policy as the lang, which I already wondered about https://github.com/clojure/clojure/pull/17#issuecomment-53628365 Try to understand, if a lib depends on another, the last one must share the same policy as the original, which is not the case, as I hardly stop accepting pull requests.
3) If you couldn't change this policy, I see "inlining" as best choice.

Now some points about the question "what will get inlined?"

4) I'm a bit surprised, that people don't share my intent to integrate it with repl like this https://github.com/greglook/whidbey, don't like colors, they simply like the layout. So coloring could be thrown away and we eliminate dependance on clansi. Or I could inline clansi (has MIT license), document a :^dynamic to turn on colors (which will get off by default)
5) Code for compact layout depends on jline and it's definitely won't get inside As such this will remain pprint/print-right-margin default value, which is 72 by default and isn't best choice in my opinion, but let it be. (or do you still run clojure on very old terminals?)
6) I don't see sense in introducing a new one :^dynamic for it, I suggest to rely on pprint/print-pretty and if it's true - use compact layout instead of pprint's default one

As such:

  • i could fill and sign a CA
  • prepare a dependent free patch for you, which includes only compact layout (and colors if you like) controlled by pprint/print-pretty and pprint/print-right-margin




[TTRACE-8] Documentation describing the features of tools.trace Created: 04/Apr/14  Updated: 14/May/14  Resolved: 14/May/14

Status: Closed
Project: tools.trace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Anders Conbere Assignee: Luc Préfontaine
Resolution: Completed Votes: 0
Labels: None


 Description   

I've been working on debugging some clojure systems and a coworker suggested that one helpful tool might be tools.trace. But the current documentation does nothing to help me understand how it might help.

For example here is a selection from the README:

```
(trace (* 2 3)) ;; To trace a value

(trace tag (* 2 3)) ;; To trace a value and assign a trace tag
```

But I as a total newb, have no idea what a trace is, why that would be useful, or why I would want to assign it to a tag (or even what a tag is).

I could pop open lein, get it installed, and play with it for a bit, but it's not clear to me why I should.



 Comments   
Comment by Luc Préfontaine [ 14/May/14 8:21 AM ]

I revamped the readme a bit but left stuff of larger scope open to experiment by the developper
(trace-ns/trace-vars and so forth).
Tracing existed for decades in Lisp, a simple Google search would give you an idea of what to expect.

Comment by Luc Préfontaine [ 14/May/14 8:23 AM ]

Modified README, no code change





[TTRACE-7] clone-throwable on type java.lang.Throwable either wastes computation or is missing cond Created: 02/Dec/13  Updated: 15/May/14  Resolved: 14/Mar/14

Status: Closed
Project: tools.trace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Andy Fingerhut Assignee: Luc Préfontaine
Resolution: Completed Votes: 0
Labels: None


 Description   

The body of clone-throwable is currently:

(extend-type java.lang.Throwable
  ThrowableRecompose
  (clone-throwable [this stack-trace args] 
    (try
      (let [ctor (.getConstructor (class this) (into-array [java.lang.String]))
            arg (first args)]
        (string? arg)
        (doto (.newInstance ctor (into-array [arg])) (.setStackTrace stack-trace))
        :else (doto (.newInstance ctor (into-array [(str arg)])) (.setStackTrace stack-trace)))
      (catch Exception e# this))))

Note that the return value of (string? arg) disappears into nowhere, as is the :else, and both doto's are always executed.

It appears that perhaps there should be a cond wrapped around the body of the let, as there is in the clone-throwable implementation for type java.nio.charset.CoderMalfunctionError earlier in the file, or these unnecessary bits of code should be removed.



 Comments   
Comment by Luc Préfontaine [ 14/Mar/14 5:14 PM ]

Yep, missing cond. Fixed.

Comment by Luc Préfontaine [ 14/Mar/14 5:53 PM ]

Missing cond added

Comment by Luc Préfontaine [ 15/Mar/14 3:53 PM ]

Official release 0.7.8, bad readme in 0.7.7





[TTRACE-6] Remove unnecessary (?) (def ~name) from deftrace Created: 29/Nov/13  Updated: 15/May/14  Resolved: 14/Mar/14

Status: Closed
Project: tools.trace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Andy Fingerhut Assignee: Luc Préfontaine
Resolution: Completed Votes: 0
Labels: None


 Description   

I was working on enhancements to a lint tool and trying it out on contrib libraries to see if any of them have redefinitions of symbols. It found that every use of deftrace causes a redefinition. Looking at deftrace's definition, I cannot see any reason for the (def ~name) before the later (defn ~name ...) expression. The later expression is done unconditionally, unlike the way deftest or defmulti always def a var, then redefine it if it was unbound.

I could easily be missing something important here, but is there a reason for the (def ~name)?



 Comments   
Comment by Nicola Mometto [ 29/Nov/13 10:52 AM ]

Removing the def, (deftrace x [] #'x) wouldn't compile anymore.

What about replacing the def with a declare?

Comment by Luc Préfontaine [ 14/Mar/14 5:13 PM ]

Switched to declare. This satisfies the compiler.

Comment by Luc Préfontaine [ 14/Mar/14 5:53 PM ]

Replaced the def by declare

Comment by Luc Préfontaine [ 15/Mar/14 4:05 PM ]

Official release 0.7.8, bad readme in 0.7.7





[TTRACE-5] test_trace.clj unnecessarily calls (run-tests) at the end Created: 29/Nov/13  Updated: 15/May/14  Resolved: 14/Mar/14

Status: Closed
Project: tools.trace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Andy Fingerhut Assignee: Luc Préfontaine
Resolution: Completed Votes: 0
Labels: None

Attachments: File ttrace-5-v1.diff    
Patch: Code

 Description   

Both 'mvn test' and 'lein test' run all tests twice with that (run-tests) near the end of the file. With that line removed, both of those commands run the set of tests once, as expected.



 Comments   
Comment by Andy Fingerhut [ 29/Nov/13 9:13 AM ]

Patch ttrace-5-v1.diff removes the unnecessary call to (run-tests). All tests pass once rather than twice after applying it

Comment by Luc Préfontaine [ 15/Mar/14 4:05 PM ]

Fixed in official release 0.7.8, bad readme in 0.7.7





[TTRACE-4] trace-ns traces all vars in the ns, not just fns Created: 12/Sep/13  Updated: 15/May/14  Resolved: 14/Mar/14

Status: Closed
Project: tools.trace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Chris Jeris Assignee: Luc Préfontaine
Resolution: Completed Votes: 0
Labels: None
Environment:

Clojure 1.5.1, tools.trace 0.7.6


Attachments: Text File tools.trace-4.patch    
Patch: Code and Test

 Description   

trace-ns traces every var in the given namespace, fns and non-fns alike. This is counterintuitive from the docstring (which says "trace all fns in the given name space") – when I have a data object like a map in a var, it may happen to implement IFn, but if I trace it I can't assoc onto it any more.

trace.demo> (def x {:foo 1 :bar 2})
#'trace.demo/x
trace.demo> (trace/trace-ns 'trace.demo)
nil
trace.demo> x
#<trace$trace_var_STAR_$fn__1682$tracing_wrapper__1683 clojure.tools.trace$trace_var_STAR_$fn__1682$tracing_wrapper__1683@2b0ce330>
trace.demo> (assoc x :baz 3)
ClassCastException clojure.tools.trace$trace_var_STAR_$fn__1682$tracing_wrapper__1683 cannot be cast to clojure.lang.Associative  clojure.lang.RT.assoc (RT.java:691)

I assume it's useful to retain the ability to trace IFn's that aren't fns at the level of trace-var. The attached patch instead changes trace-ns to check that traced values are fn? (not just ifn?) before wrapping them. Andy Fingerhut suggests that, if the ability to trace IFn's that aren't fns is not actually all that useful, it's simpler just to change ifn? to fn? in trace-var*.



 Comments   
Comment by Luc Préfontaine [ 14/Mar/14 5:19 PM ]

Agree. Fixed.

Comment by Luc Préfontaine [ 14/Mar/14 5:51 PM ]

Fix accepted. Only fns are traced.

Comment by Luc Préfontaine [ 15/Mar/14 3:52 PM ]

Official release 0.7.8, bad readme in 0.7.7





[TTRACE-3] untrace-var* removes only the highest level of tracing created by trace-var* Created: 29/Aug/13  Updated: 15/May/14  Resolved: 14/Mar/14

Status: Closed
Project: tools.trace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Luc Préfontaine Assignee: Luc Préfontaine
Resolution: Completed Votes: 0
Labels: enhancement
Environment:

Clojure 1.2-1.5.1



 Description   

Calling trace-var* (or using the macro trace-vars) multiple times on the same var(s) adds multiple tracing levels.
To remove tracing completely, untrace-vars* has to be called at least the same number of times on the traced var(s).

This behavior is annoying at the REPL where a single call to untrace-vars* would untracing much more simpler.
However such a behavior might be desirable when programatically driven.

Comments welcomed.



 Comments   
Comment by Luc Préfontaine [ 14/Mar/14 5:33 PM ]

After mulling over this, I think that the "recursive" behavior is not needed.
I see no added value even from a programmatic standpoint of allowing this behavior.

So calling trace-vars* on the same var has now on will not have any effect
after the first call.

Hence a single call to untrace-vars* will remove the trace at all times.

Took me some time but finally got through it

Comment by Luc Préfontaine [ 14/Mar/14 5:50 PM ]

trace-var* is now a noop if the fn is already traced

Comment by Luc Préfontaine [ 15/Mar/14 3:53 PM ]

Official release 0.7.8, bad readme in 0.7.7





[TTRACE-2] Eliminate a few uses of Reflection in tools.trace Created: 28/Oct/12  Updated: 15/May/14  Resolved: 01/Dec/12

Status: Closed
Project: tools.trace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Andy Fingerhut Assignee: Luc Préfontaine
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File ttrace-2-eliminate-reflection-v1.txt    
Patch: Code

 Description   

There are a few instances of reflection in tools.trace code that can be eliminated with suitable type hints.



 Comments   
Comment by Andy Fingerhut [ 28/Oct/12 10:32 PM ]

ttrace-2-eliminate-reflection-v1.txt dated Oct 28 2012 eliminates all current uses of reflection in tools.trace code by adding type hints.

Comment by Luc Préfontaine [ 14/Nov/12 6:14 PM ]

Done in 0.7.4

Comment by Luc Préfontaine [ 01/Dec/12 3:36 PM ]

Issued 0.7.5, mising readme but was fixed in 0.7.4

Comment by Luc Préfontaine [ 01/Dec/12 3:37 PM ]

Fixed in 0.7.5





[TTRACE-1] Two convenience predicate functions useful for IDE-tools Created: 23/Apr/12  Updated: 15/May/14  Resolved: 01/Dec/12

Status: Closed
Project: tools.trace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Frank Siebenlist Assignee: Luc Préfontaine
Resolution: Completed Votes: 0
Labels: enhancement

Patch: Code and Test

 Description   

In our IDE-tool clj-ns-browser, we show the user a list of vars and make it easy to add a trace by selecting a var and clicking a button.

In order to only enable the button for a var that is traceable, we use predicate "var-traceable?".

In order to change the button text from "trace" to "untrace", we use predicate "var-traced?".

Both "var-traceable?" and "var-traced?" are aware of tools.trace internals - ideally, we would like to remain oblivious to trace's inners...

Our implementations for the predicates is at: https://gist.github.com/2472261

Thanks for the great tool!



 Comments   
Comment by Luc Préfontaine [ 14/Nov/12 5:56 PM ]

Done, however the names where changed to traced? and traceable?.

Comment by Luc Préfontaine [ 14/Nov/12 6:18 PM ]

Done is 0.7.4

Comment by Luc Préfontaine [ 01/Dec/12 3:38 PM ]

Done, "good" version is 0.7.5 (readme & comments need an update in 0.7.4")





[TRDR-18] A couple of typos in doc strings Created: 21/Nov/14  Updated: 22/Nov/14  Resolved: 22/Nov/14

Status: Closed
Project: tools.reader
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Andy Fingerhut Assignee: Nicola Mometto
Resolution: Completed Votes: 0
Labels: None


 Description   

Two occurrences of SourceLoggingReader should probably be replaced with SourceLoggingPushbackReader ?



 Comments   
Comment by Nicola Mometto [ 22/Nov/14 11:56 AM ]

Fixed thanks https://github.com/clojure/tools.reader/commit/ececc05cd8d505cc5f4ff9cee408e202b6cbf402





[TRDR-17] tools.reader bug demonstrated when syntax quote contains map with key :val Created: 02/Nov/14  Updated: 03/Nov/14  Resolved: 03/Nov/14

Status: Closed
Project: tools.reader
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Andy Fingerhut Assignee: Nicola Mometto
Resolution: Completed Votes: 0
Labels: None


 Description   

Steps to reproduce with latest tools.reader 0.8.11:

user=> (require '[clojure.tools.reader :as tr])
nil
user=> (tr/read-string "(defn foo [x] `{:val x})")
UnsupportedOperationException count not supported on this type: Symbol  clojure.lang.RT.countFrom (RT.java:602)

From the partial investigation I've done so far, it appears this happens if a map inside of a syntax quote expression has a key :val. It looks like the wrong map is being passed to map-func, or even higher up the call stack, or perhaps map-func should be using coll in place of (:val coll).



 Comments   
Comment by Nicola Mometto [ 03/Nov/14 7:00 AM ]

Thanks, fixed: https://github.com/clojure/tools.reader/commit/c8758b958c3683a2e66cc6537448276082b2191e





[TRDR-16] Reading Anonymous fns without arguments throws NPE Created: 20/Jun/14  Updated: 20/Jun/14  Resolved: 20/Jun/14

Status: Closed
Project: tools.reader
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Jozef Wagner Assignee: Nicola Mometto
Resolution: Declined Votes: 0
Labels: None


 Description   

This throws:

(clojure.tools.reader/read-string "#()")


 Comments   
Comment by Jozef Wagner [ 20/Jun/14 3:31 PM ]

It works when I used most recent master from repo. My bad, you can close this ticket. Sorry.





[TRDR-15] tools.reader 0.8.4 causes clojurescript to stop working in mysterious ways Created: 03/Jun/14  Updated: 04/Jun/14  Resolved: 04/Jun/14

Status: Closed
Project: tools.reader
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: James Cash Assignee: Nicola Mometto
Resolution: Declined Votes: 0
Labels: bug, cljs
Environment:

OS X 10.9.3, Java 1.8, Clojure 1.6.0



 Description   

If tools.reader 0.8.4 is included in a project's dependencies, then clojurescript fails to work in a browser, with the error that cljs.core.PersistentArrayMap is undefined. I have created a sample project demonstrating this at https://github.com/jamesnvc/cljs-toolreader-debugging.

I also apologize that I'm not sure if this is a problem in tools.reader or clojurescript. If there is a better place for me to report this issue, please let me know!



 Comments   
Comment by Nicola Mometto [ 04/Jun/14 3:33 AM ]

This is a bootstrap issue in clojurescript.
tools.reader 0.8.4 introduced :file metadata, clojurescript needs to dissoc it here https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/analyzer.clj#L1490 to work with this version.

I suggest you open a ticket in che clojurescript jira making that change if you want to make clojurescript work with 0.8.4

Comment by James Cash [ 04/Jun/14 9:04 AM ]

Thank you very much Nicola, I opened the issue on the clojurescript JIRA





[TRDR-14] Feature Expressions Created: 15/May/14  Updated: 05/Aug/14

Status: Open
Project: tools.reader
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Ghadi Shayban Assignee: Nicola Mometto
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File tools.reader-feature-expressions-2.diff     File tools.reader-feature-expressions-3.diff     File tools.reader-feature-expressions.diff    
Patch: Code and Test

 Description   

Feature expressions based directly on Common Lisp.

Implementation based on Roman's patch for CLJS-27. #+ #- and or not are supported. Unreadable tagged literals are suppressed through *suppress-read*.

For example, reading the following with features set to #{:clj} returns :baz

#+bad/platform #foo bar :baz

Initial feature names for each platform will be clj, cljs, and cljr.

CLJ-1424 is the accompanying patch for Clojure.

Patch: tools.reader-feature-expressions-3.diff



 Comments   
Comment by Nicola Mometto [ 19/May/14 6:56 PM ]

I looked over the patch and it mostly looks good, a few comments:

  • does *suppress-read* need to be exposed?
  • can you add a docstring for *features*?
  • should *suppress-read* affect read-eval?

That said, has it been decided that this is how feature expressions will be implemented in 1.7?
If not, I'm going to wait to merge this in until a decision is taken, this is a major design point and I don't want to diverge from the official clojure take on this even for a single release: people might start using it, especially since t.r is now the default reader for cljs.

Comment by Ghadi Shayban [ 19/May/14 7:23 PM ]

Yes thank you for not merging. This is WIP and just one approach for feature expressions. There seem to be at least two couple diverging approaches emerging from the various discussion (Brandon Bloom's idea of read-time splicing being the other.) I would definitely not merge until the community and Rich weighs in.

In any case having all Clojure platforms be ready for the change is probably essential. Also backwards compatibility of feature expr code to Clojure 1.6 and below is also not trivial.

1) *suppress-read* probably doesn't need to be exposed
2) *suppress-read* should probably imply *read-eval* false. You wouldn't want read-ctor to load any classes or call any code or constructors

Comment by Alex Miller [ 04/Aug/14 12:56 PM ]

Updated patch to reflect clj and cljs as initial feature names.





[TRDR-13] Better documentation wrt interaction between reader-types and java readers Created: 11/Apr/14  Updated: 11/Apr/14

Status: Open
Project: tools.reader
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Major
Reporter: Nicola Mometto Assignee: Nicola Mometto
Resolution: Unresolved Votes: 0
Labels: None





[TRDR-12] Misplaced doc string for function newline? Created: 23/Dec/13  Updated: 23/Dec/13  Resolved: 23/Dec/13

Status: Closed
Project: tools.reader
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Andy Fingerhut Assignee: Nicola Mometto
Resolution: Completed Votes: 0
Labels: None

Attachments: File trdr-12-v1.diff    

 Description   

Found using Eastwood



 Comments   
Comment by Nicola Mometto [ 23/Dec/13 7:06 PM ]

Fixed: https://github.com/clojure/tools.reader/commit/1c9633400fdfdf6852d355edd9c44a721b0938ae





[TRDR-11] Stack overflow on whitespace in reader/read and edn/read Created: 05/Dec/13  Updated: 05/Dec/13  Resolved: 05/Dec/13

Status: Closed
Project: tools.reader
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Paul Michael Bauer Assignee: Nicola Mometto
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File fix_stack_overflow_in_reader.patch    
Patch: Code

 Description   

reader/read and edn/read call themselves recursively for several encountered character classes - whitespace, comments.
This is high-risk for stack overflow, especially for machine-generated data.

Motivating Use Case: cljs files generated via cljx are particularly prone to trigger tools.reader stack overflow errors. Cljx replaces un-included forms with homomorphic whitespace blocks to preserve line number and column errors on compilation.



 Comments   
Comment by Nicola Mometto [ 05/Dec/13 3:50 PM ]

Fixed: https://github.com/clojure/tools.reader/commit/01b53cb61b586e78cf3f70f12ba2adbbdb7abb25

Thanks, I took the liberty of changing the commit description.

Comment by Paul Michael Bauer [ 05/Dec/13 3:53 PM ]

Thanks for changing the commit message. facepalm
Missed that when I generated the patch





[TRDR-10] Add end-line and end-column metadata to read objects Created: 25/Oct/13  Updated: 08/Nov/13  Resolved: 06/Nov/13

Status: Closed
Project: tools.reader
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Alexander Redington Assignee: Alexander Redington
Resolution: Completed Votes: 0
Labels: None

Attachments: File TRDR-10-line-numbering.diff    
Patch: Code and Test

 Description   

Per TRDR-9, add end-line and end-column metadata information to every form read, when the underlying reader implements the IndexingReader protocol.



 Comments   
Comment by Alexander Redington [ 25/Oct/13 12:03 PM ]

Patch to include end-line and end-column numbers on all forms read, with tests.

Comment by Nicola Mometto [ 06/Nov/13 3:26 PM ]

Applied to master: https://github.com/clojure/tools.reader/commit/a7251e87c5530e04c3c7c4a983f30030f38730df

I'm sorry I didn't apply this earlier but JIRA didn't send me an email notifying me that you updated the patch even though I am watching the issue.

Comment by Alexander Redington [ 08/Nov/13 7:31 AM ]

I've only been working on this in Friday time, and even that has been sporadic, so nothing to worry about!

Working with you on this has been excellent, thanks for accepting the suggestions and patches!





[TRDR-9] Add more source metadata to read objects Created: 27/Sep/13  Updated: 02/Sep/14  Resolved: 20/Nov/13

Status: Closed
Project: tools.reader
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Alexander Redington Assignee: Alexander Redington
Resolution: Completed Votes: 0
Labels: None

Attachments: File TRDR-9-source-logging.diff    
Patch: Code and Test

 Description   

I'm working on a static analyzer for Clojure, where I'd like to have a reader which provides precise form coordinates and source representations.

Specifically this enables analyses where it's straightforward to bind an object like:

(fn* [p1__3615#] (inc (conj p1__3615#)))

to the source form which it was read from:

"#(inc (conj %))"

Would this kind of functionality be considered appropriate to tools.reader? Specifically I'd like to make a change such that

  • Read data is unchanged
  • When an IMeta is read, additional information is embedded in the metadata, but no existing metadata is changed
  • The additional metadata would include the source character string, the ending column, and the ending line from the source stream

As an example, reading a stream that contained the following on the first line:

#(inc (conj %))

Would return a list as follows:

(fn* [p1__3615#] (inc (conj p1__3615#)))

And would change the metadata from:

{:line 1, :column 1}

to augment it with more metadata, as follows:

{:line 1, :column 1, :end-line 1, :end-column 15, :source "#(inc (conj %))"}

I'd happily contribute a patch to include this behavior if you'd be inclined to accept it. I'd like to first see if it would be considered appropriate, and also discuss any other concerns you might have (such as having this behavior be optionally enabled, etc)



 Comments   
Comment by Nicola Mometto [ 28/Sep/13 1:48 PM ]

I would definitely take a patch to add such a behaviour, with the condition that this should not degrade the performances too much (a 2x slowdown would be acceptable though).

I don't feel like this needs to be optional behaviour, we already provide column/line info by default, if it's not needed the user might just ignore it.

Comment by Alexander Redington [ 18/Oct/13 12:49 PM ]

Just like to touch base as I've been quiet for a little while, but haven't forgotten about this. It's been my primary goal on Friday time.

I currently have a naive implementation that works, but blows our performance goal by a 5x factor. I think this is solvable with a little more finesse in source logging.

It appears that end-line and end-column information can be obtained for almost no additional computational cost. (less than 2% additional processor time).

I've committed my WIP to: https://github.com/aredington/tools.reader/tree/metadata-logging

Also worth discussing at some point in time: some tests around syntax quoting and metadata reading start failing. For syntax quote, the forms returned are significantly more verbose, including metadata assertions with the source attribution. For metadata, the metadata values read now have an additional :source key included that specifies the source string the forms were read from. In both cases, the expectations of the tests are violated, but I'm not inclined to say that the end result is incorrect without discussion.

Comment by Nicola Mometto [ 22/Oct/13 11:57 AM ]

Awesome, I haven't had a chance to read through the implementation but I'll do so as soon as possible.
If I'm reading the last commit message correctly, it looks like since your comment here you were able to reduce performance loss to 2x, that's cool.

Can you please make the end-line/end-column modifications into another ticket? I can merge that as soon as you upload the patch.

Regarding the failing tests – looks like those are inevitable, it's definitely not a bug and those tests should be fixed to match the new behaviour (fixing those should be just a matter of manually attatching the :source metadata to the forms to be read by the clojure reader)

Comment by Nicola Mometto [ 24/Oct/13 11:39 AM ]

After some more thoughts, I think it should be better to have a "InfoPushBackReader" (not sure if it's the best name) wrapping an IndexingPushBackReader that provides source data, this way we only get the performance hit when we effectively need the :source info instead of making every Reader collect source data even when we don't want/need it.

Would this approach work for your use case?

Comment by Alexander Redington [ 25/Oct/13 8:04 AM ]

Absolutely. I'll make two patches and we will have two paths for metadata:

  • Default case; if line numbering information is available, end-line and end-column will be captured. The performance hit from this is marginal (2-3% slower), but it does provide sufficient information for, e.g. editors, IDEs to precisely indicate what portions of a source file originate what forms. This patch will be submitted in a new ticket.
  • Enhanced case; using a new InfoPushbackReader (SourceInfoPushbackReader? SourceLoggingPushbackReader?) we'll log precise source information on each form read. The performance hit from this will be substantial (100% slower), but it isolates the performance impacts to those users who deliberately want more information and are willing to pay the performance cost. This patch will be attached to this issue (TRDR-9)
Comment by Alexander Redington [ 25/Oct/13 8:27 AM ]

Created TRDR-10 for end-line and end-column info.

Comment by Nicola Mometto [ 06/Nov/13 4:04 PM ]

Looks like CLJS will also beenfit from this, see http://dev.clojure.org/jira/browse/CLJS-658

+1 for SourceLoggingPushbackReader

Comment by Alexander Redington [ 15/Nov/13 1:35 PM ]

On https://github.com/aredington/tools.reader I have it nearly working save that nested collections (e.g. [1 2 [3 4 5]]) fail to read correctly.

The top collection will read correctly with the following as the source string: "[1 2 [3 4 5]]", the nested vector reads as "3 4 5]".

I tried taking the macro dispatching char and prepending it to the source log at the appropriate source logging frame, and that resolves nested collections, but causes metadata to read erroneously, e.g. {:foo :baz} 'zot will read with source string "^{:foo :baz} 'zot".

Gonna keep digging to try and resolve this because source logging is very close, but just wanted to update on current status as I'd been quiet for a while.

Comment by Nicola Mometto [ 15/Nov/13 8:02 PM ]

I'll take a look at your current implementation and see if I can help you spot what's going wrong, thanks again for the effort you're putting into this.

Comment by Nicola Mometto [ 16/Nov/13 8:49 AM ]

I opened a pull request https://github.cam/aredington/tools.reader/pull/1 that should fix all the issues you were having, as well as fixing another minor bug.

If you find my edits reasonable please merge it and then squash the commits and I'll merge the patch upstream

Comment by Alexander Redington [ 20/Nov/13 1:23 PM ]

Squashed commit with working source logging

Comment by Nicola Mometto [ 20/Nov/13 3:17 PM ]

Fixed: https://github.com/clojure/tools.reader/commit/f9e55071d82a443db3fac1c2feda059d0215bb90

Thanks

Comment by Nikita Beloglazov [ 02/Sep/14 3:40 PM ]

As I see current tools.reader still not able to return position info (line, column numbers) for strings, numbers, booleans and other primitives that don't implement IMeta interface. Is there plans to fix it somehow? I was thinking how it can be fixed and only idea I came up with was to read input data into intermediate datastructure like:

{:type :vector
:children [{:type :string
:value "hello"
:line 1
:column 2}]
:line 1
:column 4}

And then convert it to clojure datastructures: ["hello"] so changes are invisible from outside. Additionally you can pass additional option like :raw-format true and it will return those internal datastructure.

Would it be the change that will be accepted or is it too drastic? I can start working it. I think this functionality is really needed for libraries linter-like libraris that checks whether code is formatted correctly and need to know position of each lexem.

Comment by Nicola Mometto [ 02/Sep/14 4:21 PM ]

I thought about this issue a while ago and I'd be definitely more than happy to take a patch for this.
What I'd like to see is a tools.reader.parser namespace with a parse function that returns a parse tree, and then tools.reader be just a compiler from that parse tree back to clojure, however I'm worried about the performance implications of that; I've deliberately avoided writting tools.reader with multimethods in favour of direct dispatch via `case` for performance reasons, this would probably involve an unacceptable performance hit.

So probably keeping the parser and the reader separated, even at the cost of having to copy&paste most of the current implementation is the way to go; obviously if you want to try to unify the two feel free to do so.

Can you open a new enhancement ticket for this?





[TRDR-8] Fix for parsing of tagged literals Created: 12/Sep/13  Updated: 13/Sep/13  Resolved: 13/Sep/13

Status: Closed
Project: tools.reader
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Alex Coventry Assignee: Nicola Mometto
Resolution: Completed Votes: 0
Labels: bug, patch

Attachments: Text File 0001-Fix-for-ctor-reading.patch     Text File reader.patch    
Patch: Code
Approval: Ok

 Description   

Parsing of tagged literals fails at the moment, e.g.

user> (require '[clojure.tools.reader :as r] :reload) (r/read-string "#java.lang.String[\"Hi\"]")
nil
ArrayIndexOutOfBoundsException 15 clojure.lang.RT.aget (RT.java:2239)

Attached patch corrects this problem, and brings the clojure logic in line with what's in LispReader.java



 Comments   
Comment by Nicola Mometto [ 13/Sep/13 7:18 AM ]

Thanks for the patch, can you post a patch that is the result of git format-patch so that we can keep your authorship on the commit?
(apply the patch, git add <path/to/reader.clj>, git commit -m "Fix ctor reading", git format-patch origin/master)

Also, can you please put the result of (count entries) in the let so that we compute it only once?

Thanks

Comment by Alex Coventry [ 13/Sep/13 12:06 PM ]

No worries, Nicola. I've attached a revised patch.

Best regards,
Alex

Comment by Nicola Mometto [ 13/Sep/13 12:38 PM ]

fixed https://github.com/clojure/tools.reader/commit/d9374f90448a4ff52ad83a2b75be2fa520a24db8





[TRDR-7] Anonymous variadic fn arg not read properly Created: 12/Aug/13  Updated: 14/Aug/13  Resolved: 14/Aug/13

Status: Closed
Project: tools.reader
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Chas Emerick Assignee: Nicola Mometto
Resolution: Completed Votes: 0
Labels: None

Patch: Code and Test

 Description   

Compare clojure.core/read-string:

user=> (read-string "#(last %&)")
(fn* [& rest__461#] (last rest__461#))

vs. clojure.tools.reader/read-string:

user=> (reader/read-string "#(last %&)")
(fn* nil (last rest__458#))

The most immediate manifestation of this is that anonymous function forms in ClojureScript >= 0.0-1853 will not compile. I'm filing an issue to track this there as well.



 Comments   
Comment by Nicola Mometto [ 14/Aug/13 9:44 AM ]

Sorry for the delay, fixed with https://github.com/clojure/tools.reader/commit/8f48f83a5ad0dbd7f4c8e612c4ab764d418feb93

I'm releasing 0.7.6 ASAP





[TRDR-6] Some uses of reflection in tools.reader code slow it down unnecessarily Created: 13/Apr/13  Updated: 13/Apr/13  Resolved: 13/Apr/13

Status: Closed
Project: tools.reader
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Andy Fingerhut Assignee: Nicola Mometto
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File trdr-6-eliminate-reflection-with-type-hints-patch-v1.txt    
Patch: Code

 Description   

Attached patch uses type hints to eliminate several instances of reflection in the tools.reader code.

FYI, you can run 'lein check' to cause Leiningen to compile the code with warn-on-reflection true, I believe for every source file (not sure about the tests, but reflection isn't such a big deal in them anyway).



 Comments   
Comment by Nicola Mometto [ 13/Apr/13 12:13 PM ]

Andy, I don't see any patch attached to this ticket, I think you forgot to attach it.

(P.S. thanks, I didn't know about 'lein check')

Comment by Andy Fingerhut [ 13/Apr/13 12:19 PM ]

Patch trdr-6-eliminate-reflection-with-type-hints-patch-v1.txt dated Apr 13 2013 eliminates all occurrences of reflection found in latest version of tools.reader. Please check them carefully before committing them, especially the ones in default_data_readers.clj.

And I know the reflection warnings in default_data_readers.clj exist in Clojure's code, too, where you copied those from. CLJ-1080 has a patch addressing those and many other reflections within Clojure's code.

Comment by Nicola Mometto [ 13/Apr/13 12:59 PM ]

Andy, casting to char makes tools.reader crash under clojure-1.3, apparently casting to char is possible only from clojure 1.4.

Could you please submit a patch removing reflection in default_data_readers.clj and the docstring fixes only while I try to figure out a way to maintain clojure 1.3 compatibility and remove the reflection? (or if you have an idea on how to do it, you're more than welcome )

EDIT:
I edited your patch manually, and pushed a commit to type hint to char only for >clojure-1.3.0
I don't know if there is a way to avoid the reflection from clojure 1.3.0, but for the all the other versions of clojure all reflection is gone, thanks!

Comment by Andy Fingerhut [ 13/Apr/13 2:43 PM ]

Great. I was off-line there for a while, but glad you noticed the 1.3 compatibility issue where I didn't, and glad you found a different way to eliminate reflection with 1.4 and later.





[TRDR-5] Additional "Differences from LispReader.java" for README Created: 08/Apr/13  Updated: 10/Apr/13  Resolved: 10/Apr/13

Status: Closed
Project: tools.reader
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Andy Fingerhut Assignee: Nicola Mometto
Resolution: Completed Votes: 0
Labels: None


 Description   

Suggested addition to "Differences from LispReader.java" section of README.md:

read is capable of reading the symbol / with an explicit namespace, e.g. foo//, whereas clojure.lang.LispReader/read throws an exception. Refer to CLJ-873. Except for this special case, read throws an exception if a symbol contains more than one / character, whereas clojure.lang.LispReader/read allows them, returning a symbol with one or more / characters in its namespace name.



 Comments   
Comment by Nicola Mometto [ 10/Apr/13 5:20 AM ]

Thanks, fixed https://github.com/clojure/tools.reader/commit/f689cb283d1fb539a6cabbefd4036f620dabe693





[TRDR-4] Typo: Change 'end' to 'edn' in 'clojure.tools.reader.end/read-string' in README Created: 15/Mar/13  Updated: 15/Mar/13  Resolved: 15/Mar/13

Status: Closed
Project: tools.reader
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Andy Fingerhut Assignee: Nicola Mometto
Resolution: Completed Votes: 0
Labels: None


 Description   

Just a miniscule typo



 Comments   
Comment by Nicola Mometto [ 15/Mar/13 9:50 AM ]

Fixed, thanks





[TRDR-3] Make read-line available in clojure.tools.reader and/or clojure.tools.reader.edn namespace? Created: 15/Mar/13  Updated: 15/Mar/13  Resolved: 15/Mar/13

Status: Closed
Project: tools.reader
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Andy Fingerhut Assignee: Nicola Mometto
Resolution: Completed Votes: 0
Labels: None


 Description   

You mention the enhanced version of read-line that takes a reader argument in the README, but it seems like it is only available in the clojure.tools.reader.reader-types namespace. Is that intentional? Perhaps making it available in the clojure.tools.reader and/or clojure.tools.reader.edn namespace might be more convenient for the user of the library? I understand that the current implementation of read-line is Java-specific, so maybe this is the only reasonable way to expose it.

In any case, documenting the namespace in which this enhanced read-line is available in the README would be good.



 Comments   
Comment by Nicola Mometto [ 15/Mar/13 10:26 AM ]

I added more documentation on the readme, making it clear in which namespace read-line is defined.
(https://github.com/clojure/tools.reader#public-api, https://github.com/clojure/tools.reader#differences-from-lispreaderjava)

read-line definitely belongs in the c.t.r.reader-types namespace, since it works on reader-types and returns a string, so it's not a reader function.

I hope this addresses this ticket, I'm closing it, feel free to reopen it you think the doc needs to be more clear.

Comment by Andy Fingerhut [ 15/Mar/13 7:04 PM ]

Looks thoroughly documented to me – above and beyond what I would have asked for. Thanks.





[TRDR-2] Fix a few README typos Created: 14/Feb/13  Updated: 01/Aug/13  Resolved: 15/Feb/13

Status: Closed
Project: tools.reader
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Andy Fingerhut Assignee: Nicola Mometto
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File trdr-2-fix-readme-typos-patch-v1.txt    
Patch: Code

 Description   

There are a few typos in the README



 Comments   
Comment by Andy Fingerhut [ 14/Feb/13 3:16 PM ]

Nothing major here. Just a few typo fixes for the README that I found.

Comment by Nicola Mometto [ 15/Feb/13 4:19 AM ]

Thanks, applied





[TRDR-1] read-char returns nil for some input types, on first attempt to read a char Created: 12/Feb/13  Updated: 01/Aug/13  Resolved: 12/Feb/13

Status: Closed
Project: tools.reader
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Andy Fingerhut Assignee: Andy Fingerhut
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File trdr-1-correct-read-char-v1.txt    
Patch: Code

 Description   

The return value from Java's .read method is -1 for EOF. A couple of condition checks in the code appear to be reversed. See the patch.



 Comments   
Comment by Nicola Mometto [ 12/Feb/13 4:47 PM ]

Thanks, tests for reader-types are now on the TODO list





[TNS-27] c.t.n.repl/refresh fails if :main is specified in project.clj Created: 14/Nov/14  Updated: 14/Nov/14  Resolved: 14/Nov/14

Status: Resolved
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Keisuke Fukuda Assignee: Stuart Sierra
Resolution: Declined Votes: 0
Labels: None
Environment:

[org.clojure/tools.namespace "0.2.7"]
REPL-y 0.1.0-beta10
Clojure 1.6.0
Leiningen 2.0.0-preview10 on Java 1.8.0_20 Java HotSpot(TM) 64-Bit Server VM



 Description   

If ":main" is specified in a project.clj file, c.t.n.repl/refresh function fails.

Here's the procedure to reproduce the error.

$ cat ~/.lein/profiles.clj
{:user
{:dependencies [[org.clojure/tools.namespace "0.2.7"]
[spyscope "0.1.3"]
[criterium "0.4.1"]]
:injections [(require '(clojure.tools.namespace repl find))
(try (require 'spyscope.core)
(catch RuntimeException e))]
:plugins [ ]}}

$ lein new myproj
$ cd myproj
$ lein deps
$ lein repl
nREPL server started on port 52414
REPL-y 0.1.0-beta10
Clojure 1.6.0
Exit: Control+D or (exit) or (quit)
Commands: (user/help)
Docs: (doc function-name-here)
(find-doc "part-of-name-here")
Source: (source function-name-here)
(user/sourcery function-name-here)
Javadoc: (javadoc java-object-or-class-here)
Examples from clojuredocs.org: [clojuredocs or cdoc]
(user/clojuredocs name-here)
(user/clojuredocs "ns-here" "name-here")
user=> (use '[clojure.tools.namespace.repl :only (refresh)])
nil
user=> (refresh)
:reloading (myproj.core myproj.core-test)
:ok

;;; ** it works without :main in project.clj

$ vi project.clj
$ cat project.clj
(defproject myproj "0.1.0-SNAPSHOT"
:description "FIXME: write description"
:url "http://example.com/FIXME"
:license {:name "Eclipse Public License"
:url "http://www.eclipse.org/legal/epl-v10.html"}
:main myproj.core
:dependencies [[org.clojure/clojure "1.6.0"]])

$ lein repl

<snip>

myproj.core=> (use '[clojure.tools.namespace.repl :only (refresh)])
nil
myproj.core=> (refresh)
:reloading (myproj.core myproj.core-test)
:error-while-loading myproj.core-test
#<CompilerException java.lang.Exception: namespace 'myproj.core' not found, compiling:(myproj/core_test.clj:1:1)>



 Comments   
Comment by Stuart Sierra [ 14/Nov/14 10:44 AM ]

This is a known Clojure issue: reloading AOT-compiled code doesn't work.

:main in Leiningen's project.clj file triggers AOT-compilation.

Documented here: https://github.com/clojure/tools.namespace/blob/122e3d1d4fb01e4f1412d5cc7ec80dce76e8778a/README.md#warnings

Comment by Keisuke Fukuda [ 14/Nov/14 10:48 AM ]

Stuart,
I took some time to googling but couldn't reach the information. Sorry for submitting a known issue and thanks for your time.





[TNS-26] Attempted call to clojure.core/alias appears to be shadowed by local binding to name 'alias' Created: 29/Sep/14  Updated: 10/Oct/14  Resolved: 10/Oct/14

Status: Resolved
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Andy Fingerhut Assignee: Stuart Sierra
Resolution: Completed Votes: 0
Labels: None


 Description   

I discovered this while testing a new linter for Eastwood [1], not by running code, so I don't know what the run-time effect of this issue is, but I would guess it would cause an exception to be thrown if the last line of function recover-ns is ever executed.

Either that or I am really confused by the last line of function recover-ns

[1] https://github.com/jonase/eastwood



 Comments   
Comment by Stuart Sierra [ 10/Oct/14 9:59 AM ]

Good catch, thanks! Fixed in 122e3d1d

It didn't throw an exception because the local binding of alias was a symbol. When you invoke a symbol, it behaves like keywords or get, so there was no error.





[TNS-25] A few doc string typos Created: 09/Sep/14  Updated: 19/Sep/14  Resolved: 19/Sep/14

Status: Resolved
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Andy Fingerhut Assignee: Stuart Sierra
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File tns-25-v1.patch    
Patch: Code

 Description   

Attached patch corrects a few misspellings in tools.namespace doc strings



 Comments   
Comment by Stuart Sierra [ 19/Sep/14 9:06 AM ]

Applied in https://github.com/clojure/tools.namespace/commit/09e1e1fd81faed4e24399bf68bf78e87b32eb68b

Thanks.





[TNS-24] Broken tracker after mis-named namespace Created: 07/Sep/14  Updated: 11/Sep/14

Status: Open
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Stuart Sierra Assignee: Stuart Sierra
Resolution: Unresolved Votes: 0
Labels: None


 Description   

A tools.namespace dependency tracker can get into an inconsistent state after attempting to load a file with a namespace declaration that does not match the file's name.

Steps to reproduce:

In a new Clojure project, create a file foo.clj on the classpath containing the namespace declaration (ns wrong-ns-declaration-for-foo)

Then at the REPL:

user=> (use 'clojure.tools.namespace.repl)
nil
user=> (refresh)
:reloading (wrong-ns-declaration-for-foo)
:error-while-loading wrong-ns-declaration-for-foo
#<FileNotFoundException java.io.FileNotFoundException: Could not locate wrong_ns_declaration_for_foo__init.class or wrong_ns_declaration_for_foo.clj on classpath: >

Edit the file foo.clj so that its namespace declaration is correct: (ns foo)

But at the REPL, refresh still doesn't work:

user=> (refresh)
:reloading (foo wrong-ns-declaration-for-foo)
:error-while-loading wrong-ns-declaration-for-foo
#<FileNotFoundException java.io.FileNotFoundException: Could not locate wrong_ns_declaration_for_foo__init.class or wrong_ns_declaration_for_foo.clj on classpath: >

Since tools.namespace 0.2.5, the workaround is to call clear:

user=> (clear)
{}
user=> (refresh)
:reloading (foo)
:ok


 Comments   
Comment by Andy Fingerhut [ 11/Sep/14 6:38 PM ]

Such files introduce problems for all kinds of tools, not just tools.namespace. I don't know all of the consequences, but two are listed here: https://github.com/jonase/eastwood#check-consistency-of-namespace-and-file-names

I have added warnings in Eastwood whenever it finds files like this, and doesn't do any linting checks on any files until such things are corrected.





[TNS-23] Refresh breaks when changing priority of conflicting methods in `prefer-method` Created: 30/Aug/14  Updated: 07/Sep/14  Resolved: 04/Sep/14

Status: Closed
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Jeff Terrell Assignee: Stuart Sierra
Resolution: Declined Votes: 0
Labels: bug
Environment:

Clojure 1.6, CTN 0.2.5.



 Description   

Try defining a record that implements the clojure.lang.IDeref protocol. Create an instance, then try to print it, and you'll get an exception like so:

java.lang.IllegalArgumentException: Multiple methods in multimethod 'print-method' match dispatch value: class <record-class-name> -> interface clojure.lang.IPersistentMap and interface clojure.lang.IDeref, and neither is preferred

The solution to this is to include this line after the defrecord form:

(prefer-method print-method clojure.lang.IDeref clojure.lang.IPersistentMap)

This is all fine so far. The problem is that if you change the preference and call (ctn/refresh), you get an exception:

Error refreshing environment: java.lang.IllegalStateException: Preference conflict in multimethod 'print-method': interface clojure.lang.IPersistentMap is already preferred to interface clojure.lang.IDeref, compiling<source-file.clj>:164:22)



 Comments   
Comment by Stuart Sierra [ 04/Sep/14 4:18 PM ]

This is because prefer-method is a global side-effect at the top-level of a namespace.

The only way to work around this would be to call remove-method before reloading the namespace. Knowing to do that would require deep analysis far outside the scope of tools.namespace, but it can be done in application-specific code in a wrapper around refresh.

Comment by Jeff Terrell [ 05/Sep/14 2:05 PM ]

Fair enough. Thanks for looking into it.

Is this limitation worth documenting somewhere? Perhaps either in the API docs or in the c.t.n README?

Comment by Stuart Sierra [ 05/Sep/14 2:32 PM ]

Will add a note in README

Comment by Stuart Sierra [ 07/Sep/14 10:37 AM ]

Added warning to README in [commit 57e5658c8d9154711979019d85b279ad5f6898c9](https://github.com/clojure/tools.namespace/commit/57e5658c8d9154711979019d85b279ad5f6898c9).





[TNS-22] Function ns-file-name makes incorrect call to clojure.string/replace when File/separator is "\\" (e.g. Windows) Created: 24/Aug/14  Updated: 07/Sep/14  Resolved: 07/Sep/14

Status: Closed
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Andy Fingerhut Assignee: Stuart Sierra
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File tns-22-v1.patch    
Patch: Code

 Description   

On Windows, where File/separator is a string consisting of a single backslash character:

user=> (require '[clojure.string :as str])
nil

;; This works as expected, because str/replace with second arg being a string handles arbitrary strings as the third arg
user=> (str/replace "foo.bar" "." File/separator)
"foo\\bar"

;; This gives an exception, because str/replace with second arg being a regex treats backslash characters in third arg specially.
user=> (str/replace "foo.bar" #"\." File/separator)
StringIndexOutOfBoundsException String index out of range: 1  java.lang.String.charAt (String.java:658)

;; This works as expected, but is more complex than the first alternative above
user=> (str/replace "foo.bar" #"\." (str/re-quote-replacement File/separator))
"foo\\bar"

I'd recommend using str/replace calls like the first one above when the things you want to replace are constant strings.



 Comments   
Comment by Andy Fingerhut [ 24/Aug/14 7:01 PM ]

Patch tns-22-v1.patch changes the only 2 calls to clojure.string/replace I found in tools.namespace that could have their second arg be a constant string rather than a regex.

Comment by Stuart Sierra [ 07/Sep/14 11:32 AM ]

Thanks.

Patch applied in https://github.com/clojure/tools.namespace/commit/a44f709e9e0a35a0eac6ec8eb6a2845c54efb97f

Comment by Stuart Sierra [ 07/Sep/14 2:39 PM ]

In release 0.2.6





[TNS-21] tools.namespace ignores ns form dependencies inside vectors Created: 23/Aug/14  Updated: 04/Sep/14  Resolved: 04/Sep/14

Status: Closed
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Andy Fingerhut Assignee: Stuart Sierra
Resolution: Declined Votes: 0
Labels: None


 Description   

Cross reference: TLOG-13

For example, tools.logging has this ns form (metadata omitted):

(ns clojure.tools.logging
  [:use
   [clojure.string :only [trim-newline]]
   [clojure.pprint :only [code-dispatch pprint with-pprint-dispatch]]]
  [:require [clojure.tools.logging.impl :as impl]])

tools.namespace ignores these dependencies, only processing those that are in list subforms of the ns form.

Not sure if the best way to handle this is to update tools.logging or tools.namespace. tools.logging seems pretty unusual in using vectors like this, but the Clojure compiler seems to accept it just fine.



 Comments   
Comment by Stuart Sierra [ 24/Aug/14 1:29 PM ]

The ns macro docstring is quite clear that references inside it should be lists. I've seen a lot of weird ns forms, and this is the first time I've seen vectors used this way.

I would say I'm surprised that vectors work here, except nothing weird about the ns macro surprises me any more.





[TNS-20] tracker's unload order sometimes incorrect Created: 23/Aug/14  Updated: 02/Oct/14

Status: Open
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Andy Fingerhut Assignee: Stuart Sierra
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File tns-20-v1.patch     Text File tns-20-v2.patch    

 Description   

The attached patch contains a new test namespace clojure.tools.namespace.load-unload-order-test that demonstrates the incorrect unload order, if you leave out the proposed fix in file track.clj



 Comments   
Comment by Andy Fingerhut [ 23/Aug/14 3:02 PM ]

Patch tns-20-v1.patch contains a test that demonstrates the bug, and contains a proposed fix. Please scrutinize the proposed fix carefully, as I haven't yet convinced myself 100% that it is the right fix.

Also adds one more dependency check in dependency_test.clj that I discovered while reviewing those tests, to model my dependency checking tests on.

Comment by Andy Fingerhut [ 24/Aug/14 4:52 PM ]

I've got some code that checks that the load and unload orders in a tools.namespace tracker are consistent with its dependencies. It is similar to the checks done now in dependencies_test.clj, except it assumes that the dependencies in the tracker are correct and verifies the load and unload orders against those.

If you would be interested in a patch that added these checks to tools.namespace, perhaps along with a debug/consistency-check flag that when true causes these checks to be run every time a tracker is updated, let me know.

Comment by Stuart Sierra [ 07/Sep/14 11:11 AM ]

Thanks for this, Andy. I will need to study it further to convince myself this is the correct behavior.

To help, can you describe what the observed problem would be from a user's point of view?

We cannot assume that the dependencies in the tracker are always correct. I have demonstrable cases where they are wrong. Usually that takes the form of nonexistent namespaces left in the dependency graph from files that were deleted or had an invalid ns declaration.

Comment by Andy Fingerhut [ 07/Sep/14 8:10 PM ]

I have not used tools.namespace for the reload workflow that it was originally developed for, so I can't say with certainty what the effect on a user would be, but I can make some guesses that you can probably confirm more quickly than I can.

The test case in the patch is one where a few Clojure source files are added to a tracker using function c.t.n.dir/scan-all. No other changes are made to the tracker. At that point, the dependencies are completely correct, and the load order calculated from those dependencies honors them, but the unload order does not.

If that tracker were then used in a call to c.t.n.reload/track-reload, track-reload would call remove-lib on a library B before calling remove-lib on a library A, even though A requires or uses B.

I guess your question confuses me a bit. Do you believe tools.namespace should only create trackers that have load and unload orders that honor the dependencies? Or is that a wrong assumption I was making from reading the implementation?

Comment by Andy Fingerhut [ 08/Sep/14 5:52 PM ]

Attaching slightly cleaned up patch tns-20-v2.patch. Identical to tns-20-v1.patch except it avoids copying a function into the new test namespace by adding a dependency on the test namespace where it is defined.

Also updates the name of a deftest I had copied but not renamed in v1 of the patch.

Comment by Stuart Sierra [ 19/Sep/14 4:19 PM ]

I think I'm starting to get a handle on this. There's a lot going on here, and it's been more than two years since I wrote most of this code, so bear with me.

At track.clj line 78 we compute the dependency order for unloading namespaces based on the dependency graph (the local deps) as it existed before the most recent set of changes. I believe that this is correct, or at least the correct intention. If we change a file such that its dependencies are different, the order in which we unload namespaces should reflect the namespaces in memory, as they were before we changed the file.

When using c.t.n.dir/scan to detect and reload changed files this works correctly, at least most of the time.

When adding files to a new tracker for the first time, the old dependency graph is empty, so the unload order is undefined. This is arguably incorrect but effectively meaningless, since those namespaces have not yet been loaded.

In release 0.2.6, there was a bad commit which mistakenly changed c.t.n.dir/scan and scan-all to remove the files which have changed before adding them again. As a result, the dependencies of changed files were removed from the tracker's dependency graph before the unload order could be calculated, so unload order was always undefined. I have reverted that change — thanks for drawing my attention to it!

Now the unload order should be "correct" after scan or scan-all, except for the first time files are added to the tracker, when unload order is undefined.

The tracker doesn't currently have a way to distinguish between changed files which need unload+load and new files which only need load. Even if there were a way to distinguish between new and changed files, we can't be certain that a namespace has not been loaded by other means (e.g. require at the REPL) so it's safest to unload everything before reloading.

In general, unload order shouldn't matter at all. Clojure doesn't care when namespaces are removed: the Vars are still referenced from wherever they are used.

Comment by Andy Fingerhut [ 20/Sep/14 1:08 PM ]

OK, makes sense. Is it already documented anywhere that the unload order is independent of the dependencies after scan-all (and perhaps other calls)? I can create a separate ticket for that if you think it is worth adding such documentation.

It sounds like for the application I had in mind, the reverse of the load order is always defined, and is a correct unload order. You are welcome to close this ticket.

Comment by Stuart Sierra [ 26/Sep/14 9:44 AM ]

After release 0.2.7, which fixed the regression in 0.2.6, :unload order should be correct in all cases except the first time files are added to a new tracker, even with scan-all. I would appreciate it if you can confirm this in your application.

It may be possible to fix the new-tracker case too. For example, commit c0b6b93d, currently on the branch TNS-20-fix-initial-unload-order. This works for the tests in your patch.

I'm not sure if the same change is needed at track.clj line 104 as well.

Comment by Andy Fingerhut [ 02/Oct/14 5:12 PM ]

My application right now is very simple compared to the component workflow – simply use dir/scan-all to get the namespaces and their dependencies, and then print them in a particular order consistent with a correct unload order. I'm sorry, but I don't have the interest right now in testing whether the unload order is correct after doing additional operations on the tracker, since it isn't needed in my application.

I do have a sanity check in my application that confirms the load order is consistent with the dependencies in my application, and will file a bug if I ever see that trigger anything, but I don't expect there is a bug there.

I have switched to using the reverse of the load order in my application for what I believe should always be a correct unload order.

I may look into the branch change you made, but it may fall off my radar unless my needs change.





[TNS-19] deps-from-ns-decl can return nil Created: 06/Jun/14  Updated: 19/Sep/14  Resolved: 11/Jul/14

Status: Closed
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Ambrose Bonnaire-Sergeant Assignee: Stuart Sierra
Resolution: Completed Votes: 0
Labels: None


 Description   

I expect this to be #{}.

(ns-parse/deps-from-ns-decl '(ns cljs.core.typed "Internal functions for CLJS" (:refer-clojure :exclude [IFn]) (:require-macros [clojure.core.typed.bootstrap-cljs :as boot])))
;=> nil


 Comments   
Comment by Stuart Sierra [ 13/Jun/14 5:04 PM ]

Fixed in commit e9394727 on Git master. Please verify.

Comment by Stuart Sierra [ 11/Jul/14 4:42 PM ]

Merged on Git master; will be in 0.2.5 release.

Comment by Stuart Sierra [ 19/Sep/14 9:08 AM ]

Included in release 0.2.6





[TNS-18] Use linear time algorithm to calculate transitive dependencies, instead of current exponential time algorithm Created: 03/Jun/14  Updated: 19/Sep/14  Resolved: 11/Jul/14

Status: Closed
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Andy Fingerhut Assignee: Stuart Sierra
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File demo-exponential-algo.patch     Text File linear-time-no-debug.patch    
Patch: Code

 Description   

The function clojure.tools.namespace.dependency/transitive takes exponential time for cases where there are an exponential number of paths in the DAG (directed acyclic graph) of dependencies. I discovered this when using tools.namespace on the namespaces of project core.typed, where the total time for finding a topological order was several minutes on a reasonably fast machine.

It is easy to calculate transitive dependents/dependencies in linear time in the worst case (linear in the number of dependency arcs in the DAG).



 Comments   
Comment by Andy Fingerhut [ 03/Jun/14 3:18 PM ]

Patch demo-exponential-algo.patch only adds some test cases that demonstrate the exponential running time of the current algorithm for calculating transitive dependencies.

Comment by Andy Fingerhut [ 03/Jun/14 3:20 PM ]

Patch linear-time-no-debug.patch is one way to implement transitive dependencies in linear time. It also introduces additional protocol functions, which may not be desired. I am definitely open to suggestions on what kind of change you would like to see here (assuming you want any changes at all).

Comment by Stuart Sierra [ 06/Jun/14 10:27 AM ]

This is a valuable improvement — thanks, Andy!

I don't want to add more methods to the protocol, but I don't see any way to avoid it right now. Ideally, transitive-dependencies would change to take a set, but that breaks backwards-compatibility.

I'll give it a week or so to think about naming:

  • transitive-dependencies-of-node-set is too long
  • transitive-dependencies-all ?
  • transitive-dependencies-set ?
  • all-transitive-dependencies ?
Comment by Stuart Sierra [ 11/Jul/14 4:42 PM ]

Merged on master as of commit 3f946380. Will be in release 0.2.5.

New functions are named transitive-dependencies-set and transitive-dependents-set.

Comment by Stuart Sierra [ 19/Sep/14 9:08 AM ]

Included in release 0.2.6





[TNS-17] doesn't track namespace when ns decl isn't first in file Created: 04/Feb/14  Updated: 19/Sep/14  Resolved: 11/Jul/14

Status: Closed
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Trevor Wennblom Assignee: Stuart Sierra
Resolution: Completed Votes: 0
Labels: bug, enhancement


 Description   

This was first filed for ns-tracker https://github.com/weavejester/ns-tracker/issues/15 but I understand it's more specific to tools.namespace.

(I realize this is likely a very low priority, but it'd still be nice to see fixed if possible. Thanks!)


It appears ns-tracker tools.namespace will cease following a namespace when there's non-commented content before the namespace declaration.

works:

;-begin
(ns myproject.core)
(println "hi")
;-end

untracked — (any of the four possibilities when uncommented, for example):

;-begin
1
;;; (println "before ns")
;;; (println "loading" *file*)
;;; :abc
(ns myproject.core)
(println "hi")
;-end


 Comments   
Comment by Stuart Sierra [ 13/Jun/14 4:43 PM ]

What's the use case for having a file in which the ns declaration isn't the first non-comment form?

Comment by Trevor Wennblom [ 13/Jun/14 5:53 PM ]

In the case that triggered the issue for me, it can be useful to see the load-order of files based on the dependencies given in ns for debugging. That is, to print a message before and after the completion of loading the namespace dependencies.

I'm not sure how difficult or involved the issue would be to solve.

Comment by Andy Fingerhut [ 13/Jun/14 6:42 PM ]

Would the :verbose option to require perhaps give you the load-order info you want? Try something like this at a REPL, with a namespace you care about:

(require '[clojure.core.typed :as ct] :verbose)

In general, I think Stuart's concern would be questions like: How many forms would you expect tools.namespace to skip over before giving up? If it is a fixed number of forms before it gives up, why that number and not some other?

If it is based upon some other condition to give up looking for an ns form, what would that condition be?

Or would you expect it to be able to find the ns form in the body of a 'when' or 'if' form, and not at the top level?

The criteria can easily start to sound kind of arbitrary, and/or complex to implement.

Comment by Stuart Sierra [ 11/Jul/14 5:04 PM ]

Fixed on master as of commit 3c08b722. This turned out to be an easy change.

I wouldn't recommend putting anything before the ns declaration, but it's no longer required to be first. A file could also have multiple ns declarations, although clojure.tools.namespace.track will only look at the first one.

Comment by Stuart Sierra [ 19/Sep/14 9:08 AM ]

Included in release 0.2.6





[TNS-16] Some tests depend upon Clojure hash for ordering Created: 30/Jan/14  Updated: 19/Sep/14  Resolved: 31/Jan/14

Status: Closed
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Andy Fingerhut Assignee: Stuart Sierra
Resolution: Completed Votes: 0
Labels: None

Attachments: File tns-16-v1.diff    

 Description   

In particular there are several that do a topological sort of a dependency graph, and compare the returned node sequence against a single node order. In general there are many correct results for a topological sort, and the implementation's return order depends upon Clojure's hash function, recently changed.



 Comments   
Comment by Andy Fingerhut [ 30/Jan/14 9:40 AM ]

Patch tns-16-v1.diff is one way to make the tests independent of Clojure's hash function. It implements a topo-check function that determines whether a particular node sequence order is in a valid dependency order, i.e. is a subsequence of at least one topological sorting of the nodes.

Comment by Stuart Sierra [ 31/Jan/14 9:09 AM ]

Given that the current tests only use very small, hand-drawn graphs, I'm going to go with the brute-force approach for now.

But thanks for this patch. I'd like to hold on to it for possible future use with generated graphs and/or test.check.

Comment by Stuart Sierra [ 19/Sep/14 9:08 AM ]

Included in release 0.2.6





[TNS-15] Can't find files when their absolute path contains a space Created: 12/Dec/13  Updated: 10/Jan/14  Resolved: 13/Dec/13

Status: Closed
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Ryan Senior Assignee: Stuart Sierra
Resolution: Completed Votes: 0
Labels: None

Attachments: File directories-with-a-space.diff    
Patch: Code

 Description   

The URLClassloader getURLs call is returning URLs that contain %20 instead of spaces. The current code calls .getPath, which leaves the %20 in there and ultimately can't find the directory. Changing the .getPath call to .toURI calls a different java.io.File constructor and fixed the problem for me.



 Comments   
Comment by Stuart Sierra [ 13/Dec/13 9:22 AM ]

Applied in commit 8f116ba8c6944acc347cba97db0244e247021016

Comment by Stuart Sierra [ 10/Jan/14 11:26 AM ]

Mark old resolved issues as 'closed'





[TNS-14] Wasted call to str in read-ns-decl Created: 02/Dec/13  Updated: 10/Jan/14  Resolved: 02/Dec/13

Status: Closed
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Andy Fingerhut Assignee: Stuart Sierra
Resolution: Declined Votes: 0
Labels: None


 Description   

The function read-ns-decl (both copies in each of two namespaces) contain the expression:

(doto (read rdr) str)

This is equivalent to (read rdr), plus a wasted call to str whose return value is discarded.



 Comments   
Comment by Stuart Sierra [ 02/Dec/13 12:07 PM ]

The call to str was added to force the Clojure reader to throw exceptions immediately when it encounters a syntax error. See TNS-1

Comment by Andy Fingerhut [ 02/Dec/13 4:20 PM ]

Sorry for the noise. I was trying out pre-release version of Eastwood linter on as many contrib libraries as possible, and it found that odd-looking code. It didn't occur to me to look through the commit history to see when it became like it was, and why. Subtle.

Comment by Stuart Sierra [ 10/Jan/14 11:26 AM ]

Mark old resolved issues as 'closed'





[TNS-13] Refresh function not working properly Created: 17/Oct/13  Updated: 10/Jan/14  Resolved: 21/Oct/13

Status: Closed
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Pierre Allix Assignee: Stuart Sierra
Resolution: Declined Votes: 0
Labels: None
Environment:

Linux, lein 2.2.0, Clojure 1.5.1



 Description   

It seems the clojure.tools.namespace.repl/refresh functions has a problem reloading my namespaces.

user=> (refresh)
:reloading (tikkba.core lacij.model.history tikkba.utils.dom tikkba.apps.svgbrowser lacij.model.graph-history tikkba.dom lacij.utils.core lacij.view.core lacij.view.rectnodeselection lacij.view.utils.style lacij.view.utils.text lacij.view.rectnodeview lacij.view.circlenodeview lacij.view.nodeview lacij.geom.distance lacij.view.straightedgeview tikkba.utils.xml lacij.view.nodelabelview lacij.model.edge lacij.view.graphview lacij.view.segmentededgeview lacij.model.node tikkba.swing lacij.model.graph lacij.view.edgelabelview lacij.edit.graph lacij.examples.swing carneades.engine.uuid carneades.engine.utils carneades.engine.statement carneades.engine.dublin-core carneades.engine.unify carneades.engine.argument carneades.engine.argument-generator carneades.config.config carneades.engine.theory.zip carneades.engine.theory.namespace carneades.engine.theory carneades.project.admin carneades.database.db carneades.database.argument-graph carneades.web.pack lacij.examples.simple carneades.engine.argument-graph carneades.engine.translation carneades.engine.theory.translation carneades.database.export carneades.database.import carneades.policy-analysis.web.core carneades.engine.search carneades.engine.argument-evaluation carneades.engine.dung carneades.engine.caes carneades.database.evaluation carneades.engine.aspic carneades.engine.policy carneades.policy-analysis.web.logic.questions carneades.engine.dialog carneades.policy-analysis.web.controllers.reconstruction lacij.opt.annealing carneades.xml.graphml.export carneades.engine.theory.namespace-test lacij.geom.intersect lacij.layouts.core lacij.layouts.randomlayout lacij.layouts.utils.position carneades.maps.format-statement carneades.engine.triplestore carneades.web.license-analysis.model.triplestore carneades.maps.lacij-params lacij.layouts.naivelayout lacij.layouts.hierarchicallayout lacij.layouts.radiallayout carneades.maps.subset-ag lacij.layouts.layout carneades.maps.lacij-export carneades.engine.ask carneades.engine.sandbox carneades.engine.argument-builtins carneades.engine.argument-construction carneades.engine.shell carneades.policy-analysis.web.logic.askengine carneades.maps.lacij carneades.web.license-analysis.model.analysis carneades.web.license-analysis.model.debug-analysis carneades.policy-analysis.web.views.pages carneades.database.case carneades.policy-analysis.web.controllers.policy-simulation carneades.policy-analysis.web.routes carneades.web.outline carneades.web.license-analysis.routes.service carneades.web.vote carneades.policy-analysis.web.jetty carneades.xml.caf.export carneades.web.project carneades.web.info carneades.web.service carneades.analysis.web.routes-dev carneades.analysis.web.routes-selfexe tikkba.utils.test.dom lacij.edit.dynamic lacij.examples.listener carneades.engine.dnf carneades.engine.test-dnf carneades.analysis.web.routes-war carneades.policy-analysis.web.main lacij.examples.styles lacij.examples.textwrapping lacij.examples.undoredo carneades.owl.owl carneades.owl.import-axioms lacij.examples.radiallayout2 carneades.engine.argument-edit carneades.project.admin-test carneades.web.liverpool-schemes carneades.engine.owl.rule lacij.examples.circle lacij.examples.def carneades.maps.graphviz lacij.examples.hierarchicallayout2 lacij.examples.layout carneades.engine.test-evaluation carneades.engine.test-theory lacij.examples.radiallayout tikkba.examples.dynamic carneades.analysis.web.system carneades.owl.import-language carneades.owl.import carneades.engine.triplestore-test carneades.web.test.service carneades.xml.lkif.lkif tikkba.examples.output-string lacij.examples.dynamic lacij.examples.hierarchicallayout tikkba.examples.writefile tikkba.examples.image carneades.owl.import-language-test lacij.examples.display-xml lacij.examples.bug tikkba.examples.analemma carneades.engine.test-scheme lacij.test.core carneades.engine.test-unify carneades.database.test.admin carneades.engine.test-statement lacij.examples.radialloop carneades.xml.lkif.import lacij.examples.custom tikkba.examples.core tikkba.examples.svgattr lacij.examples.graphdeps user tikkba.test.core carneades.engine.owl.reasoner carneades.policy-analysis.web.logic.server-properties carneades.xml.lkif.export carneades.xml.lkif.validator)
:error-while-loading carneades.engine.argument-generator
#<Exception java.lang.Exception: No namespace: carneades.engine.argument>

user=> (require 'carneades.engine.argument)
nil
user=> (require 'carneades.engine.argument-generator)

Exception No namespace: carneades.engine.argument clojure.core/refer (core.clj:3832)
user=>

Just (require 'carneades.engine.argument-generator) from a newly started REPL works so there is a problem with refresh.

I use checkouts directory in Lein and various use and requires forms.



 Comments   
Comment by Stuart Sierra [ 17/Oct/13 11:02 AM ]

Do you have any AOT-compiled namespaces in your project?

`refresh` does not work with AOT-compiled namespaces.

Comment by Pierre Allix [ 21/Oct/13 4:20 AM ]

I had one aot namespace, which I removed. I still get one strange error:

user=> (compile 'carneades.web.service)
carneades.web.service
user=> (refresh)
:reloading (tikkba.core lacij.model.history tikkba.utils.dom tikkba.apps.svgbrowser lacij.model.graph-history tikkba.dom lacij.utils.core lacij.view.core lacij.view.rectnodeselection lacij.view.utils.style lacij.view.utils.text lacij.view.rectnodeview lacij.view.circlenodeview lacij.view.nodeview lacij.geom.distance lacij.view.straightedgeview tikkba.utils.xml lacij.view.nodelabelview lacij.model.edge lacij.view.graphview ...)
:error-while-loading carneades.web.service
#<ExceptionInInitializerError java.lang.ExceptionInInitializerError>
user=> (pst)
IllegalStateException Alias dc already exists in namespace eu.markosproject.licensing.oss-licensing-theory, aliasing carneades.engine.dublin-core
clojure.lang.Namespace.addAlias (Namespace.java:224)
clojure.core/alias (core.clj:3870)
clojure.core/load-lib (core.clj:5387)
clojure.core/apply (core.clj:619)
clojure.core/load-libs (core.clj:5413)
clojure.core/apply (core.clj:619)
clojure.core/require (core.clj:5496)
eu.markosproject.licensing.oss-licensing-theory/eval19048/loading-4910auto---19049 (oss_licensing_theory.clj:4)
eu.markosproject.licensing.oss-licensing-theory/eval19048 (oss_licensing_theory.clj:4)
clojure.lang.Compiler.eval (Compiler.java:6619)
clojure.lang.Compiler.eval (Compiler.java:6608)
clojure.lang.Compiler.load (Compiler.java:7064)
nil

The namespace eu.markosproject.licensing.oss-licensing-theory is loaded dynamically with load-file. Could this be the source of the problem?

Comment by Stuart Sierra [ 21/Oct/13 3:46 PM ]

JIRA is not a good venue for debugging problems with one specific application. If you have an isolated test case, please create a new ticket with instructions to reproduce the failure. Until then, please direct all questions to the Clojure mailing list: https://groups.google.com/forum/#!forum/clojure

To answer the latest question: yes, clojure.tools.namespace.repl/refresh is incompatible with other code which generates or loads namespaces on-the-fly.

Comment by Stuart Sierra [ 10/Jan/14 11:26 AM ]

Mark old resolved issues as 'closed'





[TNS-12] Duplicate jar-file? definition in find.clj Created: 16/Sep/13  Updated: 10/Jan/14  Resolved: 17/Sep/13

Status: Closed
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Alex Coventry Assignee: Stuart Sierra
Resolution: Completed Votes: 0
Labels: patch

Attachments: Text File 0001-Remove-duplicate-of-jar-file-fn.patch    
Patch: Code

 Description   

There are two copies of the jar-file? function in find.clj.



 Comments   
Comment by Stuart Sierra [ 17/Sep/13 7:24 AM ]

Fixed in commit 0e4fe44be8ded594d0374d4cc21d6f42a697c18e

Comment by Stuart Sierra [ 10/Jan/14 11:26 AM ]

Mark old resolved issues as 'closed'





[TNS-11] c.t.n.p/deps-from-ns-decl does not like (:use [some-root-ns module1 module2]) Created: 02/Jul/13  Updated: 10/Jan/14  Resolved: 02/Jul/13

Status: Closed
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Thomas Christensen Assignee: Stuart Sierra
Resolution: Duplicate Votes: 0
Labels: None
Environment:

Clojure 1.5.1 ; Clojure.tools.namespace 0.2.3



 Description   

I am experiencing issues using the c.t.n.r/refresh on my project.

I believe that I have tracked down the issue to the way deps-from-ns-decl parses :use; that is: it is different from Clojure's.

I have several places used ns-forms of the shape:

(ns some-root-ns.module1
       (:use [some-root-ns module2 module3])

Where Clojure would load some-root-ns.module2 and some-root-ns.module3.

However, this is what deps-from-ms-decl gets:

(clojure.tools.namespace.parse/deps-from-ns-decl '(ns some-root-ns.module1
       (:use [some-root-ns module2 module3])))
;; => #{some-root-ns}

Note that while it is of course easily fixed by using this ns-form:

(clojure.tools.namespace.parse/deps-from-ns-decl '(ns some-root-ns.module1
       (:use [some-root-ns.module2])
       (:use [some-root-ns.module3])))
;; => #{some-root-ns.module3 some-root-ns.module2}

It would probably be desirable to be equivalent with Clojure's behavior.



 Comments   
Comment by Stuart Sierra [ 02/Jul/13 7:44 AM ]

c.t.namespace.parse does not currently support all the varied argument forms that Clojure's use and require will accept.

In general, c.t.namespace.parse adheres to the docstrings of the use and require functions, which state that prefixed namespaces are written as a list, not a vector. The use and require functions do not enforce this, leading to widespread variation in usage.

This is a duplicate of TNS-9.

Until it is resolved, you can write your prefixed namespaces as a list, like:

(ns some-root-ns.module1
  (:use (some-root-ns module2 module3)))

In general, I recommend against using prefix lists in ns declarations because they are harder to identify using text-based search tools.

Comment by Stuart Sierra [ 10/Jan/14 11:26 AM ]

Mark old resolved issues as 'closed'





[TNS-10] Stack overflow on cyclic dependency Created: 15/Jun/13  Updated: 10/Jan/14  Resolved: 17/Jun/13

Status: Closed
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Ambrose Bonnaire-Sergeant Assignee: Stuart Sierra
Resolution: Completed Votes: 0
Labels: None


 Description   

If a cyclic dependency is introduced between a namespace and itself, it results in a StackOverflow when calculating transitive-dependencies.

(transitive-dependencies (-> (graph)
                             (depend 'two 'two))
                         'two)
; => StackOverflow

I believe clojure.tools.namespace.dependency/transitive should maintain a set of seen namespaces and check if we have already seen the namespace at each step.



 Comments   
Comment by Stuart Sierra [ 15/Jun/13 10:33 AM ]

Already fixed in 0.2.4-SNAPSHOT by forbidding cyclic dependencies: https://github.com/clojure/tools.namespace/commit/85e73af04fdb497db1600ef3bf21aef5ed676619

Comment by Stuart Sierra [ 10/Jan/14 11:26 AM ]

Mark old resolved issues as 'closed'





[TNS-9] Warn or error when parsing illformed prefix list Created: 14/Jun/13  Updated: 10/Jan/14  Resolved: 19/Jul/13

Status: Closed
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Ambrose Bonnaire-Sergeant Assignee: Stuart Sierra
Resolution: Completed Votes: 0
Labels: None

Attachments: File parse-fail.diff    

 Description   

Currently a ns entry (:require [foo [bar]]) where the prefix list is a vector is silently ignored.

Preferred behaviour is a warning or an error.



 Comments   
Comment by Ambrose Bonnaire-Sergeant [ 15/Jun/13 11:27 PM ]

The patch throws an exception on several ill-formed expressions.

Comment by Stuart Sierra [ 19/Jul/13 8:30 AM ]

Fixed in 0.2.4-SNAPSHOT

Commits e37221fecaf71ac0ca7e3022e3ee2cadbfbc1a45 and 930833dce8949a0154382038ac5fde85c5fb0abd

Comment by Stuart Sierra [ 10/Jan/14 11:26 AM ]

Mark old resolved issues as 'closed'





[TNS-8] move-ns changes the timestamps on files whose content doesn't change Created: 14/Jun/13  Updated: 10/Jan/14  Resolved: 19/Jul/13

Status: Closed
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Simon Katz Assignee: Stuart Sierra
Resolution: Completed Votes: 0
Labels: enhancement


 Description   

The summary says it all.
It's a minor annoyance; fixing it would be trivial.



 Comments   
Comment by Stuart Sierra [ 19/Jul/13 8:29 AM ]

Fixed in 0.2.4-SNAPSHOT

Commit 9b43f4a07e4d7264368d64a0dbbd3a2bc3b04097

Comment by Stuart Sierra [ 10/Jan/14 11:26 AM ]

Mark old resolved issues as 'closed'





[TNS-7] Stack overflow on refresh after circular dependency detected Created: 17/Apr/13  Updated: 17/Apr/13

Status: Open
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Stuart Sierra Assignee: Stuart Sierra
Resolution: Unresolved Votes: 1
Labels: None
Environment:

tools.namespace 0.2.3



 Description   
user> (refresh)
Exception Circular dependency between com.example.foo and com.example.bar  clojure.tools.namespace.dependency.MapDependencyGraph (dependency.clj:74)
user> (refresh)
StackOverflowError   clojure.lang.PersistentHashMap$BitmapIndexedNode.index (PersistentHashMap.java:576)
user> (clojure.repl/pst)
StackOverflowError 
	clojure.lang.PersistentHashMap$BitmapIndexedNode.index (PersistentHashMap.java:576)
	clojure.lang.PersistentHashMap$BitmapIndexedNode.find (PersistentHashMap.java:676)
	clojure.lang.PersistentHashMap$ArrayNode.find (PersistentHashMap.java:396)
	clojure.lang.PersistentHashMap.valAt (PersistentHashMap.java:152)
	clojure.lang.PersistentHashMap.valAt (PersistentHashMap.java:156)
	clojure.lang.RT.get (RT.java:645)
	clojure.tools.namespace.dependency/transitive (dependency.clj:52)
	clojure.tools.namespace.dependency/transitive/fn--7043 (dependency.clj:51)
	clojure.core.protocols/fn--6022 (protocols.clj:79)
	clojure.core.protocols/fn--5979/G--5974--5992 (protocols.clj:13)
	clojure.core/reduce (core.clj:6177)
	clojure.tools.namespace.dependency/transitive (dependency.clj:52)
nil





[TNS-6] Attempt to reload deleted file Created: 14/Dec/12  Updated: 14/Dec/12

Status: Open
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Stuart Sierra Assignee: Stuart Sierra
Resolution: Unresolved Votes: 1
Labels: None


 Description   

I can't identify the exact circumstances, but I have seen events where a source code file has been deleted but clojure.tools.namespace.repl/refresh still tries to reload it. Because the file doesn't exist, there's an exception when you try to load it, so you're stuck.






[TNS-5] Allow any valid .clj* source file to be parsed/analysed Created: 01/Nov/12  Updated: 07/Sep/14

Status: Open
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Max Penet Assignee: Stuart Sierra
Resolution: Unresolved Votes: 1
Labels: enhancement

Attachments: Text File e3cd6d1fa6e0c900bc1086e4a93bbc9cb343a820.patch    

 Description   

This broadens the allowed file types to anything ending with #"\.clj.?$", meaning this would work for clj, cljs, cljc, cljx and possibly other Clojure implementations with their own extension in the future.

This allows libraries such as codox (and possibly autodoc) to work with ClojureScript and others implementations without any modification.

Note: My CA is on the way, I sent it a week ago, I am not sure if it arrived yet (it was sent from Switzerland with normal mail, with an ETA of 1 week).



 Comments   
Comment by Max Penet [ 02/Nov/12 6:51 AM ]

CA received it seems (I am listed on http://clojure.org/contributing ).

Comment by Stuart Sierra [ 02/Nov/12 3:23 PM ]

I'm not sure about this. If you're only using c.t.n.find in isolation, it's fine. But if you're using code-reloading and c.t.n.repl, it could incorrectly try to reload .cljs files in JVM Clojure.

We really need [Feature Expressions]http://dev.clojure.org/display/design/Feature+Expressions or something like it to get away from multiple file extensions.

Until then, I think it has to be optional. I don't know how best to achieve this. The APIs in c.t.n.repl and c.t.n.dir are not amenable to extension. I'll think about it.

Comment by Max Penet [ 06/Nov/12 6:11 PM ]

True, I didn't realize that.

Maybe using a dynamic var to hold the regex pattern (or a predicate?) could be a reasonable solution in the meantime, this would allow to rebind it in the case of codox & similar libs.
I don't really "like" to use dynamic vars (or regexes!), but in this case it might make sense.
Not to mention it would also allow more flexibility on projects where you don't want to have your clj codoxed, but only your cljs for instance.

Comment by Max Penet [ 24/Nov/12 7:47 AM ]

Any thoughts on my last comment/edit? I don't think there is a single doc lib that works with cljs at the moment, it is a bit painful to be honest.

Comment by Stuart Sierra [ 24/Nov/12 4:25 PM ]

Yes, I think a dynamic var would be OK. However, I would like to know of a real use case, not just a potential one.

Comment by Max Penet [ 13/Dec/12 2:40 AM ]

The idea was to be able to use codox on cljs files, I tried locally but there are other problems with this approach to be able to get to cljs vars metadata. So in the end I think you were right, maybe it's better to wait for feature expressions for this.





[TNS-4] Eliminate several uses of reflection in tools.namespace Created: 28/Oct/12  Updated: 10/Jan/14  Resolved: 28/Oct/12

Status: Closed
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Andy Fingerhut Assignee: Stuart Sierra
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File tns-4-eliminate-reflection-v1.txt    

 Description   

There are several reflection warnings when compiling tools.namespace, all of which can be eliminated with fairly straightforward additions of type hints.



 Comments   
Comment by Andy Fingerhut [ 28/Oct/12 8:08 PM ]

tns-4-eliminate-reflection-v1.txt dated Oct 28 2012 adds the necessary type hints to eliminate reflection warnings in tools.namespace.

Comment by Stuart Sierra [ 28/Oct/12 9:14 PM ]

Patch applied. Thanks!

Comment by Stuart Sierra [ 10/Jan/14 11:26 AM ]

Mark old resolved issues as 'closed'





[TNS-3] refresh-dirs not used Created: 08/Oct/12  Updated: 10/Jan/14  Resolved: 08/Oct/12

Status: Closed
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Stuart Sierra Assignee: Stuart Sierra
Resolution: Completed Votes: 0
Labels: None


 Description   

Submitted by Mika Raento as a GitHub pull request: https://github.com/clojure/tools.namespace/pull/1



 Comments   
Comment by Stuart Sierra [ 08/Oct/12 4:59 PM ]

Fixed in commit bc9d5c1a6f191070a425b04feda9e2d3c2eb6928

https://github.com/clojure/tools.namespace/commit/bc9d5c1a6f191070a425b04feda9e2d3c2eb6928

Comment by Stuart Sierra [ 10/Jan/14 11:26 AM ]

Mark old resolved issues as 'closed'





[TNS-2] Add function to parse dependencies from namespace declarations Created: 16/May/12  Updated: 10/Jan/14  Resolved: 16/May/12

Status: Closed
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Stuart Sierra Assignee: Stuart Sierra
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File TNS-2-0001.patch    
Patch: Code

 Comments   
Comment by Stuart Sierra [ 10/Jan/14 11:26 AM ]

Mark old resolved issues as 'closed'





[TNS-1] Workaround to Clojure 1.2 reader bug Created: 14/Dec/11  Updated: 10/Jan/14  Resolved: 24/Apr/12

Status: Closed
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Sam Ritchie Assignee: Stuart Sierra
Resolution: Completed Votes: 2
Labels: None
Environment:

Mac OS X, Clojure 1.2.1, Leiningen 1.6.2


Attachments: Text File 0001-Workaround-to-Clojure-1.2-reader-bug.patch    
Patch: Code and Test

 Description   

The clojure 1.2 reader will allow invalid forms like {:key} to be read in, and only throw an exception on printing. Currently clojure.tools.namespace calls (read rdr) within a try form; the bug means that this particular type of error is never caught. This patch forces the reader to try and resolve with str, allowing clojure.tools.namespace to catch and bury the error.

I was running into this with moustache templates from lein-newnew on the classpath; these contain namespace headers that look like (ns name.core). This would cause (clojure.tools.namespace/find-namespaces-on-classpath) to fail when printing its results but not when actually running.



 Comments   
Comment by Sam Ritchie [ 14/Dec/11 4:55 PM ]

Funny, jira picked up the moustache markup. a bad namespace looks like (ns { { name } } . core).

Comment by Sam Ritchie [ 11/Jan/12 3:59 PM ]

Ping – Stuart, any thoughts on this?

Comment by Stuart Sierra [ 11/Jan/12 6:09 PM ]

Why should tools.ns do this? If the syntax is wrong, it's wrong.

Comment by Sam Ritchie [ 11/Jan/12 6:43 PM ]

Because without this patch, it's impossible to catch and bury errors from invalid reader syntax. I believe this comes from a bug in the reader that was fixed with 1.2.

Comment by Stuart Sierra [ 27/Jan/12 9:44 AM ]

Declined. It is not the responsibility of this library to catch errors in old versions of Clojure.

Comment by Stuart Sierra [ 24/Apr/12 2:01 PM ]

Reopening because this is still a visible issue for some libraries. I still don't like it, but I'm going to include it.

Comment by Stuart Sierra [ 24/Apr/12 2:04 PM ]

Patch applied.

Comment by Sam Ritchie [ 24/Apr/12 2:07 PM ]

Great, thanks!

Comment by Stuart Sierra [ 10/Jan/14 11:26 AM ]

Mark old resolved issues as 'closed'





[TMACRO-7] Extra paren in expansion Created: 10/Nov/14  Updated: 10/Nov/14

Status: Open
Project: tools.macro
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Oskar Kvist Assignee: Konrad Hinsen
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Clojure 1.6, tools.macro 0.1.2



 Description   

(defmacro event-handler [argsv & body]
`(reify EventHandler (~'handle ~argsv ~@body)))

(macroexpand
'(macrolet [(add-handler [& body]
`(event-handler
[~'this ~'e] ~@body))]
(add-handler 1)))

results in (notice the extra paren after `handle` and before `[this e]`):

(do (reify* [javafx.event.EventHandler] (handle ([this e] 1))))






[TMACRO-6] with-symbol-macros fails to preserve set sortedness Created: 04/Jul/14  Updated: 04/Jul/14

Status: Open
Project: tools.macro
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: François-René Rideau Assignee: Konrad Hinsen
Resolution: Unresolved Votes: 0
Labels: None
Environment:

clojure 1.6.0



 Description   

with-symbol-macros fails to preserve set sortedness of the literals it encounters in the code it has expanded. This notably causes it to improperly munge the code from the case macro.

See the full discussion on the clojure mailing-list: https://groups.google.com/forum/#!topic/clojure/HdmkjLcyqWQ






[TMACRO-5] Misplaced doc string for function protected? Created: 23/Dec/13  Updated: 23/Dec/13

Status: Open
Project: tools.macro
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Andy Fingerhut Assignee: Konrad Hinsen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File tmacro-5-v1.diff    

 Description   

Not a big deal, but I was testing a pre-release version of the Eastwood Clojure lint tool on many libraries, and found this issue among many others.



 Comments   
Comment by Andy Fingerhut [ 23/Dec/13 6:34 PM ]

Patch tmacro-5-v1.diff simply moves the doc string to the correct location.





[TMACRO-4] README.md still says 0.1.2 is latest stable release Created: 17/Nov/13  Updated: 17/Nov/13

Status: Open
Project: tools.macro
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Andy Fingerhut Assignee: Konrad Hinsen
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Perhaps that is correct, since 0.1.5 is not considered stable? If not, it would be nice to update it.






[TMACRO-3] Handling of namespaced symbols Created: 04/Sep/13  Updated: 10/Sep/13

Status: Open
Project: tools.macro
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Dmitry Groshev Assignee: Konrad Hinsen
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Here is a snippet:

(def spec-map
  {:int {:type 'ints}})

(defmacro specialize [type body]
  `(symbol-macrolet [type$ ~(-> spec-map type :type)]
     ~(w/postwalk
       (fn [form]
         (if-let [tag (-> form meta :tag)]
           (if (= tag 'type$)
             (with-meta form {:tag (-> spec-map type :type)})
             form)
           form))
       (mexpand-all body))))

(defmacro caster [x]
  `(type$ ~x))

(specialize :int
 (defn test-getter [x]
   (let [^type$ x x]
     (prn "my type" type$)
     (caster x)
     (aget x 0))))

It's broken, the error is "Unable to resolve symbol: type$ in this context". Why is it so? Let's macroexpand it:

(clojure.tools.macro/symbol-macrolet 
    [clojure.core.matrix.impl.ndarray-magic/type$ ints] 
    (def test-getter 
      (fn* ([x] (let* [x x] (prn "my type" type$) (clojure.core.matrix.impl.ndarray-magic/type$ x) (aget x 0))))))

Now, the reason is obvious: symbol-macrolet expects namespaced symbol, and in expanded form "type$" is once namespaced and once not. I think that symbol-macrolet can (should?) ignore namespacing here.



 Comments   
Comment by Konrad Hinsen [ 10/Sep/13 4:09 AM ]

After a bit of thought, I think the right solution is to treat symbol macros like ordinary symbols in evaluation. This means that global definitions (defsymbolmacro) use namespaced symbols, whereas local symbol definitions (symbol-macrolet) accept only plain symbols and raise an exception for qualified symbols, just like let does.

With those rules, the way tools.macro handles namespaces in correct code is correct, but it doesn't do the required error handling because it lets you symbol-macrolet qualified symbols.

Note also that according to those rules, your example code is not correct. You have to write

(symbol-macrolet [~'type$ ...] ...)

and

(defmacro caster [x]
  `(~'type$ ~x))

although you might then prefer to write the latter as

(defmacro caster [x]
   (list 'type$ x))

but that's a matter of taste.





[TMACRO-2] protect let-bound symbols from macrolet expansion Created: 26/Nov/12  Updated: 27/Nov/12  Resolved: 27/Nov/12

Status: Resolved
Project: tools.macro
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Tom Jack Assignee: Konrad Hinsen
Resolution: Completed Votes: 1
Labels: bug, patch

Attachments: Text File 0001-Don-t-apply-macro-expansion-to-protected-forms.patch     File macrolet-protect.diff    
Patch: Code and Test

 Description   

As discussed here: https://groups.google.com/d/msg/clojure-dev/-/UheAzkyI_WcJ

Patch macrolet-protect.diff fixes the issue and provides a test.



 Comments   
Comment by Tom Jack [ 26/Nov/12 2:55 PM ]

D'oh, patch macrolet-protect.diff has a bug — it doesn't macroexpand-1 if the symbol is protected.

Comment by Tassilo Horn [ 27/Nov/12 1:38 AM ]

This patch supersedes Tom's patch as discussed on the clojure-dev mailinglist. See

Message-ID: <CADygAw4ArNq4Z2=ZJmT6MwkBw160ShJmfQwoFEh4VOiwxfjDKQ@mail.gmail.com>

for reference.

The patch also adds letfn* as a form introducing protected symbols. That one was completely missing.

I added test cases for both let as well as letfn.

Comment by Konrad Hinsen [ 27/Nov/12 8:40 AM ]

Patch applied: https://github.com/clojure/tools.macro/commit/19c8197e10079f04e55d81c26884b9248762c2ca

Thanks!





[TMACRO-1] Preserve metadata for (at least) unexpanded forms Created: 02/Apr/12  Updated: 02/Apr/12

Status: Open
Project: tools.macro
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Stephen Compall Assignee: Konrad Hinsen
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Clojure 1.3.0, tools.macro 0.1.1, algo.monads 0.1.0



 Description   

In algo.monads:

(with-monad blah some-code...)

causes all of some-code to report as being on the line of the with-monad form. Could, perhaps, expand-all copy over metadata from the original form, at least in the common case that the form-at-point was not itself expanded?






[TLOG-13] tools.logging ns form :require and :use subforms unrecognized by tools.namespace Created: 23/Aug/14  Updated: 20/Sep/14  Resolved: 20/Sep/14

Status: Closed
Project: tools.logging
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Andy Fingerhut Assignee: Alexander Taggart
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File tlog-13-1.patch    

 Description   

Cross-reference: TNS-21

I am not sure whether this is something that should be changed in tools.logging or tools.namespace. To be sure, at most one of them should change, if either. Stuart Sierra has mentioned in TNS-21 that the ns doc string shows :require :use etc. subforms always in parentheses, and not vectors. It might be best to change tools.logging's ns form.

tools.namespace expects (:require ...) and (:use ...) subforms of the ns form to be in lists, not vectors, otherwise it ignores them.

tools.logging has ns forms that use vectors like [:require ...] and [:use ...], and are thus ignored by tools.namespace.

The Clojure compiler seems to handle these dependencies just fine, but this may be just another case where it accepts more things than it is documented to accept.



 Comments   
Comment by Andy Fingerhut [ 29/Aug/14 7:42 PM ]

Patch tlog-13-1.patch dated Aug 29 2014 simply replaces [] with () around subforms of ns forms.

Comment by Alexander Taggart [ 20/Sep/14 1:35 PM ]

https://github.com/clojure/tools.logging/commit/67d2fa8411e1f226bf1e77f3e5f6326a00e5b954





[TLOG-12] Reflection Warnings when using tools.logging Created: 30/Apr/13  Updated: 07/Jun/14  Resolved: 13/May/14

Status: Closed
Project: tools.logging
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Trevor Bernard Assignee: Alexander Taggart
Resolution: Not Reproducible Votes: 0
Labels: None

Attachments: Text File 0001-Fix-TLOG-7-Remove-reflection-warnings.patch    
Waiting On: Trevor Bernard

 Description   

A few of my projects are still reflecting using tools.logging 0.2.6 and slf4j-log4j12 "1.7.2".

lein check produces the following output:

...
Reflection warning, curator_clj/zookeeper.clj:1:1 - reference to field isTraceEnabled can't be resolved.
Reflection warning, curator_clj/zookeeper.clj:1:1 - reference to field isDebugEnabled can't be resolved.
Reflection warning, curator_clj/zookeeper.clj:1:1 - reference to field isInfoEnabled can't be resolved.
Reflection warning, curator_clj/zookeeper.clj:1:1 - reference to field isWarnEnabled can't be resolved.
Reflection warning, curator_clj/zookeeper.clj:1:1 - reference to field isErrorEnabled can't be resolved.
Reflection warning, curator_clj/zookeeper.clj:1:1 - reference to field isErrorEnabled can't be resolved.
...



 Comments   
Comment by Alexander Taggart [ 30/Apr/13 5:37 PM ]

I've made a quick test project: https://github.com/ataggart/reflect-log-test

Cloning that repo, running lein check yields no reflection warnings.

Please provide a way for me to reproduce what you see.

Comment by Trevor Bernard [ 30/Apr/13 5:46 PM ]

Forking that project and will try to find a simple test case that reproduces it.





[TLOG-11] tools.logging doesn't AOT Created: 22/Mar/13  Updated: 30/Apr/13  Resolved: 30/Apr/13

Status: Closed
Project: tools.logging
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Allen Rohner Assignee: Alexander Taggart
Resolution: Declined Votes: 0
Labels: None

Waiting On: Allen Rohner

 Description   

tools.logging does not work if used in a project that is AOT'd.

To reproduce, in a project using tools.logging:

lein jar
lein run

I can workaround it in my project by including

(when *compile-files*
  (compile 'clojure.tools.logging)
  (compile 'clojure.tools.logging.impl))


 Comments   
Comment by Alexander Taggart [ 30/Apr/13 5:11 PM ]

It's not clear to me what "does not work" means.

I've made a quick test project: https://github.com/ataggart/aot-log-test

Cloning that repo, running lein jar then lein run yields the output I would expect.

Further details appreciated.

Comment by Allen Rohner [ 30/Apr/13 6:32 PM ]

This isn't a bug; User Error.

I thought I had fully isolated the bug in my app's codebase, but I forgot about an :injection that we have that jams a debug macro into clojure.core.

for posterity, it was caused by doing

(binding [*ns* (find-ns 'clojure.core)]
  (require 'clojure.tools.logging)
  (defmacro inspect
    "prints the expression '<name> is <value>', and returns the value"
    [value]
    `(do
       (let [value# (quote ~value)
             result# ~value]
         (println value# "is" (with-out-str (clojure.pprint/pprint result#)))
         (clojure.tools.logging/infof "%s is %s" value# result#)
         result#))

Moving the require call to inside the macroexpansion resolved the bug. (And yes, I'm moving to https://github.com/dgrnbrg/spyscope now that I'm aware of it)

Sorry to waste your time, Alex.

Comment by Alexander Taggart [ 30/Apr/13 7:06 PM ]

No worries.

On a related note, do you see any utility in keeping the spy macro? It's pretty much what you're doing. Do you use inspect for logging or as an in-repl debugging tool? Perhaps spy makes more sense in tools.trace.

Comment by Allen Rohner [ 30/Apr/13 7:12 PM ]

TBH, I didn't know about spy. Also, my inspect macro predates clojure.tools.logging, and might even predate clojure.contrib.logging, if you can believe that.

I use inspect as a debugging tool. The main value to jamming it into clojure.core is that I can use it in any file, without having to stop and muck with the ns declaration. Therefore if I was going to use anything else, it would need to have that property.





[TLOG-10] Double-evaluation of arguments Created: 04/Feb/13  Updated: 04/Feb/13  Resolved: 04/Feb/13

Status: Closed
Project: tools.logging
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Chris Perkins Assignee: Alexander Taggart
Resolution: Completed Votes: 0
Labels: None
Environment:

tools.logging 0.2.5-SNAPSHOT


Attachments: File double-eval.diff    

 Description   

The expansion of the log macros can double-evaluate the first argument.

For example, the following call:

(log/info (expensive-call foo) bar)

Expands to this:

(let*
[logger_278auto_
(get-logger logger-factory #<Namespace scratch.core>)]
(if (enabled? logger_278auto_ :info)
(if (instance? java.lang.Throwable (expensive-call foo))
(log*
logger_278auto_
:info
(expensive-call foo)
(print-str bar))
(log*
logger_278auto_
:info
nil
(print-str (expensive-call foo) bar)))))

Note the calls to "expensive-call" both in the instance check and in the log* call.



 Comments   
Comment by Alexander Taggart [ 04/Feb/13 10:28 PM ]

Fixed in 0.2.6.





[TLOG-9] spy macro (or new spy macro) that accepts a message Created: 08/Dec/12  Updated: 07/Jun/14  Resolved: 07/Jun/14

Status: Closed
Project: tools.logging
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Dave Sann Assignee: Alexander Taggart
Resolution: Completed Votes: 0
Labels: None


 Description   

I think it might be useful to have spy be able to take a message to be logged in addition to the data. (or have a similar macro showing an optional message instead of the code).

(spy "doing x" (blah))
(spy :doing-x (blah) ; because it is easier to type sometimes

with spy this may conflict with the log-level.

I find this sort of function with a message useful and use it quite often. Much more often than plain logging. Generally - I would't log the code unless it logged file and line number too.



 Comments   
Comment by Alexander Taggart [ 07/Jun/14 7:10 PM ]

Added spyf, which takes a format string.

https://github.com/clojure/tools.logging/commit/91bc737ac37e1dbf8f4d658aa304154afac7a1dc





[TLOG-8] README.md still refers to using version 0.2.3 of tools.logging Created: 24/Nov/12  Updated: 04/Feb/13  Resolved: 04/Feb/13

Status: Closed
Project: tools.logging
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Trevor Bernard Assignee: Alexander Taggart
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File TLOG-8.patch    

 Description   

The latest version is 0.2.4 and should reflected in the README



 Comments   
Comment by Alexander Taggart [ 04/Feb/13 10:27 PM ]

Fixed in 0.2.6.





[TLOG-7] Reflection warnings Created: 16/Apr/12  Updated: 07/Jun/14  Resolved: 05/May/12

Status: Closed
Project: tools.logging
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Sean Corfield Assignee: Alexander Taggart
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-Fix-TLOG-7-Remove-reflection-warnings.patch     Text File TLOG-7.1.patch    

 Description   

Reflection warning, clojure/tools/logging.clj:270 - reference to field isTraceEnabled can't be resolved.
Reflection warning, clojure/tools/logging.clj:270 - reference to field isDebugEnabled can't be resolved.
Reflection warning, clojure/tools/logging.clj:270 - reference to field isInfoEnabled can't be resolved.
Reflection warning, clojure/tools/logging.clj:270 - reference to field isWarnEnabled can't be resolved.
Reflection warning, clojure/tools/logging.clj:270 - reference to field isErrorEnabled can't be resolved.
Reflection warning, clojure/tools/logging.clj:270 - reference to field isFatalEnabled can't be resolved.
Reflection warning, clojure/tools/logging.clj:270 - call to trace can't be resolved.
Reflection warning, clojure/tools/logging.clj:270 - call to debug can't be resolved.
Reflection warning, clojure/tools/logging.clj:270 - call to info can't be resolved.
Reflection warning, clojure/tools/logging.clj:270 - call to warn can't be resolved.
Reflection warning, clojure/tools/logging.clj:270 - call to error can't be resolved.
Reflection warning, clojure/tools/logging.clj:270 - call to fatal can't be resolved.



 Comments   
Comment by Alexander Taggart [ 05/May/12 3:26 PM ]

Adds type-hints to all implementation-wrapping fns.

Comment by Trevor Bernard [ 23/Apr/13 4:39 PM ]

A few of my projects are still reflecting using tools.logging 0.2.6 and slf4j-log4j12 "1.7.2". I've attached a patch that I've started using to fix this issue.

Comment by Trevor Bernard [ 23/Apr/13 4:41 PM ]

I'm a registered contributor to clojure.

http://clojure.org/contributing

Comment by Andy Fingerhut [ 24/Apr/13 12:33 PM ]

Trevor, since this ticket was already closed and marked as completed some time ago, it is probably best to create a new ticket and attach your patch to that. I know the issue is similar to this existing ticket, but best to have new tickets for new work.





[TLOG-6] tools.logging does not log to stdout when run from swank-clojure Created: 07/Feb/12  Updated: 07/Jun/14  Resolved: 07/Feb/12

Status: Closed
Project: tools.logging
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Trevor Bernard Assignee: Alexander Taggart
Resolution: Declined Votes: 0
Labels: None
Environment:

tools.logging "0.2.3", Mac OS X Lion, emacs 23.4, swank-clojure 1.3.4, slime 20100404



 Description   

Trying to evaluate (info "Hello, World!") doesn't print to stdout when evaluating from within emacs. It works if I `lein run` however.



 Comments   
Comment by Trevor Bernard [ 07/Feb/12 8:44 PM ]

It works when I use the inferior-lisp process but not in slime

Comment by Alexander Taggart [ 07/Feb/12 9:07 PM ]

Configuring swank/slime to work with wherever your log implementation writes its output is not a tools.logging issue.





[TLOG-5] Improvement of sample code in README.md Created: 14/Oct/11  Updated: 07/Jun/14  Resolved: 17/Oct/11

Status: Closed
Project: tools.logging
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Jingguo Yao Assignee: Alexander Taggart
Resolution: Completed Votes: 0
Labels: None


 Description   

README.md uses the following code to show the usage of tools.logging.

(ns example.core
  (:use [clojure.tools.logging :only (info error)]))

(defn divide [one two]
  (try
    (info "Calculated" (/ one two))
    (catch Exception ex
      (error ex "There was an error in calculation"))))

If log4j.rootLogger in the given log4j.properties is updated to "ERROR, A1". "(divide 10 0)" won't evaluate "(error ex "There was an error in calculation")". The reason is that INFO logging level is lower than ERROR logging level. So the info macro will not evaluate "(/ one two)" since the specific INFO logging level is not in effect.

New tools.logging users often expect "(divide 10 0)" to produce logging with ERROR level. They are confused with the above behaviour. So I suggest to make some change to the sample code. Maybe the following code:

(ns example.core
  (:use [clojure.tools.logging :only (info error)]))

(defn divide [one two]
  (try 
    (info "Calculating" one two)
    (/ one two)
    (catch Exception ex
      (error ex "There was an error in calculation"))))





[TLOG-4] Provided implementations of logging protocols fail. Created: 29/Sep/11  Updated: 07/Jun/14  Resolved: 29/Sep/11

Status: Closed
Project: tools.logging
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Blocker
Reporter: Alexander Taggart Assignee: Alexander Taggart
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File fix-broken-extend.patch    
Patch: Code and Test

 Description   

Example of error when trying to call a Logger protocol function:

java.lang.IllegalArgumentException: No implementation of method: :enabled? of protocol:
#'clojure.tools.logging.impl/Logger found for class: org.apache.log4j.Logger

The bug is the result of combining syntax-quoting with extend-type, e.g.:

`(do
   (extend-type org.apache.commons.logging.Log
     Logger
     (enabled? [logger# level#] ... )))

The syntax-quote turns the above enabled? into clojure.tools.logging/enabled? protocol function name, and then the extend-type turns that into the keyword :clojure.tools.logging/enabled?, which does not match the expected :enabled?.

Attached patch fixes this by using extend maps rather than extend-type. Also adds some tests.



 Comments   
Comment by Alexander Taggart [ 29/Sep/11 3:36 PM ]

Committed:

https://github.com/clojure/tools.logging/commit/f9cb0a7232d3a11cb7e74e4c13c3adc054e7e7a2





[TLOG-3] Move implementation-specific details out of main ns Created: 06/Jul/11  Updated: 07/Jun/14  Resolved: 07/Jul/11

Status: Closed
Project: tools.logging
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Alexander Taggart Assignee: Alexander Taggart
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File refactor-namespaces.patch    
Patch: Code and Test
Approval: Ok

 Description   

The protocols which allow the main functions/macros to adapt to implementation specific logging libraries do not need to be in the clojure.tools.logging namespace since they are not intended to be used by consumers.

Plan:

  • Move protocols into clojure.tools.logging.impl
  • Move implementation-specific code (e.g., the extentions to the protocols) into separate namespaces, e.g., clojure.tools.logging.log4j.


 Comments   
Comment by Alexander Taggart [ 07/Jul/11 6:49 PM ]

https://github.com/clojure/tools.logging/commit/42dfba84a373e52a4e80af6467e47761e6f20678





[TLOG-2] Change "log" semantics to "logger" semantics Created: 05/Jul/11  Updated: 07/Jun/14  Resolved: 06/Jul/11

Status: Closed
Project: tools.logging
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Laurent Petit Assignee: Laurent Petit
Resolution: Completed Votes: 0
Labels: None

Attachments: File tools.logging-TLOG-2-v2.diff    
Patch: Code and Test
Approval: Ok

 Description   

The relevant patch is named tools.logging-TLOG-2-v2.diff

Changes "log" semantics to "logger" semantics.

Log protocol becomes LoggerProtocol
LogFactory protocol becomes LoggerFactoryProtocol
log-factory becomes logger-factory
impl-get-log becomes impl-get-logger
log function argument name becomes logger function argument name
log# macro autosym becomes logger# macro autosym



 Comments   
Comment by Alexander Taggart [ 06/Jul/11 3:00 PM ]

Committed to master:
https://github.com/clojure/tools.logging/commit/0a5f9bd3148cf078dd0b62aece5f92a970b5aba3





[TLOG-1] Non-optimized, no-exception path for logp calls log* with wrong arity Created: 01/Jun/11  Updated: 07/Jun/14  Resolved: 01/Jun/11

Status: Closed
Project: tools.logging
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Alexander Taggart Assignee: Alexander Taggart
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File TLOG-1.patch    
Patch: Code and Test

 Description   

Per Laurent Petit



 Comments   
Comment by Alexander Taggart [ 01/Jun/11 11:34 AM ]

https://github.com/clojure/tools.logging/commit/687c1833c59ad1204c2a41cf0ed1f53be4a0fa8d





[TGEN-5] Defspec leaks :tag from args into generated code Created: 13/Dec/13  Updated: 13/Dec/13

Status: Open
Project: test.generative
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Nicola Mometto Assignee: Stuart Halloway
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-Don-t-leak-tag-from-defspec-args-into-generated-code.patch    
Patch: Code

 Description   

See http://dev.clojure.org/jira/browse/TANAL-24 and https://groups.google.com/d/msg/clojure-dev/hRZFuaiB_50/mzKLirgZWmUJ for more info






[TGEN-4] Typo in defspec docstring Created: 11/Mar/13  Updated: 11/Mar/13

Status: Open
Project: test.generative
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Ambrose Bonnaire-Sergeant Assignee: Stuart Halloway
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The docstring for `defspec` misspells "metdata".






[TGEN-3] binding *seed* does not change the basis Created: 13/Oct/12  Updated: 03/Feb/13  Resolved: 03/Feb/13

Status: Closed
Project: test.generative
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Ben Smith-Mannschott Assignee: Stuart Halloway
Resolution: Completed Votes: 0
Labels: None


 Description   

This is what I'm observing:

user=> (binding [gen/*seed* 7] (gen/uniform))
-6880819372607069048
user=> (binding [gen/*seed* 7] (gen/uniform))
5065957067589062886

This is what I'm expecting:

user=> (binding [gen/*seed* 7] (gen/uniform))
-4967725919621401576
user=> (binding [gen/*seed* 7] (gen/uniform))
-4967725919621401576

The work-around is to bind *rnd* to a new java.util.Random constructed appropriately:

(binding [gen/*rnd* (java.util.Random. 7)] (gen/uniform))

I'm not sure: Is this is a bug in the doc string of *seed*? Is the bug the very existence of *seed*? (*seed* is never used.) Should the doc-string for *rnd* suggest something like the code above as a way to get a consistent basis? Should generators provide a with-seed macro to do this for us?



 Comments   
Comment by Stuart Halloway [ 03/Feb/13 7:13 AM ]

this is fixed on master in the data.generators project





[TGEN-2] ASCII DEL (0x7f, 127) is not a printable character Created: 13/Oct/12  Updated: 14/Oct/12  Resolved: 14/Oct/12

Status: Resolved
Project: test.generative
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Ben Smith-Mannschott Assignee: Ben Smith-Mannschott
Resolution: Completed Votes: 0
Labels: None

Attachments: File ascii-127-is-not-printable.diff    
Patch: Code
Approval: Accepted

 Description   

ASCII 127 is a control character.

Dec Hex    Dec Hex    Dec Hex  Dec Hex  Dec Hex  Dec Hex   Dec Hex   Dec Hex  
  0 00 NUL  16 10 DLE  
  1 01 SOH  17 11 DC1  
  2 02 STX  18 12 DC2  
  3 03 ETX  19 13 DC3  
  4 04 EOT  20 14 DC4  
  5 05 ENQ  21 15 NAK  
  6 06 ACK  22 16 SYN  
  7 07 BEL  23 17 ETB  
  8 08 BS   24 18 CAN  
  9 09 HT   25 19 EM   
 10 0A LF   26 1A SUB  
 11 0B VT   27 1B ESC  
 12 0C FF   28 1C FS   
 13 0D CR   29 1D GS   
 14 0E SO   30 1E RS   
 15 0F SI   31 1F US                                                 127 7F DEL


 Comments   
Comment by Ben Smith-Mannschott [ 14/Oct/12 11:14 AM ]

Fixed by a73c11e3 on test.generative.





[TGEN-1] generators/shuffle violates contract of Comparable Created: 13/Oct/12  Updated: 14/Oct/12  Resolved: 14/Oct/12

Status: Resolved
Project: test.generative
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Ben Smith-Mannschott Assignee: Ben Smith-Mannschott
Resolution: Completed Votes: 0
Labels: None
Environment:

Mac OS X 10.8.2
java version "1.7.0_06"
Java(TM) SE Runtime Environment (build 1.7.0_06-b24)
Java HotSpot(TM) 64-Bit Server VM (build 23.2-b09, mixed mode)


Attachments: File fisher-yates-shuffle.diff    
Patch: Code
Approval: Accepted

 Description   

JDK 7's sort function is stricter about verifying that implementations of the comparison function actually obey their contract. It will even helpfully throw an exception when this is not the case:

java.lang.IllegalArgumentException: Comparison method violates its general contract!

The root cause is this gem:

generators.clj
(defn shuffle
      "Shuffle coll"
      [coll]
      (sort-by (fn [_] (long)) coll))

Which does not work on JDK7 since TimSort is clever enough to detect that the ordering function is behaving inconsistently, resulting in the previously mentioned exception.

Also, this is just a bad idea:

A variant of the above method that has seen some use in languages that support sorting with user-specified comparison functions is to shuffle a list by sorting it with a comparison function that returns random values. However, this is an extremely bad method: it is very likely to produce highly non-uniform distributions, which in addition depends heavily on the sorting algorithm used.

(wikipedia)



 Comments   
Comment by Ben Smith-Mannschott [ 14/Oct/12 11:13 AM ]

Fixed by 5a59bf0f on test.generative.





[TEMJVM-10] Fix line number info Created: 22/Oct/14  Updated: 22/Oct/14

Status: Open
Project: tools.emitter.jvm
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Nicola Mometto Assignee: Nicola Mometto
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Relevant tickets

http://dev.clojure.org/jira/browse/CLJ-1561
http://dev.clojure.org/jira/browse/CLJ-1568






[TEMJVM-9] Change fn name munging scheme to match the one introduced with CLJ-1330 Created: 13/Oct/14  Updated: 13/Oct/14

Status: Open
Project: tools.emitter.jvm
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Major
Reporter: Nicola Mometto Assignee: Nicola Mometto
Resolution: Unresolved Votes: 0
Labels: None





[TEMJVM-8] try expressions in statement position are broken Created: 24/Jul/14  Updated: 25/Jul/14  Resolved: 25/Jul/14

Status: Closed
Project: tools.emitter.jvm
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Nicola Mometto Assignee: Nicola Mometto
Resolution: Completed Votes: 0
Labels: None


 Description   
(eval '(ƒ [] (do
                (try
                  1
                  (catch Exception e))
                1)))
Illegal exception table range in class file clojure/tools/emitter/jvm$fn__23378
  [Thrown class java.lang.ClassFormatError]





[TEMJVM-7] lift loops in expr context into methods Created: 21/Jul/14  Updated: 24/Jul/14  Resolved: 24/Jul/14

Status: Closed
Project: tools.emitter.jvm
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Nicola Mometto Assignee: Nicola Mometto
Resolution: Completed Votes: 0
Labels: None


 Description   

see http://dev.clojure.org/jira/browse/CLJ-701






[TEMJVM-6] Bindings analysis defect Created: 28/May/14  Updated: 28/May/14  Resolved: 28/May/14

Status: Closed
Project: tools.emitter.jvm
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Reid McKenzie Assignee: Nicola Mometto
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File ERROR.txt    

 Description   

Initially encountered in the attached snippet. It seems like letfn's parallel bindings are not correctly handled by the emitter/analyzer.



 Comments   
Comment by Nicola Mometto [ 28/May/14 4:52 PM ]

fixed in the tools.analyzer collect pass: https://github.com/clojure/tools.analyzer/commit/60e271ca7066c59d04d683c7c403045d3e048669





[TEMJVM-5] Optimize constant metadata emission Created: 27/Apr/14  Updated: 27/Apr/14

Status: Open
Project: tools.emitter.jvm
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Nicola Mometto Assignee: Nicola Mometto
Resolution: Unresolved Votes: 0
Labels: None





[TEMJVM-4] Double args compilation on protocol-invoke Created: 15/Feb/14  Updated: 15/Feb/14  Resolved: 15/Feb/14

Status: Closed
Project: tools.emitter.jvm
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Nicola Mometto Assignee: Nicola Mometto
Resolution: Completed Votes: 0
Labels: None


 Description   

This leads to problems if an args is e.g. a fn



 Comments   
Comment by Nicola Mometto [ 15/Feb/14 12:11 PM ]

Fixed: https://github.com/clojure/tools.emitter.jvm/commit/e62d0a5b77f6dce1ff2bcd53cdb96ffb7349dabc





[TEMJVM-3] Optimize emission of empty collections Created: 28/Jan/14  Updated: 29/Jan/14  Resolved: 29/Jan/14

Status: Closed
Project: tools.emitter.jvm
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Nicola Mometto Assignee: Nicola Mometto
Resolution: Completed Votes: 0
Labels: None


 Comments   
Comment by Nicola Mometto [ 29/Jan/14 12:12 PM ]

https://github.com/clojure/tools.emitter.jvm/commit/b2747ef7bad4805644d24767b2af175c73ac0457





[TEMJVM-2] Fix class naming in type defs Created: 27/Jan/14  Updated: 27/Jan/14  Resolved: 27/Jan/14

Status: Closed
Project: tools.emitter.jvm
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Nicola Mometto Assignee: Nicola Mometto
Resolution: Completed Votes: 0
Labels: None


 Description   
clojure.tools.emitter.jvm> (eval '(byte-array 1))
clojure.lang.Numbers.byte_array(Ljava/lang/Object;)Ljava/lang/[B;
  [Thrown class java.lang.NoSuchMethodError]


 Comments   
Comment by Nicola Mometto [ 27/Jan/14 7:57 PM ]

fixed: https://github.com/clojure/tools.emitter.jvm/commit/af17ed4705c82407efa554b15369b3b2998c1a7d





[TEMJVM-1] Fix classloader handling Created: 23/Jan/14  Updated: 27/Jan/14  Resolved: 27/Jan/14

Status: Closed
Project: tools.emitter.jvm
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Blocker
Reporter: Nicola Mometto Assignee: Nicola Mometto
Resolution: Completed Votes: 0
Labels: None


 Description   

This currently throws:

(let [a (reify)] (fn [] a))


 Comments   
Comment by Nicola Mometto [ 27/Jan/14 1:43 PM ]

Fixed: https://github.com/clojure/tools.emitter.jvm/commit/8a5de2b884eaa5ce9b3afc44a6d50a147b528bad





[TCLI-10] potential incorrect parsing Created: 23/Jul/14  Updated: 10/Aug/14

Status: Open
Project: tools.cli
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Travis Camechis Assignee: Gareth Jones
Resolution: Unresolved Votes: 0
Labels: None


 Description   

If i have these options defined

(def cli-options [ ["-i" "--interactive" "interactive cli mode"]
["-H" "--help" "show help"]
["h" "-host host" "hostname"
["-u" "--user user" "username to login to rulegate"]])

if I enter a command as follows

myprog -h myhost -u foo

my option output looks like { :host myhost :user foo }

however if I leave the host value off
{:host -u}

It appears to be treating the -u as the value for -h. I would expect a missing argument error. Is this the correct behavior?



 Comments   
Comment by John Walker [ 10/Aug/14 3:01 PM ]

I am not able to reproduce this behavior. Would you create a gist or use code tags?





[TCLI-9] Support multiple validations Created: 08/Feb/14  Updated: 22/Apr/14

Status: Open
Project: tools.cli
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Jean Niklas L'orange Assignee: Gareth Jones
Resolution: Unresolved Votes: 0
Labels: None


 Description   

In many cases, I have input arguments where I want to perform multiple validations, and return different error messages based upon that. Would it be reasonable to have multiple validation functions, or being able to have a validation function returning nil if no errors exists, and an error string if not?

If this seems like an interesting idea, I believe that the easiest and simplest solution would be to support a single validation function returning the actual error message. The reationale for this is that multiple validations may require a complex rule system (ordering? should it short-circuit, or should it return multiple error messages?), and as such, it's better to have those rules evident in the validation function.



 Comments   
Comment by Sung Pae [ 13/Feb/14 10:35 PM ]

My apologies for this delayed response. I thought that I would be emailed on creation of new issues, but this does not appear to be the case. This is the second time this has happened, so I will clearly have to rig something up to keep on top of this board.

> In many cases, I have input arguments where I want to perform multiple validations, and return different error messages based upon that. Would it be reasonable to have multiple validation functions, or being able to have a validation function returning nil if no errors exists, and an error string if not?

> If this seems like an interesting idea, I believe that the easiest and simplest solution would be to support a single validation function returning the actual error message. The reationale for this is that multiple validations may require a complex rule system (ordering? should it short-circuit, or should it return multiple error messages?), and as such, it's better to have those rules evident in the validation function.

The single largest reason I opted against implementing validations like this was that I dislike the corresponding linguistic and syntactic awkwardness of inverting the logic:

:validate [pred "Must satisfy pred"] ;; Error message is optional

vs

:validate #(when-not (pred %) "Must satisfy pred")

The first approach looks like a standard assertion, while the second requires an explicit branch and must return a truthy value to register a failure.¹

Now, the second example is more powerful for the reasons you outline, but since the vast majority of command line option arguments are semantically simple values such as numbers, filenames, and hostnames, I chose to optimize for the common case in order to have the terser syntax.

That said, I am happy to be convinced otherwise. Could you please provide an example of an option specification where returning different error messages for different failures is significantly better than something like the following?

["o" "-option EXAMPLE"
:validate [#(and (foo? %) (bar? %) (baz? %))
"Must be a foo that is also a bar and a baz"]]

I am currently of the mind that a per-option error summary is sufficient for an advanced user interface like the command line, and becomes complete with a link to a documentation URL (or man page).

Thank you.

¹ I usually name functions of the second type `validation-errors` instead of `validate` because I find this confusing:

(when-not (validate input) "This seems awkward.")

Comment by Jean Niklas L'orange [ 14/Feb/14 9:42 AM ]

Hello Sung,

No worries about the response time.

It's not necessarily the fact that it is more verbose, just that I would like to give good, descriptive error messages back. While it's easy for each of the different errors, I think it's better to say exactly what the problem is. For instance, if I want to take in an input directory path, I want to ensure that is is a legal path, and if the file already exists, it must be a directory:

["-i" "--input-directory DIR"
 :validate 
 #(if-not (ok-path? %)
    (format "Input directory path '%s' is malformed" %)
    (let [f (File. %)]
      (if (and (.exists f) (not (.isDirectory f)))
        (format "Input directory '%s' is a file, not a directory" %))))]

However, I can understand that such functionality can be a bit over the top for a CLI library. As long as you've compared/considered the different possibilities, I am content with whatever choice you take.

Comment by Sung Pae [ 17/Feb/14 11:17 PM ]

Saying yes to this kind of enhancement should be quite easy, but I only have a single reservation: adding a second type of validation function that is a logical inverse of the existing type seems confusing.

What do you think about using exceptions to carry error messages?

https://gist.github.com/guns/9065033
(gaah I don't know how to use JIRA at all)

The existing implementation already catches Exceptions, throws away the error message, and returns nil. This is more verbose than fn -> optarg -> nil | String, but it is aligned to the current logical convention.

Comment by Sung Pae [ 22/Apr/14 12:21 AM ]

Hello Jean,

I've implemented your first suggestion (support multiple validations) and pushed it to master. I think it's a nice compromise.

From the patch header:

This is an implemenation (sic) of the first suggestion, multiple validation
functions.

The :validation entry can now have multiple entries, which are
partitioned into pairs, then split into the vector entries
:validation-fn and :validation-msg. It would be nice to have
a pluralized key name here, but we decline this for backwards
compatibility.

Non-collection values of :validate-fn and :validate-msg continue to be
supported by conditional wrapping.

Example:

    [["-f" "--file PATH"
      :parse-fn io/file
      :validate [#(.exists %) "File does not exist"
                 #(.isFile %) "Path is not a regular file"
                 #(.canRead %) "File is unreadable"]]]

Addresses TCLI-9.

https://github.com/clojure/tools.cli/commit/56ecd5f305d444bbf6dcd28f4f4adce58ebc0de4





[TCLI-8] Correct test that trivially always passes Created: 31/Dec/13  Updated: 02/Jan/14  Resolved: 02/Jan/14

Status: Resolved
Project: tools.cli
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Andy Fingerhut Assignee: Sung Pae
Resolution: Completed Votes: 0
Labels: None

Attachments: File tcli-8-v1.diff    

 Description   

The following tools.cli test always trivially passes, because (= expr) is always true:

(is (= {:verbose false}))

Found using pre-release version of Eastwood Clojure lint tool.



 Comments   
Comment by Andy Fingerhut [ 31/Dec/13 1:57 AM ]

Patch tcli-8-v1.diff corrects the test.

Comment by Sung Pae [ 02/Jan/14 1:33 AM ]

Thank you, patch applied!





[TCLI-7] summarize throws when called with an empty sequence of options Created: 17/Dec/13  Updated: 02/Jan/14  Resolved: 02/Jan/14

Status: Resolved
Project: tools.cli
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Hugo Duncan Assignee: Sung Pae
Resolution: Completed Votes: 0
Labels: None


 Description   

When calling opt-parse with an empty sequence of options, summarize throws an exception.

actual: clojure.lang.ArityException: Wrong number of args (1) passed to: core$map
 at clojure.lang.AFn.throwArity (AFn.java:437)
    clojure.lang.RestFn.invoke (RestFn.java:412)
    clojure.lang.AFn.applyToHelper (AFn.java:161)
    clojure.lang.RestFn.applyTo (RestFn.java:132)
    clojure.core$apply.invoke (core.clj:619)
    clojure.tools.cli$summarize.invoke (cli.clj:375)

https://github.com/hugoduncan/tools.cli/compare/feature;fix-summary-no-options?expand=1



 Comments   
Comment by Sung Pae [ 02/Jan/14 1:32 AM ]

Thank you, patch applied!

I am sorry for the delay; I mistakenly thought I would receive email notifications on new issues so have not been polling JIRA.

I hope to ameliorate this soon.





[TCLI-6] Merge optparse-clj and increase modularity Created: 20/Nov/13  Updated: 10/Dec/13  Resolved: 10/Dec/13

Status: Resolved
Project: tools.cli
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Sung Pae Assignee: Sung Pae
Resolution: Completed Votes: 0
Labels: enhancement


 Description   

On Sun 25 Aug 2013 at 09:05:15PM -0500, gaz jones wrote:

> Hey, i am the current maintainer of tools.cli - i have very little
> time to make any changes to it at the moment (kids ). I'm not sure
> what the process is for adding you as a developer or transferring
> ownership etc but if I'm happy to do so as I have no further plans for
> working on it.

Hello Gareth,

Sorry for delay in action. I submitted my Clojure CA that week and have
been casually awaiting confirmation ever since.

Only today have I noticed that my name has appeared on
http://clojure.org/contributing, so I suppose that is confirmation
enough.

This is my proposal for tools.cli:

  • Merge the CLI arguments lexer from github.com/guns/optparse-clj to
    support GNU option parsing conventions.

Examples:
https://github.com/guns/optparse-clj#features

guns.cli.optparse/tokenize-arguments:
https://github.com/guns/optparse-clj/blob/master/src-cljx/guns/cli/optparse.cljx#L25-74

GNU options parsing spec:
https://www.gnu.org/software/libc/manual/html_node/Argument-Syntax.html

  • Adapt tools.cli/cli to use the arguments lexer, then freeze it to
    maintain backwards compatibility.
  • Create a new function tools.cli/parse-opts, based largely on the
    design of guns.cli.optparse/parse, that supports the following
    features:
  • Granular options specification map.

Given the following setup:

(ns my.ns
...)

(def cli-options
[["s" "-server HOSTNAME" "Remote server"
:default (java.net.InetAddress/getByName \"example.com\")
:default-desc "example.com"
:parse-fn #(java.net.InetAddress/getByName %)
:assert-fn (partial instance? Inet4Address)
:assert-msg "%s is not an IPv4 host"]
[...]
])

A call to (clojure.tools.cli/compile-option-specs cli-options)
will result in the following PersistentArrayMap:

{option-id ; :server
{:short-opt String ; "-s"
:long-opt String ; "--server"
:required String ; "HOSTNAME"
:desc String ; "Remote server"
:default Object ; #<Inet4Address example.com/93.184.216.119>
:default-desc String ; "example.com"
:parse-fn IFn ; #(InetAddress/getByName %)
:assoc-fn IFn ; assoc
:assert-fn IFn ; (partial instance? Inet4Address)
:assert-msg String ; "%s is not an IPv4 host"
}}

The optspec compiler will verify uniqueness of option-id,
:short-opt, and :long-opt values and throw an AssertionError on
failure.

The optspec map is a PAM to preserve options ordering for summary
generation.

  • Customizable options summary.

tools.cli/parse-opts will return an options summary string to
the caller. Printing the summary with a banner will be the
responsibility of the caller.

The default options summary will look like:

-p, --port NUMBER 8080 Remote port
-s, --server HOSTNAME example.com Remote server
--detach Detach and run in the background
-h, --help

The above format can be changed by supplying an optional :summary-fn
flag that will receive the optspec map values from above and return
a summary string. The default summary-fn will be a public var.

This addresses TCLI-3.

  • Optional in-order options processing, with trailing options parsed
    by default.

This is necessary for managing different option sets for
subcommands. Indirectly addresses TCLI-5.

  • No runtime exceptions.

While parse-opts will throw an AssertionError for duplicate
option-id, :short-opt, and :long-opt values during compilation,
option parsing errors will no longer throw exceptions.

Instead, a map of {option-id error-string} will be provided, or nil
if there are no errors. Correspondingly, parse-opts will have the
following function prototype:

parse-opts:
[argument-seq [& option-vectors] & compiler-flags]
->
{:options PersistentArrayMap ; optspec above
:arguments PersistentVector ; non-optarg arguments
:summary String ; options summary produced by summary-fn
:errors {Keyword String} ; error messages by option
}

The expected usage of this function will look like:

(def usage-banner
"Usage: my-program [options] arg1 arg2\n\nOptions:\n")

(defn exit [status msg]
(println msg)
(System/exit status))

(defn -main [& argv]
(let [{:keys [options arguments summary errors]}
(cli/parse-opts argv cli-options :in-order true)]
(when (:help options)
(exit 0 (str usage-banner summary)))
(when (not= (count arguments) 2)
(exit 1 (str usage-banner summary)))
(when errors
(exit 1 (string/join "\n" (cons "The following errors have occured:\n"
(vals errors)))))
(apply my-program! options arguments)))

  • ClojureScript support.

github.com/guns/optparse-clj currently supports CLJS/node.js via
the cljx preprocessor and an extern file for the Closure Compiler
`process` namespace.

If this is desirable for tools.cli, a similar setup will be applied,
except that I will attempt to avoid cljx for easier hacking.

Comments are appreciated, and I am eager to amend this proposal to gain
community acceptance.

Cheers,
Sung Pae



 Comments   
Comment by Sung Pae [ 20/Nov/13 6:27 PM ]

A more nicely formatted version of the above is available here:

https://gist.github.com/guns/7573819/raw

EDIT:

There is an error in the function prototype above. The actual return value should be:

```
{:options {Keyword Object} ; {:server "my-server.com"}
:arguments PersistentVector ; non-optarg arguments
:summary String ; options summary produced by summary-fn
:errors {Keyword String} ; error messages by option
}
```

Specifically, :options will of course return a PHM of option keywords to their parsed values.

Comment by Gareth Jones [ 20/Nov/13 9:22 PM ]

Sounds good. One thing to add - there is an open issue on github to allow a single dash as an argument (https://github.com/clojure/tools.cli/issues/17) e.g. to indicate you wish to use stdin to read from. This seems to be used as a convention in several *nix cli utilities (such as tar for example). I was intending on adding a patch to support that by simply allowing '-' to be a valid argument name. If you can work that (or something equivalent) into your changes too that would be great!

Thanks,

Gareth

Comment by Sung Pae [ 22/Nov/13 3:57 PM ]

Sorry for the delayed response; I am finding JIRA a bit confusing.

In my experience with options parsers from other languages, '-' as
stdin is typically left to the discretion of the caller as there are
situations where you might want '-' to be a normal argument (e.g. for
describing arithmetic expressions or ranges).

Therefore I believe the proper answer to issue 17 is for the user to
call

`(replace {"-" *in*} rest-args)`

on the remaining arguments vector, and not to bake this behavior into
tools.cli.

I will be happy to make this argument to kyptin on Github if you find
this reasoning persuasive.

Meanwhile, I will wait patiently for someone on the core team to respond
to your post on clojure-dev.

Thank you!

Sung Pae

Comment by Sung Pae [ 10/Dec/13 1:20 PM ]

clojure.tools.cli/parse-opts completes the conceptual merge of optparse-clj.





[TCLI-5] Optionally allow unkown options Created: 25/Oct/13  Updated: 02/Jan/14  Resolved: 02/Jan/14

Status: Resolved
Project: tools.cli
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Hugo Duncan Assignee: Sung Pae
Resolution: Declined Votes: 0
Labels: None

Attachments: File allow_unkown_options    

 Description   

When building a cli with subcommands, it can be useful to allow passing through unkown options as arguments, so they can be processed in a subcommand.

The current API doesn't allow for passing any configuration to the `cli` command, so this adds an `allow-unknown-opts?` var, which when bound to true, will pass through unknown options.



 Comments   
Comment by Hugo Duncan [ 31/Oct/13 8:03 AM ]

A patched version of tools.cli is available on clojars as `[com.palletops/tools.cli "0.2.4"]`

Comment by Sung Pae [ 10/Dec/13 1:50 PM ]

Hello,

In the next version of tools.cli, parse-opts accepts an :in-order option to process arguments sequentially until the first unknown token.

The README has an explanation:

https://github.com/clojure/tools.cli#in-order-processing-for-subcommands

This is not the same as passing through unknown options, but it is the convention I am familiar with in large programs with subcommands.

Is this solution satisfactory for you?

The only times I have ever implemented a pass-through option for an options parser was for programs that wrap other command line programs. In these cases, the wrapper had a possibly intersecting superset of options, and I wanted to provide access to the underlying command's options without specifying them in my wrapper.

This worked okay, but the resulting options interface was complex, and since the relationship of the wrapper and underlying command was implicit, a bit fragile.

A major problem with the superset parser was that all unknown options were assumed to require arguments. This is necessary because it is impossible to tell whether `-ab` is meant to be interpreted as `-a -b` or `-a "b"` without knowing whether `-a` requires an argument. We can work around this by requiring that pass-through options always specify option arguments with an equal sign, but now we have introduced subtle differences for different tiers of options.

In contrast to all that, the subcommand options processing style of the git program and others like it allow many intersecting sets of options to be present in a single command invocation simply by following the easily understood convention:

cmd [cmd-opts] subcmd1 [subcmd1-opts]

I hope you find this argument persuasive, but I am happy to discuss it further!

Comment by Hugo Duncan [ 17/Dec/13 9:45 AM ]

The new `:in-order` option provides a better solution.

Comment by Sung Pae [ 02/Jan/14 1:36 AM ]

Thank you for your thoughts on this.





[TCLI-4] Paramters without value. Created: 11/Sep/13  Updated: 15/Dec/13  Resolved: 15/Dec/13

Status: Resolved
Project: tools.cli
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Ivan Boldyrev Assignee: Sung Pae
Resolution: Completed Votes: 0
Labels: None


 Description   

Currently there is only one type of parameters without value: boolean parameters introduced with :flag true.

But what if parameters define actions that have to be performed, and some actions need no argument? Then presence "-no-action1" with "-action" and help message would be strange at least, if not misleading.



 Comments   
Comment by Paul Stadig [ 02/Oct/13 8:01 AM ]

This is basically the case with a help option. If you specify

["-h" "--help" "Display usage" :default false :flag true]

as an option, then you get banner like this

Switches               Default     Desc                    
 --------               -------     ----                    
 -h, --no-help, --help  false       Display usage
Comment by Sung Pae [ 10/Dec/13 1:19 PM ]

In the upcoming version of tools.cli, parse-opts does not print a default value for boolean options or options without a specified :default.

In addition, option summaries may be customized with the :summary-fn option:

cf. https://github.com/clojure/tools.cli/issues/13#issuecomment-30258137

Comment by Sung Pae [ 15/Dec/13 3:14 PM ]

I am going to close this issue as it has been addressed by 0.3.0.

Thank you!





[TCLI-3] Change contract to provide access to banner on parse error Created: 09/Feb/13  Updated: 15/Dec/13  Resolved: 15/Dec/13

Status: Resolved
Project: tools.cli
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Philip Aston Assignee: Sung Pae
Resolution: Completed Votes: 1
Labels: None


 Description   

If a user provides an invalid option, most applications would want to display the banner.

tools.cli throws an Exception with minimal information, and (banner-for) is marked private.

The exception could be replaced with one that provides access to the banner, but I think it would be better (easier to use, more idiomatic?) to change the contract so that the banner is always returned. Given the exception string is also useful, perhaps change cli to return a map with keys [:options :extra-args :banner :parse-failure]?



 Comments   
Comment by Gunnar Völkel [ 08/Mar/13 4:19 AM ]

I agree with the reporter. Throwing an exception for invalid arguments ("[...] is not a valid argument") is not appropriate since you usually want to display that error message along with the banner.

Comment by Sung Pae [ 10/Dec/13 1:26 PM ]

In the upcoming 0.3.0 release, parse-opts does not throw exceptions on parse errors, but collects them into an errors vector which the caller checks to see if parsing was successful.

parse-opts does have a :post assertion on the uniqueness of :id, :short-opt, and :long-opt entries in the compiled option maps, but these errors are meant to help development, and may even be switched off by disabling assert at compile time.

Comment by Sung Pae [ 15/Dec/13 3:15 PM ]

I am going to close this issue as it has been addressed by 0.3.0.

Thank you!





[TCLI-2] Allow caller-supplied parse-fn Created: 11/Nov/12  Updated: 06/Aug/13  Resolved: 06/Aug/13

Status: Resolved
Project: tools.cli
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Pierre-Yves Ritschard Assignee: Gareth Jones
Resolution: Completed Votes: 0
Labels: enhancement

Attachments: File TCLI-2.diff    
Patch: Code and Test

 Description   

As for :parse-fn, a function can be supplied, this
is useful for standard use-cases such as: -vv, or when
you want to build a list from values.

PR: https://github.com/clojure/tools.cli/pull/11



 Comments   
Comment by Andy Fingerhut [ 11/Nov/12 12:38 PM ]

Pierre-Yves, there are instructions for creating patches under the headings "Development" and "Adding patches" on this page: http://dev.clojure.org/display/design/JIRA+workflow

Submissions to this module do require the author to sign a CA. Instructions here: http://clojure.org/contributing

Comment by Pierre-Yves Ritschard [ 11/Apr/13 9:48 AM ]

Suggested Patch

Comment by Pierre-Yves Ritschard [ 11/Apr/13 9:48 AM ]

Hello, now that I'm a registered contributor, I attached a file as suggested in the workflow wiki

Comment by Gareth Jones [ 06/Aug/13 1:13 PM ]

Applied this patch, and will release as 0.2.4 shortly. Thanks.

Comment by Gareth Jones [ 06/Aug/13 1:13 PM ]

Applied





[TCLI-1] Do not include keys when no value provided and no :default Created: 18/May/12  Updated: 24/Aug/12  Resolved: 24/Aug/12

Status: Closed
Project: tools.cli
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Stuart Sierra Assignee: Stuart Sierra
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File TCLI-1-0001.patch     Text File TCLI-1-0002.patch    
Patch: Code and Test

 Description   

I was trying to use tools.cli in conjunction with a configuration map
loaded from a file, and use clojure.core/merge to combine the results.

This didn't work because tools.cli always uses a default value of
nil, even when no default value is specified. The nil always
overrides defaults from another source.

Example before the patch:

(def my-defaults
  {:foo 1})

(merge my-defaults
       (first (clojure.tools.cli/cli
               [] ; no arguments given
               ["--foo"]))) ; no default specified
;;=> {:foo nil}

This enhancement modifies tools.cli to completely omit arguments which
have no default specified and no value given on the command line.

After the patch, the above example returns:

;;=> {:foo 1}


 Comments   
Comment by Stuart Sierra [ 20/Jul/12 7:41 AM ]

New patch file 0002 fixes syntax error in the previous patch; updated description to show correct results.

Comment by Stuart Sierra [ 24/Aug/12 8:31 AM ]

Patch applied in commit https://github.com/clojure/tools.cli/commit/f9d92395cb788dd08cb144035a9d5fd8706d10b5





[TCHECK-54] shuffle assumes vector arg Created: 18/Nov/14  Updated: 18/Nov/14

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Steve Miner Assignee: Reid Draper
Resolution: Unresolved Votes: 0
Labels: None


 Description   

(gen/sample (gen/shuffle (range 3)))

ClassCastException clojure.lang.LazySeq cannot be cast to clojure.lang.IFn clojure.test.check.generators/swap

(gen/sample (gen/shuffle (vec (range 3)))) works



 Comments   
Comment by Steve Miner [ 18/Nov/14 9:20 AM ]

The easy fix is to change gen/swap...

(defn- swap
  [coll [i1 i2]]
  (let [v (vec coll)]
    (assoc v i2 (v i1) i1 (v i2))))

I can provide a patch if desired.

Comment by Steve Miner [ 18/Nov/14 9:25 AM ]

Or change the documentation to say that only a vector is allowed as opposed to a collection.





[TCHECK-53] property creating function similar to map as alternative to for-all* Created: 17/Nov/14  Updated: 17/Nov/14

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Steve Miner Assignee: Reid Draper
Resolution: Unresolved Votes: 0
Labels: None


 Description   

As an alternative to the `for-all*` syntax, I suggest using a function signature that looks more like `map` with the predicate as the first arg and the generators as the rest of the args.

For example, these forms would all be equivalent:

(property (comp integer? *) gen/int gen/int gen/int) ;; proposed style

(for-all* [gen/int gen/int gen/int] (comp integer? *)) ;; current style



 Comments   
Comment by Steve Miner [ 17/Nov/14 8:20 AM ]

It's easy enough to write for myself. I'll make a patch if you want it.

(defn property 
    ([pred gen] (prop/for-all* [gen] pred))
    ([pred gen1 gen2] (prop/for-all* [gen1 gen2] pred))
    ([pred gen1 gen2 & more] 
        (prop/for-all* (list* gen1 gen2 more) pred)))




[TCHECK-52] defspec in 0.6.0 doesn't allow symbolic options Created: 15/Nov/14  Updated: 17/Nov/14  Resolved: 17/Nov/14

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Steve Miner Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File TCHECK-52-fix-defspec-options.patch    
Patch: Code and Test

 Description   

The new 0.6.0 release has a `defspec` that no longer allows symbolic or calculated options. Basically, the macro was improperly defined to require literals as options. In previous releases, the number of trials for a `defspec` could be a regular Clojure expression that was evaluated at runtime. My project broke on the new test.check because I was using my own var to define the number of trials for all my defspecs. Easy enough to work around once I understood the issue, but it's still needlessly backward incompatible.



 Comments   
Comment by Steve Miner [ 15/Nov/14 6:54 PM ]

fix for defspec

Comment by Steve Miner [ 15/Nov/14 6:55 PM ]

patch and test attached

Comment by Reid Draper [ 15/Nov/14 11:09 PM ]

Thanks Steve, this is definitely a bug. I'll review the patch in the morning, and get a 0.6.1 release out ASAP.

Comment by Steve Miner [ 16/Nov/14 5:55 AM ]

clean up patch

Comment by Reid Draper [ 16/Nov/14 12:28 PM ]

It will probably take some time to make it up to maven central, but 0.6.1 has been released with this patch. Thanks!

Comment by Steve Miner [ 16/Nov/14 4:01 PM ]

version 0.6.1 fixed my problem. Thanks.

Comment by Reid Draper [ 17/Nov/14 10:21 PM ]

Fixed in 30673274b035d78163efa83878515ced67eee474.





[TCHECK-51] Add set generator Created: 12/Nov/14  Updated: 15/Nov/14

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Daniel Compton Assignee: Reid Draper
Resolution: Unresolved Votes: 0
Labels: None


 Description   

test.check has generators for maps, vectors, and lists. Can we also add sets as a generator with the same arguments as vector?



 Comments   
Comment by Reid Draper [ 15/Nov/14 1:53 PM ]

There are a few subtleties to adding a set generator with the same arguments as vector, particularly, asking for a particular size set. For example, what should the set generator do with the following arguments: gen/set gen/boolean 5? The domain of the boolean generator is only two elements, yet we've asked for a set with 5 elements. A vector can have duplicates, so this is no problem.

In the meantime, a set can be created:

(defn set-gen
  [vector-gen]
  (gen/fmap set vector-gen))




[TCHECK-50] Sampling one in the obvious way could return a more random value Created: 06/Nov/14  Updated: 06/Nov/14

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Jessica Kerr Assignee: Reid Draper
Resolution: Unresolved Votes: 0
Labels: generator


 Description   

To get values from a generator I need to call gen/sample. This returns a sequence of values of increasing size, starting from 0.

From the outside, the obvious way to get one value from a generator is (first (gen/sample g)). This always produces a value of generator's size 0. If it's a vector or string, it's empty. If it's an integer, it's 0.

This is not documented or obvious.

There are many uses for generators outside of testing. At work we use them to populate test databases. And for example tests that are in the process of becoming property tests but not there yet. Let's make that harder to screw up.

suggestions: 1) specify this in docs around sample
2) either reverse sample OR
3) create a sample-one function that provides one value with a size like the default max size in checked properties (currently 200).



 Comments   
Comment by Jessica Kerr [ 06/Nov/14 5:09 PM ]

didn't mean to make it Major. Three attempts to resubmit later...

Comment by Andy Fingerhut [ 06/Nov/14 11:59 PM ]

Jessica, you only get one chance to pick the ticket attributes when you create one, unless you have been granted permission to edit them. This permission is given to people who have signed a Clojure Contributor Agreement: http://clojure.org/contributing

No worries, though. Those responding to tickets can easily modify them if they wish.





[TCHECK-49] StackOveflowError when generating large vectors Created: 03/Nov/14  Updated: 17/Nov/14  Resolved: 17/Nov/14

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: James Aley Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None
Environment:

java version "1.7.0_71"
OpenJDK Runtime Environment (IcedTea 2.5.3) (Arch Linux build 7.u71_2.5.3-1-x86_64)
OpenJDK 64-Bit Server VM (build 24.65-b04, mixed mode)

Clojure 1.6.0
test.check 0.5.9


Attachments: Text File TCHECK-49-p1.patch    

 Description   

If you attempt to generate a vector with tens of thousands of elements, you'll get a StackOverflowError:

(gen/sample (gen/vector gen/int 1 20000))
;; StackOverflowError   clojure.test.check.generators/gen-bind/fn--10435 (generators.clj:77)

Because it's fun to be meta, here's a test.check test to reproduce it

(defspec does-not-overflow 1000
  (prop/for-all [max (gen/fmap (partial apply *)
                               (gen/tuple gen/s-pos-int gen/s-pos-int))]
                (< 0 (count (gen/sample (gen/vector gen/int 1 max))))))

I'm generating the the max parameter with that product, because it seems gen/s-pos-int doesn't actually generate large enough values to produce the error by itself.

Further, I'm actually just looking for a way to generate two things:

  • Byte arrays under 250kb
  • Bute arrays over 250kb

My app has different expected behaviour for each case. I was going about this by generating vectors and fmapping byte-array over it, but hit this overflow issue. I couldn't see a nice way to just generate data with the given size constraints directly. Perhaps I missed something? This is actually an issue I have quite often with test.check – integers in a particular range, strings of some minimum or maximum length, etc. Am I doing it wrong?



 Comments   
Comment by James Aley [ 03/Nov/14 12:06 PM ]

Oh - ignore my question about integers - I just found gen/choose

Comment by Reid Draper [ 08/Nov/14 2:04 PM ]

Thanks for filing this issue. First, the StackOverflow is indeed a problem. I believe I've figured out the root cause, but figuring out an acceptable fix may take some time. However, I believe I have some workarounds for you.

It sounds like your function/system has a magic number at 250Kb. I've run into some tests like this before, and what I've been able to do is to actually make this magic number parameterizable, at least for tests. During testing, could you just set the magic number to say, 250 bytes? This will work around the stack overflow issue, and make your tests both faster and less resource-intensive.

If you do end up needing to generate large values, you can get around this by simply combining several smaller generators into a large one. The stack overflow only happens when one collection generator is large. So instead of:

(def my-gen (gen/vector gen/int 10000))

try:

(def my-gen (gen/fmap flatten (gen/vector (gen/vector gen/int 100) 100)))

You're just flattening together 100 one-hundred element vectors, effectively doing the same thing.

Does this help?

Comment by Gary Fredericks [ 16/Nov/14 1:26 PM ]

I'm thinking the cleanest thing to do is to abandon the generic impl of sequence and replace it with smarter seq-of-gens->gen-of-seqs code (since that's all sequence is used for anyhow). Any objections to this?

Comment by Reid Draper [ 16/Nov/14 1:29 PM ]

I have no objection to that.

Comment by Gary Fredericks [ 16/Nov/14 1:33 PM ]

Patch is forthcoming.

Comment by James Aley [ 16/Nov/14 1:38 PM ]

Reid - sorry, the activity on this thread just reminded me that I still owe you a reply!

Yes, you're absolutely right about the magic number. I did end up refactoring the code to make it a parameter, and so the work-arounds weren't necessary.

Thanks for the great work on test.check, by the way. I'm really enjoying breaking my code with it!

Comment by Gary Fredericks [ 16/Nov/14 1:58 PM ]

Attached TCHECK-49-p1.patch.

Comment by Reid Draper [ 17/Nov/14 10:23 PM ]

Resolved in 40ccf07085bf987dd977306a3d9b69d8ce4a18ca.





[TCHECK-48] Add sublist generator Created: 28/Oct/14  Updated: 28/Oct/14

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Reid Draper Assignee: Reid Draper
Resolution: Unresolved Votes: 0
Labels: None





[TCHECK-47] tuple generator accepts bad arguments, then throws NullPointerException when sampled Created: 13/Oct/14  Updated: 28/Oct/14  Resolved: 28/Oct/14

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Alistair Roche Assignee: John Walker
Resolution: Completed Votes: 0
Labels: None
Environment:

[org.clojure/test.check "0.5.9"]
org.clojure/clojure "1.6.0"]


Attachments: Text File TCHECK-47-p1.patch    

 Description   

test-reqs.generate=> (gen/sample (gen/tuple "asdf"))

test-reqs.generate=> (print-stack-trace *e 3)
java.lang.NullPointerException:
clojure.test.check.generators/gen-bind/fn generators.clj: 77
clojure.test.check.generators/gen-bind/fn generators.clj: 79
clojure.test.check.generators/gen-bind/fn generators.clj: 77
test-reqs.generate=>



 Comments   
Comment by John Walker [ 18/Oct/14 8:33 PM ]

Bad things can be passed to all generators, and they'll do the same thing when they're sampled.

Comment by John Walker [ 18/Oct/14 9:29 PM ]

We could address this by adding :pre and :post conditions where generators are expected.

This is similar to http://dev.clojure.org/jira/browse/TCHECK-46

(defn tuple
  "Create a generator that returns a vector, whose elements are chosen
  from the generators in the same position. The individual elements shrink
  according to their generator, but the value will never shrink in count.

  Examples:

      (def t (tuple gen/int gen/boolean))
      (sample t)
      ;; => ([1 true] [2 true] [2 false] [1 false] [0 true] [-2 false] [-6 false]
      ;; =>  [3 true] [-4 false] [9 true]))
  "
  [& generators]
  {:pre [(every? generator? generators)]}
  (gen-bind (sequence gen-bind gen-pure generators)
            (fn [roses]
              (gen-pure (rose/zip core/vector roses)))))

Usage:

(ns experimental-clojure.tcheck.jira49
  (:require [clojure.test.check :as tc]
            [clojure.test.check.generators :as gen]
            [clojure.test.check.properties :as prop]))

(gen/tuple 5)
CompilerException java.lang.AssertionError: Assert failed: (every? generator? generators), compiling:(/home/john/development/experimental-clojure/src/experimental_clojure/tcheck/jira47.clj:4:55) 
Comment by Reid Draper [ 21/Oct/14 9:55 PM ]

I'd definitely be interested in a patch for this. If not, I'd be happy to add it myself, just may be a little while.

Comment by Gary Fredericks [ 21/Oct/14 10:17 PM ]

John Walker did you assign this to yourself because you're working on a patch?

I'll throw one together if not.

Comment by John Walker [ 21/Oct/14 10:25 PM ]

Yes. I don't mind if you take it, though.

Comment by Gary Fredericks [ 25/Oct/14 9:43 AM ]

Uploaded TCHECK-47-p1.patch.

Comment by John Walker [ 28/Oct/14 10:05 PM ]

Looks good to me.

Comment by Reid Draper [ 28/Oct/14 10:43 PM ]

Thanks! Applied in 8ae6a077b6b0d22d89b37c6108fbd323b53bd2d6.





[TCHECK-46] Better error message when accidentally using something else as a generator Created: 07/Oct/14  Updated: 28/Oct/14  Resolved: 28/Oct/14

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Gary Fredericks Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None


 Description   

Currently if you accidentally use something else as a generator the error is a NullPointerException:

(gen/sample 42) ;; => throws NullPointerException

This could either be fixed with explicit type checking in the base functions that take generators, or alternatively be converting those same functions into a protocol and letting the normal protocol dispatch exceptions do the work.



 Comments   
Comment by Reid Draper [ 21/Oct/14 9:57 PM ]

See TCHECK-47http://dev.clojure.org/jira/browse/TCHECK-47 too. A precondition seems like a reasonable fix.

Comment by Reid Draper [ 28/Oct/14 10:48 PM ]

Fixed in 78b7cf8b3fbf6f96c3d5c706134fb67c40dcbb7e.





[TCHECK-45] Add ns to such-that example in README.md Created: 22/Sep/14  Updated: 24/Sep/14  Resolved: 24/Sep/14

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Ryan Fowler Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None

Attachments: File add-ns-for-such-that.diff    
Patch: Code

 Description   

The clojure.test integration example is missing a "gen/" in front of the such-that function call.

The attached patch just makes it slightly easier to copy/paste the example into a project.



 Comments   
Comment by Reid Draper [ 24/Sep/14 11:23 AM ]

Thanks. I ended up just pushing a commit to use gen/not-empty, which is a convenience function. Fixed in e54646f667af10034cdc637adeb96df3948b2b2c.





[TCHECK-44] for-all should support nesting Created: 22/Sep/14  Updated: 28/Oct/14

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Michael Sperber Assignee: Reid Draper
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File for-alls-nest.diff    
Patch: Code and Test

 Description   

Haskell QuickCheck allows for-all expressions to nest. This is useful when there are dependencies between generated values. test.check should allow this, too.

Currently, nested for-alls always succeed, which is somewhat pernicious.

I've added a patch that implements this.



 Comments   
Comment by Reid Draper [ 24/Sep/14 11:32 AM ]

Thanks Michael. I appreciate the patch, but there's a few design details that could be discussed before we get to code-level detail. As a separate, but related issue, I've been wanting to implement something like your CheckResult type, but as a record with several more fields. These fields would hold things like a plain-text description of any errors found, statistics (ala QuickCheck's collect, classify, etc.). I'd also like to write a protocol that allows basic types like booleans to be turned into this record. This would be analogous to Haskell QuickCheck's Testable Type Class. While this is technically a separate issue, I think it would behoove us to solve it in conjunction with nestable for-alls, particularly since nested for-alls can be simulated by just using bind at the generator level. Does this make sense?

Comment by Michael Sperber [ 25/Sep/14 2:19 AM ]

Absolutely.

I personally would start this patch, and work from there, unless you want to do things fundamentally differently rather than add more stuff.

Either way, how can I help make it happen?

Comment by Reid Draper [ 25/Sep/14 11:10 PM ]

Great question. Let me think on that and get back to you ASAP. I'd also love to make this happen soon.

Comment by Reid Draper [ 21/Oct/14 10:23 PM ]

Sorry for the delay, here's my sketch I've been working with:

diff --git a/src/main/clojure/clojure/test/check/properties.clj b/src/main/clojure/clojure/test/check/properties.clj
index 99b5222..139ae9a 100644
--- a/src/main/clojure/clojure/test/check/properties.clj
+++ b/src/main/clojure/clojure/test/check/properties.clj
@@ -8,13 +8,47 @@
 ;   You must not remove this notice, or any other, from this software.
 
 (ns clojure.test.check.properties
+  (:import clojure.test.check.generators.Generator)
   (:require [clojure.test.check.generators :as gen]))
 
+(defrecord Result [result pass? message stamps])
+
+(defprotocol ToResult
+  (to-result [a]))
+
+(extend java.lang.Object
+  ToResult
+  {:to-result (fn [b]
+               ;; not checking for caught exceptions here
+               (->Result b (not (false? b)) nil nil))})
+
+(extend nil
+  ToResult
+  {:to-result (fn [b]
+               (->Result b false nil nil))})
+
+(extend java.lang.Boolean
+  ToResult
+  {:to-result (fn [b]
+               (->Result b b nil nil))})
+
+(extend Generator
+  ToResult
+  {:to-result identity})
+
+(extend Result
+  ToResult
+  {:to-result identity})
+
+(defn message
+  [m property]
+  (assoc property :message m))
+
 (defn- apply-gen
   [function]
   (fn [args]
-    (let [result (try (apply function args) (catch Throwable t t))]
-      {:result result
+    (let [result (to-result (try (apply function args) (catch Throwable t t)))]
+      {:result (:result result)
        :function function
        :args args})))
 
@@ -29,9 +63,18 @@
   (for-all* [gen/int gen/int] (fn [a b] (>= (+ a b) a)))
   "
   [args function]
-  (gen/fmap
-    (apply-gen function)
-    (apply gen/tuple args)))
+  (gen/bind
+    (apply gen/tuple args)
+    (fn [a]
+      (let [result ((apply-gen function) a)]
+        (cond (gen/generator? result) (gen/fmap (fn [r] (println "foo") (update-in r :args #(conj % a))) result)
+              ;; NOTE: quick note to myself before I leave this code for the night,
+              ;; this :else is getting hit because we're wrapping the result
+              ;; with a {:result ...} map. Should probably do that conditionally.
+              ;; We also need two result types I think, a result to return from
+              ;; a property itself, and a reuslt that tacks the 'args' on top of this.
+              :else (do (println "bar") (gen/return result)))))
+    ))
 
 (defn binding-vars
   [bindings]
Comment by Michael Sperber [ 22/Oct/14 2:00 AM ]

Looks OK. However, it's difficult to see why that would get you more quickly where you said you want to go than my patch ...

Comment by Reid Draper [ 28/Oct/14 10:55 PM ]

Looks OK. However, it's difficult to see why that would get you more quickly where you said you want to go than my patch ...

Fair enough. Part of this it that it was easier for me to write up a sketch. The main things I'm trying to cover when supporting nested generators are making sure:

  1. We also support the upcoming ability to collect and return statistics about a test, ala collect from Haskell QuickCheck
  2. We have a sane way of returning failing tests to a user. Right now, in the :fail and :smallest keys of the returned map, we tell the user the failing arguments. They're always wrapped in at least one vector, since you may use more than one generator using prop/for-all. What do we do with nested properties? How do we distinguish between multiple generators at the 'same level', vs nested properties? Or do we not need to distinguish? Can whatever we decide to do be backward compatible?

Point being, I want to make sure we're not committing ourselves to nested-properties until we have some of those answers, and for me personally, it's easier to try and play with these things together, and see how they will fit together.





[TCHECK-43] Test.check lacks a generator for all floats and floats in a range Created: 17/Sep/14  Updated: 17/Sep/14

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Adam Clements Assignee: Reid Draper
Resolution: Unresolved Votes: 0
Labels: enhancement

Attachments: File float-generators.diff    
Patch: Code and Test

 Description   

Test.check lacks generators for floats and float ranges. I have implemented a float generator which shrinks to zero based on gen/ratio and also a float-in-range which is bounded by size.






[TCHECK-42] Interruptibility Created: 03/Sep/14  Updated: 21/Oct/14

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Gary Fredericks Assignee: Reid Draper
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File TCHECK-42.patch    

 Description   

Both the main test loop and the shrink loop should have checks for thread interrupts

Patch coming imminently.



 Comments   
Comment by Gary Fredericks [ 03/Sep/14 3:55 PM ]

Attached TCHECK-42.patch.

Comment by Reid Draper [ 21/Oct/14 9:54 PM ]

Thanks, sorry for the delay. Maybe this is a lein repl thing, but when I ctrl-c a test, while using this patch, I get a java.lang.ThreadDeath exception as the :result. Maybe the repl spawns your code in another thread, unlike running lein test?





[TCHECK-41] Add helpers for deprecating generators Created: 01/Sep/14  Updated: 01/Sep/14

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: John Walker Assignee: Reid Draper
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File deprecation-helpers.diff    
Patch: Code

 Description   

It should be clearly communicated to the user when generators are deprecated. This patch supplies helpers for easily deprecating generators.

def-throwing-generator receives the metadata that its generator will throw if its called. As an example, suppose deprecated-generator was deprecated in 0.5.9.

(def-throwing-generator deprecated-generator
  "deprecated-generator has been deprecated. Use current-generator instead."
  {:deprecated "0.5.9"})

Then if the user looked up its documentation, they would see from cider:

clojure.test.check.generators/deprecated-generator
Deprecated in 0.5.9
  deprecated-generator has been deprecated. Use current-generator instead.

or if they used the generator and ran cider-test, they would see:

Test Summary
foobar.core-test

Ran 1 tests, in 1 test functions
1 errors


Results

Error in a-test
FIXME, I fail.
expected: (= "a" (sample deprecated-generator 5))
  actual: clojure.lang.ExceptionInfo: deprecated-generator has been deprecated. Use current-generator instead. {:deprecated "0.5.9"}





[TCHECK-40] Typo: alpha-numeric should be alphanumeric Created: 25/Aug/14  Updated: 17/Nov/14  Resolved: 17/Nov/14

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: John Walker Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None

Attachments: File alphanumeric2.diff    
Patch: Code and Test

 Description   

This replaces all instances of alpha-numeric with alphanumeric. This includes renaming the following functions:

char-alpha-numeric -> char-alphanumeric
string-alpha-numeric -> string-alphanumeric



 Comments   
Comment by Reid Draper [ 26/Aug/14 2:22 PM ]

test.check hasn't reached 1.0, so we can break backward compatibility. However, I want to still communicate clearly to users. Maybe we can leave a definition of the hyphenated versions that raises a 'deprecated/removed' exception?

Comment by John Walker [ 27/Aug/14 10:44 AM ]

This seems reasonable. Are generators of this form alright?

(defn make-deprecated-generator
  ([msg map]
     (make-gen
      (fn [_ _]
        (throw (ex-info msg map)))))
  ([msg map cause]
     (make-gen
      (fn [_ _]
        (throw (ex-info msg map cause))))))

(def
  ^{:deprecated "TODO"}
  char-alpha-numeric
  "DEPRECATED - use char-alphanumeric instead."
  (make-deprecated-generator "DEPRECATED - use char-alphanumeric instead." {:deprecated "TODO"}))

If so, I can supply a macro to create these things. It would attach the metadata like docstrings and the deprecation version to the var, and usage might look like

(def-deprecated-generator char-alpha-numeric 
  "DEPRECATED - use char-alphanumeric instead."
  {:deprecated "TODO"})
Comment by Reid Draper [ 01/Sep/14 10:31 PM ]

I think I may be changing my mind here. I'm thinking we should have one release where the two names are both present. Make some noise about it in the release notes, and attach the :deprecated tag on the metadata. Would you mind making a patch to just add the new names, and add a ^{:deprecated ...} tag on the existing names? Sorry for the back and forth.

Comment by John Walker [ 02/Sep/14 2:11 PM ]

Thats fair. How should we handle it after that release?

Comment by John Walker [ 02/Sep/14 4:26 PM ]

This patch replaces alpha-numeric with alphanumeric, and attaches :deprecated "0.6.0" to char-alpha-numeric and string-alpha-numeric in favor of char-alphanumeric and string-alphanumeric.

Comment by Reid Draper [ 02/Sep/14 7:59 PM ]

Patch applied in 6def75ebc80cc5c1ab66ae956c76a7b402198852.





[TCHECK-39] such-that stops after 10 tries Created: 23/Aug/14  Updated: 26/Aug/14  Resolved: 26/Aug/14

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Paul Lange Assignee: Reid Draper
Resolution: Declined Votes: 0
Labels: None
Environment:

clojure 1.5.1, test.check 0.5.9



 Description   

The documentation for such-that states that "it will keep trying forever" till it finds values that satisfy f.

I use it with a prime? predicate to generate primes: (such-that prime? gen/pos-int)

This sometimes results in this error when running a tc/quick-check which contradicts the docs:
ExceptionInfo Couldn't satisfy such-that predicate after 10 tries. clojure.core/ex-info (core.clj:4327)



 Comments   
Comment by John Walker [ 25/Aug/14 5:27 PM ]

Where do you see this in the documentation?

https://github.com/clojure/test.check/blob/d6edf70cfa6cb20316f9377edf9630772ccef969/src/main/clojure/clojure/test/check/generators.clj#L268

"By default, `such-that` will try 10 times to generate a value that
satisfies the predicate. If no value passes this predicate after this number
of iterations, a runtime exception will be throw." [sic]

Comment by Paul Lange [ 26/Aug/14 2:52 AM ]

Thanks, saw it here (which seem out of date): https://clojure.github.io/test.check/clojure.test.check.generators.html#var-such-that

Comment by Reid Draper [ 26/Aug/14 2:19 PM ]

I've just updated that page. Apologies on it being out of date. Would be nice to have a little version selector thing on it, so you can know you're viewing docs for the version of test.check that you use.





[TCHECK-38] Generators for functions? Created: 15/Aug/14  Updated: 27/Aug/14

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Michael Sperber Assignee: Reid Draper
Resolution: Unresolved Votes: 0
Labels: None


 Description   

test.check has generators for first-order types, but not for functions.

In the original QuickCheck, an additional type class ("Arbitrary") handles
functions.

If I implemented this and submitted a patch, would it be likely to be included?

Background: I wrote my own Clojure QuickCheck clone here:

https://bitbucket.org/sperber/deinprogramm.quickcheck

This has arbitraries and function generators, but not all of test.check's goodies.

I'm trying to decide whether to continue maintaining my QuickCheck clone or
instead to contribute ot test.check, so feedback would be much appreciated.



 Comments   
Comment by Reid Draper [ 21/Aug/14 2:16 PM ]

I'd love a patch for this! Would you like to write up a quick implementation proposal first, so we can iron out any architectural details before looking at the code?

Comment by Michael Sperber [ 25/Aug/14 2:06 AM ]

Would you be willing to look at my QuickCheck clone as a starting point? Its general implementation approach is very similar to yours, so I think you should feel right at home.

The caveat is that it introduces a type separate from generators - the "arbitrary", corresponding to the `Arbitray' type class in the Haskell code. Doing this to `test.check' would change the API, so maybe there, the `Generator' type should be extended by an optional `transformer' field.

Comment by Reid Draper [ 26/Aug/14 2:26 PM ]

Sure, I've already read through it a bit, but need to read in more detail. How does your implementation handle printing (showing) functions? Do functions shrink? Have you seen Showing and Shrinking Functions by Claessen ?

Comment by Michael Sperber [ 27/Aug/14 2:21 AM ]

Yes. Haven't done that yet, but it would be on my list. Given that functions are objects in Clojure, I think printing should be a little easier than in Haskell.





[TCHECK-37] Make random-number generation in test.check thread-safe Created: 15/Aug/14  Updated: 05/Sep/14

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Michael Sperber Assignee: Reid Draper
Resolution: Unresolved Votes: 0
Labels: enhancement

Attachments: File your-patch-file.diff    
Patch: Code and Test

 Description   

test.check uses the Java random-number generator, which is imperative.

This makes the testing non-deterministic in many settings.

It would be better if test.check used a purely functional rng, just
like the original Haskell version of QuickCheck.

I've attached a patch that does this. All but one of the original test
pass; I've modified the only failing test to not rely on an imperative
rng.

Note that it still has quick-check/make-rng supply a random seed.
IMHO, it should supply a fixed seed.



 Comments   
Comment by Reid Draper [ 15/Aug/14 5:45 PM ]

Thanks for the patch! I haven't had a chance to look it over in detail, but I do have a few introductory questions. You're correct that test.check currently uses a mutable RNG. While it may not be ideal, I haven't run into any user-facing cases where the use is non-deterministic. Do you have some particular example in mind where test.check is not deterministic now, given the same RNG seed? One place this could be an issue in the future is if we changed the way we walk the shrink tree, which would cause different values to be generated. I'd be keen on fixing this, but it's not an issue at the moment.

tl;dr: Under what circumstances, given the same seed, is test.check non-deterministic?

Comment by Michael Sperber [ 20/Aug/14 6:50 AM ]

Thanks for the prompt answer, and sorry for taking so long to reply.

The answer to your question is: None I'm currently encountering.

My motivation for the patch was a bit different, though:

1. I don't see an easy way to make things deterministic with defspec, running tests from, say, Leiningen.

2. While I can get reproducibility specifying the seed explicitly, this complicates the testing process, which should be as smooth as possible.

3. With the purely-functional rng, the generator monad is splittable: This enables me to write deterministic tests for concurrent programs.

#1 and #2 could also be addressed with the imperative RNG, not #3, though.

Comment by Reid Draper [ 21/Aug/14 2:15 PM ]

I don't see an easy way to make things deterministic with defspec, running tests from, say, Leiningen.

Indeed, this is simply an omission in the defspec macro. Calling the quick-check function directly allows you to pass in a seed. You can see the tests for this.

While I can get reproducibility specifying the seed explicitly, this complicates the testing process, which should be as smooth as possible.

This is how Haskell QuickCheck works too. If you don't specify a seed, tests are random. Would you prefer to get the same result, even without explicitly supplying a seed?

With the purely-functional rng, the generator monad is splittable: This enables me to write deterministic tests for concurrent programs.

You can write tests for concurrent programs now. The generated values are completely constructed before your code that may use threads is called. What am I missing?

Comment by Michael Sperber [ 25/Aug/14 2:02 AM ]

Ah, I didn't realize the shipped QuickCheck does this - sorry. But yes, I would greatly prefer to get the same result, unless explicitly instructed otherwise: This would solve the `defspec' problem.

Also, some test suites will pass or fail depending on the seed - particularly those involving floating-point may need a lot of test cases before they fail, and non-reproducibility might convince you that they pass - for a while.

You can write tests for concurrent programs now.

I was being less than clear: I meant concurrent tests for concurrent programs, where the concurrency is initiated by the test itself. In that case, you may want to call `quickcheck' from different threads, and would thus like to give each its own random-number generator.

Comment by Reid Draper [ 04/Sep/14 9:00 PM ]

Just noticed that newer Haskell QuickCheck uses this random number generator now .

Comment by Reid Draper [ 05/Sep/14 1:45 PM ]

Some more discussion of the problems with the proposed implementation:





[TCHECK-36] doc typo: 'excepts' should be 'accepts' Created: 05/Aug/14  Updated: 17/Nov/14  Resolved: 17/Nov/14

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Steve Miner Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File typo-excepts-should-be-accepts.patch    
Patch: Code

 Description   

typo in doc/intro.md



 Comments   
Comment by Reid Draper [ 07/Aug/14 11:51 AM ]

Thanks! Pushed in d2f46ad2fba3e7363cc0f22f40272088f2f01eb5.





[TCHECK-35] Improve symbol and keyword generators Created: 11/Jul/14  Updated: 23/Jul/14  Resolved: 23/Jul/14

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Eric Normand Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: enhancement, generator

Attachments: File namespaced-symbols-2014-07-11-14-38.diff     File namespaced-symbols-2014-07-23.diff    
Patch: Code

 Description   

Create a generator for symbols.

Also modify the keyword generator to allow :'s (as per the spec) (http://clojure.org/reader)

Create generators for namespaced symbols and keywords, as these were not tested before.

The new generators are added to gen/any.

A patch (namespaced-symbols-2014-07-11-14-38.diff) is attached.

Also a suggestion (but not in the patch): the edn-roundtrips test should be run separately on each type in gen/any to adequately search the space for errors.



 Comments   
Comment by Reid Draper [ 22/Jul/14 8:20 PM ]

Aside from +or--digit?, are all of the rest of the definitions in this patch intended to be public? It's a little unclear to me which of these are public generators and which are just helpers.

Comment by Eric Normand [ 23/Jul/14 2:06 PM ]

This patch is the same as the previous one with the non-interface functions set as private.

Patch: namespaced-symbols-2014-07-23.diff

Comment by Reid Draper [ 23/Jul/14 7:08 PM ]

Thanks! Merged in bc1650d4136d3453a6c16a363c1d5db36ad9a6b1.





[TCHECK-34] clojure.test reporting is uninformative Created: 21/Jun/14  Updated: 11/Aug/14

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Philip Potter Assignee: Reid Draper
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The clojure.test reporting results in output like:

FAIL in (always-fail) (clojure_test.clj:18)
expected: result
  actual: false

Note that the file and line number are always clojure_test.clj:18 and the expected value is always the literal string "result".

I'm not sure what the right output would be for expected/actual, but the incorrect file and line reporting means that clojure-test-mode consistently highlights the wrong line.



 Comments   
Comment by Philip Potter [ 21/Jun/14 9:10 AM ]

I think this is because the assertion is checked by calling clojure.test/is. clojure.test/is is being called for its return value, but it has the side effect of reporting failures. It's a macro, and it takes the literal symbol "result" as its expected expression, and takes the current file and line number to attach to the failure. It's really designed to be called by user code, not library code.

Comment by John Walker [ 11/Aug/14 3:55 PM ]

Well, one solution would involve taking advantage of the report functionality.

https://github.com/clojure/clojure/blob/master/src/clj/clojure/test.clj#L204
https://github.com/clojure/clojure/blob/master/src/clj/clojure/test.clj#L371





[TCHECK-33] test.check is entangled with clojure.test Created: 21/Jun/14  Updated: 21/Jun/14

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Philip Potter Assignee: Reid Draper
Resolution: Unresolved Votes: 0
Labels: None


 Description   

test.check is tightly coupled to clojure.test; this means that it's difficult to use it with other test frameworks.

There is a dependency cycle between clojure.test.check and clojure.test.check.clojure-test.

quick-check uses clojure.test/report to print dots to the screen, meaning that quick-check necessarily depends on clojure.test



 Comments   
Comment by Philip Potter [ 21/Jun/14 8:54 AM ]

I wonder if the solution is to allow you to provide your own reporting functions to quick-check to report successes, failures, and trials? That way different frameworks can inject different behaviour as required.





[TCHECK-32] Default sizing on gen/any needs re-evaluation Created: 13/Jun/14  Updated: 04/Aug/14

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Luke VanderHart Assignee: Reid Draper
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The following innocuous-looking test blows the heap and then crashes a 4GB JVM with an out-of-memory error:

(tc/defspec merge-is-idempotent
  100
  (prop/for-all [m (gen/map gen/any gen/any)]
    (= m (merge m m))))

I understand how this happens, and how to fix it by adjusting the size parameters.

However, it would be great if using the defaults did not have the potential for such a nasty failure mode (particularly as the unwary user will have trouble determining that the fault is not in their code).



 Comments   
Comment by Philip Potter [ 21/Jun/14 8:29 AM ]

I was going to raise a separate ticket for this, but it seems like it might be related: gen/any (and any-printable) seem to take exponential time in the size of the input. See for example this session:

user> (time (dorun (gen/sample (gen/resize 50 gen/any-printable))))
"Elapsed time: 2204.284643 msecs"
nil
user> (time (dorun (gen/sample (gen/resize 55 gen/any-printable))))
"Elapsed time: 2620.717337 msecs"
nil
user> (time (dorun (gen/sample (gen/resize 60 gen/any-printable))))
"Elapsed time: 5923.636336 msecs"
nil
user> (time (dorun (gen/sample (gen/resize 65 gen/any-printable))))
"Elapsed time: 9035.762191 msecs"
nil
user> (time (dorun (gen/sample (gen/resize 70 gen/any-printable))))
"Elapsed time: 15393.687184 msecs"
nil
user> (time (dorun (gen/sample (gen/resize 75 gen/any-printable))))
"Elapsed time: 9510.571668 msecs"
nil
user> (time (dorun (gen/sample (gen/resize 80 gen/any-printable))))
"Elapsed time: 39591.543565 msecs"
nil

Apart from the anomaly at 75, adding 10 to the size seems to increase runtime by almost 3x.

Comment by Reid Draper [ 23/Jun/14 5:07 PM ]

I believe I should have a fix for this today, as well as an easier way to write custom, recursive generators.

Comment by Reid Draper [ 23/Jun/14 5:21 PM ]

This isn't fixing the OOM yet, but is making a big difference in making the size have a linear relationship with the number of elements generated, in a recursive generator. Here's branch: https://github.com/clojure/test.check/compare/feature;recursive-generator-helpers.

Comment by Reid Draper [ 23/Jun/14 7:16 PM ]

Fixed in https://github.com/clojure/test.check/commit/19ca756c95141af3fb9caa5e053b9d01120e5d7e. I'll try and get a snapshot build up tonight. Will be 0.5.9-SNAPSHOT. Check out the commit-message for a full explanation of how this now works.

Comment by Philip Potter [ 28/Jun/14 3:26 PM ]

awesome work! The new recursive-gen fn is great, too: I can immediately see a use case for generating arbitrary JSON-compatible data (ie vectors, maps, strings, numbers, bools, but no rationals, characters, symbols...).

Comment by Philip Potter [ 29/Jun/14 6:48 AM ]

Just re-tested on my machine; performance is vastly improved though still superlinear:

clojure.test.mode> (time (dorun (gen/sample (gen/resize 100 gen/any-printable))))
"Elapsed time: 101.907628 msecs"
nil
clojure.test.mode> (time (dorun (gen/sample (gen/resize 200 gen/any-printable))))
"Elapsed time: 302.341697 msecs"
nil
clojure.test.mode> (time (dorun (gen/sample (gen/resize 400 gen/any-printable))))
"Elapsed time: 1154.098163 msecs"
nil
clojure.test.mode> (time (dorun (gen/sample (gen/resize 800 gen/any-printable))))
"Elapsed time: 2954.889396 msecs"
nil
clojure.test.mode> (time (dorun (gen/sample (gen/resize 1600 gen/any-printable))))
"Elapsed time: 22335.200578 msecs"
nil

although since the default max-size is 200, this is very much no big deal.

Comment by Reid Draper [ 03/Jul/14 10:52 AM ]

I'm not too surprised that performance is still superlinear. What should be linear is the number of leaf nodes in the generated tree. We may eventually be able to do something a little more complex and have the total number of nodes be linear (including internal nodes). For now, however, it should be considered a bug if the relationship between leaf nodes doesn't grow linearly with the size parameter.

Comment by Reid Draper [ 04/Aug/14 8:35 PM ]

I'm still seeing that the original example is OOMing. I'm not sure the best way to solve that. For a single layer of nested generators, the above patch seems to be making a big difference. But when you make a map of gen/any, map doesn't know that it's arguments are themselves recursive generators. This means you might create 100 keys and values, each themselves, large recursive generators. There's a balance that has to be had between making the default recursive generators be 'large enough' by themselves, but not too large when used like this. Hmm.. I think I'll go ahead and release 0.5.9, and keep on thinking of ways to make this even better.





[TCHECK-31] Doc improvements: how to separate "size" from "range" Created: 11/Jun/14  Updated: 20/Nov/14

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Michael Nygard Assignee: Reid Draper
Resolution: Unresolved Votes: 1
Labels: None


 Description   

Size is discussed mainly in terms of recursive generators. However, it is crucial to understanding other aspects such as vector length and integer ranges.

Some examples like these would be helpful:

  • How to get a small number of samples with a large range.
  • How to get a large number of samples with a small range.


 Comments   
Comment by Reid Draper [ 12/Jun/14 9:13 AM ]

Agree. I'll try and get a patch up in the next couple of days. Do you just have something in mind going in the doc/ dir?

Comment by Brian Craft [ 12/Nov/14 12:05 AM ]

I have exactly this problem. Are there any examples from projects using test.check?

Comment by Reid Draper [ 12/Nov/14 9:12 AM ]

Brian, can you be more specific about your problem? Some helpful functions regarding sizing and domain-generation are documented here. Take a look specifically at gen/choose, gen/resize, and gen/sized.

Comment by Brian Craft [ 12/Nov/14 10:29 AM ]

I don't need to test, for example, strings that are 500 chars long, but do need to test lists of 500 strings. If I have a generator that builds lists of strings & size it larger, both the lists and strings get larger. Then I start hitting irrelevant resource limits, like db field widths, or jvm memory. It seems like I need a way to specify how different data elements grow. Just setting the range, like (gen/vector ... 0 5000), seems wrong, because then it always picks up to 5000, while I suspect for shrinking it needs to respond to "size". Something like this?

(gen/sized (fn [size] (gen/resize (* size size) (gen/vector (gen/resize size gen/int)))))

forcing the size of the vector to increase more quickly than the size of the elements in the vector.

I gave this a try, but hit the vector stack overflow problem. I will try the fmap/flatten workaround, but am wondering if this is the right approach. Also wondering about the range of "size". From the code it looks like it goes to 100 due to sample-seq. So functions of size passed to gen/sized should expect range from 0-99, and return the largest meaningful data when given size parameter 99?

Comment by Reid Draper [ 15/Nov/14 12:03 PM ]

I don't need to test, for example, strings that are 500 chars long, but do need to test lists of 500 strings. If I have a generator that builds lists of strings & size it larger, both the lists and strings get larger.

Indeed, by default, all generators will 'grow' together. And as you've correctly noticed, you can use gen/sized and gen/resize to further control this.

Just setting the range, like (gen/vector ... 0 5000), seems wrong, because then it always picks up to 5000, while I suspect for shrinking it needs to respond to "size". Something like this?

Shrinking is completely separate from the size parameter. You can safely use (gen/vector ... 0 n) without negatively affecting shrinking.

(gen/sized (fn [size] (gen/resize (* size size) (gen/vector (gen/resize size gen/int)))))

forcing the size of the vector to increase more quickly than the size of the elements in the vector.

I gave this a try, but hit the vector stack overflow problem. I will try the fmap/flatten workaround, but am wondering if this is the right approach.

Your example looks fine. And you can avoid the stack overflow problem by simply not allowing size to grow above a certain limit, say 5000: (gen/resize (min 5000 (* size size)) ...).

(gen/sized (fn [size] (gen/resize (* size size) (gen/vector (gen/resize size gen/int)))))

forcing the size of the vector to increase more quickly than the size of the elements in the vector.

Also wondering about the range of "size". From the code it looks like it goes to 100 due to sample-seq. So functions of size passed to gen/sized should expect range from 0-99, and return the largest meaningful data when given size parameter 99?

When testing, the default max size is 200. However, you can override this simply by passing in a :max-size n argument to tc/quick-check. For example: (tc/quick-check 100 my-prop :max-size 75).

Hope this helps, and don't hesitate to ask anything else.

Comment by Brian Craft [ 18/Nov/14 11:41 AM ]

At the moment the hardest bit for me is understanding the shrinking. Composing generators, I inadvertently build things that won't shrink. E.g. to generate a 2d matrix of numbers with row & column names, I make use of bind so I can size vectors of names to match the matrix size, but then the resulting matrix/vectors will not shrink.

Here's a small example. Generate a vector of short strings, and test that they are all unique.

(tc/quick-check 40 (prop/for-all [f (gen/bind gen/int (fn [i] (gen/vector (gen/resize 5 (gen/such-that not-empty gen/string-ascii)) i)))] (do (println "f" f "count" (count f)) (= (count f) (count (set f))))))

Run this a few times & it will hit a duplicate string, failing the test. It is then unable to shrink the vector.

Here's a case generating two vectors of the same size by generating one vector, then using bind to generate a second of the same size. Testing uniqueness on the second vector, it is able to shrink, however it does so by regenerating the 2nd vector for every test.

(tc/quick-check 40 (prop/for-all [f (gen/bind (gen/vector (gen/resize 5 (gen/such-that not-empty gen/string-ascii))) (fn [v] (gen/hash-map :a (gen/return v) :b (gen/vector gen/int (count v)))))] (do (println "f" f "count" (count (:a f))) (= (count (:b f)) (count (set (:b f)))))))

I expect this will shrink poorly in many cases, because regenerating the second vector destroys the (possibly very rare) failure condition. I suspect this is why my 2d matrices shrink poorly. One really wants to be able to shrink by dropping the same positional element from both vectors. In the case of a fixed dimension (two), you can rewrite it to generate a vector of tuples. For a 2d matrix, though, I'm not sure what to do. Ideally it would shrink by dropping either a column or a row, and the corresponding column or row label. Perhaps I need to write a recursive generator like vector?

Are there any docs/papers describing how the shrinking works?

Comment by Gary Fredericks [ 20/Nov/14 9:45 AM ]

I've included a generator called cap-size in the test.chuck library.





[TCHECK-30] Make Namespacing Consistent in generators.clj Docstrings Created: 31/May/14  Updated: 17/Nov/14  Resolved: 17/Nov/14

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Michael S. Daines Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: docstring

Attachments: Text File prefix.patch    
Patch: Code

 Description   

Three of the docs with examples in generators.clj aren't namespaced with "gen/", whereas everything else is. Add these in to make the documentation consistent and easy for people learning the library to cut and paste to test.






[TCHECK-29] Docstring typo Created: 31/May/14  Updated: 17/Nov/14  Resolved: 17/Nov/14

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Gary Fredericks Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File docstring-typo.patch    

 Description   

Patch attached.



 Comments   
Comment by Reid Draper [ 17/Nov/14 10:28 PM ]

Fixed in 6c553aaa891e0b3c3bc1c6fba3791d75664c530e.





[TCHECK-28] gen/elements should work with arbitrary collections Created: 23/May/14  Updated: 23/May/14  Resolved: 23/May/14

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Gary Fredericks Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File TCHECK-28.patch    

 Description   

gen/elements currently fails for any collection on which nth fails.

We can fix this by calling vec on the input, which has the added benefit of ensuring that the subsequent indexed lookups (one per generated value) are constant time.

A patch to that effect is imminent.



 Comments   
Comment by Gary Fredericks [ 23/May/14 8:52 AM ]

Added TCHECK-28.patch.

Comment by Reid Draper [ 23/May/14 4:52 PM ]

Applied in 1df5a635bcf195c2f98717da3929f6c0d832ee0e.





[TCHECK-27] stateful PRNG + bind => nondeterministic shrink tree Created: 10/May/14  Updated: 11/May/14

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Gary Fredericks Assignee: Reid Draper
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File TCHECK-27-p1.patch    

 Description   

bind inherently needs to consume randomness while shrinking, since the inner generator has to be run multiple times. Since the shrink tree is lazy and there's only one Random object providing the randomness, the resulting shrank values are sensitive to what order the tree is walked in.

I don't believe this affects normal usage, but there are various modifications I've made that I think are useful (e.g., resuming slow shrinks) that are thwarted by this nondeterminism.

Ideally we could stop using a stateful PRNG, but I think this would be a rather extreme change. A much smaller fix is to change bind to create a new Random object each time the inner function is called, based on a seed from the outer Random. I will have a patch that does this shortly, and it seems to fix the issues I've had.

Code for reproducing the issue:

(ns user
  (:require [clojure.test.check.generators :as gen]
            [clojure.test.check.rose-tree :as rose]))

(def the-gen
  (gen/bind gen/nat
            (fn [n]
              (gen/fmap (fn [m] [n m]) gen/nat))))

(defn reproduced?
  [seed]
  (let [make-rose-tree #(gen/call-gen the-gen (java.util.Random. seed) 100)
        rose-1 (make-rose-tree)
        rose-2 (doto (make-rose-tree)
                 ((fn [rt1]
                    ;; force the tree to be realized in a different way
                    (dorun (for [rt2 (rose/children rt1)
                                 rt3 (rose/children rt2)]
                             42)))))
        last-child #(-> % rose/children last rose/root)]
    ;; when these two are not equal, we've reproduced the problem
    (not= (last-child rose-1)
          (last-child rose-2))))

(frequencies (map reproduced? (range 10000)))
;; => {false 9952, true 48}


 Comments   
Comment by Gary Fredericks [ 10/May/14 2:16 PM ]

Attached TCHECK-27-p1, which makes the change I described in the description to the bind-helper function.

Comment by Gary Fredericks [ 11/May/14 8:27 AM ]

The only function that uses bind-helper is bind; however, bind is also implemented with gen-bind, and several other functions use gen-bind. So we'll have to check if gen-bind is sensitive to this as well (e.g., perhaps frequencies has this issue?). If so, I'm not sure what that means about the best way to fix it. Maybe there's an alternative fix that could happen in gen-bind, or maybe the other functions could be rewritten with bind?





[TCHECK-26] defspec does not respect (is (thrown? ...)) Created: 20/Apr/14  Updated: 13/May/14

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Sung Pae Assignee: Reid Draper
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The following test does not pass:

(quick-check 1
  (for-all [x g/any]
    (is (thrown? Throwable (assert false)))))

This can be worked around by wrapping the (is ...) form with a call to boolean:

(quick-check 1
  (for-all [x g/any]
    (boolean (is (thrown? Throwable (assert false))))))

The (is (thrown? ...)) form does not return true|false, but will return the Exception itself when it passes.

Thank you!



 Comments   
Comment by Reid Draper [ 13/May/14 6:14 PM ]

This is correct. And I'm not yet sure what the best way forward is, but my hunch is that we'll need to support non-boolean return values from tests. Maybe something that implements a ->pass? function.





[TCHECK-25] shuffle generator Created: 17/Apr/14  Updated: 26/May/14

Status: Open
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Gunnar Völkel Assignee: Reid Draper
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Adding a generator that shuffles the output of a given collection generator would be really useful.
Short example:

(gen/sample (gen/shuffle (gen/vector get/nat))) ;=> ([5 2 4] [9 1 7 8] ...)

This could be used to add a generator for permutations of integers 1,2,...,n which I also missed several times already.



 Comments   
Comment by Reid Draper [ 13/May/14 6:17 PM ]

Yeah this would be cool. Should it shrink toward the original collection-ordering?

Comment by Gunnar Völkel [ 26/May/14 10:55 AM ]

Yes, that was the idea when we talked about that topic on #clojure. Otherwise, you can just use gen/int to create a seed and use java.util.Random together with java.util.Collections/shuffle.





[TCHECK-24] Add a retry-limited version of `such-that` Created: 17/Apr/14  Updated: 13/May/14  Resolved: 13/May/14

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Jean Niklas L'orange Assignee: Reid Draper
Resolution: Completed Votes: 0
Labels: None


 Description   

The docstring of such-that states that

Eventually we will add another generator combinator that only tries N times before giving up.

So I found it reasonable to add an issue just so it's not forgotten. Seems reasonable to have it throw a RuntimeException with an explanation that such-that attempted to do more than N tries.

Additionally, I think it would be valuable to have a runtime-error in the general such-that implementation, e.g. on 10000 retries, possibly with an explanation on how to override the default retry-limit. I've heard of many people (myself included) not being really sure where their infinite loop is within their test.check code, and I guess such-that is the culprit in very many cases. This might help debugging those issues, although it would break backwards compatibility.



 Comments   
Comment by Reid Draper [ 13/May/14 6:12 PM ]

Fixed in 719d64d60419638f22e3ceca5e37fe075df652ea. Should be in the next release.





[TCHECK-23] Clojure Keywords must begin with a non-digit. Created: 15/Apr/14  Updated: 16/Apr/14  Resolved: 16/Apr/14

Status: Resolved
Project: test.check
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Eric Normand Assignee: