Custom operations are at the top of completion list & remove active pattern values from the list

[vasily-kirichenko]

This fixes #4829

[vasily-kirichenko]

It also fixes #4832

(VS NUnit 3 Test Adapter is unable to discover any test, so I cannot run the new tests locally, chances are they will fail on CI).

[forki]

That's great news vasily. Hope to see it in FCS soon since we are about to do a lot of Saturn soon. Vasily Kirichenko <notifications@github.com> schrieb am Sa., 5. Mai 2018, 10:11:

…

It also fixes #4832 <#4832> [image: image] <https://user-images.githubusercontent.com/873919/39660907-14bb699a-5051-11e8-898d-1c17bdfa658d.png> (VS NUnit 3 Test Adapter is unable to discover any test, so I cannot run the new tests locally, chances are they will fail on CI). — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#4831 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADgNLU2BR5xQ3FWiXg88UdykP4eqiDqks5tvV6rgaJpZM4TzNcg> .

[vasily-kirichenko]

@forki To move custom operations to the top of completion list in Ionide, it must sort items by kind itself (in FSAC, I think). This PR expose CustomOperation kind for items, but the sorting is done in FSharp.Editor, which is not a part of FCS and works in VS only.

[forki]

Ok thanks. Paging @Krzysztof-Cieslak who is conveniently in charge of ionide and Saturn ;-) Vasily Kirichenko <notifications@github.com> schrieb am Sa., 5. Mai 2018, 10:31:

…

@forki <https://github.com/forki> To move custom operations to the top of completion list in Ionide, it must sort items by kind itself (in FSAC, I think). This PR expose CustomOpation kind for items, but the sorting is done in FSharp.Editor, which is not a part of FCS and works in VS only. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4831 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADgNA1qneMS2VTC4eClYyhJbXe80jibks5tvWNhgaJpZM4TzNcg> .

[cartermp]

VS NUnit 3 Test Adapter is unable to discover any test, so I cannot run the new tests locally, chances are they will fail on CI

Nope 😄

[cartermp]

Testing this out right now. I have two thoughts, neither of which are related to the PR:

1. Interesting that CompletionItemKind doesn't have a notion of a local, which you may want at the top of the completion list under some circumstances. But the placement of the priority makes sense to me given what exists.
2. WOOF, while we're here can we just make bestAlmostIncludedSoFar a mutable value instead of the ref cell? Pretty sure @dsyme wouldn't mind that change even if it's not related to this PR directly...

[cartermp]

Hmmm, building this PR against master on 15.7 results in #4832 and #4829 are still there. Maybe I just screwed up installing the VSIX into VS because I'm tired, but the version is 42.42.42.42 and your commits are in the log on the branch I built off of...

[vasily-kirichenko]

@cartermp I confirm the sorting does not work on Saturn's application CE, however it does work on query CE.

[vasily-kirichenko]

Kind is wrong for custom operations:

[F#][INFO 12:21:42 PM] First 10 completion items:
[|("abs", Field); ("acceptHtml", Field); ("acceptJson", Field);
  ("acceptMultipart", Field); ("acceptXml", Field); ("acos", Field);
  ("application", Field); ("array2D", Field); ("asin", Field); ("async", Field)|]

[vasily-kirichenko]

The fix for #4832 was wrong. It turns out for the following code

let _ = 
  query {
    for i in 1..10 do
    <* cursor *>
  }

at the cursor position there is no name resolution environment for the entire CE, and the "smart" algorithm of GetBestEnvForPos picks the first lhs one, which spreads from { to do and has eIsComputationExpression set to true. My fix was to not pick such nenvs specifically, to avoid polluting completion list with custom operations outside of the last CE. It results with no custom operations at cursor position above. So, I've reverted the fix and left only the sorting stuff.

[vasily-kirichenko]

It's ready.

[vasily-kirichenko]

I also remove active patterns (as values) from completion list altogether.

Before

After

[cartermp]

@TIHan / @KevinRansom / @brettfo / @Pilchie we should consider this for 15.8 P4

[TIHan]

This is a very reasonable change. Looks good to me.

[KevinRansom]

@dotnet-bot test this please

[KevinRansom]

@dotnet-bot no longer works :-( so closing and reopen is the way until we get a new way.

[KevinRansom]

@vasily-kirichenko thanks for this mate. Nice work.

Kevin

[Krzysztof-Cieslak]

❤️