<< Back to previous view

[CLJS-1292] Add IPrintWithWriter implementation for TaggedLiteral Created: 28/May/15  Updated: 28/May/15

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

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

Attachments: Text File 0001-CLJS-1292-Add-IPrintWithWriter-implementation-for-Ta.patch    




[CLJS-1291] pprint whitespace/letter checks are incomplete Created: 28/May/15  Updated: 28/May/15  Resolved: 28/May/15

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

Type: Defect Priority: Minor
Reporter: Jonathan Boston Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-1291.patch    

 Description   

pprint uses a simple is-whitespace? and is-letter? check that doesn't cover all cases (e.g. unicode chars). Using Google Closure fns are a better choice.



 Comments   
Comment by Jonathan Boston [ 28/May/15 9:36 AM ]

Patch added.

Comment by David Nolen [ 28/May/15 10:17 AM ]

fixed https://github.com/clojure/clojurescript/commit/25e4945cb5c7f0ed4322a377d0e12a62ffea3087





[CLJS-1290] :refer does not work with Closure JS namespaces Created: 27/May/15  Updated: 27/May/15

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 0.0-3269
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: newbie


 Description   
(require '[goog.string :refer [startsWith]])

fails due to attempted var checking. We just need to disable var existence checks if we don't have a ClojureScript namespace.






[CLJS-1289] Revert clj->cljc file conversion Created: 26/May/15  Updated: 26/May/15  Resolved: 26/May/15

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

Type: Defect Priority: Critical
Reporter: Thomas Heller Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: None


 Description   

Commit [1] changed some .clj files over to .cljc files but no other changes were made to actually make these "common" files. Most of the files are not actually compatible (and probably never will be, eg. assuming sync streaming IO) and some even cause conflicts src/clj/cljs/repl.cljc vs. src/main/cljs/cljs/repl.cljs both trying to provide cljs.repl namespaces.

Any objections to changing these back as they provide no gain as of now?

I know other build tools are not a concern but these "faulty" files cause "issues" in shadow-build.

[1] https://github.com/clojure/clojurescript/commit/11113cea2f599d685b60f8fbdcb56d944240ea25



 Comments   
Comment by David Nolen [ 26/May/15 1:13 PM ]

The change was intentionally made now that Clojure has reader conditionals. The intent is for portions of these ClojureScript files to be made so that we can compile to Node.js gradually over time.





[CLJS-1288] compiler doesn't emit "goog.require" for foreign library when optimization level is not set Created: 25/May/15  Updated: 28/May/15  Resolved: 28/May/15

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

Type: Defect Priority: Major
Reporter: Maria Neise Assignee: Maria Neise
Resolution: Completed Votes: 0
Labels: bug, foreign-libs

Attachments: Text File CLJS-1288-2.patch     Text File CLJS-1288.patch    
Patch: Code

 Description   

When the optimization level is not set explicitly, meaning that :none will be used as a default, the compiler doesn't emit a goog.require for foreign libraries. An example project reproducing the problem can be found here: https://github.com/MNeise/hello_world.

From my understanding of the code, the compiler environment gets created with the initial options and doesn't get updated with the default optimization level when the options don't include an optimization level. The following check in the load-libs function in the compiler.cljc file then fails, because optimizations is nil.

compiler.cljc
(cond
        (ana/foreign-dep? lib)
        (let [{:keys [target optimizations]} (get @env/*compiler* :options)]
          ;; we only load foreign libraries under optimizations :none
          (when (= :none optimizations)
            (if (= :nodejs target)
              ;; under node.js we load foreign libs globally
              (let [{:keys [js-dependency-index options]} @env/*compiler*
                    ijs-url (get-in js-dependency-index [(name lib) :url])]
                (emitln "cljs.core.load_file(\""
                  (str (io/file (util/output-directory options) (util/get-name ijs-url)))
                  "\");"))
              (emitln "goog.require('" (munge lib) "');"))))


 Comments   
Comment by Maria Neise [ 25/May/15 11:23 PM ]

I've attached a possible patch for this.

Comment by David Nolen [ 26/May/15 5:16 PM ]

The patch looks OK but I think we should just merge all-opts with :options in the compiler-env otherwise later there will be some other case missed

Comment by Maria Neise [ 26/May/15 6:43 PM ]

That makes sense I've update the patch (CLJS-1288.patch).

I was also wondering, if it would make sense to pull emit-constants into add-implicit-options, maybe similar to CLJS-1288-2.patch?

Comment by David Nolen [ 28/May/15 10:09 AM ]

fixed https://github.com/clojure/clojurescript/commit/f75da4ec93add6030ab57d4e2b3e1ad4a1cf9d4d





[CLJS-1287] CLJS exceptions should use same format as Clojure 1.7 (#exception {...}) Created: 25/May/15  Updated: 25/May/15

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 0.0-3269
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: Daniel Skarda Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Recent Clojure 1.7-RC adds new print format compatible with EDN syntax:

(ex-info "foobar" {:bar :baz})
#error {
 :cause "foobar"
 :data {:bar :baz}
 :via
 [{:type clojure.lang.ExceptionInfo
   :message "foobar"
   :data {:bar :baz}
   :at [clojure.core$ex_info invoke "core.clj" 4591]}]
 ...
}

(try (.indexOf 0 "baz") (catch Throwable e e))
#error {
 :cause "No matching method found: indexOf for class java.lang.Long"
 :via
 [{:type java.lang.IllegalArgumentException
   :message "No matching method found: indexOf for class java.lang.Long"
   :at [clojure.lang.Reflector invokeMatchingMethod "Reflector.java" 53]}]
 ...
}

Unfortunatelly ClojureScript format is different (and sometimes even not compatible with EDN):

(ex-info "foobar" {:bar :baz})
#ExceptionInfo{:message "foobar", :data {:bar :baz}}

(try (.indexOf 0  "baz") (catch js/Error e e))
#<TypeError: Object 0 has no method 'indexOf'>

The request to unify the output of exceptions might sound like "nitpicking".

However please consider applications, which share (lots of) code between server and client.
Any arbitrary difference between Clojure and ClojureScript adds future accidental complexity.

Hint: looking for green light and answer patches welcome

Last thing to consider is how to deal with stacktraces in the patch. Recent CLJS parses have a parser implemented on server side in Clojure. Should the patch move parsing to ClojureScript?



 Comments   
Comment by David Nolen [ 25/May/15 10:14 AM ]

Yes patch welcome. Thanks!





[CLJS-1286] REPL environment should be able to provide advice if source mapping fails Created: 23/May/15  Updated: 25/May/15

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 0.0-3269
Fix Version/s: Next

Type: Defect Priority: Minor
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

For example browser REPL will often need users to supply :host-port, :host, and :asset-path in order to correctly parse files from stacktraces.






[CLJS-1285] load-file regression Created: 23/May/15  Updated: 23/May/15  Resolved: 23/May/15

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 0.0-3269
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Description   

We need to make load-file more robust. Currently we don't properly check goog.dependencies.written_ for the sourcePath, we always delete the basePath + source path.



 Comments   
Comment by David Nolen [ 23/May/15 5:12 PM ]

fixed https://github.com/clojure/clojurescript/commit/22ba5a6d0b780183d21bd69a64492e2216a7b379





[CLJS-1284] IndexedSeq -seq implementation incorrect for i >= alength internal array Created: 23/May/15  Updated: 23/May/15  Resolved: 23/May/15

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 0.0-3269
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: Daniel Skarda Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None
Environment:

ClojureScript 0.0-3291



 Description   
(defn foo-ok
  ([x0 & xs] [x0 xs]))

(defn foo-fail
  ([] nil)
  ([x0 & xs] [x0 xs]))


(foo-ok 0) => [0 nil]     ;; OK
(foo-ok 0 1) => [0 (1)]   ;; OK

(foo-fail) => nil         ;; OK
(foo-fail 0) => [0 (nil)] ;; INCORRECT
(foo-fail 0 1) => [0 (1)] ;; OK


 Comments   
Comment by David Nolen [ 23/May/15 1:53 PM ]

This is a bug with IndexedSeq, not fn arities.

Comment by David Nolen [ 23/May/15 2:10 PM ]

fixed https://github.com/clojure/clojurescript/commit/f165420d2b25008924d6655ca4e79bf9901c2125

Comment by David Nolen [ 23/May/15 2:15 PM ]

I cut a 0.0-3297 interim release with this fix.

Comment by Daniel Skarda [ 23/May/15 5:33 PM ]

David, thank you for very fast analysis, fix and release!





[CLJS-1283] (long \7) produces different result on Clojure/ClojureScript Created: 22/May/15  Updated: 25/May/15

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

Type: Defect Priority: Minor
Reporter: Sean Corfield Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

In Clojure (long \7) produces 55 which is the ASCII code value for the character 7.

In ClojureScript, it seems this just converts the single character string "7" to a number and you get 7 as the answer. This might make writing portable string manipulation code rather hazardous?

(reported by irctc_ on IRC)



 Comments   
Comment by David Nolen [ 22/May/15 11:23 PM ]

Coercions like long only exist to simplify porting code. In general such things should be replaced.

Comment by Sean Corfield [ 25/May/15 4:24 PM ]

What would be your recommended, portable, way to turn characters into ASCII then? (that would work in both CLJS and CLJ)

Comment by David Nolen [ 25/May/15 10:27 PM ]

I don't have any recommendation. It seems intractable to me.





[CLJS-1282] Add a :pprint option to the default reporter in cljs.test Created: 22/May/15  Updated: 22/May/15

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 0.0-3269
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Sebastian Bensusan Assignee: Sebastian Bensusan
Resolution: Unresolved Votes: 0
Labels: test


 Description   

Now that cljs.pprint has landed, cljs.test could report failures and exceptions with it. The exact API is TBD.






[CLJS-1281] preserve test order Created: 21/May/15  Updated: 22/May/15

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 0.0-3269
Fix Version/s: Next

Type: Enhancement Priority: Minor
Reporter: David Nolen Assignee: Sebastian Bensusan
Resolution: Unresolved Votes: 0
Labels: newbie


 Description   

We can keep tests sorted by :line var meta information.






Generated at Thu May 28 15:27:17 CDT 2015 using JIRA 4.4#649-r158309.