Skip to content

Add the host support to the PowerShell kernel #202

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

Merged
merged 8 commits into from
Mar 3, 2020

Conversation

daxian-dbw
Copy link
Contributor

@daxian-dbw daxian-dbw commented Feb 26, 2020

Fix #190
Fix #159

Implement the PowerShell host interfaces for the PowerShell kernel, so as to provide a rich interactive experience to the PS kernel.
The light-up features are:

  • The general support to foreground and background;
  • Write-Host with foreground/background color, and properly deal with -NoNewLine;
  • Prompt for input request for Read-Host and Get-Credential;
  • Progress rendering similar to the console experience;
  • The same concise error view as in the console.

The rich experience is shown as below:

screen-good

@TylerLeonhardt
Copy link
Contributor

@daxian-dbw Where do we craft StandardOutputValueProduced and send to the client? Does dotnet-interactive do this automatically?

@jonsequitur
Copy link
Contributor

@TylerLeonhardt dotnet-interactive captures console output via the middleware in KernelBase and emits these events for you.

@daxian-dbw
Copy link
Contributor Author

@TylerLeonhardt It used to be taken care of by individual kernel, but @jonsequitur made a change recently to take care of that in general for all kernels, the relevant code is here:

private void AddCaptureConsoleMiddleware()
{
AddMiddleware(async (command, context, next) =>
{
using var console = await ConsoleOutput.TryCaptureAsync(c =>
{
return new CompositeDisposable
{
c.Out.Subscribe(s => context.PublishStandardOut(s, command)),
c.Error.Subscribe(s => context.PublishStandardError(s, command))
};
});
await next(command, context);
});
}

@TylerLeonhardt
Copy link
Contributor

Nice. That change flew under my radar. Thanks!

@daxian-dbw
Copy link
Contributor Author

@TylerLeonhardt Your comments are addressed. Can you please take another look when you get a chance?

Copy link
Contributor

@TylerLeonhardt TylerLeonhardt left a comment

Choose a reason for hiding this comment

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

one comment, otherwise LGTM - it's pretty wild how much code needs to brought in to truly host PowerShell. I wish we could do something about that but I'm not really sure what.

@daxian-dbw
Copy link
Contributor Author

Thanks @TylerLeonhardt!
@colombod can you please take a look? We hope to get this in soon 😄

@TylerLeonhardt
Copy link
Contributor

@daxian-dbw any idea if this will work with native executables that accept input/password? Like func new or sudo

@daxian-dbw
Copy link
Contributor Author

Rebased to resolve conflicts.
@TylerLeonhardt this won't work for native utilities that requests for input/password. Those are native API calls and cannot be handled by the PowerShell host. It's the same as in PowerShell remoting.

@daxian-dbw
Copy link
Contributor Author

daxian-dbw commented Mar 2, 2020

I had a offline discussion with @jonsequitur and @colombod about the concern on the test coverage.
Opened the issue #219 to track adding comprehensive tests for the PS kernel host implementation.

@daxian-dbw
Copy link
Contributor Author

Rebased the changes to resolve conflict.

@jonsequitur jonsequitur merged commit 09eb5b0 into dotnet:master Mar 3, 2020
@daxian-dbw daxian-dbw deleted the host-final branch March 3, 2020 00:57
@daxian-dbw
Copy link
Contributor Author

Thanks @jonsequitur! I will get #219 done ASAP.

@jonsequitur
Copy link
Contributor

Thanks to you @daxian-dbw! This is great stuff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No interactive credential prompt PowerShell kernel is adding blank lines to all the output
3 participants