Add properties priority for completion#32266
Add properties priority for completion#32266sheetalkamat merged 8 commits intomicrosoft:masterfrom fuafa:properties-priorities
Conversation
sheetalkamat
left a comment
sheetalkamat
left a comment
There was a problem hiding this comment.
Thank you for working on this.
src/services/completions.ts
Outdated
|
|
||
| // Set SortText to MemberDeclaredBySpreadAssignment if it is fulfilled by spread assignment | ||
| function setSortTextToMemberDeclaredBySpreadAssignment(membersDeclaredBySpreadAssignment: Symbol[], contextualMemberSymbols: Symbol[]): void { | ||
| for (const fulfilledSymbol of membersDeclaredBySpreadAssignment) { |
There was a problem hiding this comment.
Short circuit for filteredSymbols .length === 0 is needed.
src/services/completions.ts
Outdated
|
|
||
| if (isBindingElement(m) && m.propertyName) { | ||
| if (isSpreadAssignment(m)) { | ||
| const expression = m.expression; |
There was a problem hiding this comment.
Ok last one change needed.
Scope out this login into another function that sets the members in map so that it can be shared for object members and jsx attributes
|
Anything I miss? @DanielRosenwasser @sheetalkamat 🆙 🚀 |
src/services/completions.ts
Outdated
| return contextualMemberSymbols; | ||
| } | ||
|
|
||
| const membersDeclaredBySpreadAssignment = createMap<boolean>(); |
There was a problem hiding this comment.
This needs to be Map since we don't use actual value.
There was a problem hiding this comment.
But it is a Map already, and what do you mean we don't use actual value?
There was a problem hiding this comment.
I was on mobile and didnt realise its not showing correctly. Sorry about that. It needs to be Map<true>
src/services/completions.ts
Outdated
| } | ||
|
|
||
| // Set SortText to MemberDeclaredBySpreadAssignment if it is fulfilled by spread assignment | ||
| function setSortTextToMemberDeclaredBySpreadAssignment(membersDeclaredBySpreadAssignment: Map<boolean>, contextualMemberSymbols: Symbol[]): void { |
There was a problem hiding this comment.
Change to Map<true> here too.
3 participants
Fixes #29868