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

OrganizeImports removes entire lines #21

Closed
mcepl opened this issue Nov 30, 2013 · 1 comment
Closed

OrganizeImports removes entire lines #21

mcepl opened this issue Nov 30, 2013 · 1 comment

Comments

@mcepl
Copy link
Contributor

mcepl commented Nov 30, 2013

This is what Rope (using the Vim plugin) does when applying the RopeOrganizeImports command:

 import re
 from datetime import datetime
+
+from django.core.exceptions import ObjectDoesNotExist
 from django.db import models
-from djangosphinx.models import SphinxSearch
-from django.core.exceptions import ObjectDoesNotExist
 from django.db.models import Q
-from django.contrib.auth.models import User
+from django.template.loader import render_to_string
 from django.utils.safestring import mark_safe
 from django.utils.translation import ugettext as _
-from apps.users import signals
-from django.template.loader import render_to_string
+from djangosphinx.models import SphinxSearch

If you look at the changes, you can see that "from django.contrib.auth.models import User" as well as "from apps.users import signals" were completely removed instead of moved.

  • The signals import was removed because it wasn't used. I don't want it to be removed though, as the import has intentional side effects.
  • The User import was removed even though the User module is used in the code.
@lieryan
Copy link
Member

lieryan commented Sep 7, 2021

The signals import was removed because it wasn't used. I don't want it to be removed though, as the import has intentional side effects.

Removing unused import is an intentional behavior. rope can't know that you intend to use an import just for its side effect. Once we've implemented __all__ (#8) you can add the imported lines there to prevent it from being removed. It would also be reasonable if user can add a special comment markers (e.g. # rope: keep-unused-import) on imports lines to prevent rope from removing them during reorganization. I've separated that to a separate ticket (#368).

The User import was removed even though the User module is used in the code.

There is not enough info to reproduce this issue. Please open a new ticket if you find specific scenarios where rope removed imports that it shouldn't have.

@lieryan lieryan closed this as completed Sep 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants