Standardize toml format with taplo

[ameknite]

Objective

• Standardize fmt for toml files

Solution

• Add taplo to CI (check for fmt and diff for toml files), for context taplo is used by the most popular extension in VScode [Even Better TOML](https://marketplace.visualstudio.com/items?itemName=tamasfe.even-better-toml
• Add contribution section to explain toml fmt with taplo.

Now to pass CI you need to run taplo fmt or if you use vscode have the Even Better TOML extension with 2 spaces for indent

[alice-i-cecile]

I would like to have toml format checking in CI, and this seems like a sensible tool and solution. It's annoying to deal with inconsistent formatting, as it either means hassling contributors to not auto-format or accepting churn.

[mockersf]

what would be the default formatting, without a config file?

[ameknite]

@alice-i-cecile It is already, or what do you mean?

[alice-i-cecile]

My previous comment was expressing my endorsement of this change :) I like the intent, but I am unsure about the details.

[ameknite]

@mockersf the indent is with 2 spaces, but 4 is more used in rust. I checked other repos like wgpu and winit, and both use 4 spaces

[mockersf]

I think I would prefer without a config file.

This should be mentioned in https://github.com/bevyengine/bevy/blob/main/CONTRIBUTING.md, with links to how to install and how to use it.

I'm not sure it should be plugged in our ci rust script, as it means it will start to fail for everyone who haven't installed that tool, even if they are not touching toml files. Maybe it should be like the markdown linter instead.

[alice-i-cecile]

I like this. IMO we should avoid duplicating this info in CONTRIBUTING.md, but if others want that, I don't feel strongly.

[bushrat011899]

This is a great idea. I recently came across taplo when contributing to matchbox and it was fantastic to have a standardised Cargo.toml file.

[alice-i-cecile]

@ameknite if you can resolve conflicts, I'm happy to merge this in :)

[mockersf]

It would be better to stick to the default settings

[daxpedda]

FWIW: custom settings could be specified in a configuration file if that's desirable. The VSCode extensions also correctly detects and uses it.

[mockersf]

yup, but adding a new file on the root level adds clutter to the repository. IMO formatting is already nicer enough, aiming for the 4 spaces isn't worth either the clutter or the complexity

[rlidwka]

This is not a good change:

- bevy_reflect = { path = "../bevy_reflect", version = "0.12.0", features = ["bevy"] }
+ bevy_reflect = { path = "../bevy_reflect", version = "0.12.0", features = [
+    "bevy",
+ ] }

This is the first time I see features being split to a separate line (especially if there's only one feature), and it is less readable.

Overall, you are adding more tooling that every contributor has to install and run before making a pull request. It does not need to be standardized. It needs to be readable and easy to use, and to me it doesn't look like achieving either.

[rlidwka]

I'd suggest to go over each change and add them one-by-one (because current formatting is indeed inconsistent at times). In a year from now, if people add more inconsistencies, you can go over all the changes with your formatter again.

Enforcing this and putting it on CI is imposing a small burden onto everyone (people who edit toml will have to install the tool, and the rest of contributors will just get worse CI response time). Same argument against cargo fmt by the way, but that's a topic for separate discussion.

[bushrat011899]

Overall, you are adding more tooling that every contributor has to install and run before making a pull request. It does not need to be standardized. It needs to be readable and easy to use, and to me it doesn't look like achieving either.

By adding it to the CI, local devs don't need to have it installed. Once this is merged, the entire set of Cargo.toml files will be standardised. If a new dev wants to make a change, they're very likely to mimic the exact same formatting that is already in the file. Even if they don't mimic it (or they come across an edge case like wrapping a list), the CI will tell them exactly what's wrong anyway. The worst case scenario is amending your commit to match the feedback CI gives you.

I would prefer it if this was a feature built into cargo to ensure it was more standardised, but that's a different matter.

I think having something like this is important to avoid needless churn going forward. cargo fmt doesn't just format the code to make it look pretty, it's the final say in what's correct for the whole project. Bringing that freedom through restriction to the Cargo.toml only seems natural.

[rlidwka]

@ameknite, when I make a pull request, I grant access to that branch to bevy's maintainers ("Allowing edits by maintainers" checkbox on PR).

Can this thing be configured to format, commit and automatically push formatted files to that branch that's triggered CI (if it's pull-request and not master branch of course)?

[alice-i-cecile]

Yeah, we could add a bot of some form that either automatically does formatting, or does it on command. I'd be interested in exploring that later for Rust/markdown/toml, as long as it's always in a separate commit.

[rlidwka]

- bevy_reflect = { path = "../bevy_reflect", version = "0.12.0", features = ["bevy"] }
+ bevy_reflect = { path = "../bevy_reflect", version = "0.12.0", features = [
+    "bevy",
+ ] }

I suggest to increase column_width (e.g. --option column_width=90 or 100) specifically to keep older one-line formatting of internal modules with bevy feature enabled.

This affects 20 changes in this PR (about half of it), and it's the only change that I can argue to be worse than before.

[DasLixou]

Coming from #10663 , could we add a Todo in the description to not forget to change the tracing doc link again?

[ameknite]

@rlidwka I would like to keep a simple fmt. It is not about achieving a beautiful format but rather making it standardize and easy to use.

There reason why everyone agree to use cargo fmt is not because is pretty but because is standardize.

[ameknite]

@DasLixou Fixed, thanks for mentioned it.

[bushrat011899]

Looks good to me!

[ameknite]

oh sorry, I didn't notice, it must have been the automatic formatting that I have in vscode