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

Option to disable "move-to-top" feature #249

Closed
hydrargyrum opened this issue Oct 3, 2023 · 3 comments
Closed

Option to disable "move-to-top" feature #249

hydrargyrum opened this issue Oct 3, 2023 · 3 comments

Comments

@hydrargyrum
Copy link
Contributor

Description

I work on a project which has many function-local imports because top imports would cause import cycles. But I can't use autoimport because it insists on moving them to the top (and also produces invalid syntax in this case, but that's another story).

Possible Solution

Make "move-import-to-top" feature optional, so it can be disabled, and still keep the "remove-unused-imports" feature.

@lyz-code
Copy link
Owner

lyz-code commented Oct 3, 2023

Hi @hydrargyrum thanks for taking the time to open an issue. Have you tried reordering your code so that you don't run into import cycles? It's often suggested that imports are left on top of the file both for cleanness and performance, so that's why we move them to the top. It's also because you can write imports as you develop code wherever you are so that when you run autoimport they are moved to the top.

However, I feel you that there are some cases where it would be nice to disable this feature. I'm personally no big fan of it as I think a refactor is a better solution, but if you want to make a PR that implements this and it doesn't add too much complexity I'm fine with merging it.

@wbbradley
Copy link
Contributor

I've added a PR to address this here. Note that the tests are currently failing due to an unrelated security vulnerability detected by the security tool. I have not had time to get autoimport's development environment up and running locally, so I could use help remediating that vulnerability. Probably a version bump is needed, but I'm out of time on this for now.

@lyz-code
Copy link
Owner

Available since 1.6.0 thanks to @wbbradley

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