Add a canonical rustfmt config

[PoignardAzur]

No description provided.

[DJMcNab]

If we do decide to set this, I'd expect it to have the same value as single_line_if_else_max_width, I think.

[xStrom]

Indeed, they seem very similar and have identical default values.

[DJMcNab]

Obviously this isn't used at all in Linebender yet. This is a reasonable value, but I'd lean more towards keeping the default without very good reason, for consistency with the rest of the ecosystem.

This is certainly more taste based than the other two options here.

[PoignardAzur]

For what it's worth I copy-pasted it from the xilem repo.

[DJMcNab]

Hmm. Fair. I don't care that much either way.

[xStrom]

Yeah we approved it in xilem#305. What I said back then still applies for me: it's quite minor and so I don't personally care much either way.

I believe this was originally added by you @PoignardAzur. So if you still think it's worth having, then I'm fine with it and don't want to bikeshed it. However, if you're not arguing for its benefit, then I don't think we have anyone doing that and we should omit it by default as Daniel mentioned.

[xStrom]

Properly standardizing this across repos has been a long time coming, thanks for getting the ball actually rolling.

[xStrom]

I think we should keep the import ordering settings as part of the standard. These have been discussed at various times before and are present in the commented out form in various repos. It would be nice to skip bikeshedding them again later.

The chosen options also match what were chosen by the Rust compiler team for their own code.

The specific suggestion here is copy-pasted from Xilem.

Suggested change

	# END LINEBENDER RUSTFMT CONFIG
	# Subjectively, this combo improves readability on imports. The full prefix is always
	# visible right after `use`, and different types of imports are grouped together in
	# a predictable way (first std, then other crates, then local imports).
	# (These options are still unstable and thus commented out)
	# imports_granularity = "Module"
	# group_imports = "StdExternalCrate"
	# END LINEBENDER RUSTFMT CONFIG

[PoignardAzur]

One of the reasons I didn't include them, besides them being unstable and commented out, is that we don't actually apply them even if the repositories where they're included.

There's a bunch of cases where strictly following these settings actually makes the import code harder to read, because it doesn't take into account things like cfg attributes which may make you want to import something separately.

[xStrom]

Not actually applying them yet is true, but also I think that is a bit of a chicken and egg problem. Having it be part of this standard config would help us propagate it to all the repos that would make actually occasionally applying it much easier. I can volunteer to do a pass of all the repos to apply it once the config has been propagated.

Now not taking into account things like cfg attributes would definitely be a blocker. However I'm not aware of any cases where this actually happens. I just did a test run of this on the Xilem repo and the only file where I saw cfg attribute guarded imports is masonry/src/render_root.rs. In that file it behaved correctly, it moved both the import and its guard together.

#[cfg(not(target_arch = "wasm32"))]
use std::time::Instant;

Do you have an example of where rustfmt fails with cfg attributes?

For what it's worth I also think that the Rust compiler team adopting these settings is a strong enough signal that the remaining bugs aren't very severe.

[PoignardAzur]

I didn't say there was a bug. Just, there are some cases where I want the cfg-guarded imports to be in a separate block.

[DJMcNab]

In my mind, the reason to want these settings is to reduce merge conflicts, not to have ideal formatting in all cases (as is the case with all auto-formatting...).

I do agree that having cfg-d imports not intermingled would be better, but given that there's supposedly only one case of this in the entire Xilem repository, I agree with Kaur that we should aspire to enabling this.

[PoignardAzur]

Let's merge linebender/xilem#838, then I'll create a PR showing the remaining differences, so we have a concrete example.

[PoignardAzur]

Done: linebender/xilem#847