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.

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

handle multiple #!set commands in sungle submission #2960

Merged
merged 1 commit into from
May 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,10 @@ public async Task can_set_value_prompting_user()
{
var kernel = CreateKernel(Language.CSharp);

using var composite = new CompositeKernel();

composite.Add(kernel);
using var composite = new CompositeKernel
{
kernel
};

composite.RegisterCommandHandler<RequestInput>((requestInput, context) =>
{
Expand All @@ -50,10 +51,43 @@ public async Task can_set_value_prompting_user()
valueProduced.Value.Should().BeEquivalentTo("hello!");
}

[Fact]
public async Task can_handle_multiple_set_commands_in_single_submission()
{
using var kernel = CreateCompositeKernel();

kernel.RegisterCommandHandler<RequestInput>((requestInput, context) =>
{
context.Publish(new InputProduced("hello!", requestInput));
return Task.CompletedTask;
});

await kernel.SendAsync( new SubmitCode("""
let var1 = "a"
let var2 = "b"
""", targetKernelName:"fsharp"));

var result = await kernel.SendAsync(new SubmitCode("""
#!set --name newVar1 --value @fsharp:var1 --mime-type text/plain
#!set --name newVar2 --value @fsharp:var2 --mime-type text/plain
#!set --name newVar3 --value @input:input-please
""", targetKernelName: "csharp"));

result.Events.Should().NotContainErrors();

result = await kernel.SendAsync(new RequestValueInfos("csharp"));

var valueInfosProduced = result.Events.Should()
.ContainSingle<ValueInfosProduced>()
.Which;

valueInfosProduced.ValueInfos.Select(v => v.Name).Should().BeEquivalentTo("newVar1", "newVar2", "newVar3");
}

[Theory]
[InlineData(
"""
#!fsharp
#!fsharp
let x = 123
""",
"""
Expand Down
28 changes: 19 additions & 9 deletions src/Microsoft.DotNet.Interactive/KernelExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.CommandLine.Invocation;
using System.CommandLine.NamingConventionBinder;
using System.CommandLine.Parsing;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Reactive.Linq;
Expand Down Expand Up @@ -215,7 +216,7 @@ private static async Task HandleSetMagicCommand<T>(
T kernel,
InvocationContext cmdLineContext,
Option<string> nameOption,
Option<object> valueOption,
Option<ValueOptionResult> valueOption,
Option<string> mimeTypeOption,
Option<bool> byrefOption)
where T : Kernel
Expand All @@ -230,8 +231,14 @@ private static async Task HandleSetMagicCommand<T>(
var events = new List<ValueProduced>();

using var subscription = context.KernelEvents.OfType<ValueProduced>().Subscribe(events.Add);

var valueSource = cmdLineContext.ParseResult.GetValueForOption(valueOption);


var valueProduced = events.SingleOrDefault();
var valueProduced = valueSource switch
{ { Name: var sourceValueName, Kernel: var sourceKernelName } when !string.IsNullOrWhiteSpace(sourceKernelName) && !string.IsNullOrEmpty(sourceKernelName) && sourceKernelName != "input" => events.SingleOrDefault(e => e.Name == sourceValueName && e.Command.TargetKernelName == sourceKernelName),
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be great to split the long line...

_ => null
};

if (valueProduced is { })
{
Expand All @@ -242,15 +249,16 @@ private static async Task HandleSetMagicCommand<T>(
}
else
{
var interpolatedValue = cmdLineContext.ParseResult.GetValueForOption(valueOption);

await SendValue(kernel, interpolatedValue, null, valueName);
await SendValue(kernel,valueSource?.Value, null, valueName);
}
}
else
{
context.Fail(context.Command, new CommandNotSupportedException(typeof(SendValue), kernel));
}


Copy link
Contributor

Choose a reason for hiding this comment

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

minor: Remove blank lines

}

public static T UseValueSharing<T>(this T kernel) where T : Kernel
Expand Down Expand Up @@ -293,7 +301,7 @@ private static void ConfigureAndAddSetMagicCommand<T>(T destinationKernel) where
HtmlFormatter.MimeType,
PlainTextFormatter.MimeType);

var valueOption = new Option<object>(
var valueOption = new Option<ValueOptionResult>(
"--value",
description:
LocalizationResources.Magics_set_value_Description(),
Expand Down Expand Up @@ -355,13 +363,13 @@ destinationKernel.RootKernel is CompositeKernel root &&

destinationKernel.AddDirective(set);

object ParseValueOption(ArgumentResult argResult)
ValueOptionResult ParseValueOption(ArgumentResult argResult)
{
var valueOptionValue = argResult.Tokens.Single().Value;

if (!valueOptionValue.StartsWith("@"))
{
return valueOptionValue;
return new ValueOptionResult( valueOptionValue, null,null);
Copy link
Contributor

Choose a reason for hiding this comment

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

consider using named params for the nulls

}

bool isByref;
Expand Down Expand Up @@ -412,15 +420,17 @@ object ParseValueOption(ArgumentResult argResult)

if (isByref)
{
return valueProduced.Value;
return new ValueOptionResult(valueProduced.Value, sourceKernelName, sourceValueName);
}
else
{
return valueProduced.FormattedValue;
return new ValueOptionResult(valueProduced.FormattedValue, sourceKernelName, sourceValueName);
}
}
}

private record ValueOptionResult(object Value, string Kernel, string Name);

private static void ConfigureAndAddShareMagicCommand<T>(T kernel) where T : Kernel
{
var sourceValueNameArg = new Argument<string>(
Expand Down