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
Add cross tests for pattern only case completions, fix for typed case completions #4356
Conversation
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.
Thanks for adding the test cases! That highlighted some issues we might have
), | ||
true, | ||
false, |
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.
Could you use named arguments for all the booleans?
| } | ||
|}""".stripMargin, | ||
"""|None scala | ||
|Some[_] scala |
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.
|Some[_] scala | |
|Some[?] scala |
_
might get deprecated at some point
| case _: N@@ => | ||
| } | ||
|}""".stripMargin, | ||
"""|None scala |
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.
None is an object, so ideally we shouldn't suggest it
454ae9f
to
5800185
Compare
case (sel @ Select(qualifier, name)) :: _ | ||
if "match".startsWith(name.toString()) && text.charAt( | ||
completionPos.start - 1 | ||
) == ' ' => |
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.
it also might be a .
I found this usage recently at https://dotty.epfl.ch/docs/reference/other-new-features/indentation.html
one
+ two.match
case 1 => b
case 2 => c
+ three
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.
In the first, main PR about match case completions for scala3 we came to conclusion to show match completions after two.match
but not two.m
, two.ma
etc. because I think most users aren't using this syntax and having match completion there might be confusing (also, match
completions are exclusive, maybe we should change that?)
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 think we can make match completions not exclusive, to enable things like xxx.match@@
and completion xxx.matches()
, but it's better to not show match completions in xxx.mat@@
(only for xxx.match@@
)
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.
Ok, thx for clarifying!
case Some(query) => CompletionFuzzy.matches(query, name) | ||
|
||
def toCompletionValue( | ||
sym: Symbol, | ||
name: String, | ||
autoImports: List[l.TextEdit], | ||
)(using Context): CompletionValue.CaseKeyword = | ||
)(using Context): Option[CompletionValue.CaseKeyword] = | ||
sym.info |
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.
this line is unused
"""|head :: next scala.collection.immutable | ||
|Nil scala.collection.immutable""".stripMargin |
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.
Shouldn't these result be filtered out by fuzzy-match?
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.
There is a problem, because new
is not seen on path (the tree looks the same as for case@@
. I'm looking into it, if I won't find a solution soon I will create an issue
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.
fuzzy-match isn't working, because compiler doesn't see the Ident("new")
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've created an issue #4367
Adds a cross tests suite to check if we cover all cases like
case _: @@
,case xxx @ So@@
etc.Also fixes sorting completions when
CaseKeywordCompletions
are not exclusive.Edit: I moved all extractors to one place