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.

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

tracking std out prints #343

Merged
merged 11 commits into from
Jul 23, 2019
Merged

tracking std out prints #343

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
51827f1
tracking std out prints
colombod Jul 23, 2019
2e5a772
transient display ids must be emitted per value
colombod Jul 23, 2019
f44251d
handle last value produce correctly
colombod Jul 23, 2019
117ee6c
remove kernel name from csharpkernel tests
colombod Jul 23, 2019
24e4b9f
mime type not null
colombod Jul 23, 2019
3d4dcdf
Merge branch 'master' into feature/console_writeline
colombod Jul 23, 2019
99be41c
rename methods
colombod Jul 23, 2019
03f2fe1
use const for DefaultMimeType and remove redundant checks
colombod Jul 23, 2019
67aba14
Merge branch 'master' into feature/console_writeline
colombod Jul 23, 2019
69fd2d7
white space
colombod Jul 23, 2019
a5be738
empty or white space string are not valid mimetypes
colombod Jul 23, 2019
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
3 changes: 1 addition & 2 deletions Microsoft.DotNet.Interactive.Jupyter/CompleteRequestHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
using Microsoft.DotNet.Interactive.Commands;
using Microsoft.DotNet.Interactive.Events;
using Microsoft.DotNet.Interactive.Jupyter.Protocol;
using WorkspaceServer.Kernel;

namespace Microsoft.DotNet.Interactive.Jupyter
{
Expand All @@ -31,7 +30,7 @@ public async Task Handle(JupyterRequestContext context)

var command = new RequestCompletion(completeRequest.Code, completeRequest.CursorPosition);

var openRequest = new InflightRequest(context, completeRequest, 0, null);
var openRequest = new InflightRequest(context, completeRequest, 0);

InFlightRequests[command] = openRequest;

Expand Down
23 changes: 17 additions & 6 deletions Microsoft.DotNet.Interactive.Jupyter/ExecuteRequestHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
using Microsoft.DotNet.Interactive.Commands;
using Microsoft.DotNet.Interactive.Events;
using Microsoft.DotNet.Interactive.Jupyter.Protocol;
using WorkspaceServer.Kernel;

namespace Microsoft.DotNet.Interactive.Jupyter
{
Expand All @@ -30,9 +29,8 @@ public async Task Handle(JupyterRequestContext context)
var executionCount = executeRequest.Silent ? _executionCount : Interlocked.Increment(ref _executionCount);

var command = new SubmitCode(executeRequest.Code, "csharp");
var id = Guid.NewGuid();
var transient = new Dictionary<string, object> { { "display_id", id.ToString() } };
var openRequest = new InflightRequest(context, executeRequest, executionCount, transient);

var openRequest = new InflightRequest(context, executeRequest, executionCount);

InFlightRequests[command] = openRequest;

Expand Down Expand Up @@ -79,6 +77,13 @@ public async Task Handle(JupyterRequestContext context)
}
}

private static Dictionary<string, object> CreateTransient()
{
var id = Guid.NewGuid();
var transient = new Dictionary<string, object> {{"display_id", id.ToString()}};
return transient;
}

void OnKernelResultEvent(IKernelEvent value)
{
switch (value)
Expand Down Expand Up @@ -152,10 +157,16 @@ private static void OnValueProduced(

try
{
var transient = CreateTransient();
// executeResult data
var executeResultData = new ExecuteResult(
var executeResultData = valueProduced.IsLastValue
?new ExecuteResult(
openRequest.ExecutionCount,
transient: openRequest.Transient,
transient: transient,
data: valueProduced?.FormattedValues
?.ToDictionary(k => k.MimeType ?? "text/plain", v => v.Value))
: new DisplayData(
transient: transient,
data: valueProduced?.FormattedValues
?.ToDictionary(k => k.MimeType ?? "text/plain", v => v.Value));
Copy link
Contributor

Choose a reason for hiding this comment

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

We should no longer need a fallback here.


Expand Down
6 changes: 1 addition & 5 deletions Microsoft.DotNet.Interactive.Jupyter/RequestHandlerBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using System.Reactive.Disposables;
using Microsoft.DotNet.Interactive.Commands;
using Microsoft.DotNet.Interactive.Jupyter.Protocol;
using WorkspaceServer.Kernel;

namespace Microsoft.DotNet.Interactive.Jupyter
{
Expand Down Expand Up @@ -41,18 +40,15 @@ public void Dispose()
protected class InflightRequest : IDisposable
{
private readonly CompositeDisposable _disposables = new CompositeDisposable();
public Dictionary<string, object> Transient { get; }
public JupyterRequestContext Context { get; }
public T Request { get; }
public int ExecutionCount { get; }

public InflightRequest(JupyterRequestContext context, T request, int executionCount,
Dictionary<string, object> transient)
public InflightRequest(JupyterRequestContext context, T request, int executionCount)
{
Context = context;
Request = request;
ExecutionCount = executionCount;
Transient = transient;
}

public void AddDisposable(IDisposable disposable)
Expand Down
6 changes: 4 additions & 2 deletions Microsoft.DotNet.Interactive/Events/ValueProduced.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,18 @@ namespace Microsoft.DotNet.Interactive.Events
{
public class ValueProduced : KernelEventBase
{
public ValueProduced(
object value,
public ValueProduced(object value,
SubmitCode submitCode,
bool isLastValue = false,
colombod marked this conversation as resolved.
Show resolved Hide resolved
IReadOnlyCollection<FormattedValue> formattedValues = null) : base(submitCode)
{
Value = value;
IsLastValue = isLastValue;
FormattedValues = formattedValues;
}

public object Value { get; }
public bool IsLastValue { get; }

public IReadOnlyCollection<FormattedValue> FormattedValues { get; }
}
Expand Down
41 changes: 40 additions & 1 deletion WorkspaceServer.Tests/Kernel/CSharpKernelTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,45 @@ public async Task it_aggregates_multiple_submissions()
.Be(3);
}

[Fact]
public async Task it_produces_values_when_executing_Console_output()
{
var kernel = CreateKernel();

var kernelCommand = new SubmitCode(@"
Console.Write(""value one"");
Console.Write(""value two"");
Console.Write(""value three"");", "csharp");
colombod marked this conversation as resolved.
Show resolved Hide resolved
await kernel.SendAsync(kernelCommand);

KernelEvents.OfType<ValueProduced>()
.Should()
.BeEquivalentTo(
new ValueProduced("value one", kernelCommand, false, new[] { new FormattedValue(null, "value one"), }),
new ValueProduced("value two", kernelCommand, false, new[] { new FormattedValue(null, "value two"), }),
new ValueProduced("value three", kernelCommand, false, new[] { new FormattedValue(null, "value three"), }));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why no mime type?

Copy link
Member Author

Choose a reason for hiding this comment

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

myme types are done later on when formatting on raw value is done

Copy link
Contributor

Choose a reason for hiding this comment

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

I think FormattedValue without a mime type is confusing and probably bug prone. I think this test shows that because of the redundancy -- the formatted values ARE the raw values.

Copy link
Member Author

Choose a reason for hiding this comment

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

null value is generated by the Formatter.MimeTypeFor method

Copy link
Contributor

Choose a reason for hiding this comment

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

That's the right behavior. FormattedValue.MimeType is intentionally read-only because the expectation was that the mime type must be known at the time a FormattedValue is created. It was an oversight that the constructor didn't guard against this.

}

[Fact]
public async Task it_produces_a_final_value_if_the_code_expression_evaluates()
{
var kernel = CreateKernel();

var kernelCommand = new SubmitCode(@"
Console.Write(""value one"");
Console.Write(""value two"");
Console.Write(""value three"");
5", "csharp");
colombod marked this conversation as resolved.
Show resolved Hide resolved
await kernel.SendAsync(kernelCommand);

KernelEvents.OfType<ValueProduced>()
.Should()
.HaveCount(4)
.And
.ContainSingle(e => e.IsLastValue);

}

[Fact(Skip = "requires support for cs8 in roslyn scripting")]
public async Task it_supports_csharp_8()
{
Expand Down Expand Up @@ -280,7 +319,7 @@ await kernel.SendAsync(
.Should()
.Contain(i => i.DisplayText == "SerializeObject");
}

[Fact]
public async Task The_extend_directive_can_be_used_to_load_a_kernel_extension()
{
Expand Down
78 changes: 52 additions & 26 deletions WorkspaceServer/Kernel/CSharpKernel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
using Microsoft.DotNet.Interactive.Events;
using WorkspaceServer.LanguageServices;
using Microsoft.DotNet.Interactive.Rendering;
using WorkspaceServer.Servers.Roslyn;
using WorkspaceServer.Servers.Scripting;
using CompletionItem = Microsoft.DotNet.Interactive.CompletionItem;
using Task = System.Threading.Tasks.Task;
Expand Down Expand Up @@ -106,7 +107,7 @@ protected override async Task HandleAsync(
}

private async Task HandleSubmitCode(
SubmitCode codeSubmission,
SubmitCode codeSubmission,
KernelInvocationContext context)
{
var codeSubmissionReceived = new CodeSubmissionReceived(
Expand All @@ -120,29 +121,38 @@ private async Task HandleSubmitCode(
{
context.OnNext(new CompleteCodeSubmissionReceived(codeSubmission));
Exception exception = null;
try
var output = Array.Empty<string>();
using (var console = await ConsoleOutput.Capture())
{
if (_scriptState == null)

try
{
_scriptState = await CSharpScript.RunAsync(
code,
ScriptOptions);
if (_scriptState == null)
{
_scriptState = await CSharpScript.RunAsync(
code,
ScriptOptions);
}
else
{
_scriptState = await _scriptState.ContinueWithAsync(
code,
ScriptOptions,
e =>
{
exception = e;
return true;
});
}
}
else
catch (Exception e)
{
_scriptState = await _scriptState.ContinueWithAsync(
code,
ScriptOptions,
e =>
{
exception = e;
return true;
});
exception = e;
}
}
catch (Exception e)
{
exception = e;
output =
console.WriteOccurredOnStandardOutput
? console.GetStandardOutputWrites().ToArray()
: Array.Empty<string>();
}

if (exception != null)
Expand All @@ -155,6 +165,21 @@ private async Task HandleSubmitCode(
}
else
{
foreach (var std in output)
{
var formattedValues = new List<FormattedValue>
{
new FormattedValue(
Formatter.MimeTypeFor(std?.GetType() ?? typeof(object)), std)
};

context.OnNext(
new ValueProduced(
std,
codeSubmission,
false,
formattedValues));
}
if (HasReturnValue)
{
var writer = new StringWriter();
Expand All @@ -170,6 +195,7 @@ private async Task HandleSubmitCode(
new ValueProduced(
_scriptState.ReturnValue,
codeSubmission,
true,
formattedValues));
}

Expand All @@ -186,11 +212,11 @@ private async Task HandleSubmitCode(

private async Task HandleRequestCompletion(
RequestCompletion requestCompletion,
KernelInvocationContext context,
KernelInvocationContext context,
ScriptState scriptState)
{
var completionRequestReceived = new CompletionRequestReceived(requestCompletion);

context.OnNext(completionRequestReceived);

var completionList =
Expand All @@ -202,7 +228,7 @@ private async Task HandleRequestCompletion(
private async Task<IEnumerable<CompletionItem>> GetCompletionList(string code, int cursorPosition, ScriptState scriptState)
{
var metadataReferences = ImmutableArray<MetadataReference>.Empty;

var forcedState = false;
if (scriptState == null)
{
Expand All @@ -217,14 +243,14 @@ private async Task<IEnumerable<CompletionItem>> GetCompletionList(string code, i
buffer.AppendLine(code);
var fullScriptCode = buffer.ToString();
var offset = fullScriptCode.LastIndexOf(code, StringComparison.InvariantCulture);
var absolutePosition = Math.Max(offset,0) + cursorPosition;
var absolutePosition = Math.Max(offset, 0) + cursorPosition;

if (_fixture == null || _metadataReferences != metadataReferences)
{
_fixture = new WorkspaceFixture(compilation.Options, metadataReferences);
_metadataReferences = metadataReferences;
}

var document = _fixture.ForkDocument(fullScriptCode);
var service = CompletionService.GetService(document);

Expand All @@ -247,7 +273,7 @@ private async Task<IEnumerable<CompletionItem>> GetCompletionList(string code, i
}

private bool HasReturnValue =>
_scriptState != null &&
(bool) _hasReturnValueMethod.Invoke(_scriptState.Script, null);
_scriptState != null &&
(bool)_hasReturnValueMethod.Invoke(_scriptState.Script, null);
}
}
18 changes: 15 additions & 3 deletions WorkspaceServer/Servers/Roslyn/ConsoleOutput.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,20 @@
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System;
using System.Collections.Generic;
using System.IO;
using System.Threading;
using System.Threading.Tasks;
using Clockwise;

namespace WorkspaceServer.Servers.Roslyn
{
public class ConsoleOutput : IDisposable
{

private TextWriter originalOutputWriter;
private TextWriter originalErrorWriter;
private readonly StringWriter outputWriter = new StringWriter();
private readonly StringWriter errorWriter = new StringWriter();
private readonly TrackingStringWriter outputWriter = new TrackingStringWriter();
private readonly TrackingStringWriter errorWriter = new TrackingStringWriter();

private const int NOT_DISPOSED = 0;
private const int DISPOSED = 1;
Expand Down Expand Up @@ -66,9 +67,15 @@ public void Dispose()
}
}



public string StandardOutput => outputWriter.ToString();

public string StandardError => errorWriter.ToString();
public bool WriteOccurredOnStandardOutput => outputWriter.WriteOccurred;

public bool WriteOccurredOnStandardError => errorWriter.WriteOccurred;


public void Clear()
{
Expand All @@ -77,5 +84,10 @@ public void Clear()
}

public bool IsEmpty() => outputWriter.ToString().Length == 0 && errorWriter.ToString().Length == 0;

public IEnumerable<string> GetStandardOutputWrites()
{
return outputWriter.Writes();
}
}
}
Loading