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

reorder_imports does not respect comments #1373

Closed
malbarbo opened this issue Mar 11, 2017 · 5 comments
Closed

reorder_imports does not respect comments #1373

malbarbo opened this issue Mar 11, 2017 · 5 comments
Labels
a-comments only-with-option requires a non-default option value to reproduce poor-formatting

Comments

@malbarbo
Copy link
Contributor

// Comment 1
use std::mem;
use std::marker;

// Comment 2
use std::iter;

pub fn main() {}

rustfmt produces

// Comment 1


// Comment 2
use std::iter;
use std::marker;
use std::mem;

pub fn main() {}

The comment 1 lost their context. I would expect something like:

// Comment 1
use std::marker;
use std::mem;

// Comment 2
use std::iter;

pub fn main() {}

Maybe related with #1137

@ishitatsuyuki
Copy link

This needs some kind of discussion.

Currently there's a test that expects the behavior that comments moves together with the code below; this one doesn't follow the rule.

Example:

/// sticky
use b;
use a;

to:

use a;
/// sticky
use b;

@nrc
Copy link
Member

nrc commented May 17, 2017

@ishitatsuyuki I'm not clear - are you saying the example is expected but doesn't happen or happens but shouldn't? It looks correct to me (that is the 'sticky' comment sticks to the import of b).

@ishitatsuyuki
Copy link

@nrc this is the current behavior and it clearly conflicts with the example posted by OP.

@nrc
Copy link
Member

nrc commented May 17, 2017

Ah ok, so I guess the question is whether the first comment should be special-cased and kept at the top of the imports instead of sticking to the import it is 'on top of'? At the moment, it seems that the comment is left and newlines are inserted? There also seems to be some confusion about whether we maintain grouping of imports.

So, I think that

  • we should fix the extra newlines - that looks like just a bug
  • If there is a newline between comments and the first use, leave the comment where it is. If not, the comment should stick to the use
  • No grouping for of imports for now (at least, not by using newlines).

@topecongiro
Copy link
Contributor

Triage: the first issue is fixed, closing in favor of #2185.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-comments only-with-option requires a non-default option value to reproduce poor-formatting
Projects
None yet
Development

No branches or pull requests

4 participants