-
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
Add Xaml command hanler for CreateEventHandler command #51670
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
// See the LICENSE file in the project root for more information. | ||
|
||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Microsoft.CodeAnalysis; | ||
using Microsoft.CodeAnalysis.Host; | ||
|
||
namespace Microsoft.VisualStudio.LanguageServices.Xaml.Features.Commands | ||
{ | ||
internal interface IXamlCommandService : ILanguageService | ||
{ | ||
/// <summary> | ||
/// Execute the <paramref name="command"/> with the <paramref name="commandArguments"/> on server | ||
/// </summary> | ||
/// <param name="document">TextDocument command was triggered on</param> | ||
/// <param name="command">The command that will be executed</param> | ||
/// <param name="commandArguments">The arguments need by the command</param> | ||
/// <param name="cancellationToken">cancellationToken</param> | ||
/// <returns>True if the command has been executed, otherwise false</returns> | ||
Task<bool> ExecuteCommandAsync(TextDocument document, string command, object[]? commandArguments, CancellationToken cancellationToken); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
// See the LICENSE file in the project root for more information. | ||
|
||
using System.Collections.Immutable; | ||
|
||
namespace Microsoft.VisualStudio.LanguageServices.Xaml.Features.Commands | ||
{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Feels like this should live under Completion and not Commands. We just happen to pass it around as a command parameter. #Closed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I have been moving this struct between Commands and Completion too. Now as you vote for completion I guess completion wins. :) #Closed |
||
internal struct XamlEventDescription | ||
{ | ||
public string ClassName { get; set; } | ||
public string EventName { get; set; } | ||
public string ReturnType { get; set; } | ||
public ImmutableArray<(string Name, string ParameterType, string Modifier)> Parameters { get; set; } | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
// See the LICENSE file in the project root for more information. | ||
|
||
using System.Linq; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Microsoft.CodeAnalysis.Editor.Xaml; | ||
using Microsoft.CodeAnalysis.LanguageServer.Handler; | ||
using Microsoft.CodeAnalysis.LanguageServer.Handler.Commands; | ||
using Microsoft.VisualStudio.LanguageServer.Protocol; | ||
using Microsoft.VisualStudio.LanguageServices.Xaml.Features.Commands; | ||
using Newtonsoft.Json.Linq; | ||
using Roslyn.Utilities; | ||
|
||
namespace Microsoft.VisualStudio.LanguageServices.Xaml.Implementation.LanguageServer.Handler.Commands | ||
{ | ||
/// <summary> | ||
/// Handle the command that adds an event handler method in code | ||
/// </summary> | ||
internal class CreateEventCommandHandler : AbstractExecuteWorkspaceCommandHandler | ||
{ | ||
public override string Command => StringConstants.CreateEventHandlerCommand; | ||
|
||
public override bool MutatesSolutionState => false; | ||
|
||
public override bool RequiresLSPSolution => true; | ||
|
||
public override TextDocumentIdentifier? GetTextDocumentIdentifier(ExecuteCommandParams request) | ||
=> ((JToken)request.Arguments.FirstOrDefault())?.ToObject<TextDocumentIdentifier>(); | ||
|
||
public override async Task<object> HandleRequestAsync(ExecuteCommandParams request, RequestContext context, CancellationToken cancellationToken) | ||
{ | ||
Contract.ThrowIfNull(request.Arguments); | ||
|
||
var document = context.Document; | ||
if (document == null) | ||
{ | ||
return false; | ||
} | ||
|
||
var commandService = document.Project.LanguageServices.GetService<IXamlCommandService>(); | ||
if (commandService == null) | ||
{ | ||
return false; | ||
} | ||
|
||
// request.Arguments has two argument for CreateEventHandlerCommand | ||
// Arguments[0]: TextDocumentIdentifier | ||
// Arguments[1]: XamlEventDescription | ||
var arguments = new object[] { ((JToken)request.Arguments[1]).ToObject<XamlEventDescription>() }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see arguments[0] being used anywhere, do we actually need that item? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah it is used in line 29 GetTextDocumentIdentifier. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah I missed that. It looks like if we get the command it must have that argument, so I'd recommend First() There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good to me. |
||
return await commandService.ExecuteCommandAsync(document, request.Command, arguments, cancellationToken).ConfigureAwait(false); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
// See the LICENSE file in the project root for more information. | ||
|
||
using System; | ||
using System.Collections.Immutable; | ||
using System.Composition; | ||
using Microsoft.CodeAnalysis.Editor.Xaml; | ||
using Microsoft.CodeAnalysis.Host.Mef; | ||
using Microsoft.CodeAnalysis.LanguageServer.Handler; | ||
using Microsoft.CodeAnalysis.LanguageServer.Handler.Commands; | ||
using Microsoft.VisualStudio.LanguageServer.Protocol; | ||
|
||
namespace Microsoft.VisualStudio.LanguageServices.Xaml.Implementation.LanguageServer.Handler.Commands | ||
{ | ||
[ExportLspRequestHandlerProvider(StringConstants.XamlLanguageName), Shared] | ||
[ProvidesMethod(Methods.WorkspaceExecuteCommandName)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you just want ProvidesCommand, ProvidesMethod shouldn't be needed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch! I have removed the ProvidesMethod. |
||
[ProvidesCommand(StringConstants.CreateEventHandlerCommand)] | ||
internal class CreateEventCommandHandlerProvider : AbstractRequestHandlerProvider | ||
{ | ||
[ImportingConstructor] | ||
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] | ||
public CreateEventCommandHandlerProvider() | ||
{ | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this actually needed? We're not importing anything in here. #Closed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is required by the ExportLspRequestHandlerProvider. I will get warning if I don't have an importing constructor. #Closed |
||
public override ImmutableArray<IRequestHandler> CreateRequestHandlers() | ||
{ | ||
return ImmutableArray.Create<IRequestHandler>(new CreateEventCommandHandler()); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,12 +58,12 @@ public CompletionHandler() | |
|
||
return new VSCompletionList | ||
{ | ||
Items = completionResult.Completions.Select(c => CreateCompletionItem(c, document.Id, text, request.Position)).ToArray(), | ||
Items = completionResult.Completions.Select(c => CreateCompletionItem(c, document.Id, text, request.Position, request.TextDocument)).ToArray(), | ||
SuggestionMode = false, | ||
}; | ||
} | ||
|
||
private static CompletionItem CreateCompletionItem(XamlCompletionItem xamlCompletion, DocumentId documentId, SourceText text, Position position) | ||
private static CompletionItem CreateCompletionItem(XamlCompletionItem xamlCompletion, DocumentId documentId, SourceText text, Position position, TextDocumentIdentifier textDocument) | ||
{ | ||
var item = new VSCompletionItem | ||
{ | ||
|
@@ -89,6 +89,16 @@ private static CompletionItem CreateCompletionItem(XamlCompletionItem xamlComple | |
}; | ||
} | ||
|
||
if (xamlCompletion.EventDescription.HasValue) | ||
{ | ||
item.Command = new Command() | ||
{ | ||
CommandIdentifier = StringConstants.CreateEventHandlerCommand, | ||
Arguments = new object[] { textDocument, xamlCompletion.EventDescription }, | ||
Title = Resources.CreateEventHandler | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the title shown anywhere for completion commands? If it's not we may not need to localize it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You have a good point. I am not aware the title is shown anywhere so far. I have removed the string from the resources. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Small improvement, make it a const string in this class. In reply to: 587847403 [](ancestors = 587847403) |
||
}; | ||
} | ||
|
||
return item; | ||
} | ||
|
||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
That's just an implementation detail that's not relevant to the description of the interface #Closed
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.
Sure I will update the comment. #Closed