Rustfmt: [unstable option] merge_imports

Created on 13 Feb 2019  Â·  60Comments  Â·  Source: rust-lang/rustfmt

Tracking issue for unstable option: merge_imports

unstable option

Most helpful comment

On the topic of enum vs bool from https://github.com/rust-lang/rustfmt/issues/3362#issuecomment-637163171 and https://github.com/rust-lang/rustfmt/issues/3362#issuecomment-639811574:

I think the clearest way to frame import merging to me is in terms of the scope of what gets put into the same use; the Always/Never/Shallow terminology does not make as much sense to me in comparison.

Various styles in the real world are as follows. Note that the whitespace in everything below is not significant to this option and is governed by https://github.com/rust-lang/rustfmt/issues/3361 which is an orthogonal option.

  • Everything goes into 1 use.

    use {
        futures::{
            ready,
            stream::{Stream, StreamExt},
        },
        std::pin::{Pin, Unpin},
    };
    
  • Every crate gets 1 use.

    use futures::{
        ready,
        stream::{Stream, StreamExt},
    };
    use std::pin::{Pin, Unpin};
    
  • Every module gets 1 use. People call this "shallow" above but I think that name is pretty opaque.

    use futures::ready;
    use futures::stream::{Stream, StreamExt};
    use std::pin::{Pin, Unpin};
    
  • Every item gets 1 use. Java style. Minimizes conflicts in codebases that have a lot of people touching the same files. My work codebase is interested in this if it were implemented. Has been requested in https://github.com/rust-lang/rustfmt/issues/3361#issuecomment-632661079 and https://github.com/rust-lang/rustfmt/issues/3362#issuecomment-641125438.

    use futures::ready;
    use futures::stream::Stream;
    use futures::stream::StreamExt;
    use std::pin::Pin;
    use std::pin::Unpin;
    
  • Off; whatever the programmer wrote.

    use futures::{ready, stream::Stream};
    use futures::stream::StreamExt;
    use std::pin::{Pin, Unpin};
    

All 60 comments

Would it be possible to update this Tracking issue with more information?

For example, why is this unstable? Should this always be opt-in, or is the plan to enable it by default, what remains to be done to make this option stable, etc.

