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

[edn] Expose line/column/character span position information from parsed EDN streams #258

Closed
ncalexan opened this issue Feb 8, 2017 · 3 comments

Comments

@ncalexan
Copy link
Member

ncalexan commented Feb 8, 2017

This ticket tracks including line/column/character position information with parsed edn::Value trees. It addresses #149 (comment); it is a pre-requisite, I think, for #168.

Concretely, I think we want a pair type like:

struct edn::ValueAndSpan {
  value: edn::Value,
  span: edn::Span,
}

which encapsulates a Span of some description coming out of the PEG EDN parser. Sadly, this means we'll need to rewrite or duplicate the edn::Value type to have edn::ValueAndSpan children. I could imagine something like:

  • edn::Value<V>, where V parameterizes the children (so V = edn::Value is what we have now)

or something like

  • edn::Value<S>, where S parameterizes the span (so S = () is what we have now).

We'll want to expose this new type, and probably convert to the existing type as well (just dropping all the spans).

This is pretty awkward -- better suggestions appreciated. This could be a good first bug if the patch doesn't try to change existing code too much (i.e., just adds new types and conversions into existing types).

@rnewman
Copy link
Collaborator

rnewman commented Feb 8, 2017

Why not just make edn::Value your ValueAndSpan?

Each fork of the enum would be (value, span). Callers would need to match with _ to discard the span, but that's OK: it forces them to think about it.

@rnewman
Copy link
Collaborator

rnewman commented Feb 8, 2017

That is: we already have a type for just the internal value — the internal value itself.

@ncalexan
Copy link
Member Author

ncalexan commented Feb 8, 2017

Why not just make edn::Value your ValueAndSpan?

Two reasons:

  • it churns a lot of existing code, making this not a good first bug;
  • because it makes programatically generating edn::Value difficult. You can introduce some NullSpan, I guess, but eventually you're going to want just "raw values".

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

No branches or pull requests

3 participants