From 51827f1a8eefce89b9b3d8ded590e51e0c1507ab Mon Sep 17 00:00:00 2001 From: Diego Date: Tue, 23 Jul 2019 15:18:13 +0100 Subject: [PATCH 1/9] tracking std out prints --- .../RequestHandlerBase.cs | 1 - .../Kernel/CSharpKernelTests.cs | 21 +- WorkspaceServer/Kernel/CSharpKernel.cs | 76 +++-- .../Servers/Roslyn/ConsoleOutput.cs | 18 +- .../Servers/Roslyn/TrackingStringWriter.cs | 289 ++++++++++++++++++ 5 files changed, 374 insertions(+), 31 deletions(-) create mode 100644 WorkspaceServer/Servers/Roslyn/TrackingStringWriter.cs diff --git a/Microsoft.DotNet.Interactive.Jupyter/RequestHandlerBase.cs b/Microsoft.DotNet.Interactive.Jupyter/RequestHandlerBase.cs index a627868d7..2135e2139 100644 --- a/Microsoft.DotNet.Interactive.Jupyter/RequestHandlerBase.cs +++ b/Microsoft.DotNet.Interactive.Jupyter/RequestHandlerBase.cs @@ -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 { diff --git a/WorkspaceServer.Tests/Kernel/CSharpKernelTests.cs b/WorkspaceServer.Tests/Kernel/CSharpKernelTests.cs index 7a21a5bbf..8142dba70 100644 --- a/WorkspaceServer.Tests/Kernel/CSharpKernelTests.cs +++ b/WorkspaceServer.Tests/Kernel/CSharpKernelTests.cs @@ -178,6 +178,25 @@ 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"); + await kernel.SendAsync(kernelCommand); + + KernelEvents.OfType() + .Should() + .BeEquivalentTo( + new ValueProduced("value one", kernelCommand, new[] { new FormattedValue(null, "value one"), }), + new ValueProduced("value two", kernelCommand, new[] { new FormattedValue(null, "value two"), }), + new ValueProduced("value three", kernelCommand, new[] { new FormattedValue(null, "value three"), })); + } + [Fact(Skip = "requires support for cs8 in roslyn scripting")] public async Task it_supports_csharp_8() { @@ -280,7 +299,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() { diff --git a/WorkspaceServer/Kernel/CSharpKernel.cs b/WorkspaceServer/Kernel/CSharpKernel.cs index 39135e675..e74abab48 100644 --- a/WorkspaceServer/Kernel/CSharpKernel.cs +++ b/WorkspaceServer/Kernel/CSharpKernel.cs @@ -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; @@ -106,7 +107,7 @@ protected override async Task HandleAsync( } private async Task HandleSubmitCode( - SubmitCode codeSubmission, + SubmitCode codeSubmission, KernelInvocationContext context) { var codeSubmissionReceived = new CodeSubmissionReceived( @@ -120,29 +121,38 @@ private async Task HandleSubmitCode( { context.OnNext(new CompleteCodeSubmissionReceived(codeSubmission)); Exception exception = null; - try + var output = Array.Empty(); + 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(); } if (exception != null) @@ -155,6 +165,20 @@ private async Task HandleSubmitCode( } else { + foreach (var std in output) + { + var formattedValues = new List + { + new FormattedValue( + Formatter.MimeTypeFor(std?.GetType() ?? typeof(object)), std) + }; + + context.OnNext( + new ValueProduced( + std, + codeSubmission, + formattedValues)); + } if (HasReturnValue) { var writer = new StringWriter(); @@ -186,11 +210,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 = @@ -202,7 +226,7 @@ private async Task HandleRequestCompletion( private async Task> GetCompletionList(string code, int cursorPosition, ScriptState scriptState) { var metadataReferences = ImmutableArray.Empty; - + var forcedState = false; if (scriptState == null) { @@ -217,14 +241,14 @@ private async Task> 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); @@ -247,7 +271,7 @@ private async Task> GetCompletionList(string code, i } private bool HasReturnValue => - _scriptState != null && - (bool) _hasReturnValueMethod.Invoke(_scriptState.Script, null); + _scriptState != null && + (bool)_hasReturnValueMethod.Invoke(_scriptState.Script, null); } } \ No newline at end of file diff --git a/WorkspaceServer/Servers/Roslyn/ConsoleOutput.cs b/WorkspaceServer/Servers/Roslyn/ConsoleOutput.cs index 5c46f805d..eedde401b 100644 --- a/WorkspaceServer/Servers/Roslyn/ConsoleOutput.cs +++ b/WorkspaceServer/Servers/Roslyn/ConsoleOutput.cs @@ -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; @@ -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() { @@ -77,5 +84,10 @@ public void Clear() } public bool IsEmpty() => outputWriter.ToString().Length == 0 && errorWriter.ToString().Length == 0; + + public IEnumerable GetStandardOutputWrites() + { + return outputWriter.Writes(); + } } } diff --git a/WorkspaceServer/Servers/Roslyn/TrackingStringWriter.cs b/WorkspaceServer/Servers/Roslyn/TrackingStringWriter.cs new file mode 100644 index 000000000..5149ac9a6 --- /dev/null +++ b/WorkspaceServer/Servers/Roslyn/TrackingStringWriter.cs @@ -0,0 +1,289 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// 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.Tasks; + +namespace WorkspaceServer.Servers.Roslyn +{ + internal class TrackingStringWriter : StringWriter + { + private class Region + { + public int Start { get; set; } + public int Length { get; set; } + } + List _regions = new List(); + private bool _trakingSize; + + + public bool WriteOccurred { get; set; } + + public override void Write(char value) + { + TrackSize(() => base.Write(value)); + } + + private void TrackSize(Action action) + { + WriteOccurred = true; + if (_trakingSize) + { + action(); + return; + } + + _trakingSize = true; + var sb = base.GetStringBuilder(); + + var region = new Region + { + Start = sb.Length + }; + + _regions.Add(region); + action(); + region.Length = sb.Length - region.Start; + _trakingSize = false; + } + private async Task TrackSizeAsync(Func action) + { + WriteOccurred = true; + if (_trakingSize) + { + await action(); + return; + } + _trakingSize = true; + var sb = base.GetStringBuilder(); + + var region = new Region + { + Start = sb.Length + }; + + _regions.Add(region); + + await action(); + region.Length = sb.Length - region.Start; + _trakingSize = false; + } + + public override void Write(char[] buffer, int index, int count) + { + TrackSize(() => base.Write(buffer, index, count)); + } + + public override void Write(string value) + { + TrackSize(() => base.Write(value)); + } + + public override Task WriteAsync(char value) + { + return TrackSizeAsync(() => base.WriteAsync(value)); + } + + public override Task WriteAsync(char[] buffer, int index, int count) + { + return TrackSizeAsync(() => base.WriteAsync(buffer, index, count)); + } + + public override Task WriteAsync(string value) + { + return TrackSizeAsync(() => base.WriteAsync(value)); + } + + public override Task WriteLineAsync(char value) + { + return TrackSizeAsync(() => base.WriteLineAsync(value)); + } + + public override Task WriteLineAsync(char[] buffer, int index, int count) + { + return TrackSizeAsync(() => base.WriteLineAsync(buffer, index, count)); + } + + public override Task WriteLineAsync(string value) + { + return TrackSizeAsync(() => base.WriteLineAsync(value)); + } + + public override void Write(bool value) + { + TrackSize(() => base.Write(value)); + } + + public override void Write(char[] buffer) + { + TrackSize(() => base.Write(buffer)); + } + + public override void Write(decimal value) + { + TrackSize(() => base.Write(value)); + } + + public override void Write(double value) + { + TrackSize(() => base.Write(value)); + } + + public override void Write(int value) + { + TrackSize(() => base.Write(value)); + } + + public override void Write(long value) + { + TrackSize(() => base.Write(value)); + } + + public override void Write(object value) + { + TrackSize(() => base.Write(value)); + } + + public override void Write(float value) + { + TrackSize(() => base.Write(value)); + } + + public override void Write(string format, object arg0) + { + TrackSize(() => base.Write(format, arg0)); + } + + public override void Write(string format, object arg0, object arg1) + { + TrackSize(() => base.Write(format, arg0, arg1)); + } + + public override void Write(string format, object arg0, object arg1, object arg2) + { + TrackSize(() => base.Write(format, arg0, arg1, arg2)); + } + + public override void Write(string format, params object[] arg) + { + TrackSize(() => base.Write(format, arg)); + } + + public override void Write(uint value) + { + TrackSize(() => base.Write(value)); + } + + public override void Write(ulong value) + { + TrackSize(() => base.Write(value)); + } + + public override void WriteLine() + { + TrackSize(() => base.WriteLine()); + } + + public override void WriteLine(bool value) + { + TrackSize(() => base.WriteLine(value)); + } + + public override void WriteLine(char value) + { + TrackSize(() => base.WriteLine(value)); + } + + public override void WriteLine(char[] buffer) + { + TrackSize(() => base.WriteLine(buffer)); + } + + public override void WriteLine(char[] buffer, int index, int count) + { + TrackSize(() => base.WriteLine(buffer, index, count)); + } + + public override void WriteLine(decimal value) + { + TrackSize(() => base.WriteLine(value)); + } + + public override void WriteLine(double value) + { + TrackSize(() => base.WriteLine(value)); + } + + public override void WriteLine(int value) + { + TrackSize(() => base.WriteLine(value)); + } + + public override void WriteLine(long value) + { + TrackSize(() => base.WriteLine(value)); + } + + public override void WriteLine(object value) + { + TrackSize(() => base.WriteLine(value)); + } + + public override void WriteLine(float value) + { + TrackSize(() => base.WriteLine(value)); + } + + public override void WriteLine(string value) + { + TrackSize(() => base.WriteLine(value)); + } + + public override void WriteLine(string format, object arg0) + { + TrackSize(() => base.WriteLine(format, arg0)); + } + + public override void WriteLine(string format, object arg0, object arg1) + { + TrackSize(() => base.WriteLine(format, arg0, arg1)); + } + + public override void WriteLine(string format, object arg0, object arg1, object arg2) + { + TrackSize(() => base.WriteLine(format, arg0, arg1, arg2)); + } + + public override void WriteLine(string format, params object[] arg) + { + TrackSize(() => base.WriteLine(format, arg)); + } + + public override void WriteLine(uint value) + { + TrackSize(() => base.WriteLine(value)); + } + + public override void WriteLine(ulong value) + { + TrackSize(() => base.WriteLine(value)); + } + + public override Task WriteLineAsync() + { + return TrackSizeAsync(() => base.WriteLineAsync()); + } + + public IEnumerable Writes() + { + var src = base.GetStringBuilder().ToString(); + foreach (var region in _regions) + { + yield return src.Substring(region.Start, region.Length); + } + } + } +} \ No newline at end of file From 2e5a77258d81f35f87fd3accfc2dd8d8ef8384ba Mon Sep 17 00:00:00 2001 From: Diego Date: Tue, 23 Jul 2019 15:46:53 +0100 Subject: [PATCH 2/9] transient display ids must be emitted per value --- .../CompleteRequestHandler.cs | 3 +-- .../ExecuteRequestHandler.cs | 16 +++++++++++----- .../RequestHandlerBase.cs | 5 +---- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/Microsoft.DotNet.Interactive.Jupyter/CompleteRequestHandler.cs b/Microsoft.DotNet.Interactive.Jupyter/CompleteRequestHandler.cs index 786df2785..c49796654 100644 --- a/Microsoft.DotNet.Interactive.Jupyter/CompleteRequestHandler.cs +++ b/Microsoft.DotNet.Interactive.Jupyter/CompleteRequestHandler.cs @@ -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 { @@ -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; diff --git a/Microsoft.DotNet.Interactive.Jupyter/ExecuteRequestHandler.cs b/Microsoft.DotNet.Interactive.Jupyter/ExecuteRequestHandler.cs index 85f0c1ddb..655c0e7f1 100644 --- a/Microsoft.DotNet.Interactive.Jupyter/ExecuteRequestHandler.cs +++ b/Microsoft.DotNet.Interactive.Jupyter/ExecuteRequestHandler.cs @@ -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 { @@ -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 { { "display_id", id.ToString() } }; - var openRequest = new InflightRequest(context, executeRequest, executionCount, transient); + + var openRequest = new InflightRequest(context, executeRequest, executionCount); InFlightRequests[command] = openRequest; @@ -79,6 +77,13 @@ public async Task Handle(JupyterRequestContext context) } } + private static Dictionary CreateTransient() + { + var id = Guid.NewGuid(); + var transient = new Dictionary {{"display_id", id.ToString()}}; + return transient; + } + void OnKernelResultEvent(IKernelEvent value) { switch (value) @@ -152,10 +157,11 @@ private static void OnValueProduced( try { + var transient = CreateTransient(); // executeResult data var executeResultData = new ExecuteResult( openRequest.ExecutionCount, - transient: openRequest.Transient, + transient: transient, data: valueProduced?.FormattedValues ?.ToDictionary(k => k.MimeType ?? "text/plain", v => v.Value)); diff --git a/Microsoft.DotNet.Interactive.Jupyter/RequestHandlerBase.cs b/Microsoft.DotNet.Interactive.Jupyter/RequestHandlerBase.cs index 2135e2139..bd43725e5 100644 --- a/Microsoft.DotNet.Interactive.Jupyter/RequestHandlerBase.cs +++ b/Microsoft.DotNet.Interactive.Jupyter/RequestHandlerBase.cs @@ -40,18 +40,15 @@ public void Dispose() protected class InflightRequest : IDisposable { private readonly CompositeDisposable _disposables = new CompositeDisposable(); - public Dictionary Transient { get; } public JupyterRequestContext Context { get; } public T Request { get; } public int ExecutionCount { get; } - public InflightRequest(JupyterRequestContext context, T request, int executionCount, - Dictionary transient) + public InflightRequest(JupyterRequestContext context, T request, int executionCount) { Context = context; Request = request; ExecutionCount = executionCount; - Transient = transient; } public void AddDisposable(IDisposable disposable) From f44251dd0b8125ac4a4716beafba24667054c339 Mon Sep 17 00:00:00 2001 From: Diego Date: Tue, 23 Jul 2019 16:18:21 +0100 Subject: [PATCH 3/9] handle last value produce correctly --- .../ExecuteRequestHandler.cs | 7 ++++- .../Events/ValueProduced.cs | 6 +++-- .../Kernel/CSharpKernelTests.cs | 26 ++++++++++++++++--- WorkspaceServer/Kernel/CSharpKernel.cs | 2 ++ 4 files changed, 35 insertions(+), 6 deletions(-) diff --git a/Microsoft.DotNet.Interactive.Jupyter/ExecuteRequestHandler.cs b/Microsoft.DotNet.Interactive.Jupyter/ExecuteRequestHandler.cs index 655c0e7f1..feb3ef63c 100644 --- a/Microsoft.DotNet.Interactive.Jupyter/ExecuteRequestHandler.cs +++ b/Microsoft.DotNet.Interactive.Jupyter/ExecuteRequestHandler.cs @@ -159,8 +159,13 @@ private static void OnValueProduced( { var transient = CreateTransient(); // executeResult data - var executeResultData = new ExecuteResult( + var executeResultData = valueProduced.IsLastValue + ?new ExecuteResult( openRequest.ExecutionCount, + 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)); diff --git a/Microsoft.DotNet.Interactive/Events/ValueProduced.cs b/Microsoft.DotNet.Interactive/Events/ValueProduced.cs index fe7ce0a6f..c50ea337d 100644 --- a/Microsoft.DotNet.Interactive/Events/ValueProduced.cs +++ b/Microsoft.DotNet.Interactive/Events/ValueProduced.cs @@ -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, IReadOnlyCollection formattedValues = null) : base(submitCode) { Value = value; + IsLastValue = isLastValue; FormattedValues = formattedValues; } public object Value { get; } + public bool IsLastValue { get; } public IReadOnlyCollection FormattedValues { get; } } diff --git a/WorkspaceServer.Tests/Kernel/CSharpKernelTests.cs b/WorkspaceServer.Tests/Kernel/CSharpKernelTests.cs index 8142dba70..f27c46faf 100644 --- a/WorkspaceServer.Tests/Kernel/CSharpKernelTests.cs +++ b/WorkspaceServer.Tests/Kernel/CSharpKernelTests.cs @@ -192,9 +192,29 @@ public async Task it_produces_values_when_executing_Console_output() KernelEvents.OfType() .Should() .BeEquivalentTo( - new ValueProduced("value one", kernelCommand, new[] { new FormattedValue(null, "value one"), }), - new ValueProduced("value two", kernelCommand, new[] { new FormattedValue(null, "value two"), }), - new ValueProduced("value three", kernelCommand, new[] { new FormattedValue(null, "value three"), })); + 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"), })); + } + + [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"); + await kernel.SendAsync(kernelCommand); + + KernelEvents.OfType() + .Should() + .HaveCount(4) + .And + .ContainSingle(e => e.IsLastValue); + } [Fact(Skip = "requires support for cs8 in roslyn scripting")] diff --git a/WorkspaceServer/Kernel/CSharpKernel.cs b/WorkspaceServer/Kernel/CSharpKernel.cs index e74abab48..ad6ac4561 100644 --- a/WorkspaceServer/Kernel/CSharpKernel.cs +++ b/WorkspaceServer/Kernel/CSharpKernel.cs @@ -177,6 +177,7 @@ private async Task HandleSubmitCode( new ValueProduced( std, codeSubmission, + false, formattedValues)); } if (HasReturnValue) @@ -194,6 +195,7 @@ private async Task HandleSubmitCode( new ValueProduced( _scriptState.ReturnValue, codeSubmission, + true, formattedValues)); } From 117ee6c05fd018c96c36b58b4ceb331546fb67ab Mon Sep 17 00:00:00 2001 From: Diego Date: Tue, 23 Jul 2019 17:24:34 +0100 Subject: [PATCH 4/9] remove kernel name from csharpkernel tests --- .../Kernel/CSharpKernelTests.cs | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/WorkspaceServer.Tests/Kernel/CSharpKernelTests.cs b/WorkspaceServer.Tests/Kernel/CSharpKernelTests.cs index f27c46faf..c4c1b42cb 100644 --- a/WorkspaceServer.Tests/Kernel/CSharpKernelTests.cs +++ b/WorkspaceServer.Tests/Kernel/CSharpKernelTests.cs @@ -30,7 +30,7 @@ public async Task it_returns_the_result_of_a_non_null_expression() { var kernel = CreateKernel(); - await kernel.SendAsync(new SubmitCode("123", "csharp")); + await kernel.SendAsync(new SubmitCode("123")); KernelEvents.OfType() .Last() @@ -44,9 +44,9 @@ public async Task when_it_throws_exception_after_a_value_was_produced_then_only_ { var kernel = CreateKernel(); - await kernel.SendAsync(new SubmitCode("using System;", "csharp")); - await kernel.SendAsync(new SubmitCode("2 + 2", "csharp")); - await kernel.SendAsync(new SubmitCode("adddddddddd", "csharp")); + await kernel.SendAsync(new SubmitCode("using System;")); + await kernel.SendAsync(new SubmitCode("2 + 2")); + await kernel.SendAsync(new SubmitCode("adddddddddd")); var (failure, lastCodeSubmissionEvaluationFailedPosition) = KernelEvents .Select((error, pos) => (error, pos)) @@ -75,8 +75,8 @@ public async Task it_returns_exceptions_thrown_in_user_code() { var kernel = CreateKernel(); - await kernel.SendAsync(new SubmitCode("using System;", "csharp")); - await kernel.SendAsync(new SubmitCode("throw new NotImplementedException();", "csharp")); + await kernel.SendAsync(new SubmitCode("using System;")); + await kernel.SendAsync(new SubmitCode("throw new NotImplementedException();")); KernelEvents.Last() .Should() @@ -92,8 +92,8 @@ public async Task it_returns_diagnostics() { var kernel = CreateKernel(); - await kernel.SendAsync(new SubmitCode("using System;", "csharp")); - await kernel.SendAsync(new SubmitCode("aaaadd", "csharp")); + await kernel.SendAsync(new SubmitCode("using System;")); + await kernel.SendAsync(new SubmitCode("aaaadd")); KernelEvents.Last() .Should() @@ -109,9 +109,9 @@ public async Task it_notifies_when_submission_is_complete() { var kernel = CreateKernel(); - await kernel.SendAsync(new SubmitCode("var a =", "csharp")); + await kernel.SendAsync(new SubmitCode("var a =")); - await kernel.SendAsync(new SubmitCode("12;", "csharp")); + await kernel.SendAsync(new SubmitCode("12;")); KernelEvents.Should() .NotContain(e => e is ValueProduced); @@ -126,7 +126,7 @@ public async Task it_notifies_when_submission_is_incomplete() { var kernel = CreateKernel(); - await kernel.SendAsync(new SubmitCode("var a =", "csharp")); + await kernel.SendAsync(new SubmitCode("var a =")); KernelEvents.Should() .NotContain(e => e is ValueProduced); @@ -141,7 +141,7 @@ public async Task it_returns_the_result_of_a_null_expression() { var kernel = CreateKernel(); - await kernel.SendAsync(new SubmitCode("null", "csharp")); + await kernel.SendAsync(new SubmitCode("null")); KernelEvents.OfType() .Last() @@ -155,7 +155,7 @@ public async Task it_does_not_return_a_result_for_a_statement() { var kernel = CreateKernel(); - await kernel.SendAsync(new SubmitCode("var x = 1;", "csharp")); + await kernel.SendAsync(new SubmitCode("var x = 1;")); KernelEvents .Should() @@ -167,9 +167,9 @@ public async Task it_aggregates_multiple_submissions() { var kernel = CreateKernel(); - await kernel.SendAsync(new SubmitCode("var x = new List{1,2};", "csharp")); - await kernel.SendAsync(new SubmitCode("x.Add(3);", "csharp")); - await kernel.SendAsync(new SubmitCode("x.Max()", "csharp")); + await kernel.SendAsync(new SubmitCode("var x = new List{1,2};")); + await kernel.SendAsync(new SubmitCode("x.Add(3);")); + await kernel.SendAsync(new SubmitCode("x.Max()")); KernelEvents.OfType() .Last() @@ -186,7 +186,7 @@ public async Task it_produces_values_when_executing_Console_output() var kernelCommand = new SubmitCode(@" Console.Write(""value one""); Console.Write(""value two""); -Console.Write(""value three"");", "csharp"); +Console.Write(""value three"");"); await kernel.SendAsync(kernelCommand); KernelEvents.OfType() @@ -222,8 +222,8 @@ public async Task it_supports_csharp_8() { var kernel = CreateKernel(); - await kernel.SendAsync(new SubmitCode("var text = \"meow? meow!\";", "csharp")); - await kernel.SendAsync(new SubmitCode("text[^5..^0]", "csharp")); + await kernel.SendAsync(new SubmitCode("var text = \"meow? meow!\";")); + await kernel.SendAsync(new SubmitCode("text[^5..^0]")); KernelEvents.OfType() .Last() From 24e4b9f3b71eb6d4dfdfafa2421d25c0182b99c6 Mon Sep 17 00:00:00 2001 From: Diego Date: Tue, 23 Jul 2019 20:36:29 +0100 Subject: [PATCH 5/9] mime type not null --- Microsoft.DotNet.Interactive.Rendering/Formatter.cs | 3 +-- Microsoft.DotNet.Interactive.Rendering/PocketView.cs | 3 ++- Microsoft.DotNet.Interactive/FormattedValue.cs | 4 +++- WorkspaceServer.Tests/Kernel/CSharpKernelTests.cs | 6 +++--- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/Microsoft.DotNet.Interactive.Rendering/Formatter.cs b/Microsoft.DotNet.Interactive.Rendering/Formatter.cs index 017b05a86..81b850553 100644 --- a/Microsoft.DotNet.Interactive.Rendering/Formatter.cs +++ b/Microsoft.DotNet.Interactive.Rendering/Formatter.cs @@ -358,8 +358,7 @@ private static void TryRegisterDefault(string typeName, Action() .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"), })); + new ValueProduced("value one", kernelCommand, false, new[] { new FormattedValue("text/plain", "value one"), }), + new ValueProduced("value two", kernelCommand, false, new[] { new FormattedValue("text/plain", "value two"), }), + new ValueProduced("value three", kernelCommand, false, new[] { new FormattedValue("text/plain", "value three"), })); } [Fact] From 99be41cf66a5b7a6e43f3bdb5ff9a7a6d706f14d Mon Sep 17 00:00:00 2001 From: Diego Date: Tue, 23 Jul 2019 20:58:25 +0100 Subject: [PATCH 6/9] rename methods --- .../Servers/Roslyn/TrackingStringWriter.cs | 108 +++++++++--------- 1 file changed, 56 insertions(+), 52 deletions(-) diff --git a/WorkspaceServer/Servers/Roslyn/TrackingStringWriter.cs b/WorkspaceServer/Servers/Roslyn/TrackingStringWriter.cs index 5149ac9a6..e4d5b743e 100644 --- a/WorkspaceServer/Servers/Roslyn/TrackingStringWriter.cs +++ b/WorkspaceServer/Servers/Roslyn/TrackingStringWriter.cs @@ -16,27 +16,28 @@ private class Region public int Start { get; set; } public int Length { get; set; } } - List _regions = new List(); - private bool _trakingSize; + + readonly List _regions = new List(); + private bool _trackingWriteOperation; public bool WriteOccurred { get; set; } public override void Write(char value) { - TrackSize(() => base.Write(value)); + TrackWriteOperation(() => base.Write(value)); } - private void TrackSize(Action action) + private void TrackWriteOperation(Action action) { WriteOccurred = true; - if (_trakingSize) + if (_trackingWriteOperation) { action(); return; } - _trakingSize = true; + _trackingWriteOperation = true; var sb = base.GetStringBuilder(); var region = new Region @@ -45,19 +46,21 @@ private void TrackSize(Action action) }; _regions.Add(region); + action(); + region.Length = sb.Length - region.Start; - _trakingSize = false; + _trackingWriteOperation = false; } - private async Task TrackSizeAsync(Func action) + private async Task TrackWriteOperationAsync(Func action) { WriteOccurred = true; - if (_trakingSize) + if (_trackingWriteOperation) { await action(); return; } - _trakingSize = true; + _trackingWriteOperation = true; var sb = base.GetStringBuilder(); var region = new Region @@ -68,213 +71,214 @@ private async Task TrackSizeAsync(Func action) _regions.Add(region); await action(); + region.Length = sb.Length - region.Start; - _trakingSize = false; + _trackingWriteOperation = false; } public override void Write(char[] buffer, int index, int count) { - TrackSize(() => base.Write(buffer, index, count)); + TrackWriteOperation(() => base.Write(buffer, index, count)); } public override void Write(string value) { - TrackSize(() => base.Write(value)); + TrackWriteOperation(() => base.Write(value)); } public override Task WriteAsync(char value) { - return TrackSizeAsync(() => base.WriteAsync(value)); + return TrackWriteOperationAsync(() => base.WriteAsync(value)); } public override Task WriteAsync(char[] buffer, int index, int count) { - return TrackSizeAsync(() => base.WriteAsync(buffer, index, count)); + return TrackWriteOperationAsync(() => base.WriteAsync(buffer, index, count)); } public override Task WriteAsync(string value) { - return TrackSizeAsync(() => base.WriteAsync(value)); + return TrackWriteOperationAsync(() => base.WriteAsync(value)); } public override Task WriteLineAsync(char value) { - return TrackSizeAsync(() => base.WriteLineAsync(value)); + return TrackWriteOperationAsync(() => base.WriteLineAsync(value)); } public override Task WriteLineAsync(char[] buffer, int index, int count) { - return TrackSizeAsync(() => base.WriteLineAsync(buffer, index, count)); + return TrackWriteOperationAsync(() => base.WriteLineAsync(buffer, index, count)); } public override Task WriteLineAsync(string value) { - return TrackSizeAsync(() => base.WriteLineAsync(value)); + return TrackWriteOperationAsync(() => base.WriteLineAsync(value)); } public override void Write(bool value) { - TrackSize(() => base.Write(value)); + TrackWriteOperation(() => base.Write(value)); } public override void Write(char[] buffer) { - TrackSize(() => base.Write(buffer)); + TrackWriteOperation(() => base.Write(buffer)); } public override void Write(decimal value) { - TrackSize(() => base.Write(value)); + TrackWriteOperation(() => base.Write(value)); } public override void Write(double value) { - TrackSize(() => base.Write(value)); + TrackWriteOperation(() => base.Write(value)); } public override void Write(int value) { - TrackSize(() => base.Write(value)); + TrackWriteOperation(() => base.Write(value)); } public override void Write(long value) { - TrackSize(() => base.Write(value)); + TrackWriteOperation(() => base.Write(value)); } public override void Write(object value) { - TrackSize(() => base.Write(value)); + TrackWriteOperation(() => base.Write(value)); } public override void Write(float value) { - TrackSize(() => base.Write(value)); + TrackWriteOperation(() => base.Write(value)); } public override void Write(string format, object arg0) { - TrackSize(() => base.Write(format, arg0)); + TrackWriteOperation(() => base.Write(format, arg0)); } public override void Write(string format, object arg0, object arg1) { - TrackSize(() => base.Write(format, arg0, arg1)); + TrackWriteOperation(() => base.Write(format, arg0, arg1)); } public override void Write(string format, object arg0, object arg1, object arg2) { - TrackSize(() => base.Write(format, arg0, arg1, arg2)); + TrackWriteOperation(() => base.Write(format, arg0, arg1, arg2)); } public override void Write(string format, params object[] arg) { - TrackSize(() => base.Write(format, arg)); + TrackWriteOperation(() => base.Write(format, arg)); } public override void Write(uint value) { - TrackSize(() => base.Write(value)); + TrackWriteOperation(() => base.Write(value)); } public override void Write(ulong value) { - TrackSize(() => base.Write(value)); + TrackWriteOperation(() => base.Write(value)); } public override void WriteLine() { - TrackSize(() => base.WriteLine()); + TrackWriteOperation(() => base.WriteLine()); } public override void WriteLine(bool value) { - TrackSize(() => base.WriteLine(value)); + TrackWriteOperation(() => base.WriteLine(value)); } public override void WriteLine(char value) { - TrackSize(() => base.WriteLine(value)); + TrackWriteOperation(() => base.WriteLine(value)); } public override void WriteLine(char[] buffer) { - TrackSize(() => base.WriteLine(buffer)); + TrackWriteOperation(() => base.WriteLine(buffer)); } public override void WriteLine(char[] buffer, int index, int count) { - TrackSize(() => base.WriteLine(buffer, index, count)); + TrackWriteOperation(() => base.WriteLine(buffer, index, count)); } public override void WriteLine(decimal value) { - TrackSize(() => base.WriteLine(value)); + TrackWriteOperation(() => base.WriteLine(value)); } public override void WriteLine(double value) { - TrackSize(() => base.WriteLine(value)); + TrackWriteOperation(() => base.WriteLine(value)); } public override void WriteLine(int value) { - TrackSize(() => base.WriteLine(value)); + TrackWriteOperation(() => base.WriteLine(value)); } public override void WriteLine(long value) { - TrackSize(() => base.WriteLine(value)); + TrackWriteOperation(() => base.WriteLine(value)); } public override void WriteLine(object value) { - TrackSize(() => base.WriteLine(value)); + TrackWriteOperation(() => base.WriteLine(value)); } public override void WriteLine(float value) { - TrackSize(() => base.WriteLine(value)); + TrackWriteOperation(() => base.WriteLine(value)); } public override void WriteLine(string value) { - TrackSize(() => base.WriteLine(value)); + TrackWriteOperation(() => base.WriteLine(value)); } public override void WriteLine(string format, object arg0) { - TrackSize(() => base.WriteLine(format, arg0)); + TrackWriteOperation(() => base.WriteLine(format, arg0)); } public override void WriteLine(string format, object arg0, object arg1) { - TrackSize(() => base.WriteLine(format, arg0, arg1)); + TrackWriteOperation(() => base.WriteLine(format, arg0, arg1)); } public override void WriteLine(string format, object arg0, object arg1, object arg2) { - TrackSize(() => base.WriteLine(format, arg0, arg1, arg2)); + TrackWriteOperation(() => base.WriteLine(format, arg0, arg1, arg2)); } public override void WriteLine(string format, params object[] arg) { - TrackSize(() => base.WriteLine(format, arg)); + TrackWriteOperation(() => base.WriteLine(format, arg)); } public override void WriteLine(uint value) { - TrackSize(() => base.WriteLine(value)); + TrackWriteOperation(() => base.WriteLine(value)); } public override void WriteLine(ulong value) { - TrackSize(() => base.WriteLine(value)); + TrackWriteOperation(() => base.WriteLine(value)); } public override Task WriteLineAsync() { - return TrackSizeAsync(() => base.WriteLineAsync()); + return TrackWriteOperationAsync(() => base.WriteLineAsync()); } public IEnumerable Writes() From 03f2fe138f2c39df968844cb28c37842c172eb6c Mon Sep 17 00:00:00 2001 From: Diego Date: Tue, 23 Jul 2019 21:06:21 +0100 Subject: [PATCH 7/9] use const for DefaultMimeType and remove redundant checks --- .../ExecuteRequestHandler.cs | 12 ++++++------ Microsoft.DotNet.Interactive.Rendering/Formatter.cs | 3 ++- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Microsoft.DotNet.Interactive.Jupyter/ExecuteRequestHandler.cs b/Microsoft.DotNet.Interactive.Jupyter/ExecuteRequestHandler.cs index feb3ef63c..9440d8215 100644 --- a/Microsoft.DotNet.Interactive.Jupyter/ExecuteRequestHandler.cs +++ b/Microsoft.DotNet.Interactive.Jupyter/ExecuteRequestHandler.cs @@ -16,7 +16,7 @@ namespace Microsoft.DotNet.Interactive.Jupyter public class ExecuteRequestHandler : RequestHandlerBase { private int _executionCount; - + public ExecuteRequestHandler(IKernel kernel) : base(kernel) { } @@ -80,7 +80,7 @@ public async Task Handle(JupyterRequestContext context) private static Dictionary CreateTransient() { var id = Guid.NewGuid(); - var transient = new Dictionary {{"display_id", id.ToString()}}; + var transient = new Dictionary { { "display_id", id.ToString() } }; return transient; } @@ -101,7 +101,7 @@ void OnKernelResultEvent(IKernelEvent value) case IncompleteCodeSubmissionReceived _: case CompleteCodeSubmissionReceived _: break; - default: + default: throw new NotSupportedException(); } } @@ -160,15 +160,15 @@ private static void OnValueProduced( var transient = CreateTransient(); // executeResult data var executeResultData = valueProduced.IsLastValue - ?new ExecuteResult( + ? new ExecuteResult( openRequest.ExecutionCount, transient: transient, data: valueProduced?.FormattedValues - ?.ToDictionary(k => k.MimeType ?? "text/plain", v => v.Value)) + ?.ToDictionary(k => k.MimeType, v => v.Value)) : new DisplayData( transient: transient, data: valueProduced?.FormattedValues - ?.ToDictionary(k => k.MimeType ?? "text/plain", v => v.Value)); + ?.ToDictionary(k => k.MimeType, v => v.Value)); if (!openRequest.Request.Silent) { diff --git a/Microsoft.DotNet.Interactive.Rendering/Formatter.cs b/Microsoft.DotNet.Interactive.Rendering/Formatter.cs index 81b850553..3ec984932 100644 --- a/Microsoft.DotNet.Interactive.Rendering/Formatter.cs +++ b/Microsoft.DotNet.Interactive.Rendering/Formatter.cs @@ -18,6 +18,7 @@ namespace Microsoft.DotNet.Interactive.Rendering /// public static class Formatter { + private const string DefaultMimeType = "text/plain"; private static Func _autoGenerateForType = t => false; private static int _defaultListExpansionLimit; private static int _recursionLimit; @@ -358,7 +359,7 @@ private static void TryRegisterDefault(string typeName, Action Date: Tue, 23 Jul 2019 21:07:43 +0100 Subject: [PATCH 8/9] white space --- WorkspaceServer/Servers/Roslyn/TrackingStringWriter.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/WorkspaceServer/Servers/Roslyn/TrackingStringWriter.cs b/WorkspaceServer/Servers/Roslyn/TrackingStringWriter.cs index e4d5b743e..84ac20015 100644 --- a/WorkspaceServer/Servers/Roslyn/TrackingStringWriter.cs +++ b/WorkspaceServer/Servers/Roslyn/TrackingStringWriter.cs @@ -18,10 +18,10 @@ private class Region } readonly List _regions = new List(); - private bool _trackingWriteOperation; + private bool _trackingWriteOperation; - public bool WriteOccurred { get; set; } + public bool WriteOccurred { get; set; } public override void Write(char value) { @@ -52,6 +52,7 @@ private void TrackWriteOperation(Action action) region.Length = sb.Length - region.Start; _trackingWriteOperation = false; } + private async Task TrackWriteOperationAsync(Func action) { WriteOccurred = true; From a5be7388c5e7aac5c14be96ce3d464331a9829fa Mon Sep 17 00:00:00 2001 From: Diego Date: Tue, 23 Jul 2019 21:08:59 +0100 Subject: [PATCH 9/9] empty or white space string are not valid mimetypes --- Microsoft.DotNet.Interactive/FormattedValue.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Microsoft.DotNet.Interactive/FormattedValue.cs b/Microsoft.DotNet.Interactive/FormattedValue.cs index aac43cde2..3eae78368 100644 --- a/Microsoft.DotNet.Interactive/FormattedValue.cs +++ b/Microsoft.DotNet.Interactive/FormattedValue.cs @@ -9,7 +9,12 @@ public class FormattedValue { public FormattedValue(string mimeType, object value) { - MimeType = mimeType ?? throw new ArgumentNullException(nameof(mimeType)); + if (string.IsNullOrWhiteSpace(mimeType)) + { + throw new ArgumentException("Value cannot be null or whitespace.", nameof(mimeType)); + } + + MimeType = mimeType; Value = value; }