[ty] Sort keyword argument completions higher#22297
[ty] Sort keyword argument completions higher#22297MichaReiser merged 3 commits intoastral-sh:mainfrom
Conversation
RasmusNygren
requested review from
AlexWaygood,
MichaReiser,
carljm,
dcreager and
sharkdp
as code owners
| @@ -0,0 +1 @@ | |||
| class Foo(m<CURSOR: metaclass>) | |||
There was a problem hiding this comment.
We don't really capture the sort order of this very well in existing tests in completion.rs::tests so it probably makes sense to capture it here as an eval test
AlexWaygood
added
server
RasmusNygren
force-pushed
the
fix-argument-completion-order
branch
from
99e9db6 to
19c242f
Compare
If we have identified context-aware argument completions we know that they will always be valid and are more likely to more precise than any other completions, hence we sort them first.
MichaReiser
force-pushed
the
fix-argument-completion-order
branch
from
19c242f to
e869046
Compare
| /// If that's the case, we know this is a very precise completion | ||
| /// that should always be valid and can be preferred when | ||
| /// ordering completions. | ||
| pub is_context_specific: bool, |
There was a problem hiding this comment.
Yeah okay, I think I buy this. I'm slightly concerned about collapsing all context specific information down into a single bool, but I think this is fine for now.
There was a problem hiding this comment.
Yeah I agree, I mainly did it like that because clippy started complaining about too many bools in that struct when I added another one and I didn't really feel the need to consider a bigger change at that point when I thought this seemed fine enough for now :) (and it does seem like a reasonable lint to obey and not just slap an ignore on)
There was a problem hiding this comment.
Hah. I will happily squash that lint for structs personally. (Too many bool parameters to a function is another story though.)
There was a problem hiding this comment.
Maybe that's what I should have done instead then, for next time :)
4 participants
Summary
Fixes astral-sh/ty#2271
Test Plan
Fix existing snapshot tests