Skip to content

Merge master to master-vs-deps#2

Closed
genlu wants to merge 7 commits intomaster-vs-depsfrom
merges/master-to-master-vs-deps
Closed

Merge master to master-vs-deps#2
genlu wants to merge 7 commits intomaster-vs-depsfrom
merges/master-to-master-vs-deps

Conversation

@genlu
Copy link
Copy Markdown
Owner

@genlu genlu commented Jan 29, 2020

This is an automatically generated pull request from master into master-vs-deps.
(created from a manual run of the PR generation tool)

git fetch --all
git checkout merges/master-to-master-vs-deps
git reset --hard upstream/master-vs-deps
git merge upstream/master
# Fix merge conflicts
git commit
git push upstream merges/master-to-master-vs-deps --force

Once all conflicts are resolved and all the tests pass, you are free to merge the pull request.

CyrusNajmabadi and others added 6 commits January 18, 2020 16:17
…121.3 (dotnet#41138)

- Microsoft.DotNet.Arcade.Sdk - 5.0.0-beta.20071.3
Filter identifier name simplification according to aliases and predefined types
Moves an ArrayBuilder for DeclaredEnvironments to a single environment.
This means that a scope only declares a single closure environment for its
captured variables.

I left space for us to declare multiple closure environments
in a single scope, but that seems like over-engineering at the
moment and it makes a lot of code more complicated. We can
always add it back if it seems like it would be worth it
@genlu
Copy link
Copy Markdown
Owner Author

genlu commented Jan 29, 2020

Reset HEAD of merges/master-to-master-vs-deps to 85ea148

@genlu genlu closed this Feb 3, 2020
@genlu genlu deleted the merges/master-to-master-vs-deps branch February 3, 2020 19:39
genlu pushed a commit that referenced this pull request Jul 15, 2020
Forward integration from dotnet/roslyn
genlu added a commit that referenced this pull request Jan 27, 2022
The computation of completion items is divided into two tasks:
1. "Core" items(i.e.non - expanded) which should be included in the list regardless of the selection of expander. Right now this includes all items except those from unimported namespaces.
2 .Expanded items which only show in the completion list when expander is selected, or by default if the corresponding features are enabled. Right now only items from unimported namespaces are associated with expander.

 #1 is the essence of completion so we'd always wait until its task is completed and return the results. However, because we have a really tight perf budget in completion, and computing those items in #2 could be expensive especially in a large solution (e.g.requires syntax / symbol indices and/ or runs in OOP,) we decide to kick off the computation in parallel when completion is triggered, but only include its results if it's completed by the time task #1 is completed, otherwise we don't wait on it and  just return items from #1 immediately. Task #2 will still be running in the background (until session is dismissed/committed,) and we'd check back to see if it's completed whenever we have a chance to update the completion list, i.e.when user typed another character, a filter was selected, etc. If so, those items will be added as part of the refresh.

 The reason of adopting this approach is we want to minimize typing delays. There are two ways user might perceive a delay in typing. First, they could see a delay between typing a character and completion list being displayed if they want to examine the items available. Second, they might be typing continuously w/ o paying attention to completion list, and simply expect the completion to do the "right thing" when a commit char is typed(e.g.commit "cancellationToken" when typing 'can$TAB$'). However, the commit could be delayed if completion is  still waiting on the computation of all available items, which manifests as UI delays and in worst case timeouts in commit which results in unexpected behavior(e.g.typing 'can$TAB$' results in a 250ms UI freeze and still ends up with "can" instead of "cancellationToken".)

This approach would ensure the computation of #2 will not be the cause of such delays, with the obvious trade off of potentially not providing expanded items until later(or never) in a completion session even if the feature is enabled.Note that in most cases we'd expect task #2 to finish in time and complete result would be available from the start of the session.However, even in the case only partial result is returned at the start, we still believe this is acceptable given how critical perf is in typing scenario. Additionally, expanded items are usually considered complementary. The need for them only rise occasionally(it's rare when users need to add imports,) and when they are needed, our hypothesis is because of their more intrusive nature(adding an import to the document) users would more likely to contemplate such action thus typing slower before commit and / or spending more time examining the list, which give us some opportunities to still provide those items later before they are truly required.

In this commit we have to handle adding those delayed expanded items into completion list in Roslyn. This is because the `CompletionContext.IsIncomplete` flag isn't fully supported in classic mode. Will need to move to that API once it's implemented properly.
genlu pushed a commit that referenced this pull request May 4, 2022
Add FixAll support for code refactorings (approach #2)
genlu pushed a commit that referenced this pull request Apr 2, 2024
Moving All handling of broker service from EA.D into the implementation IVisualDiagnosticsLanguageService
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants