Skip to content

Add events for PowerShell execution status (running, completed, etc). #632

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Mar 20, 2018
Merged

Add events for PowerShell execution status (running, completed, etc). #632

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4161212
Add events for PowerShell execution status (running, completed, etc).
rjmholt Mar 12, 2018
d3bc268
Remove empty <return> doc-comments
rjmholt Mar 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/PowerShellEditorServices.Protocol/LanguageServer/ExecutionStatusChangedEvent.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
using Microsoft.PowerShell.EditorServices.Protocol.MessageProtocol;

namespace Microsoft.PowerShell.EditorServices.Protocol.LanguageServer
{
/// <summary>
/// Defines an event type for PowerShell context execution status changes (e.g. execution has completed)
/// </summary>
public class ExecutionStatusChangedEvent
{
/// <summary>
/// The notification type for execution status change events in the message protocol
/// </summary>
/// <returns></returns>
Copy link
Member

Choose a reason for hiding this comment

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

same

public static readonly
NotificationType<object, object> Type =
NotificationType<object, object>.Create("powerShell/executionStatusChanged");
}
}
15 changes: 15 additions & 0 deletions src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ public LanguageServer(
{
this.Logger = logger;
this.editorSession = editorSession;
// Attach to the underlying PowerShell context to listen for changes in the runspace or execution status
this.editorSession.PowerShellContext.RunspaceChanged += PowerShellContext_RunspaceChanged;
this.editorSession.PowerShellContext.ExecutionStatusChanged += PowerShellContext_ExecutionStatusChanged;

// Attach to ExtensionService events
this.editorSession.ExtensionService.CommandAdded += ExtensionService_ExtensionAdded;
Expand Down Expand Up @@ -1273,6 +1275,19 @@ await this.messageSender.SendEvent(
new Protocol.LanguageServer.RunspaceDetails(e.NewRunspace));
}

/// <summary>
/// Event hook on the PowerShell context to listen for changes in script execution status
/// </summary>
/// <param name="sender">the PowerShell context sending the execution event</param>
/// <param name="e">details of the execution status change</param>
/// <returns></returns>
Copy link
Member

Choose a reason for hiding this comment

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

nit: maybe delete this line unless others in the file have the same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking through the file and the codebase and the documentation, I noticed we didn't have as much documentation as in the PowerShell codebase (in fact, the web docs on the code base are out of date and refer to classes that no longer exist). I was thinking that anything I contribute I should document. Ideally I'd want to see comments documenting most things, just so we can make contributing easier, but for now I think some documentation is better than none.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@rjmholt Definitely agree! Documentation is an area needs a lot of work here :)

In this case, I believe he's referring to just the empty returns tag. I'd also prefer if they were dropped to keep it consistent. If I remember correctly even if they were public API's, the rule for returns documentation doesn't hit on properties or void methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I see. Yes agreed!

private async void PowerShellContext_ExecutionStatusChanged(object sender, ExecutionStatusChangedEventArgs e)
{
await this.messageSender.SendEvent(
ExecutionStatusChangedEvent.Type,
e);
}

private async void ExtensionService_ExtensionAdded(object sender, EditorCommand e)
{
await this.messageSender.SendEvent(
Expand Down