Which PR implemented this (e.g. https://github.com/rust-lang/rustfmt/pull/2603) ?

There is a closed fmt-rfc: https://github.com/rust-dev-tools/fmt-rfcs/issues/24 where @nrc said:

I propose that we not do this by default, but tools may offer options to do this.

But that's about it, the merged RFC does not talk about it, so I'd guess there is no merged RFC saying anything about this. Is the next step towards stabilization to merge an RFC about this ? (independently of which direction that RFC goes).

Assuming the rust compiler is behaving as intended, this can break imports. Currently, these are not semantically equivalent, but rustfmt with merge_imports will format the former into the later:

use alloc::vec; // imports *both* the macro and the module
use alloc::vec::Vec;
use alloc::vec::{self, Vec}; // only imports the module and `Vec`. `vec!` is left out.

Perhaps merge_imports shouldn't use self in a leaf position at all? So this

use alloc::vec::{self, Vec};

should always be converted to

use alloc::{vec, vec::Vec};

I wish there was a third option: unmerge imports into one import per line.

I would love merge_imports to be stabilized. I use it in all my personal and work crates. But if splitting imports is also desired, it should probably be under the same config directive, so probably good to think that through first.

With https://github.com/rust-lang/rustfmt/pull/3753 merged, is the only blocker renaming the option so it is a list of 3 values (Never merge, always merge, leave imports untouched)?

With "never merge" I suppose you mean something that splits imports, or unmerge them?

I suppose we want one value that turns

// this
use std::{ptr, mem};
// into
use std::ptr;
use std::mem;

And one value that does the opposite. And of course the third one which disables this feature altogether, not touching use statements.

I would like to see merge_imports, but it seems like there is some work to do before it is bug-free enough to do so.

One observation is that it is a little inconsistent about when it uses one line vs multiple lines (though I guess this is a problem without the option, but you notice it more with the option). Using merge_imports does give an opportunity to change behaviour in a backwards-compatible way.

E.g.,

// Formatted on one line.
use futures::{compat::Compat01As03, future::BoxFuture, prelude::*};
// Formatted on multiple lines.
use std::{
    fmt::{self, Display},
    result,
};

IMO both of those should be multi-line.

Shouldn't it just be controlled by the max line width and small_heuristics like code expressions are?

What is missing for this to be stabilized?

Hi! I'm asking the question again. What is missing exactly? I'm willing to help and make a PR if necessary. Should I try to change the behavior pointed by @nrc ?

The process for stabilizing a configuration option is detailed here. That documentation also enumerates the conditions/requirements to stabilize:

* Is the default value correct ?
* The design and implementation of the option are sound and clean.
* The option is well tested, both in unit tests and, optimally, in real usage.
* There is no open bug about the option that prevents its use.

As it relates to merge_imports, I believe at least a couple of those conditions have not yet been met.

There are several open issues related to the option in addition to some of the items discussed in this thread. There are also outstanding questions regarding the default value, or even if this option should be an enum like some other config options instead of the current boolean.

According to the stability of the implementation:
We are using this formatting option in our codebase with approx 30k lines of code and it is even integrated into our CI - so commits that do not adhere to our formatting are automatically rejected.
This works since many months now and I have never had a single problem with this formatting scheme.

Also I think it would be nice if the default was true instead of false since it is to my opinion one of the most useful formatting schemes by the amount of formatting work it takes from my shoulders.
However, I'd argue that for a start we should have false as default value to avoid changing the formatting of many crates out there.

Maybe an edition change could be used to change the default formatting rules?

Thank you @calebcartwright! I had a look. I already started to check code.

@Robbepop
I agree that it's quite stable. We're using this option in my company as well (in a "soft" way, we format with it, but CI check is using stable channel). However looks like edgecases are remaining, with comments and already invalid Rust code (?) (not sure about what should be done with that one as it sounds fair that invalid code is formatted into invalid code).
Also, I see there is an RFC to enable merge imports by default: https://github.com/rust-dev-tools/fmt-rfcs/issues/140 .
I believe the matter of the default should be discussed there.

I saw several comments in this thread about an alternative "shallow merge behavior": never import multiple modules on one line. Essentially only struct, enum, etc are merged (e.g. use std::ffi::{CStr, CString} but never use std::{convert::TryFrom, ffi::{CStr, CString}}). That's what this issue is about I believe.

Regarding the option. I believe enum-like would be a good idea.

  • we can add new values without breaking existing configuration
  • we already have multiple good values: Always, Never, Shallow, LeaveUntouched (default: LeaveUntouched)

Shallow being the aforementioned behavior.

Big question. What about imports_layout? Looks like this option is similar:

imports_layout [Vertical|Horizontal|HorizontalVertical|LimitedHorizontalVertical|Mixed] Default: Mixed

Is there any way to do the automatic unmerging with the current rustfmt version?

\
Personally I have no idea why Rust even has this import merging "feature". It's just a source of merge conflicts, makes copy-pasting code more difficult, and makes understanding of the import lines more time consuming... why is this a thing? What is the benefit? Reminds me of the Makefile syntax's itch to deduplicate every single symbol, at the heavy cost of readability and maintainability.

use futures::{compat::Compat01As03, future::BoxFuture, prelude::*};
use std::{
    fmt::{self, Display},
    result,
};

I mean what am I even looking at? Import lines should be the most straightforward piece of code. Compare the above with

use futures::compat::Compat01As03;
use futures::future::BoxFuture;
use futures::prelude::*;
use std::fmt::self;
use std::fmt::Display;
use std::result;

Wow. Boring, I know. But suddenly, if there are 2 commits adding a futures import, they won't conflict? And the imports are actually readable?

Surely I am missing something...
\

I'd really like to see this stabilized (preferably with options "Never" and "Shallow").

I think another option that I’ve seen in some codebases is the use of only one use statement:

use {
    futures::{compat::Compat01As03, future::BoxFuture, prelude::*},
    std::{
        fmt::{self, Display},
        result,
    },
};

rather than

use futures::{compat::Compat01As03, future::BoxFuture, prelude::*};
use std::{
    fmt::{self, Display},
    result,
};

I've been a big fan of merge_imports in the past. Not so much for its exact behaviour but because it provides consistency across the codebase.
AFAIK, it is the only option in rustfmt that completely re-arranges all imports to a consistent style.

In terms of actual behaviour, I would likely switch to Shallow if it gets stabilized as the current behaviour causes too many merge conflicts. Concretely, merging all imports together causes them to wrap around newlines if they get long. Removing an import can therefore effect several lines, causing merge-conflicts that might have been avoided if those imports would have been organized more shallow.

For everyone interested in stabilizing: https://github.com/rust-lang/rustfmt/issues/3362#issuecomment-637163171 explains why this has not been stabilized. I agree that the bar has not been met. Most significantly, https://github.com/rust-lang/rustfmt/issues/3112 strongly indicates that merge_imports would be the wrong name to stabilize this under, but that issue has not gotten attention recently. I would recommend directing some attention to there to make progress.

That issue is separate from the decision of whether an enum or bool is appropriate. For example under the enum possibility described in https://github.com/rust-lang/rustfmt/issues/3362#issuecomment-639811574 the Always setting would still result in imports getting unmerged which is a surprise.

On the topic of enum vs bool from https://github.com/rust-lang/rustfmt/issues/3362#issuecomment-637163171 and https://github.com/rust-lang/rustfmt/issues/3362#issuecomment-639811574:

I think the clearest way to frame import merging to me is in terms of the scope of what gets put into the same use; the Always/Never/Shallow terminology does not make as much sense to me in comparison.

Various styles in the real world are as follows. Note that the whitespace in everything below is not significant to this option and is governed by https://github.com/rust-lang/rustfmt/issues/3361 which is an orthogonal option.

  • Everything goes into 1 use.

    use {
        futures::{
            ready,
            stream::{Stream, StreamExt},
        },
        std::pin::{Pin, Unpin},
    };
    
  • Every crate gets 1 use.

    use futures::{
        ready,
        stream::{Stream, StreamExt},
    };
    use std::pin::{Pin, Unpin};
    
  • Every module gets 1 use. People call this "shallow" above but I think that name is pretty opaque.

    use futures::ready;
    use futures::stream::{Stream, StreamExt};
    use std::pin::{Pin, Unpin};
    
  • Every item gets 1 use. Java style. Minimizes conflicts in codebases that have a lot of people touching the same files. My work codebase is interested in this if it were implemented. Has been requested in https://github.com/rust-lang/rustfmt/issues/3361#issuecomment-632661079 and https://github.com/rust-lang/rustfmt/issues/3362#issuecomment-641125438.

    use futures::ready;
    use futures::stream::Stream;
    use futures::stream::StreamExt;
    use std::pin::Pin;
    use std::pin::Unpin;
    
  • Off; whatever the programmer wrote.

    use futures::{ready, stream::Stream};
    use futures::stream::StreamExt;
    use std::pin::{Pin, Unpin};
    

I have previously been a huge fan of Every crate gets 1 use. But I have started finding it a bit hard to read for some combinations of imports. I now personally prefer Every module gets 1 use and that is what I write manually most of the time. I also don't like the name "shallow" it tells me nothing. The java style singe item imports are way too verbose for me. We have not had a noticeable amount of import conflicts at work that I can remember. Maybe we are just too few developers or split our code over more modules?

What if the option has the following correct values: off, item, module, crate and one for all the cases above (in reverse order)?

If the option does not only merge imports, but rather it adjusts all imports to a given style. Sometimes it joins, sometimes it splits. Then maybe name the setting import_style?

I would love to see this option stabilized in some form at least. Every formatting option that takes decisions away from the programmer are amazing.

I now personally prefer Every module gets 1 use and that is what I write manually most of the time.

I agree that this is the best default for most small–mid sized projects and would be a good default for rustfmt.

(I used the wrong account to answer so I deleted my previous answer and post it again with the appropriate account. I apologize for the additional notification if you watch the issue)

This sounds great!

I don't have much to add on what have been said above, but I'll express my strong agreement.
First, the off, item, module, crate and one is a great naming. One can know exactly what behavior to expect.
Second, module or even item as default both sounds good. I personally have no strong argument for one or the other, but for sure crate or one would be disliked by many people as far as I can see in the related issues. I'm in favor of renaming the option to import_style as well.

As said previously, I want this option to be stabilized so I'll work on it. I'll submit PR to move in that direction as it seems promising.

I thought of a few points, I am not sure if they have been discussed before:

  • Import order must be stable, I guess merge_imports reorders them in alphabetical order.
  • The selected style should play well with use ... as ... and pub use ... and their mix.
  • It should also play well with #[cfg(...)] for conditional imports.
  • It's rare, but I sometimes have a comment for an import (most of the time something like: use ...; // this provide the xxx trait). Some libraries can be hard to reason around and having a few comments doesn't hurt. It would be great if the comment could follow the use.

I don't really know which style is better, I like the "every module gets 1 use" but for example, if I write:

use alib::prelude::*;
use alib::afeature::{TypeA, TypeB, sub::TypeC};

I might not want it to be changed to:

use alib::prelude::*;
use alib::afeature::{TypeA, TypeB};
use alib::afeature::sub::TypeC;

Because I consider A, B, C part of the same "feature module".

I understand this might be some "edge case" but this is to point to the issue that it is heavily project and personal preferences dependent.

I think that the only styles that can be consistently formatted are "1 use per file" or "java style".

And while I think the java style is horribly verbose, I think this is the best style to use in a collaborative environment, because:

  • Best merge behaviour.
  • Can have comment on the same line or above.
  • Can have a cfg or other above.
  • Can be manipulated for example wrapped into a cfg_if::cfg_if! easily, with good git merge.
  • It's easier to delete or move, as many editors (vim…) have keys to delete or move lines, if in my above example, you want to move TypeB us in another file (and not A and C), it's just a line cut with java style, but it's much more work with other style (have to delete TypeB from original file and have to delete TypeA from target file).

I realize those might not be all objective arguments, but I feel it might help in the discussion.

Thank you, those are all good points!
I just want to add my opinion on the following point.

I don't really know which style is better, I like the "every module gets 1 use" but for example, if I write:

use alib::prelude::*;
use alib::afeature::{TypeA, TypeB, sub::TypeC};

I might not want it to be changed to:

use alib::prelude::*;
use alib::afeature::{TypeA, TypeB};
use alib::afeature::sub::TypeC;

Because I consider A, B, C part of the same "feature module".

I understand this might be some "edge case" but this is to point to the issue that it is heavily project and personal preferences dependent.

I think you should probably set merge_imports to off if you care for import organization in such details. I understand your point is that we can't cover all edge cases, and that's true. As long as the offered options are reasonable for most projects, I'm not sure we want the option to become anymore complex.
Also, as an alternative, you may do pub use sub::TypeC inside the afeature module if you want to achieve a flat namespace (if sub module is not made public, the only import path effectively becomes alib::afeature::TypeC).
This sounds reasonable if A, B, C are to be considered part of the same "feature module".

I think it's good the option always gives the same result regardless of how you organize your imports beforehand. All I want is to throw new use on the top, and let the tool do the boring stuff in a predictable way so that when I go back to check my use statements I know how I need to read the block immediately.

EDIT: I realize that maybe your point was that the default value should rather be item (java style) and not module. If that's the case, my answer above probably missed the point and I apologize.

@CBenoit actually you got my points, but I wasn't really clear.

What I meant is that the only styles that can be "standardized/automated" are one or "item (java style)", as the styles "in between" are subject to preferences and interpretation.

As you said, if we care about the "aesthetic" so much it might be better to just turn merge_imports to off and organize manually.

The "item (java style)" style seems to have some support https://github.com/rust-lang/rustfmt/issues/3361#issuecomment-632661079 https://github.com/rust-lang/rustfmt/issues/3362#issuecomment-641125438 .

Now, while I don't like the verbosity of the "item (java style)", it's the only style that check all the boxes, especially those in larger projects, where import_style would be used the most.

As @CBenoit said (https://github.com/rust-lang/rustfmt/issues/3362#issuecomment-642871553) what seems the best course of action is to rename the option to import_style and default it with "item (java style)". We can keep crate and module but I suggest to document those styles with a short summary of what has been discussed in this thread as it might be tempting to change the default based on personal taste without realizing problems could arise.

I really think module is a sane default. As backed up by at least https://github.com/rust-lang/rustfmt/issues/3362#issuecomment-642572944. If we really can't agree on a good default style then I think off is the only sane default.

For what it’s worth, I spent a minute or two looking through the standard library’s documentation, and, out of the code samples I looked at, each one that used more than one use statement used module. Here are the examples I found: 1, 2, 3, 4.

Some of these aren’t unambiguously module though – they could also be item.

I’m not sure about the precedent here, but would it be considered a ‘good idea’ to create some sort of poll, perhaps with GitHub reaction emojis, to determine the overall community’s preference for import styles?

I don't think it's a good idea to base many style choices at all on documentation examples. Examples are all tiny, usually sub 20 lines of code and they are written to showcase one very very specific thing. So they usually go to great lengths showing that very thing in as little code as possible. That is rarely a good representation of "real" code. In real projects there is a ton more code and there are usually many features and crates interacting with each other.

For example: In documentation examples no one wants a very long list of use statements in the beginning, because it can drown the actual code being showcased. And no one cares about merge conflicts in examples since they are tiny and rarely modified. (I also don't want my real code to drown in use statements, which is why I personally like module :) )

I don't think that the documentation using module is an argument against module, I just think it's not a good argument in favor of it.

Personally I have no idea why Rust even has this import merging "feature".

One thing that I don't think is called out enough in this thread is the magical ability to do "set union" on two different pasted blocks of imports using this feature. That, together with automatic warning & fixing of unused imports, makes module-level refactors way more pleasant IMHO.

So if others feel like me, the specific choice of which option is default is less important than just having this set merging functionality. I've found it very odd that without this flag, rustfmt will reorder my imports alphabetically but not eliminate duplicates. (They're clearly errors, why leave them?)

Providing control in the of {off, item, module, crate and one} seems great.

I just wanted to throw in another vote for the "item (java style)" as the default.
I think there is no point in arguing over aesthetics of the different variants (different people like different things), but this one has the clear factual benefit, that it reduces merge conflicts.

Example:

# developer 1
-use alib::afeature::{TypeB, TypeC};
+use alib::afeature::{TypeA, TypeB, TypeC};
# developer 2
-use alib::afeature::{TypeB, TypeC};
+use alib::afeature::{TypeB, TypeC, TypeD};

no conflict with the "item (java style)"

+use alib::afeature::TypeA;
use alib::afeature::TypeB;
use alib::afeature::TypeC;
+use alib::afeature::TypeD;

What @timonbimon said is exactly my point.

I mean, we should not design rust and its tools around personal preference of a few rust programmers. We should really aim to what is best for the community, this means beginners, programmer coming from other languages...

The success of rust depends on its adoption, and the item has the lowest friction. This language has tremmendous potential and already achieved a lot, but any little thing that can lower the learning curve and increase the ease of adoption in the "enterprise" is good.

Folks, please post your thoughts about the various options and preferred default in the corresponding RFC in the Style Guide repo (https://github.com/rust-dev-tools/fmt-rfcs/issues/140).

The RFC process used for the Style Guide is precisely for these types of discussions and soliciting input, wheras this issue is primarily intended for us to be able to discuss and track any outstanding issues in rustfmt relative to the implementation of the config option and being able to stabilize it.

rustfmt's defaults are based on what's codified in the Style Guide, and we'll implement/update things in rustfmt accordingly based on the Style Guide; we don't make the decisions here.

We've got some good work in flight to get us to a point where we can support the various strategies discussed here. The next step is determining the new config option and variant names and as such I'd like to use this thread to get feedback from folks and then finalize those names. Posting the proposals here because the topic has been most heavily discussed in this thread and this thread likely has the highest number of folks subscribed/following.

Please utilize the reactions feature to indicate your support/objection to each name proposal below and refrain from any noisy comments (e.g. +1). The ask is for feedback on the _names_ and whether they sufficiently reflect the option/style, not whether the associated style is one you'd like/use or prefer to see as the default.

Feel free to suggest an alternative name, just be sure to indicate what it's an alternative to. These proposals are largely based on the excellent summary above in https://github.com/rust-lang/rustfmt/issues/3362#issuecomment-642564290 so please be sure to review that for framing on these proposed names.

Finally, note that this is not necessarily an exhaustive list (additional styles/strategies could potentially be added down the road), and there will need to be a separate stabilization review for this new option once delivered.

Config option name: imports_style

(alternative names proposed below, imports_blocks/imports_blocks_style, imports_granularity, and group_imports_style)

Variant: One - Everything goes into 1 use

use {
    futures::{
        ready,
        stream::{Stream, StreamExt},
    },
    std::pin::{Pin, Unpin},
};

(alternative name proposed below, Single)

Variant: Crate - Every crate gets 1 use (this maps to the current merge_imports = true)

use futures::{
    ready,
    stream::{Stream, StreamExt},
};
use std::pin::{Pin, Unpin};

Variant: Module - Every module gets 1 use

use futures::ready;
use futures::stream::{Stream, StreamExt};
use std::pin::{Pin, Unpin};

Variant: Item - Every item gets 1 use

use futures::ready;
use futures::stream::Stream;
use futures::stream::StreamExt;
use std::pin::Pin;
use std::pin::Unpin;

Variant: Preserve - Whatever the programmer wrote (this maps to the current default rustfmt behavior, merge_imports = false)

use futures::{ready, stream::Stream};
use futures::stream::StreamExt;
use std::pin::{Pin, Unpin};

(alternative names proposed below, Manual and Disable)

Your first two examples have indentation issues. I guess they are not there on purpose?

One:

use {
    futures::{
        ready,
        stream::{Stream, StreamExt},
    },
    std::pin::{Pin, Unpin},
};

Crate:

use futures::{
    ready,
    stream::{Stream, StreamExt},
};
use std::pin::{Pin, Unpin};

I only realized the question was about naming after a second reading, I think it would be helpful to put more emphasis on it in the original question.

Also, alternate naming suggestion for One: Single

Your first two examples have indentation issues. I guess they are not there on purpose?

Thanks for pointing it out (indentation got lost in copy/paste), though spacing/indentation isn't really the question at hand

Config option name: imports_style

I would like to propose imports_blocks (or imports_blocks_style if we want to retain the "use style postfix for enum-based options" convention).

We have several options for changing the formatting of imports in some way:

  • imports_layout
  • group_imports
  • reorder_imports
  • imports_ident

    I think imports_style by itself does not convey enough how it is different from the others.

Why imports_**blocks**_style?

I found the following terminology being used within existing rustfmt options:

  1. An "import group" is a set of imports separated by a newline. See the recently added group_imports option.
  2. imports_layout calls all imports within a single use an "import block".

(2) suggests that our new option controls how imports are organized into blocks:

  • one: only have one import block
  • module: create one import block per module
  • crate: create one import block per crate
  • item: create one import block per item

This looks fairly consistent with one exception: I believe syntactially, a block is identified by curly braces { }. With the item variant, there would not be any curly braces and hence the terminology of block is slightly off.

I think imports_style by itself does not convey enough how it is different from the others.
I would like to propose imports_blocks (or imports_blocks_style

I understand that thinking, and have gone through a few iterations of it myself. However, I think the usage of block within the name is a bad idea because there's far too many other connotations within the rustfmt context that aren't applicable here.

Though the imports_style name doesn't necessarily convey how it differs from other import options, imports_block_* would almost certainly be a source of confusion due to overlap, both with import specific options and more broadly.

I think an additional modifier in the config option name could be helpful, but I'm opposed to block being that modifier personally

I think an additional modifier in the config option name could be helpful, but I'm opposed to block being that modifier personally

I agree with that, block is just the best thing I could come up with. I am completely open to use a different modifier.

Edit: Based on the number of upvotes, people also seem to be happy with the imports_style name. I am also happy with that and actually super excited about this issue making progress!

imports_granularity would accurately describe that this option determines what goes in one import statement vs another.

imports_layout = "Horizontal"
imports_granularity = "Module"

imports_granularity would accurately describe that this option determines what goes in one import statement vs another.

Agreed. I like this one a lot more.

Given the work in flight, we'll first need to finalize the config option name and the name of the variants that map to the current true/false values for merge_imports, but can bikeshed on the other variant names if needed for a while. I've updated the items that have proposed alternatives and would encourage folks to only vote for one/change votes as needed (for example if you'd prefer imports_granularity over imports_style then remove any positive reaction from the imports_style comment)

Alternative suggestions for Preserve, depending on how the option is called in the end: Disable, Manual.

I think the verb "grouping" best describes what this option does. Something like group_imports_style or something maybe?

Alternative suggestions for Preserve, depending on how the option is called in the end: Disable, Manual.

Preserve variants are used within rustfmt config options to cover scenarios where rustfmt's maintains/does not modify the original formatting, so this was chosen intentionally for consistency.

Obviously a variant names applicability is dependent upon the option itself, but with the main option names discussed so far Disable looks pretty odd to me: imports_style = Disable, imports_granularity = Disable. Manual seems reasonable on its own IMO, but would introduce some inconsistency in the broader rustfmt configuration ecosystem

I think the verb "grouping" best describes what this option does. Something like group_imports_style or something maybe?

Understand the thinking with this one, but it's another case where it'd be problematic in the broader context since there's also a desire (and a new, unstable config option group_imports) to support moving/regrouping use items separately from the granularity (refs #4107)

Sorry to hear that there are problems with Manual since I find it much better than Preserve or Disable. Alternate suggestion: Free?

Alternate suggestion for One or Single (I still like the latter): Factorized.

Further notes regarding consistency:

Preserve is used in several config option names and variants already. Disable and Manual are not used anywhere.

single is used in several config names but one is not.

single is used in several config names but one is not.

Understand the thrust of the point, but this does need a minor correction. It's true that there are no option _variants_ that utilize Single, and there are no option _names_ that have One, but there is an existing option that has One as a variant

https://github.com/rust-lang/rustfmt/blob/rustfmt-1.4.31/Configurations.md#version
https://rust-lang.github.io/rustfmt/?version=v1.4.30&search=

Thanks to everyone who weighed in and voted! At this point I feel we have more than sufficient consensus to move forward with the first portion of this (#4600) to make the new configuration option and a couple of the variants available for consumption.

The new config option will be named imports_granularity, and will initially include the Crate, Module, and Preserve variants.

After incorporating the new option into the codebase I'll open up new individual issues to track the implementation of the respective additional variants and their associated styles, and then at some point down the road we'll start a new thread to track the stabilization of imports_granularity

Happy to let the discussion continue here on the names of the other variants (e.g. One vs. Single) until such time as an implementation is proposed, but we're going to go ahead and proceed with the option and ones enumerated above.

Since Item isn't contested anywhere in the discussion, would it be possible to include that variant as well?

Since Item isn't contested anywhere in the discussion, would it be possible to include that variant as well?

It's not really about whether or not there's contestation. The actual implementation already exists for the ones I mentioned above (rustfmt is capable of doing the corresponding formatting), but there is no such implementation for the others. If anyone is interested in doing the work to actually implement support for Item that'd be great, but we obviously can't just add an empty variant.

We're proceeding with what's implemented and possible today, and the others will be added at some point down the road. We just want to go ahead and start letting folks use what's already available now, as opposed to making everyone wait until everything is done to be able to use _any_ variant.

Was this page helpful?
0 / 5 - 0 ratings