Skip to content
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

Merged
merged 1 commit into from Mar 8, 2019

Conversation

sharwell
Copy link
Member

@sharwell sharwell commented Mar 2, 2019

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.

@sharwell sharwell removed the Blocked label Mar 3, 2019
@sharwell sharwell marked this pull request as ready for review March 3, 2019 20:23
@sharwell sharwell requested a review from a team as a code owner March 3, 2019 20:23
@@ -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))
Copy link
Contributor

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();

Copy link
Contributor

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?

Copy link
Contributor

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

Copy link
Member Author

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.

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);
Copy link
Member

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?

Copy link
Contributor

@ivanbasov ivanbasov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#33862 - to revert this PR when an Editor fix become available
#33863 - to support the behavior with a unit test (actually, completion test harness does not support Shift-Enter now)

@sharwell sharwell merged commit 516a25d into dotnet:dev16.0-vs-deps Mar 8, 2019
@sharwell sharwell deleted the line-ender branch March 8, 2019 17:25
@sharwell sharwell mentioned this pull request Apr 3, 2019
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.

None yet

5 participants