VS2017 RC - Edit.CompleteWord should not block

[Pilchie]

Repro steps

1. Invoke Edit.CompleteWord (Ctrl+Space)

Expected behavior

It asynchronously computes completion.

Actual behavior

It synchonously blocks

Known workarounds

Using Edit.ListMembers (Ctrl+J) instead should start an asynchronous computation, but not block until commit happens.

Related information

We can/should tweak the command handler to just do the ListMembers behavior.

A further concern however is that Roslyn doesn't have any cancellation support in completion once an operation has triggered a commit - it will block until it knows what to commit. Given the existence of type providers, we may need to change Roslyn to accommodate that.

Tagging @CyrusNajmabadi and @rchande for thoughts.

[dsyme]

@vladima Any pointers on this? Thanks

[CyrusNajmabadi]

I think we should have no problem making this command asynchronous. Note, we should not use a cancelable wait dialog here. It would massively impede topping, possibly capturing keystrokes.

We may simply want to put a timeout for the blocking here. I'd like it if could also listen for esc, but I'm not sure how we'd do that if we're currently blocked waiting for completion to complete. Can we somehow still register and hear about those keystrokes if we've blocked the UI thread?

[CyrusNajmabadi]

I'm also thinking we might need a way for a completion service to say that it should never block. For providers that simply can't guarantee fast completion time. I know Venus wants this as they may make network calls when trying to compute completion items.

[dsyme]

know Venus wants this as they may make network calls when trying to compute completion items.

The same applies to F# type providers

What is Venus?

[isaacabraham]

Exactly. The Azure Storage TP can sometimes take a few seconds to return depending on number of blobs in a folder etc. so you won't want that to block whilst it's waiting to return.

[CyrusNajmabadi]

'Venus' is the MS name for the web stack technologies. So things like HTML/json/CSS completions.

[CyrusNajmabadi]

tagging @rchande @mattwar

