Merge main to features/rename_ui_rework #59521
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
Was broken by previous refactoring
So we can always wait for unimported items and force index creation
to decide if we want to wait for expand items to be calculated
Since we are using Editor's responsive completion for this purpose now
… duplicating them to the guest in LiveShare
and exclude them from import completion
Backport diagnostics error list property for liveshare to 16.11
…merges/release/dev17.1-vs-deps-to-release/dev17.2
Let Import Completion check for global usings auto-generated by SDK
We had a case where a workspace change during the middle of a CodeModel operation meant that this replace might throw not-obvious exceptions later when we tried to reformat the tree.
…to-release/dev17.2 Merge release/dev17.1-vs-deps to release/dev17.2
…led-tryapplychanges Add a few asserts/throws to our CodeModel implementation
Merge release/dev17.2 to main
* NavigationOptions refactoring * TypeScript fixes
Don't block completion for items from unimported namespaces
dotnet-bot
force-pushed
the
merges/main-to-features/rename_ui_rework
branch
from
d3e2815 to
5193908
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an automatically generated pull request from main into features/rename_ui_rework.
Once all conflicts are resolved and all the tests pass, you are free to merge the pull request. 🐯
Troubleshooting conflicts
Identify authors of changes which introduced merge conflicts
Scroll to the bottom, then for each file containing conflicts copy its path into the following searches:
Usually the most recent change to a file between the two branches is considered to have introduced the conflicts, but sometimes it will be necessary to look for the conflicting lines and check the blame in each branch. Generally the author whose change introduced the conflicts should pull down this PR, fix the conflicts locally, then push up a commit resolving the conflicts.
Resolve merge conflicts using your local repo
Sometimes merge conflicts may be present on GitHub but merging locally will work without conflicts. This is due to differences between the merge algorithm used in local git versus the one used by GitHub.
git fetch --all git checkout -t upstream/merges/main-to-features/rename_ui_rework git reset --hard upstream/features/rename_ui_rework git merge upstream/main # Fix merge conflicts git commit git push upstream merges/main-to-features/rename_ui_rework --force