<< Back to previous view

[TRDR-26] Port tools.reader to Clojurescript Created: 09/Jun/15  Updated: 09/Jun/15  Resolved: 09/Jun/15

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

Type: Enhancement Priority: Major
Reporter: Andrew Mcveigh Assignee: Nicola Mometto
Resolution: Completed Votes: 0
Labels: cljs, enhancement, patch

Attachments: Text File 0001-Port-tools.reader-to-Clojurescript.patch    
Patch: Code and Test

 Description   

As part of the effort to get the Clojurescript compiler bootstrapped,
this is a patch porting org.clojure/tools.reader to clojurescript.

The port follows the Clojure implementation quite closely, except in the case of:

  • Numbers: all numbers are reduced to js/Number
  • Chars: reduced to string as in Clojurescript
  • Macros: only used to compile in/out features (not) available in cljs builds
  • There is no `(defn read [] ...)` zero arity implementation as we don't have `in`
  • Custom implementation of clojure.lang.RT/nextID using an atom
  • Run tests using: `lein cljsbuild test`

Contributors: Andrew Mcveigh, Jonathan Boston, Nicola Mometto and Thomas G. Kristensen



 Comments   
Comment by Nicola Mometto [ 09/Jun/15 5:06 AM ]

Andrew, thanks a lot. Can you please attach a patch created with `git format patch`? This is what clojure requires, to track authorship: http://dev.clojure.org/display/community/Creating+Tickets

It will need to be a single commit. You can use a `Also-by:` field to share authorship

Comment by Andrew Mcveigh [ 09/Jun/15 5:34 AM ]

Resubmit patch in required format

Comment by Nicola Mometto [ 09/Jun/15 7:03 AM ]

Pushed with a couple of changes, most notably it depends on cljs >=3308 rather than trying to backport.

Comment by Andrew Mcveigh [ 09/Jun/15 7:08 AM ]

Ah, good. I was a bit uncomfortable about the version check macros being there, but decided to take the lead of the Clojure version.

Comment by Nicola Mometto [ 09/Jun/15 7:18 AM ]

cljs is a moving target and updates much frequently then clj, no interest in keeping compatibility with older versions (cljs itself depends on clj 1.7.0-rc1)





[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





Generated at Sat Jul 04 17:37:00 CDT 2015 using JIRA 4.4#649-r158309.