[CLJS-321] Support with-out-str Created: 18/Jun/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: | Enhancement | Priority: | Major |
| Reporter: | Brandon Bloom | Assignee: | Paul deGrandis |
| Resolution: | Completed | Votes: | 0 |
| Labels: | patch, patch, | ||
| Attachments: |
cljs-321-with-out-str.diff
cljs-321-with-out-str.diff
with-out-str.patch
|
| Patch: | Code and Test |
| Description |
|
Attached patch implements with-out-str macro in terms of goog.string.StringBuffer and print-fn via .append The attached patch also fixes |
| Comments |
| Comment by David Nolen [ 18/Jun/12 8:44 AM ] |
|
Patch looks mostly OK. Did you do any basic benchmarking? It would be nice to know that this doesn't slow down printing of Clojure objects. |
| Comment by Brandon Bloom [ 18/Jun/12 5:03 PM ] |
|
Yes, I ran this: (time (dotimes [i 5000] (prn-str {:foo [1 "two" 'three]})) before patch: 29988 msecs Effectively identical. |
| Comment by David Nolen [ 18/Jun/12 8:56 PM ] |
|
on V8? JSC? SM? |
| Comment by Brandon Bloom [ 19/Jun/12 12:35 AM ] |
|
I had run it in a browser repl, but I forget which. I added a benchmark and re-ran it in all three. It was a tiny bit slower, so I went to investigate and discovered that I haven't the slightest clue how to predict or interpret Javascript engine performance numbers. Hell, Chrome's console seems to have completely random performance, where as the Node.js repl seems to be more consistent. I'll get my shit together and see if I can fix this up. Hopefully will have time to look at it tomorrow. |
| Comment by Laszlo Török [ 19/Jun/12 3:54 AM ] |
|
jsperf.com was a great help to me when prototyping PersistentVector's first CLJS implementation it's very-very handy and easy to use and keep updated if necessary |
| Comment by David Nolen [ 29/Aug/12 8:42 PM ] |
|
Thanks Brandon, let's hold off on this one until we can resolve |
| Comment by Paul deGrandis [ 22/Oct/12 4:57 PM ] |
|
I attached an updated patch (that I'm using internally for a personal project) of just the with-out-str macro code. |
| Comment by David Nolen [ 22/Oct/12 5:00 PM ] |
|
Thanks Paul. Could we get a patch with tests included? |
| Comment by Paul deGrandis [ 22/Oct/12 5:02 PM ] |
|
No problem - on it right now. |
| Comment by Paul deGrandis [ 22/Oct/12 5:18 PM ] |
|
Updated with one additional piece I needed to bring over from my branch (:dynamic on print-fn) and two tests: one using print and one using the raw print-fn. |
| Comment by David Nolen [ 22/Oct/12 6:06 PM ] |
|
fixed, http://github.com/clojure/clojurescript/commit/eab6032e6ba22571e5c4f821707bf5de8075e757 |