Can it support eon？

[jellybobbin]

Can it support eon？

eon

[epage]

So far we've taken a fairly liberal approach to formats so long as they don't cause problems (see #573). However, we are re-evaluating enabling all of them by default (#621)

[jellybobbin]

I have tried it locally, but config::value::ValueKind cannot adapt well to eon::Value::Variant:

config::value::ValueKind

#[derive(Debug, Clone, PartialEq)]
pub enum ValueKind {
    Nil,
    Boolean(bool),
    I64(i64),
    I128(i128),
    U64(u64),
    U128(u128),
    Float(f64),
    String(String),
    Table(Table),
    Array(Array),
}

eon::Value

#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub enum Value {
    /// Special `null` value
    Null,

    /// `true` or `false`
    Bool(bool),

    /// An integer or floating point number
    Number(Number),

    /// A string value, like `"Hello, world!"`
    ///
    /// Also commonly used as the key in a [`Map`].
    ///
    /// Strings are also used for simple sum-type (enum) variants values, e.g. `"Maybe"`.
    /// See [`Self::Variant`] for more complex sum-type (enum) variants.
    String(String),

    /// A list of values.
    List(Vec<Value>),

    /// Maps strings to values, i.e. like a `struct`.
    Map(Map),

    /// A sum-type (enum) variant containing some data, like `"Rgb"(255, 0, 0)`.
    ///
    /// For simple enum types (e.g. `enum Maybe { Yes, No }`),
    /// the variants will be represented as [`Self::String`] instead.
    Variant(Variant),
}

#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct Variant {
    /// The name of the variant, like `Rgb`.
    pub name: String,

    /// The contents of the variant.
    ///
    /// Note that this cannot be empty.
    /// A variant with no contents is represented as a [`Value::String`].
    pub values: Vec1<Value>,
}

[epage]

Could you clarify what the concern is?

If its numbers, that is similar to json: https://github.com/rust-cli/config-rs/blob/07f13ff25f18e6a7bd40a2d8a2f1e036a473736a/src/file/format/json.rs#L20-L28

If its variants, then I would represent that like serde does to get consistent deserialization: { name = <values> }, so a single element Table.

[polarathene]

You could look at my stale PR: https://github.com/rust-cli/config-rs/pull/472#issuecomment-1770151034

In the format.rs file I contributed a unified way to manage the various formats, simplifying each individual implementation. At least for any config format crate that relied upon serde this seemed like a better approach.

You'll see that the PR replaced the format specific Value mappings, with both Ron and JSON support having a similar Number variant type to handle.

Your eon parse() method would then just be these two lines:

let value = format::from_parsed_value(uri, eon::from_str(text)?);
format::extract_root_table(uri, value)

---

I haven't been able to return to that PR yet (or others I have for config-rs) as I lack the spare hardware to context switch from my existing tasks (my current dev system is often at 95% memory usage, low disk space and prone to crashing). If you'd like to rebase my PR and get it through review again, it might be useful to get merged?

I think there was still some concerns I wanted to address, so if you (or someone else) does tackle pushing that PR forward before I get back to it be sure to go through my commentary, notably at least:

• I wanted to verify some number types being handled correctly: https://github.com/rust-cli/config-rs/pull/472#issuecomment-1772298302
• Two formats handled but that PR may not be viable:
  • YAML was switched to serde-yaml to benefit from using serde, however that crate later became unmaintained with no suitable alternative so config-rs continues to use yaml-rust2 (which is now only minimally maintained), if saphyr eventually lands serde support then the PR could adopt that crate for YAML support instead but it's been a long wait (there is some hope).
  • RON, I expressed a concern with a new type introduced (unreleased at the time). A separate PR instead was raised using the existing parse support in config-rs updated to support the updated lossless number type (more granular), but the PR did not implement support for the new Bytes type.

So given all that, rather than a "common" parser for the formats to benefit from, it'd be better revised as one specific to formats supporting and leaning into serde 😅 (but as noted with Ron, and other feedback in the PR, there was some tradeoffs or at least there was at the time due to limitations / behaviour of serde)

[jellybobbin]

@epage @polarathene

Specifically, there is no corresponding variant for eon::Value::Variant(value) in config::ValueKind

src/file/format/eon.rs:

use std::error::Error;
use eon::NumberImpl;
use crate::format;
use crate::map::Map;
use crate::value::{Value, ValueKind};

pub(crate) fn parse(
    uri: Option<&String>,
    text: &str,
) -> Result<Map<String, Value>, Box<dyn Error + Send + Sync>> {
    let value = from_eon_value(uri, eon::from_str(text)?)?;
    format::extract_root_table(uri, value)
}

fn from_eon_value(
    uri: Option<&String>,
    value: eon::Value,
) -> Result<Value, Box<dyn Error + Send + Sync>> {
    let kind = match value {
        eon::Value::Null => ValueKind::Nil,
        eon::Value::Bool(value) => ValueKind::Boolean(value),
        eon::Value::Number(eon::Number(value)) => match value {
            NumberImpl::F32(value) => ValueKind::Float(value as f64),
            NumberImpl::F64(value) => ValueKind::Float(value),
            NumberImpl::I128(value) => ValueKind::I128(value),
            NumberImpl::U128(value) => ValueKind::U128(value),
        },
        eon::Value::String(value) => ValueKind::String(value),
        eon::Value::List(values) => {
            let array = values
                .into_iter()
                .map(|value| from_eon_value(uri, value))
                .collect::<Result<Vec<_>, _>>()?;

            ValueKind::Array(array)
        },
        eon::Value::Map(values) => {
            let map = values.map
                .iter()
                .map(|(key, value)| -> Result<_, Box<dyn Error + Send + Sync>> {
                    let key = key.to_string();
                    let value = from_eon_value(uri, value.clone())?;

                    Ok((key, value))
                })
                .collect::<Result<Map<_, _>, _>>()?;

            ValueKind::Table(map)
        }
        eon::Value::Variant(value) =>  {
            // let array = value.values
            //     .into_iter()
            //     .map(|value| from_eon_value(uri, value))
            //     .collect::<Result<Vec<_>, _>>()?;

            // let map = Map::from([(value.name, Value::new(uri, ValueKind::Array(array)))]);
            // ValueKind::Table(map)

            ValueKind::??
        }
    };

    Ok(Value::new(uri, kind))
}

[jellybobbin]

Is it possible to integrate and enumerate the values of other crates without constructing config::ValueKind

config::Value

pub enum Value {
    Toml(toml::Value),
    Json(serde_json::Value),
    Ron(ron::Value),
    ...
}

[epage]

Is it possible to integrate and enumerate the values of other crates without constructing config::ValueKind

Note that for at least some of those crates (e.g. serde_json), you can deserialize directly into a custom type, so there is no difference between our own type and serde_json::Value.

toml now has a native type it parses into (toml::de::DeTable) but its not one really meant for us to carry around though we could if we wanted to.

Even where it would make sense to do so, we'd have to re-work how we do value merging which would require ways to walk the data structures. We'd have to track the source as we walked each one which is doable. We'd then have to deal with the lack of extensibility. For example, I was wondering if we should more thoroughly track the source and then extend that to also track the Span for the formats that support it.

[epage]

Specifically, there is no corresponding variant for eon::Value::Variant(value) in config::ValueKind

So yes. we'd handle variant like serde_json::Value handles variant, its a single-element map.