Skip to content

using value_repr is clunky, as it forces me to think about document formatting #130

@sodiboo

Description

@sodiboo

I want to create a numeric KDL entry with arbitrary precision (converting to KDL), like so:

let mut entry = KdlEntry::new(KdlValue::Float(0.0));
entry.set_format(KdlEntryFormat {
    value_repr: some_potentially_really_long_numeric_string,
    ..Default::default()
});

But, doing so actually doesn't work if i format my document, as autoformat() removes all formatting, including the value_repr, so all my values just become 0.0.

That's okay. I can tell autoformat not to touch this entry:

+ entry.keep_format()

But now, the generated document (after autoformatting) is no longer valid.

Error: 
  × Found invalid node name
    ╭─[10:37]
  9 │ }
 10(development)config debug=#trueport=8080
    ·                                     ──┬─
    ·                                       ╰── not node name
    ╰────
  help: This can be any string type, including a quoted, raw, or multiline string, as well as a plain identifier string.

This is because autoformat() now keeps the value_repr, but for reasons that are beyond me it also keeps leading, which i didn't ever intend to touch.

kdl-rs/src/entry.rs

Lines 184 to 190 in 439aa63

#[cfg(feature = "v1")]
self.ensure_v2();
self.format = self.format.take().map(|f| KdlEntryFormat {
value_repr: f.value_repr,
leading: f.leading,
..Default::default()
});

In the ensure_v1/ensure_v2 functions, there's this code, which seemingly acknowledges this issue:

kdl-rs/src/entry.rs

Lines 272 to 286 in 439aa63

if let Some(value_repr) = value_repr.as_ref() {
self.format = Some(
self.format
.clone()
.map(|mut x| {
x.value_repr = value_repr.into();
x
})
.unwrap_or_else(|| KdlEntryFormat {
value_repr: value_repr.into(),
leading: " ".into(),
..Default::default()
}),
)
}

If there's no format on the node, and it sets the value_repr, it also sets the leading to a singular space.

But that branch is unreachable? This format.unwrap_or_else(...) is in an if statement, which runs if and only if the format is also Some(_).

So, in my code, i must copy this unreachable branch and set leading: " ".into(). But, i don't want to have to care about this.

I would expect KdlEntryFormat::default() to contain sensible values, like having a leading space (because, without any leading or trailing whitespace, a given KdlEntryFormat isn't valid inside of a full document). But, the format of an Entry shouldn't need to contain leading/trailing spaces to be valid. impl Display for KdlEntry actually doesn't behave weirdly at all on its own, even when there's empty leading/trailing formatting. The issue only occurs when stringifying a KdlNode. Currently, a space is inserted if and only if there is no explicit format set for the entry:

kdl-rs/src/node.rs

Lines 846 to 852 in 439aa63

for entry in &self.entries {
if entry.format().is_none() {
write!(f, " ")?;
}
write!(f, "{}", entry)?;
space_before_children = entry.format().is_none();
}

A lot of places like this check for a single field of the format struct. It would perhaps make a lot more sense, given this usage, for every format field to be Option<T>. There's no way to specify "an entry with a specific value repr, but unspecified leading whitespace"; which is what i really want; i'd like all the implicit printing behaviour of adding leading spaces, please.

Also, the space before the children check here basically is always ignored, because, when creating a node, the format is Some(_) by default, which overrides the space_before_children check. With autoformatting, the space is explicitly inserted, so this quirk is easy to miss, unlike with KdlEntryFormat.leading which is not mitigated by autoformat.


I'm aware of #77, and i suppose this is a "duplicate"/subset of that issue; this is related to interaction with autoformat or more generally the formatting options. "it's completely borked right now" yeah i noticed.

But i want to use kdl-rs as a fully data-oriented KDL document builder without caring at all about formatting. I can almost do that with autoformat as-is. Most of its problems don't affect me at all, as my documents never contain comments, raw/multiline strings, etc.

If i don't care about numeric precision, kdl-rs is already usable like this. As soon as i try to set value_repr, i must start manually formatting. :(

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions