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

Fix input prompt quoting issue #3360

Merged
merged 4 commits into from
Dec 4, 2023
Merged
Show file tree
Hide file tree
Changes from 3 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 @@ -29,9 +29,6 @@
<NamedTestSelector>
<TestName>Microsoft.DotNet.Interactive.PowerShell.Tests.PowerShellKernelTests.GetCorrectProfilePaths</TestName>
</NamedTestSelector>
<NamedTestSelector>
<TestName>Microsoft.DotNet.Interactive.PowerShell.Tests.PowerShellKernelTests.When_code_produces_errors_then_the_command_fails</TestName>
</NamedTestSelector>
</IgnoredTests>
</Settings>
</ProjectConfiguration>
36 changes: 22 additions & 14 deletions src/Microsoft.DotNet.Interactive.PowerShell/PowerShellKernel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ async Task IKernelCommandHandler<SubmitCode>.HandleAsync(
{
var success = RunSubmitCodeLocally(code);

if (!success || pwsh.HadErrors)
if (!success)
{
context.Fail(context.Command);
}
Expand Down Expand Up @@ -383,23 +383,31 @@ private bool RunSubmitCodeLocally(string code)

pwsh.InvokeAndClear();

pwsh.AddScript("$error");
var output = new List<string>();
try
if (pwsh.HadErrors)
{
pwsh.Invoke(input: null, output: output);
}
finally
{
pwsh.Clear();
succeeded = false;
}

if (output.Count > _errorCount)
else
{
succeeded = false;
// certain kinds of errors aren't signaled by the HadErrors so we can check using $error
pwsh.AddScript("$error");
var output = new List<string>();
try
{
pwsh.Invoke(input: null, output: output);
}
finally
{
pwsh.Clear();
}

if (output.Count > _errorCount)
{
succeeded = false;
}

_errorCount = output.Count;
}

_errorCount = output.Count;
}
catch (Exception e)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System;
using System.Collections.Generic;
using System.CommandLine;
using System.IO;
using System.Threading.Tasks;
Expand All @@ -10,25 +11,31 @@
using Microsoft.DotNet.Interactive.Commands;
using Microsoft.DotNet.Interactive.CSharp;
using Microsoft.DotNet.Interactive.Events;
using Microsoft.DotNet.Interactive.Tests.Utility;
using Xunit;

namespace Microsoft.DotNet.Interactive.Tests;

public class InputsWithinMagicCommandsTests : IDisposable
{
private readonly CompositeKernel kernel;

private RequestInput _receivedRequestInput = null;
private string _receivedUserInput = null;

private readonly List<string> _receivedUserInput = new();

private readonly Command _shimCommand;

private readonly Queue<string> _responses = new();

public InputsWithinMagicCommandsTests()
{
kernel = CreateKernel();

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

Expand All @@ -41,7 +48,7 @@ public InputsWithinMagicCommandsTests()
};
_shimCommand.SetHandler(context =>
{
_receivedUserInput = context.ParseResult.GetValueForOption(stringOption);
_receivedUserInput.Add(context.ParseResult.GetValueForOption(stringOption));
});

kernel.FindKernelByName("csharp").AddDirective(_shimCommand);
Expand Down Expand Up @@ -85,11 +92,27 @@ public async Task Input_token_in_magic_command_includes_requested_value_name()
}

[Fact]
public async Task Input_token_in_magic_command_prompts_user_passes_user_input_to_directive__to_handler()
public async Task Input_token_in_magic_command_prompts_user_passes_user_input_to_directive_to_handler()
{
_responses.Enqueue("one");

await kernel.SendAsync(new SubmitCode("#!shim --string @input:input-please", "csharp"));

_receivedUserInput.Should().Be("hello!");
_receivedUserInput.Should().ContainSingle().Which.Should().Be("one");
}

[Fact]
public async Task Input_token_in_magic_command_prompts_user_passes_user_input_to_directive_to_handler_when_there_are_multiple_inputs()
{
_responses.Enqueue("one");
_responses.Enqueue("two");

await kernel.SendAsync(new SubmitCode("""
#!shim --string @input:input-please
#!shim --string @input:input-please
""", "csharp"));

_receivedUserInput.Should().BeEquivalentTo("one", "two");
}

[Fact]
Expand All @@ -109,11 +132,27 @@ public async Task Input_token_in_magic_command_prompts_user_for_password()
}

[Fact]
public async Task An_input_type_hint_is_set_for_file_inputs()
public async Task An_input_type_hint_is_set_for_file_inputs_when_prompt_is_unquoted()
{
_shimCommand.Add(new Option<FileInfo>("--file"));

await kernel.SendAsync(new SubmitCode("#!shim --file @input:file-please\n// some more stuff", "csharp"));
await kernel.SendAsync(new SubmitCode("""
#!shim --file @input:file-please
// some more stuff
""", "csharp"));

_receivedRequestInput.InputTypeHint.Should().Be("file");
}

