Fix #4390: Sort REPL autocomplete suggestions #4417
Conversation
dottybot
left a comment
dottybot
left a comment
There was a problem hiding this comment.
Hello, and thank you for opening this PR! 🎉
All contributors have signed the CLA, thank you! ❤️
Have an awesome day! ☀️
allanrenucci
left a comment
allanrenucci
left a comment
There was a problem hiding this comment.
Can you add a test case to dotty.tools.repl.TabcompleteTests. Something like:
@Test def sortedCompletions =
fromInitialState { implicit state =>
compiler
.compile("class Foo { def comp3 = 3; def comp1 = 1; def comp2 = 2 }")
.stateOrFail
}
.andThen { implicit state =>
val expected = List("comp1", "comp2", "comp3")
assertEquals(expected, tabComplete("(new Foo).comp").suggestions)
}| } | ||
| interactiv.println(i"completion with pos = $pos, prefix = $prefix, termOnly = $termOnly, typeOnly = $typeOnly = ${completions.toList}%, %") | ||
| (completionPos, completions.toList) | ||
| (completionPos, completions.toList.sortBy(_.name.show)) |
There was a problem hiding this comment.
You can use _.name.toString
|
Done! |
|
|
||
| @Test def sortedCompletions: Unit = | ||
| fromInitialState { implicit state => | ||
| val src = """class Foo { def comp1 = 1; def compa = 3; def compA = 4 }""" |
There was a problem hiding this comment.
Use simple quotes. Triple quotes are not needed here.
There was a problem hiding this comment.
I prefer the code snippet I gave you. I think the ordering is clearer:
class Foo { def comp3 = 3; def comp1 = 1; def comp2 = 2 }val expected = List("comp1", "comp2", "comp3")
allanrenucci
left a comment
allanrenucci
left a comment
There was a problem hiding this comment.
LGTM! Thanks @ankitson 🎉
| } | ||
| interactiv.println(i"completion with pos = $pos, prefix = $prefix, termOnly = $termOnly, typeOnly = $typeOnly = ${completions.toList}%, %") | ||
| (completionPos, completions.toList) | ||
| (completionPos, completions.toList.sortBy(_.name.toString)) |
There was a problem hiding this comment.
@smarter We have an ordering for Name that differs from the default String ordering. I am wondering if we should use it here? The Name ordering will not group type names and term names together
There was a problem hiding this comment.
I think they should be grouped together, but we should also not display type completions if we're not in a type context
There was a problem hiding this comment.
Sure but that's probably out of the scope of this PR
4 participants
This seems to be a quick fix for #4390. Its my first PR to dotty so I might be missing something.