search

JonyEpsilon / gorilla-repl

A rich REPL for Clojure in the notebook style.
http://gorilla-repl.org
MIT License
887 stars 104 forks source link

Use keywords rather than strings in the Vega representations #74

Closed cldellow closed 10 years ago

cldellow commented 10 years ago

Would you be open to a PR that switches to using keywords rather than strings as the keys in the maps used by vega.clj and friends? They should render the same way in cheshire. Additionally, it would simplify some graph-munging that I find myself doing and tidy up a bit of the gorilla-repl code itself, too.

Since you use keywords as map keys elsewhere in the project, I figured I'd ask first in case there's something I'm missing.

JonyEpsilon commented 10 years ago

I think the reason that they're strings at the moment is that with the old renderer, the data was passed to the client as EDN, and then parsed by jsedn. And I think the problem was I couldn't get jsedn to map keywords into the strings I wanted.

But now that the new renderer JSONs everything server side, then I think there's no reason not to keywordify (?) the code. It would be much nicer to work with/look at. I can foresee one minor compatibility problem with saved worksheets - the plots should display ok, but if the user were to copy and paste the value from a plot, then it would be in the wrong form to work with the new compose say. So, maybe that would suggest a 0.3.x version bump, or maybe we ignore it because it's such a niche problem!

So, yeah, I'd be delighted to see a PR! Sounds like a tedious job getting rid of all those quotes, but you'll probably tell me it's a simple one-liner in Vim :-)

cldellow commented 10 years ago

Ah, thanks for the history.

I submitted a PR. I don't know how to link it to this issue, so I'll just close this one.

cldellow commented 10 years ago

Oops, I forgot to respond to your other concern -- I would bump the version / roll this out with the next version bump.

People who manipulate the Vega map in Clojure will find their code breaking here, too.

Granted, it may well be that no one does that, but it's nice to be predictable.