Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify recommended ordering of groups, even without rustfmt support? #131

Closed
daboross opened this issue Jun 5, 2018 · 5 comments
Closed

Comments

@daboross
Copy link

daboross commented Jun 5, 2018

There's an original issue, #24, which discussed imports and ordering of imports.

It was closed with #129, but that pull request seems to only address the part of the issue which rustfmt can currently format (ordering within import groups).

I'm particularly partial to the ordering stated in #24 (comment). Specifically, imports should be split into at least three groups, consisting of:

  • imports from 'std'
  • imports from external crates
  • imports from within the same crate

There don't seem to be any major objections to this in #24 - the only problem is that rustfmt doesn't (or didn't at the time) have enough information to enforce this.

If a PR were made, could fmt-rfcs take stance on this issue and recommend these three groups?

If not, does anyone know a more appropriate place where this convention could be established?

@JeanMertz
Copy link

I was looking for the same functionality from rustfmt. I don't believe this was ever implemented, was it? Given the lack of comments, I guess this slipped through, and won't be considered anymore for a 1.0 release? Is this something that can be added later?

I believe this comment by @alexcrichton is solved with the new import rules in Rust 2018, right?

I imagine it could be hard for rustfmt to distinguish between internal/external crate imports, however :(, so it may not end up making it all the way through.

@daboross
Copy link
Author

daboross commented Oct 1, 2018

If I recall correctly, it was a conscious decision to not re-order import groups as there was no consensus?

https://github.com/rust-lang/rfcs/blob/master/text/2437-rustfmt-stability.md does allow for some formatting changes in minor version bumps so this could be included later.

@calebcartwright
Copy link
Member

Understand why folks would like some prescriptions for this, but I don't see this ever coming to any kind of consensus and rustfmt simply won't have sufficient information to be able to do it well/cover everything.

We have introduced a new(ish), unstable, config option in rustfmt called group_imports that provides a non-default variant to support that std/external/crate regrouping. I'm skeptical that many (if any) other substantive groupings and variants will be possible, but motivated users should feel free to experiment and submit a PR to rustfmt with new variants/groupings for that option if they'd like something different

@gibfahn
Copy link

gibfahn commented Jan 17, 2022

The group_imports option works great, thanks for adding it!

Is there an issue to track stabilising the option? I'm not in any rush, especially as you can configure rust-analyser to group imports, and to use nightly rustfmt, just curious as to how features progress from rustfmt nightly -> stable.

@calebcartwright
Copy link
Member

The group_imports option works great, thanks for adding it!

Is there an issue to track stabilising the option? I'm not in any rush, especially as you can configure rust-analyser to group imports, and to use nightly rustfmt, just curious as to how features progress from rustfmt nightly -> stable.

rust-lang/rustfmt#5083
https://github.com/rust-lang/rustfmt/blob/master/Processes.md#stabilising-an-option

Stabilization of options is a different beast than the standard propagation of changes (config options or otherwise) from nightly -> beta -> release.

To be fully transparent, there's a number of outstanding issues afflicting the various import related options, and I doubt any of them will be candidates for stabilization any time soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants