Format members Array of Cargo.toml in workspace

[detoxifiedplant]

Problem

While adding new members with cargo new member_name in a workspace, the members list should be updated with the new member being added.

Current implementation sorts the members list upon adding new member. Prefix of " "(an empty space) gets added before name of each member/display_path resulting in improper formatted output of members list.

Executing mentioned steps should result in Cargo.toml file as below

[workspace]
members = [ "crates/lib_auth","crates/lib_core"]

The members list have inconsistent format.
Correct Format:

[workspace]
members = ["crates/lib_auth", "crates/lib_core"]

Steps

1. mkdir work_space
2. cd work_space
3. echo "[workspace]" > Cargo.toml
4. cargo new crates/lib_core
5. cargo new crates/lib_auth

Possible Solution(s)

Format the members list which is Array struct of toml_edit lib.

I have fixed the issue and have raised the PR.

Notes

No response

Version

cargo 1.84.0 (66221abde 2024-11-19)
release: 1.84.0
commit-hash: 66221abdeca2002d318fde6efff516aab091df0e
commit-date: 2024-11-19
host: x86_64-unknown-linux-gnu
libgit2: 1.8.1 (sys:0.19.0 vendored)
libcurl: 8.9.0-DEV (sys:0.4.74+curl-8.9.0 vendored ssl:OpenSSL/1.1.1w)
ssl: OpenSSL 1.1.1w  11 Sep 2023
os: Arch Linux [64-bit]

[epage]

Detecting the current style and replicating it, especially as some format styles call for changes based on size, is difficult. cargo fix does not produce fully styled Rust code but requires the running of cargo fmt. We've been taking a similar approach here for editing of Cargo.toml, whether that be cargo new, cargo add, cargo remove, or cargo fix. This is dependent on cargo fmt gaining support for Cargo.toml which is being tracked in rust-lang/rustfmt#4091.

As for the example given, I am surprised that there is a leading space that is inserted. We maintain sort order if the array is already sorted. This means we put lib_auth before lib_core. As a new entry, lib_auth should be getting the default decor of a first entry.

[detoxifiedplant]

@epage thank you for responding,
i have raised the PR solving the issue.
the proposed solution in PR removes the user formatting and comments, hence closed.

[epage]

When we push a unformatted value
https://github.com/toml-rs/toml/blob/e92c3b66b05d1043dafad4366645599b4e4f6a24/crates/toml_edit/src/array.rs#L177-L180

toml_edit for some reason applies some default formatting
https://github.com/toml-rs/toml/blob/e92c3b66b05d1043dafad4366645599b4e4f6a24/crates/toml_edit/src/array.rs#L378-L391

When we sort, that default is no longer relevant.

I had thought we had deferred on all formatting decisions until render time. I had forgotten that that code was in there.

A quick fix for that leading space is for us to create a toml_edit::Value without decor and pass that in. I wonder if this design from toml_edit should be revisited. It was added back in toml-rs/toml#88 and apparently not revisited since then. I created toml-rs/toml#826 to track this

[detoxifiedplant]

yes, the issue arises when the toml_edit applies the default formatting when pushing the Value.
the issue arises when we sort the toml_edit::Array, the decorators gets sorted with the Value.

thanks for opening the issue,
but the logic seems very straight, prefix every Value with " " when pushed.

[detoxifiedplant]

@epage to fix the leading space we can change the prefix of the first element post the push,
if you allow, i can raise the PR for fixing it, it wont change the user formatting/comments.

Happy to help 😃

[epage]

What I would recommend is using push_formatted with a manually constructed Value that has no formatting.

In implementing this, we generally ask that tests be written and preferably with the test added in its own commit (and passes) and then the follow up commit fixes the behavior and updates the test so it also passes.

[detoxifiedplant]

NOTE: the mentioned issue only occurs if the newly added member becomes the first member in the members array, in order to use push_formatted we need to know whether the new member will move to first position and then conditionally handle the case.

Solution: we let the push work as it is, after members being sorted, we can conditionally strip the prefix if the new member moves to first position.

Can write-up tests and solution as mentioned once the Issues gets marked as accepted.

[epage]

NOTE: the mentioned issue only occurs if the newly added member becomes the first member in the members array, in order to use push_formatted we need to know whether the new member will move to first position and then conditionally handle the case.

No we don't need to know that. Values are "default formatted" by default. This defers formatting until we render output. Our problem is that push is applying formatting beyond the default.

Solution: we let the push work as it is, after members being sorted, we can conditionally strip the prefix if the new member moves to first position.

Trying to out-guess the sort like this is tricky

[detoxifiedplant]

No we don't need to know that. Values are "default formatted" by default. This defers formatting until we render output. Our problem is that push is applying formatting beyond the default.

as value_op is used to push the members, it is going to apply formatting anyhow.

Trying to out-guess the sort like this is tricky

not the guess, we can conditionally check it after the sort and strip away the prefix if it matches.

[epage]

as value_op is used to push the members, it is going to apply formatting anyhow.

Not with push_formatted

https://github.com/toml-rs/toml/blob/e92c3b66b05d1043dafad4366645599b4e4f6a24/crates/toml_edit/src/array.rs#L194-L196

[detoxifiedplant]

can you please share, how exactly do you suggest the workaround with it?

[epage]

Change from

 members.push(display_path);

to

 members.push_formatted(display_path.into());

[detoxifiedplant]

Replacing push_formatted and adding lib_core -> lib_auth -> lib_utils would produce like this

[workspace]
members = ["crates/lib_auth","crates/lib_core", "crates/lib_utils"]

which would still have inconsistent format

[epage]

There are two formatting issues here:

• the leading space which that fixes
• the lack of leading space which we've been punting to external formatters for handling

[detoxifiedplant]

Lack of leading space can only occur when the member from first position moves to next,
in those cases we can have a conditional check which can insert additional space.

would it be worth the trouble?

[epage]

As I said, we've been punting on detecting formatting styles and making new content consistent with it.