<< Back to previous view

[CLJS-400] Stop using goog.string.quote to escape strings for printing Created: 19/Oct/12  Updated: 22/Oct/12  Resolved: 22/Oct/12

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

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

Attachments: File CLJS-400.diff    
Patch: Code and Test

 Description   

The contract of goog.string.quote is to produce a "valid JavaScript string", which can end up being quite different from what is expected by e.g. the Clojure reader and other edn-compatible readers. This has been discussed in a variety of threads/issues:

The most significant issue is that characters > 127 and < 256 are outputted with an \x.. escape sequence, which no other reader supports. Further, other Unicode characters are always escaped using the \u.... notation; this is not a functional difference, but can be a stumbling block, e.g. when viewing data that had been pr-str'd that contains high codepoint characters.

The most straightforward fix is to take control of printed string escaping to ensure the results conform first to the expectations of existing readers (primarily Clojure's and ClojureScript's), and to the formal [edn specification](https://github.com/edn-format/edn/) as that matures.

This ticket supersedes http://dev.clojure.org/jira/browse/CLJ-1025.

 Comments   
Comment by Chas Emerick [ 19/Oct/12 7:50 PM ]

Patch attached

Comment by David Nolen [ 19/Oct/12 8:13 PM ]

Thanks. Performance things - can we avoid instantiating the RegExp everytime, can we switch char-escapes to a JS object and use aget?

Comment by Chas Emerick [ 19/Oct/12 9:11 PM ]

Patch CLJS-400-fast.diff added with suggested perf tweaks. Please excuse my (still) absolutely shoddy cljs instincts.

Comment by David Nolen [ 20/Oct/12 9:42 AM ]

Using the let won't work here. That will be emit top-levels that are not namespaced. Edge case in the compiler that should be sorted out on another ticket. Just define top levels for these thanks.

Comment by Chas Emerick [ 22/Oct/12 2:03 PM ]

New patch attached with requested changes, CLJS-400.diff.

Comment by David Nolen [ 22/Oct/12 6:06 PM ]

fixed, http://github.com/clojure/clojurescript/commit/3b32345966a65f66e49563a3a40a2877d0435c5d

Generated at Thu May 23 03:35:32 CDT 2013 using JIRA 4.4#649-r158309.