-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Restore original behavior of Shift+Enter during completion #33828
Conversation
cb7649d
to
990b50f
Compare
@@ -58,6 +63,22 @@ public VSCommanding.CommandState GetCommandState(AutomaticLineEnderCommandArgs a | |||
|
|||
public void ExecuteCommand(AutomaticLineEnderCommandArgs args, Action nextHandler, CommandExecutionContext context) | |||
{ | |||
// Completion will only be active here if this command wasn't handled by the completion controller itself. | |||
if (_asyncCompletionBroker.IsCompletionActive(args.TextView)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_asyncCompletionBroker [](start = 16, length = 22)
Can it be null if the async completion is turned off?
var softSelection = computedItems.SuggestionItemSelected || computedItems.UsesSoftSelection; | ||
var behavior = session.Commit('\n', context.OperationContext.UserCancellationToken); | ||
session.Dismiss(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fix look weird. It seems that the old completion (Roslyn) supports IChainedCommandHandler but the new one (Editor) does not support it.
Maybe we should make a fi on the Editor side?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the fix definitely belongs on the editor side
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fix can be reverted if the editor side fixes the issue, but it will not break the functionality to use it as a temporary workaround.
src/VisualStudio/IntegrationTest/IntegrationTests/CSharp/CSharpIntelliSense.cs
Show resolved
Hide resolved
var session = _asyncCompletionBroker.GetSession(args.TextView); | ||
var computedItems = session.GetComputedItems(context.OperationContext.UserCancellationToken); | ||
var softSelection = computedItems.SuggestionItemSelected || computedItems.UsesSoftSelection; | ||
var behavior = session.Commit('\n', context.OperationContext.UserCancellationToken); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we do go with taking this workaround, I presume the code here should have a comment linking to the tracking bug to track the proper fix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes #33823
This change is a workaround for https://devdiv.visualstudio.com/DevDiv/_workitems/edit/809579, since we will not be able to fix and validate that from the editor side prior to 16.0 RTM.