Add some mutable accessors to Array

[sunshowers]

I found these to be quite convenient while working with toml_edit.

(Could you also do a new release once this is accepted? Thanks!)

[ordian]

@sunshowers thanks for the PR! I think the reason I omitted mutable accessors to Array is to enforce type-safety, so that it would be impossible to create an invalid toml document. In TOML Arrays contain values of the same data type https://github.com/toml-lang/toml#user-content-array, but with mutable accessors it would be possible e.g. to swap a string with a number as Value type contains both.

We already have a mutating function push which checks the types (in push_value). Unfortunately, if we provide a user a mutable reference to a value, it would be impossible to check the types if a user will use e.g. https://doc.rust-lang.org/std/mem/fn.swap.html.

I don't know what would be the good solution here, will need to think about it.

[sunshowers]

ahhh I see! Yeah that's a problem.

Some ways I can think about resolving this:

• instead of an array being a vector of values, make it an enum of vector of strings, vector of integers etc — basically make those illegal states unrepresentable. (Rust doesn't have dependent types which would probably make this easier to model).
• do a consistency check while serializing arrays.

The core problem I was trying to solve was change elements of an array while preserving existing formatting.

[sunshowers]

Another way might be to return a wrapper around &mut Value instances that does validation for any replacement values. That might be a cleaner way to achieve the same goals, and the wrapper type would prevent mem::replace (which I was definitely using, heh)

[ordian]

Another way might be to return a wrapper around &mut Value instances that does validation for any replacement values. That might be a cleaner way to achieve the same goals.

That sounds like an interesting idea to try, but I'm not quite sure how the mutating API would look like?

It's possible already to get the value formatting with value.decor and create another value with the given decor with formatted.rs/decorated.

The reason Value has a as_str, but not as_str_mut method is because there are multiple ways to represent a string in TOML (single quoted, double quoted, etc, but it's not just about quotes themselves). That's why we store the repr.raw_value string representation in decor.rs/Formatted along with the (rust) value. If we change the (rust) value, we would also need to update its string representation (repr.raw_value).

Feel free to push commits implementing your ideas and I'll give you feedback.

[ordian]

Another idea would be to implement a bunch of set_integer(&mut self, index: usize, value: i64) -> Result<(), Error>, set_string... on Array.

[sunshowers]

Thanks! I ended up adding methods to insert and replace values in an array, both formatted and unformatted versions. Let me know what you think!

[sunshowers]

(These are breaking changes, I hope that's fine because I think Result is a better API than bool anyway :) )

[sunshowers]

Oh, and if you're curious what I'm using toml_edit for, it's in combination with the Cargo dependency analysis library guppy to implement operations on a workspace. The first one I worked on was mv to move crates around within a workspace.

[ordian]

Looks great, thank you!

I wonder what happened to CI...
Let's try to summon
bors r+

[ordian]

I wonder what happened to CI...

Looks like it was a mistake on my part https://github.com/ordian/toml_edit/blob/520bfe2c81af48664cfe5de16f5357d931aa0832/.github/workflows/ci.yml#L3.

[ordian]

(These are breaking changes, I hope that's fine because I think Result is a better API than bool anyway :) )

Yeah, I wanted to change that myself, sometimes you look at your code that you wrote 3 years ago and wonder... :)

[bors]

Build succeeded:

• Test (ubuntu-latest)
• Test (macOS-latest)
• Test (windows-latest)