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

Handle inline comments for imports #125

Closed
Bengt opened this issue Feb 19, 2014 · 8 comments
Closed

Handle inline comments for imports #125

Bengt opened this issue Feb 19, 2014 · 8 comments

Comments

@Bengt
Copy link

Bengt commented Feb 19, 2014

Sometimes it is useful to comment imports, e.g. to clarify renames:

from a import f as f_small  # more space efficient implementation
from b import f as f_quick  # more time efficient implementation

Currently isort removes such inline comments:

from a import f as f_small
from b import f as f_quick

isort should reorder and reformat inline comments properly.

@timothycrosley
Copy link
Member

Agreed,

historically isort hasn't done this because it adds a lot more complexity.

For instance given:

from a import b # comment 1
from a import c # comment 2

Considering the imports end up as:

from a import b, c

Where should the comments go? And what about dealing with line length limits etc?
However, I think at this stage of the project - it's worth it to make sure this works as best as possible:

from a import b, c # comment 1, comment 2

is after all, much better then stripping potentially useful comments.

Thank you for bringing this up, will fix and then deploy a new release.

~Tim

@Bengt
Copy link
Author

Bengt commented Feb 21, 2014

Glad you see it that way. I see the complexity of this, which gets worse with the various multi-line modes. I think though, comments in imports are fairly common, so handling them seems worth an effort.

Yes, concatenating comments seems to be the best possible solution. That way no comments get lost and the user can edit them manually, if necessary. Concatenating with a comma might lead to ambiguity when the comments already contain commas:

from a import b  # bar, foos a foobar
from a import f  # foo, bars a foobar

That would become:

from a import b, f  # bar, foos a foobar, foo, bars a foobar

Maybe better use a semicolon, as it is far less commonly used:

from a import b, f  # bar, foos a foobar; foo, bars a foobar

@timothycrosley
Copy link
Member

@Bengt, you're right a comma is too overused in comments to use as a separator here. I'll switch to a semicolon as suggested, thanks for pointing this out!

~Tim

@timothycrosley
Copy link
Member

Fixed in version 3.6.0 :)

@Bengt
Copy link
Author

Bengt commented Feb 23, 2014

Thanks for implementing this. It works almost as expected. I opened an issue for the remaining detail. See #129.

@dandavison
Copy link

Going back to the original example,

from a import f as f_small  # more space efficient implementation
from b import f as f_quick  # more time efficient implementation

If the user has specified --force_single_line_imports then these lines should simply be reordered, maintaining comments. Currently however, the comments are completely lost. I'd like to suggest re-opening this issue to address that problem.

@dandavison
Copy link

Sorry, the above comment was slightly inaccurate. In fact, this is not an issue with --force_single_line_imports; this is an issue with the default behavior:

from a import b  # b comment
from a import c  # c comment

is left unchanged, whereas

from a import b as bb  # b comment
from a import c as cc  # c comment

has its comments stripped.

@dandavison
Copy link

I've proposed a fix for the above issue in PR #179.

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

3 participants