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

Current issues with Value #122

Open
1 of 6 tasks
Tracked by #397
torkleyy opened this issue Aug 5, 2018 · 10 comments
Open
1 of 6 tasks
Tracked by #397

Current issues with Value #122

torkleyy opened this issue Aug 5, 2018 · 10 comments

Comments

@torkleyy
Copy link
Contributor

torkleyy commented Aug 5, 2018

  • tuples / tuple structs not supported (Ron does not properly round trip through Value #121)
  • enums not supported
  • struct names don't get stored
  • units represented with struct names
  • Make sure errors make sense, esp after a unit struct (e.g. UnitStruct: as input)
  • figure out what to do about tuples becoming sequences
@Boscop
Copy link

Boscop commented Feb 26, 2020

Can some of these boxes be ticket now? It seems at least the first one can..

@github-actions
Copy link

Issue has had no activity in the last 180 days and is going to be closed in 7 days if no further activity occurs

@github-actions github-actions bot added the stale label Nov 18, 2021
@kvark kvark reopened this Dec 3, 2021
@kvark kvark reopened this Dec 17, 2021
@kvark
Copy link
Collaborator

kvark commented Dec 25, 2021

comment

@wiiznokes
Copy link

Any plans on improving this or this is out of scope ?

@juntyr
Copy link
Member

juntyr commented Nov 6, 2024

Yes, many plans, little time (research + small child) :/ If you can help with any part, I'd really appreciate it :)

@wiiznokes
Copy link

Yes, many plans, little time (research + small child) :/ If you can help with any part, I'd really appreciate it :)

I'm interested in making the conversion "ron str" -> ron::Value lossless. Maybe i could try to implement it, but i think it would help if you can give some hint about the change needed in the Value type.

@juntyr
Copy link
Member

juntyr commented Nov 23, 2024

Yes, many plans, little time (research + small child) :/ If you can help with any part, I'd really appreciate it :)

I'm interested in making the conversion "ron str" -> ron::Value lossless. Maybe i could try to implement it, but i think it would help if you can give some hint about the change needed in the Value type.

Essentially the Value type would need to become an AST-like type for everything the current grammar can represent. Then ron string -> Value -> string would be lossless.

You can find the grammar here: https://github.com/ron-rs/ron/blob/master/docs/grammar.md (it even has a Value section).

@wiiznokes
Copy link

Ok, i've got smtg like this:

pub enum Value {
    Bool(bool),
    Char(char),
    Map(Map),
    Struct(Option<String>, Map),
    Number(Number),
    Option(Option<Box<Value>>),
    String(String),
    Bytes(Vec<u8>),
    Seq(Vec<Value>),
    Tuple(Option<String>, Vec<Value>),
    Unit,
}

@juntyr
Copy link
Member

juntyr commented Nov 23, 2024

Ok, i've got smtg like this:

pub enum Value {
    Bool(bool),
    Char(char),
    Map(Map),
    Struct(Option<String>, Map),
    Number(Number),
    Option(Option<Box<Value>>),
    String(String),
    Bytes(Vec<u8>),
    Seq(Vec<Value>),
    Tuple(Option<String>, Vec<Value>),
    Unit,
}

I’d probably make the following changes:

  • rename Seq to List
  • struct should use a map with string keys
  • we also need a named unit variant with a string field (e.g. for unit structs and unit variants)

@wiiznokes
Copy link

we also need a named unit variant with a string field (e.g. for unit structs and unit variants)

I was thinking of using Struct(None, empty::map) for "()", and Struct(Some("name"), empty::map) for unit_struct with indent.
For enum_variant_unit, we can use Tuple(Some("name"), empty::vec).

Do you still think it's better to add a new variant ? In that case, maybe UnitStruct(Option<String>)

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

5 participants