[WIP] Merge imports from the same module #2475
Conversation
Note that `merge_imports` takes effect only when paired with `reorder_imports`.
|
(I think that this option is required in RFC somewhere, but I forget where and cannot find one) |
topecongiro
force-pushed
the
merge-imports
branch
from
f222ae4
to
144e595
Compare
| use super::{contains_comment, rewrite_comment, CharClasses, CodeCharKind, CommentCodeSlices, | ||
| FindUncommented, FullCodeCharKind}; | ||
| use shape::{Indent, Shape}; | ||
| use CharClasses; |
There was a problem hiding this comment.
Does not handle use super::{/* .. */}; properly.
topecongiro
changed the title
|
A couple of comments: I don't think we need this to work when reorder is not enabled - I can't imagine someone would want to merge but not reorder. We need to think a bit about how we want to use nested imports. My personal feeling (and the intention on the RFC) was that nested imports should be used rarely and shallowly. But that is not a great guideline for an automatic tool. To start with it might be good to have different levels of merging, rather than just true/false. E.g., no merging/merge with no nesting/merge with some nesting (?)/merge with maximal nesting. I'm also not sure if we should un-merge at all. E.g., if you specify 'merge with no nesting' and there is a nested import, should we split it? btw, there is also a compile error. |
|
@nrc Thank you for you reviews. Unfortunately I will not have enough time to work on rustfmt for a while, so I am going to close this PR for now. I will come back on this at some point, thank you for taking time to review on WIP PR! |
This PR adds a
merge_importsconfig option.When this option is enabled, rustfmt will merge
usedeclarations from the same module.The implementation is limited in several ways:
Themerge_importsonly works whenreorder_importsis set toture.Closes #1383.
Closes #2452.
EDIT:
merge_importsdoes take effect on its own, but still need some work and more tests.