Ok. I've investigated this a bit. While i don't think it would be hard, we def need to do some cleanup in completion around where we do blocking. I think Ravi, Matt or myself could definitely do this (though Ravi might be best as he's done a lot of recent work in the completion controller).

Right now we've scattered 'blocking' functionality in a few places in completion, and we likely would need to carefully unify these areas, and ensure acceptable behavior with a host language that wanted to never block. The places where we block today are:

First, some clarifying terms:

1. 'Completion Session' (or just 'Session'). The term we use for the lifetime of work related to the completion list. It starts when we kick off the background work to get the completion results, and ends when completion is finally dismissed. Note: the Session starts before the actual presentation of results. i.e. when <dot> is hit, we kick off the work in the background to get the results. So there is an active 'Session' even though we haven't even computed any actual items, and we certainly haven't displayed anything.
2. 'Completion Model' (or just 'Model'). The immutable data we've computed about which items to show. We create an 'Initial' 'Model' that contains all the initial items we want to present, and we then fork that model as the user interacts with Completion to produce the current items to display. i.e. when the user hits <dot> we'll compute the initial Model with all items in it. Then, as they type, we'll filter that Model, producing new Model instances. These new Models contain the entire initial list, the current filtered list, and lots of cached computation data to help make things faster when the user types.

Completion has a few 'State's which can generally be expressed in terms of the above two concepts. These states are not explicitly called out in code (but would likely be valuable to do so).

1. Inactive: There is no Session at all. i.e. User is just typing, and we haven't heard anything that would make us think that completion was triggered.
2. Active, no-initial-model: We had something that we though might trigger completion (like <dot> or the start of a new identifier). We have kicked off the background work, but it has not come back yet. Ergo, from the user's perspective, completion is not 'up' (visible). During this time, if certain actions happen (like user hitting up/down). We will treat them as user actions in the buffer, and not user actions that would affect the completion list. Because the user is interacting with the buffer, we will then cancel the background completion work, and we will end the session, as we assume the user isn't interested in completion. i.e. hitting <dot><up> quickly, will result in an inactive session with no completion.
3. Active, initial-model-computed: We have gotten the initial set of completion results from the completion service. Completion is up (or will be up the next time the UI thread has a chance to run). At this point, we assume that many commands now affect the completion list. i.e. if the user hits <up> we will change the completion selection rather than move the caret.

Ok, given the above, here the places where we currently block:

1. If we have a session, and the user types a non-alpha-numeric character (like <dot>). In this case we block because the character may be intended to 'commit' the session. i.e. the user use "Console" and quickly types ".WL(". First, the dot will cause the session to start, then "WL" will kick off backgroun "filter" tasks. Then, when we see the "(", we have to decide what we want to do. For C#/VB we want this to complete to "WriteLine" (in order to not break muscle memory). As such, we Block until we've computed the Model and filtered it appropriately. Then we can see if ( is considered a Commit character for the currently selected item.
2. If we have a session and tab or enter are typed. This is similar to case '1'.
3. If we have a session and the caret is moved manually (i.e. not through normal typing). We have some complex logic here that i don't quite understand and should likely be done in another manner. i.e. if the caret moves to the start of the word being typed, we switch to soft-selection mode, so that committing won't insert that word. This likely can be done in a much simpler manner, without needing this explicit logic.
4. Complete-Word is invoked. If there is no session, one will be created. We then wait for the model. We then the selected item if it considered 'unique' (i.e. the text typed so far only matches a single item).

Ok, given the above, i think we should change our behavior specifically in the "Active, no-initial-model" state. Recall, this is the state where the completion service is computing items, but has not returned anything yet. In this state the user effectively thinks there is no completion and they'll have a very bad experience if completion takes many seconds and we end up blocking. We should allow specific completion servics to opt out of the blocking experience while in this state. So, if we're in "Active, no-initial-model" and any of the first three places where we block happens, then we just immediately cancel completion so that we don't end up blocking for something which may take a long time.

This leaves the fourth case ("Complete-Word" is invoked). If we have an , this is simple, we're in "Active, initial-model-computed" then we just commit the single selected item if it has one. However, if we're in Inactive, what should we do? The user has invoked the keystroke which is them telling us "i want to complete out this word" but in order to do that, we need to actually compute the completions in order to determine what to commit to. Perhaps with languages like F#, this command is simply not supported while completion is Inactive or is in "Active, no-initial-model". Or maybe it does not have any effect but to kick off the completion work. Then, once we get to "Active, initial-model-computed" the user can hit complete-word again to actually commit. This means that in F# you'd need to hit complete-word twice if completion wasn't active. In essence complete-word would act like "list members" initially. And only if you had the list up would it commit.

[dsyme]

@CyrusNajmabadi Super analysis, thanks

This leaves the fourth case ("Complete-Word" is invoked). ... However, if we're in Inactive, what should we do? The user has invoked the keystroke which is them telling us "i want to complete out this word" but in order to do that, we need to actually compute the completions in order to determine what to commit to.
Perhaps with languages like F#, this command is simply not supported while completion is Inactive or is in "Active, no-initial-model". Or maybe it does not have any effect but to kick off the completion work.
Then, once we get to "Active, initial-model-computed" the user can hit complete-word again to actually commit. This means that in F# you'd need to hit complete-word twice if completion wasn't active. In essence complete-word would act like "list members" initially. And only if you had the list up would it commit.

In VS2015 I think the behaviour for Ctrl-Space is that, if the user types something else or does anything to move away before the completion is committed, then the completion request is ignored and cancelled. There is no blocking up to that point

So from our perspective it feels like we want to do the part "actually compute the completions in order to determine what to commit to" asynchronously,. Then check the cursor location and document version are still matching on async-completion of the request

[CyrusNajmabadi]

Hey @dsyme . That makes a lot of sense. And i think we should be able to provide that without too much difficulty. Thanks!

[CyrusNajmabadi]

I have a PR out for this now. Will try to get it in soon!

[dsyme]

That's great!

Could you take a,look at #1808 (comment) too please? We basically need a way to request an on-idle re-analysis of a file due to a detected change outside of Roslyn. Something that triggers semantic error checking to happen again.

[CyrusNajmabadi]

Fixed with dotnet/roslyn#15572

[dsyme]

@brettfo This got auto-closed - I think we still need to do this once we pick up the new Roslyn packages:

@dsyme @OmarTawfik You will be able to control this by setting "CompletionOptions.BlockForCompletionItems" to 'false' for the F# language.

[Pilchie]

And auto-closed again.

[vasily-kirichenko]

The flag is in "approved for RC3" status dotnet/roslyn#15572 (comment)

We must not forget to set it to false as it's available!

[vasily-kirichenko]

See what I've found dotnet/roslyn#15924

/cc @forki @dsyme

[vasily-kirichenko]

A good news is that the blocking autocomplete is definitely a problem for us:

and I hope dotnet/roslyn#15572 (comment) will fix it.

[Pilchie]

@vasily-kirichenko would it be possible to grab the option by reflection and set it to validate before the API makes it to F#?

[Pilchie]

Alternatively, try updating the Roslyn packages to some newer version on https://dotnet.myget.org/F/roslyn to get at the option?