lilactown / punk

A data REBL built for the web
Eclipse Public License 2.0
148 stars 5 forks source link

Reader literals #2

Closed lilactown closed 5 years ago

lilactown commented 5 years ago

Currently, Punk uses cljs.tools.reader.edn/read-edn which just barfs if it's a reader-literal it doesn't recognize.

Common ones like #inst, #js, #error are not supported by default either.

Ideally we would at the very least just pass these through unmarred.

lilactown commented 5 years ago

74f43d232650eab0ef7e88f86393521dfdd8d61a fixed

ingesolvoll commented 5 years ago

Hi! Very interesting library, need to get cracking at this right away. The issue here was my first problem, and I was very happy to see that you fixed it. I'm assuming this fix is in the latest release (0.0.4)?

I'm using browser-only version, and still get errors.

Here are my errors when testing with 0.0.4:

Is there a way that I can plug in readers for these types in my own system?

lilactown commented 5 years ago

Hm. I've found an issue where, when navigating to a nested structure, it errors.

Reprodunction:

  1. (tap> {:asdf #uuid "97bda55b-6175-4c39-9e04-7c0205c709dc"})
  2. Click on that in the Entries pane
  3. Click on :asdf in Current pane to bring #uuid "..." into Next pane

When #uuid "..." should be rendered in the Next pane, I see this error:

...
data: 
{:type :reader-exception, :ex-kind :reader-error}
...
message: "No reader function for tag uuid."

Is this the same problem you're experiencing, @ingesolvoll ?

ingesolvoll commented 5 years ago

That looks right, yes.

lilactown commented 5 years ago

The error was occurring because I had configured the EDN reader on the UI application side, but not in the adapter. I will update the web and node adapter to read EDN correctly.

The issue with extending Punk's reader to user-defined literals is that it has to be extended in two places: the adapter (easy) and the UI application (not currently possible).

This might be a symptom of an architectural failing. Perhaps the UI application should defer more of its functionality to the adapter, which would enable you to extend it further. I'll have to think about this.

lilactown commented 5 years ago

I've updated the adapters to read #inst, #uuid, and #queue correctly.

I've also updated the UI and the adapter packages to simply pass through as plain EDN any tags it doesn't know (e.g. #js, #my-app/Foo, etc.).

This fix should be available in v0.0.5.

ingesolvoll commented 5 years ago

Nice, thank you!