Make imports consistent.

[sunli829]

I don't know if this is a coding style standard, but I know that many popular projects use this import order. 😁

[davidpdrsn]

The order of imports isn't something I'm very concerned with tbh. The one used today is the rustfmt's default. Can you configure rustfmt to use this ordering?

[davidpdrsn]

It seems rustfmt has an group_imports option which when set to StdExternalCrate should match this style. That option isn't stable yet however.

Until rustfmt stable is able to automatically format imports the way we want I'm reluctant to change the formatting as it creates churn and will not be maintained unless done automatically.

Do you know of a way to maintain this style automatically?

[sunli829]

rustfmt can't do this automatically, I did it manually, but I saw other projects do it, which seems more clear. 😁

[davidpdrsn]

I did it manually

In that case I think we should wait with this. Even though I'm sorry for discarding all this tedious manual work 😕

The order used now is the default ordering used by rustfmt when all the imports are in one block (no newlines between) so at least that gives some consistency.

[sunli829]

I respect your decision. 😁

[jplatte]

What I've done in a bunch of projects before is have one Stable CI job that runs cargo test (and cargo check for different feature sets) and a one Nightly CI job that runs cargo fmt and cargo clippy (benefitting from new lints as they come out, with the downside that there's also false positives now and then).

[davidpdrsn]

What do you do for running rustfmt locally then? Override it to use nightly somehow?

[jplatte]

Well stable rustfmt still applies all non-experimental formatting options for me and with the codebase being consistently formatted I hardly ever accidentally introduce something that nightly rustfmt would format differently. Occasionally I might cargo +nightly fmt.

[ZhangHanDong]

The group_imports option doesn't solve this problem either, and you end up having to manually group the imported modules in order to improve code readability. Of course these tasks can be done after the project is stable.

[jplatte]

@ZhangHanDong What do you mean by that? I'm aware of two bugs affecting group_imports, but it doesn't sound like that's what you're referring to.

[ZhangHanDong]

@jplatte

What I mean is that relying on group_imports alone doesn't achieve the desired effect either.

The only option currently available for group_imports is StdExternalCrate, which can be divided into three groups.

1. std, core and alloc,
2. external crates.
3. self, super and crate imports.

https://rust-lang.github.io/rustfmt/?#group_imports

But wouldn't external crates have to be manually grouped by the developer?

[jplatte]

Not sure what additional grouping of external crates you're talking about. From a quick glance at this PR it seems like it implements exactly what rustfmt would do too with group_imports = "StdExternalCrate".

[ZhangHanDong]

@jplatte You'll understand if you look at the changes in this pr.

group_imports = "StdExternalCrate " is useless for this kind of grouping and needs to be manually adjusted by the developer.

Does this make it clear?

[jplatte]

But why do you think that the previous version would be preferred? I was under the impression that @davidpdrsn was agreeing with changing things in the way that this PR does, but that the churn is not worth it if it can't be enforced in the future.