[Fact]
public async Task An_input_type_hint_is_set_for_file_inputs_when_prompt_is_quoted()
{
_shimCommand.Add(new Option<FileInfo>("--file"));

await kernel.SendAsync(new SubmitCode("""
#!shim --file @input:"file please"
// some more stuff
""", "csharp"));

_receivedRequestInput.InputTypeHint.Should().Be("file");
}
Expand All @@ -128,6 +167,104 @@ public async Task Unknown_types_return_type_hint_of_text()
_receivedRequestInput.InputTypeHint.Should().Be("text");
}

[Fact]
public async Task multiple_set_commands_with_inputs_can_be_used_in_single_submission()
{
using var kernel = new CompositeKernel
{
new CSharpKernel().UseValueSharing()
};

var responses = new Queue<string>();
responses.Enqueue("one");
responses.Enqueue("two");
responses.Enqueue("three");

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

var result = await kernel.SendAsync(new SubmitCode("""
#!set --name value1 --value @input:"input-please "
#!set --name value2 --value @input:"input-please "
#!set --name value3 --value @input:"input-please"
""", targetKernelName: "csharp"));

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

var csharpKernel = (CSharpKernel)kernel.FindKernelByName("csharp");

csharpKernel.TryGetValue("value1", out object value1)
.Should().BeTrue();
value1.Should().Be("one");

csharpKernel.TryGetValue("value2", out object value2)
.Should().BeTrue();
value2.Should().Be("two");

csharpKernel.TryGetValue("value3", out object value3)
.Should().BeTrue();
value3.Should().Be("three");
}

[Fact]
public async Task additional_properties_of_input_request_are_set_by_input_properties_when_prompt_is_quoted()
{
using var kernel = new CSharpKernel();

var command = new Command("#!test")
{
new Argument<string>()
};
kernel.AddDirective(command);

RequestInput requestInput = null;
kernel.RegisterCommandHandler<RequestInput>((input, context) =>
{
requestInput = input;

return Task.CompletedTask;
});

var magicCommand = """#!test @input:"pick a number",save,type=file """;

await kernel.SendAsync(new SubmitCode(magicCommand));

requestInput.Prompt.Should().Be("pick a number");
// FIX: requestInput.Persistent.Should().BeTrue();
requestInput.InputTypeHint.Should().Be("file");
}

[Fact(Skip = "Evaluating different syntax approaches")]
public async Task additional_properties_of_input_request_are_set_by_input_properties_when_prompt_or_field_name_is_not_quoted()
{
using var kernel = new CSharpKernel();

var command = new Command("#!test")
{
new Argument<string>()
};
kernel.AddDirective(command);

RequestInput requestInput = null;
kernel.RegisterCommandHandler<RequestInput>((input, context) =>
{
requestInput = input;

return Task.CompletedTask;
});

var magicCommand = """#!test @input:promptOrFieldName,save,type=file """;

await kernel.SendAsync(new SubmitCode(magicCommand));

requestInput.Prompt.Should().Be("promptOrFieldName");
// FIX: requestInput.Persistent.Should().BeTrue();
requestInput.InputTypeHint.Should().Be("file");
}

private static CompositeKernel CreateKernel() =>
new()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
<FixtureTestSelector>
<FixtureName>Microsoft.DotNet.Interactive.Tests.LanguageKernelPackageTests</FixtureName>
</FixtureTestSelector>
<FixtureTestSelector>
<FixtureName>Microsoft.DotNet.Interactive.Tests.StdIoKernelConnectorTests</FixtureName>
</FixtureTestSelector>
</IgnoredTests>
</Settings>
</ProjectConfiguration>
Original file line number Diff line number Diff line change
Expand Up @@ -77,19 +77,19 @@ public async Task can_set_value_prompting_user_for_password()
}

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

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

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

var result = await kernel.SendAsync(new SubmitCode("""
Expand All @@ -107,6 +107,20 @@ public async Task can_handle_multiple_set_commands_in_single_submission()
.Which;

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

var csharpKernel = (CSharpKernel)kernel.FindKernelByName("csharp");

csharpKernel.TryGetValue("newVar1", out object newVar1)
.Should().BeTrue();
newVar1.Should().Be("one");

csharpKernel.TryGetValue("newVar2", out object newVar2)
.Should().BeTrue();
newVar2.Should().Be("two");

csharpKernel.TryGetValue("newVar3", out object newVar3)
.Should().BeTrue();
newVar3.Should().Be("three");
}

[Fact]
Expand Down
Loading
Loading