Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

replace context: mustache.Context object with data: JsonNode object #102

Open
pietroppeter opened this issue Jul 2, 2022 · 4 comments
Open

Comments

@pietroppeter
Copy link
Owner

currently NbDoc and NbBlock have both a context field of type Context taken from mustache to collect unstructured data. This is further used by the html render backend to render stuff and one thing I do not like is that we modify the context both of doc and block when we render with the backend. Also none of the particular stuff of Context object is used.

Instead we should have a data object which is a standard JsonNode and when rendering with mustache we should generate appropriate context. This would be a first step in a general refactoring of backends which would in general be the target for the next big version (0.4). It should allows us to have a new html backend based on nimja instead of nim mustache for example.

@HugoGranstrom
Copy link
Collaborator

This sounds great! What I like the most is the ability to just do context["someArray"].add newValue to add an element to an existing json array. That's something the current context really isn't well suited for.

@pietroppeter
Copy link
Owner Author

Yes, another good reason for the change is Json api is much better and more well known than the Context one.

@pietroppeter
Copy link
Owner Author

pietroppeter commented Oct 26, 2022

while thinking about api for table of contents (#58), I had some thoughts about this.

what if we introduct a new type for field nb.data:

type
  NbData = distinct JsonNode

instead of directly using JsonNode we could use a distinct version where we implement other stuff.
In particular, thanks to jsony one could implement a way to (put in and) get out of NbData arbitrary objects:

nb.data.get(MyCustomObject, "custom_object_key")

(and one can think on implements failures when in key there is not the correct type or you get a default ...)

To go back to the original point, for the toc case we might have a TocOptions object that we put in when we create nbToc and we need to get out when we render the toc. In principle also stuff like seq[Heading] could be implemented with this mechanism (although in that case it might make less sense.

One of the tricky aspect would be visibility of jsony (does not play well with some other json stuff, but visibility is required to use hooks). Unclear also if really needs to be a distinct object (or could just be a JsonNode).

@HugoGranstrom
Copy link
Collaborator

I don't see why we would have to make it a distinct, then we lose support for every library working with JsonNode. We can still implement our own API on top of it, we just have to make sure it doesn't clash with the existing one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants