rope icon indicating copy to clipboard operation
rope copied to clipboard

AutoImport imports if already imported

Open erikw opened this issue 7 years ago • 10 comments

AutoImport should not import the given name if it is already imported. Issuing this command many times on the same symbol will just append more imports

from X import Y
from X import Y
from X import Y
from X import Y

erikw avatar Dec 09 '16 14:12 erikw

I'll take a look at this soon, it's definitely a bug.

Thanks for reporting!

soupytwist avatar Dec 09 '16 16:12 soupytwist

Hey Nick. I'll check it on next week. Please assign this issue to me.

sergeyglazyrindev avatar Dec 09 '16 20:12 sergeyglazyrindev

Github won't let me do that, lame. It's all yours!

soupytwist avatar Dec 09 '16 20:12 soupytwist

@soupytwist , probably, @sergeyglazyrindev should be added as member first.

emacsway avatar Dec 10 '16 14:12 emacsway

Hey guys! I am not sure this is an issue with rope Autoimport stuff. It seems like an issue in ropemode. https://github.com/python-rope/ropemode/blob/master/ropemode/interface.py#L606-L613 Ropevim uses ropemode to communicate with rope and editor. All what does ropemode is simply asking a list of globally registered modules https://github.com/python-rope/rope/blob/master/rope/contrib/autoimport.py#L56-L62 and then if such name appears in current module and if there's only one such name, it automatically inserts import It seems to me it's not an issue but it works by design. According to what I understand in ropemode it works following way: https://github.com/python-rope/ropemode/blob/master/ropemode/interface.py#L605 it asks vim for currently cursored word and looking for such global module and if it exists inserts import though if there are two global modules with the same name, it asks which module would you like to insert. That's why I think it works as designed. But it raises few more questions: what if we decide we want to add here workaround, we need to add it to ropemode because:

  1. in theory it should be a job connected to the code where we have an access to currently edited text (edited text could be not saved, so we can't rely on observing changes in a file, that's why it can't be handled in rope), ropemode provides us such access, we may add such workaround to ropemode: https://github.com/python-rope/ropemode/blob/master/ropemode/interface.py#L659 if (this.env.current_file.has_text('from X import Y')): return

sergeyglazyrindev avatar Dec 22 '16 11:12 sergeyglazyrindev

if (this.env.current_file.has_text('from X import Y')): return

Great analysis! Thank you. Just one thought: don't we have some better way how to find which imports are present than simple text search?

mcepl avatar Dec 23 '16 17:12 mcepl

Yes, I thought about much deeper analysis. But I need to test it with ropevim and vim. Actually, I use emacs, so that's what I could to get just investigating the code sources. But as far as I understand, anyway, much proper solution should be related to analyzing text in code editor because the file could be changed and not saved, so, if we decide to implement it only in rope (which is not by design of autoimport module, as far as I understand) we will need anyway fallback to checking if such import statement present in code editor's opened file (to make it 100% working). So, I can tell a bit more about this problem in a week or so. Right now finalizing one important project.

sergeyglazyrindev avatar Dec 23 '16 20:12 sergeyglazyrindev

Doesn’t ropemacs use ropemode as well?

mcepl avatar Dec 24 '16 09:12 mcepl

Yes, it does. I'll try to do much deeper analysis a bit later.

sergeyglazyrindev avatar Dec 24 '16 09:12 sergeyglazyrindev

In the sqllite implementation under #469 there is an option to pass a list of ignored names. I used this in https://github.com/python-lsp/python-lsp-server/pull/199 to ignore names already defined in the file.

bagel897 avatar Apr 29 '22 16:04 bagel897