Skip to content
This repository has been archived by the owner on Sep 12, 2018. It is now read-only.

Expose rest api for edn parsing #248

Closed
victorporof opened this issue Feb 6, 2017 · 7 comments
Closed

Expose rest api for edn parsing #248

victorporof opened this issue Feb 6, 2017 · 7 comments

Comments

@victorporof
Copy link
Contributor

Similar to #247, a method for at least making sure some string is valid edn would be useful for the web tooling. Bonus points for adding additional information like line/column error, what token was expected etc.

@jsantell anything special you'd need?

@victorporof victorporof self-assigned this Feb 6, 2017
@rnewman
Copy link
Collaborator

rnewman commented Feb 6, 2017

Does this need to be an (attackable, documented, etc) API? The endpoints will already complain about malformed EDN, so presumably this is for live editing. Won't a JS EDN parser (or our Rust one transpiled) suffice?

@victorporof
Copy link
Contributor Author

I'm seeing this useful for highlighting errors in the web frontend that @jsantell is working on. Not sure if it's worth looking into transpiling our parser when we can just expose a method on the server to access it. A JS edn parser would probably work too.

IIRC the plan a few weeks ago was to go this route to add live editing feedback in the web frontend. I guess now the decision needs to be: should it be through our own parser or through a third party one written in js. The end result is the same in both cases.

I'm in favor of using our own parser for this, just to make sure we consistently fail everywhere in the same way, if there's some error.

@rnewman
Copy link
Collaborator

rnewman commented Feb 6, 2017

My opinion: either expose the full-fledged parser for what's being entered (tx or query) in order to get the full set of possible errors — e.g., "invalid :find expression: expected ?variable" — or just do EDN parsing locally.

The middle ground, handling round-trip strings in order to achieve basic structured EDN editing, doesn't really make sense to me. I'm trying to be mindful of not expanding our API surface unnecessarily, both from a bloat perspective and also a vulnerability perspective, so exposing what is essentially an EDN validator/pretty-printer doesn't seem like a decent tradeoff.

It might be sensible to build language support as a separate utility altogether, perhaps speaking LSP or similar. We have distinct crates that can be combined to do this…

@victorporof victorporof removed their assignment Feb 6, 2017
@victorporof
Copy link
Contributor Author

The transactor or query parsers aren't ready yet. I thought we could've started somewhere, and that somewhere was exposing the edn parser. Unassigning.

@victorporof
Copy link
Contributor Author

@ncalexan your thoughts on this, especially after #258 ?

@ncalexan
Copy link
Member

ncalexan commented Feb 9, 2017

@ncalexan your thoughts on this, especially after #258 ?

I see no particular value in an "Is this valid EDN?" endpoint, nor in a pretty-printing endpoint. I don't think #258 is relevant -- that would just help provide better information for invalid EDN.

However, there's value in plumbing an API -- any API -- all the way from HTTP REQUEST, through Nickel, and down to Mentat and back. If this is the functionality that's ready to expose, then I'm not against doing that work.

@victorporof
Copy link
Contributor Author

Let's file separate issues for that.

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

No branches or pull requests

3 participants