-
Notifications
You must be signed in to change notification settings - Fork 1.1k
completions: do not complete package #20532
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
Conversation
@rochala btw, do you know if it should be backported to metals or not? |
Hey, so the situation is as follows. Short answer: there is no need Long answer: Metals is no longer publishing mtags for >= 3.4.0. We will also stop releasing them for LTS line in the near future (as soon as we backport most of the latest changes to the LTS branch). Hopefully it will be 3.3.4 |
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.
I left small comment
@@ -211,6 +211,8 @@ object Completion: | |||
val completer = new Completer(mode, pos, untpdPath, matches0) | |||
|
|||
val result = adjustedPath match | |||
// Ignore `package foo@@` and `package foo.bar@@` | |||
case ((_: tpd.Select) | (_: tpd.Ident)):: (_ : tpd.PackageDef) :: _ => Map.empty |
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.
The check is correct, but it may be better to make it in Completion.completionMode
instead of this place. The reason is that despite filtering compiler completions, completion mode is also used for the rest of the completions in presentation compiler.
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.
fixed
@dos65 this should be close to finishing, do you want still do it? |
@tgodzik oops sorry, I totally forgot about it 😅 will do it asap |
No worries and no hurry! I closed an another older PR of yours, but that one might have needed a total rework, so I figured that it might not make sense to keep it open |
There is an issue with completions for package in Metals. ```scala // code package one@@ // compeltions oneCURSOR ``` It seems there is no need in completions for Package at all.
49408b1
to
7c76ded
Compare
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.
LGTM
There is an issue with completions for package in Metals.
It seems there is no need in completions for Package at all.