-
Notifications
You must be signed in to change notification settings - Fork 34
Don't insert imports when method calls are created through searcher #1209
Conversation
…om/enso-org/ide into wip/mwu/no-import-for-methods-1178
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, the reality is more complex than I expected. Actually there is scenario which works wrong with the current solution: press tab without any node selected, pick Table.new and press enter.
the new is a method, but it is a method of not imported module "Table". So actually we should skip importing only when we pick a method for this argument.
@farmaazon @mwu-tow it's even more complex! Even if you have the
|
As for point 2: in scope of this task is only checking if we literally have the same import (as we do with other suggestions). |
@farmaazon sure, point 1 is the most important one, because it could lead you towards overoptimization – i.e. not importing extension methods, which would break programs (and not just make them ugly by too many imports) |
I have not read this code btw. it's very possible you've already taken everything into account :P Just saw this discussion and randomly commented |
No, no, the point 1 in your comment is important (cc @mwu), and no, we don't handle it in our code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, update the branch so I could easily test it with all new changes.
…-import-for-methods-1178
Pull Request Description
When method call is inserted as a suggestion from searcher, the import of the relevant module won't be added anymore.
It is not needed and may cause trouble with name collisions, like #1145.
Issue #1178
Important Notes
Checklist
Please include the following checklist in your PR:
CHANGELOG.md
was updated with the changes introduced in this PR.