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

Feat: "One" import granularity #11361

Closed
CAD97 opened this issue Jan 28, 2022 · 9 comments · Fixed by #16372
Closed

Feat: "One" import granularity #11361

CAD97 opened this issue Jan 28, 2022 · 9 comments · Fixed by #16372
Assignees
Labels
A-assists A-config configuration C-feature Category: feature request E-has-instructions Issue has some instructions and pointers to code to get started S-actionable Someone could pick this issue up and work on it right now

Comments

@CAD97
Copy link
Contributor

CAD97 commented Jan 28, 2022

The current options for rust-analyzer.assist.importGranularity are "preserve", "crate", "module", and "item". Personally, I group all of my imports into a single use block (and don't really understand why nobody else seems to do so); rust-analyzer should support (enforcing) this grouping along the existing use groupings.

@lnicola lnicola added C-feature Category: feature request A-assists S-actionable Someone could pick this issue up and work on it right now labels Jan 28, 2022
@flodiebold
Copy link
Member

This should be called One like in rustfmt.

@Veykril Veykril added A-config configuration E-has-instructions Issue has some instructions and pointers to code to get started labels Jan 28, 2022
@petr-tik
Copy link
Contributor

petr-tik commented Feb 7, 2022

can you please assign this issue to me?

@CAD97 CAD97 changed the title Feat: "all" import granularity Feat: "One" import granularity Feb 7, 2022
@petr-tik
Copy link
Contributor

petr-tik commented Feb 8, 2022

We also should adjust our guessing infra to be able to understand that form here

What's the most resilient way to identify the correct ImportGranularityGuess?

Looking at the rustfmt implementation, it looks like having merged use trees one for each visibility level is obviously One, but what about mixed cases? What should it guess?

https://github.com/rust-lang/rustfmt/pull/4680/files#diff-7bd4f14a75a37432af3ab896bbbe38f88428b490b5d76ed1586f7f473a745e98R1772-R1781

@Veykril
Copy link
Member

Veykril commented Feb 14, 2022

To identify we probably just want to check if there is only one tree that starts as use { ... };

If mixed it should fall back to Unknown I'd say.

@petr-tik
Copy link
Contributor

i was thinking about writing a test that checks for one tree for each of the possible publicity types

use { ... };
pub use { ... };
pub(crate) use { ... };

thoughts?

@Veykril
Copy link
Member

Veykril commented Feb 15, 2022

Sounds good

@petr-tik
Copy link
Contributor

Some personal stuff made me quite unproductive in February. Looks like @jeremyBanks is actively working on it, so it's only fair I am un-assigned from this issue

@petr-tik petr-tik removed their assignment Jul 21, 2022
irh added a commit to koto-lang/koto that referenced this issue Aug 8, 2023
Koto's been using the `one` import granularity style [1], but this doesn't play
well with Rust Analyzer [2] which has made wish for a change, and the `crate`
style seems like a decent alternative.

[1] https://rust-lang.github.io/rustfmt/?search=#imports_granularity
[2] rust-lang/rust-analyzer#11361
irh added a commit to koto-lang/koto that referenced this issue Aug 8, 2023
Koto's been using the `one` import granularity style [1], but this doesn't play
well with Rust Analyzer [2] which has made wish for a change, and the `crate`
style seems like a decent alternative.

[1] https://rust-lang.github.io/rustfmt/?search=#imports_granularity
[2] rust-lang/rust-analyzer#11361
@davidsemakula
Copy link
Contributor

@rustbot claim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-assists A-config configuration C-feature Category: feature request E-has-instructions Issue has some instructions and pointers to code to get started S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants