ClojureScript

Stop using goog.string.quote to escape strings for printing

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • 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.

Activity

Hide
Chas Emerick added a comment -

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

Show
Chas Emerick added a comment - New patch attached with requested changes, CLJS-400.diff.
Hide
David Nolen added a comment -

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.

Show
David Nolen added a comment - 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.
Hide
Chas Emerick added a comment - - edited

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

Show
Chas Emerick added a comment - - edited Patch CLJS-400-fast.diff added with suggested perf tweaks. Please excuse my (still) absolutely shoddy cljs instincts.
Hide
David Nolen added a comment -

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

Show
David Nolen added a comment - Thanks. Performance things - can we avoid instantiating the RegExp everytime, can we switch char-escapes to a JS object and use aget?
Hide
Chas Emerick added a comment -

Patch attached

Show
Chas Emerick added a comment - Patch attached

People

Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: