martinohmann / hcl-rs

HCL parsing and encoding libraries for rust with serde support
Apache License 2.0
121 stars 14 forks source link

Remove static newline suffix #271

Closed denfren closed 1 year ago

denfren commented 1 year ago

might fix #270

I added a test for the body roundtrip that terminates without a newline as a regression test.

martinohmann commented 1 year ago

Hi there, thanks for the PR, this is actally a good spot.

However, moving the trailing newline into the Decor essentially makes it optional for Structures that are constructed programatically. Encoding a Body that contains these now would produce invalid HCL. Users would need to manually add this to the decor suffix of every structure that they add to a Body to make it emit valid HCL.

One way to fix this I see is to inline this:

for structure in body {
    structure.encode_decorated(buf, NO_DECOR)?;
    buf.write_char('\n')?;
}

at https://github.com/martinohmann/hcl-rs/blob/b92d3d49920683c83bf70008c8c6f832c0cf5481/crates/hcl-edit/src/encode/structure.rs#L55 and then change https://github.com/martinohmann/hcl-rs/blob/b92d3d49920683c83bf70008c8c6f832c0cf5481/crates/hcl-edit/src/encode/structure.rs#L10-L13 to something like this:

let len = self.len();

for (i, structure) in self.iter().enumerate() {
    structure.encode_decorated(buf, NO_DECOR)?;

    if i < len-1 {
        buf.write_char('\n')?;
    }
}

This will only enforce the newlines for every structure of the top-level body, except the last one.

But there we'll have another problem that we need to keep track if the body was newline terminated or not somehow.

For me this smells a bit like splitting the body parser into one for the block body and one for the top-level body to handle this edge case.

martinohmann commented 1 year ago

See my other comment in #270 as well. I think my proposed solution isn't ideal either.

denfren commented 1 year ago

Closing in favor of #273