rendering keywords with spaces fails about clerk

[borkdude]

I'm not super familiar with the trade-offs

I think one answer could be: we're not trying to be better than Clojure, EDN or JSON and protect you from their pitfalls.

[philomates]

As far as I understand, Clerk uses EDN to transport data from the JVM to the browser and spaces in keywords doesn't seem to work with EDN (some context in taoensso/sente#251)

Transit does support this but it looks like there is already a lot of code in place around EDN, via edamame, so I imagine that migrating to transit isn't worth it.

I'm not super familiar with the trade-offs between the two formats, so I'm curious: would it be theoretically possible to switch from EDN to Transit without loosing existing features? @mk and @borkdude, do you know?

[borkdude]

Evaluating:

[(hash-map (keyword "N hello") [1 2 3])]
;;=> [{:N hello [1 2 3]}]

Parsing [{:N hello [1 2 3]}] is what you see the error message about.

[philomates]

I think it breaks at several places, but one I expect would be usages of edamame in nextjournal.clerk.hashing/read-string
where (nextjournal.clerk.hashing/read-string ":a a") ;; => :a
The other somehow the notebook shows the viewer/value datastructure instead of rendering the keyword. When it renders this datastructure it shows the error mentioned by @behrica

[borkdude]

I'll look at this later today.

[mk]

I think the problem is that programmatically constructed keywords or symbols (via keyword and symbol) must not be readable so unsure if there's something actionable here.

See also Why is it possible to create keywords that can’t be read by the reader?

[philomates]

I think one answer could be: we're not trying to be better than Clojure, EDN or JSON and protect you from their pitfalls.

yeah, I was thinking that that would be a fine place to leave it. Like, Clerk depends on transferring data via EDN, so it will choke on whatever EDN doesn't support

[behrica]

The philosophical questions is more, how "compatible" Clerk need to be with "an arbitrary .clj file".

So far I have the expectation that any valid .clj file can be rendered/executed as well by Clerk without error.
(Otherwise it is a bug in Clerk)

But maybe it is a too wide expectation , but current Clerk announces as

Clerk takes a Clojure namespace and turns it into a notebook:

which implies "takes any namespace".

I made similar comments regarding the fact that the Clerk caching cannot cache everything.

Eventually we should communicate more clearly, that Clerk has certain requirements for being able to execute / render a Clojure namespace. (and keywords with spaces would then be unsupported to render).

I would still say, Clerk should "guaranty" to "execute" all lines of any .clj file (if valid Clojure).
It might "not render some values" (or not cache them), but we should strive for "successful execution".

[behrica]

Maybe it would make sense to catch the "sci read error" on ClojureScript side and show in the browser a more friendly message, which indicates that the current value "cannot be rendered by Clerk".

[behrica]

I just learned that "spaces are illegal in keywords", even though they can be created via the keyword function.
In this case I would suggest as well to close this issue.

[mk]

I would suggest as well to close this issue.

I agree 🙃. Also agree that there's plenty of holes in the documentation that need to be filled but I don't want to track those in issues just yet.

[mk]

Since dropping the edn in edn encoding for results this leads to a global failure (instead of being contained to a single result). This occurred again today https://clojurians.slack.com/archives/C035GRLJEP8/p1670105206417629