Fix input prompt quoting issue (#3360)

* fix issue with multiple #!set commands using @input in one submission

* cleanup

* fix quoted prompt bug

* fix warning

src/Microsoft.DotNet.Interactive.PowerShell.Tests/Microsoft.DotNet.Interactive.PowerShell.Tests.v3.ncrunchproject

@@ -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>

src/Microsoft.DotNet.Interactive.PowerShell/PowerShellKernel.cs

@@ -280,7 +280,7 @@ async Task IKernelCommandHandler<SubmitCode>.HandleAsync(
         {
             var success = RunSubmitCodeLocally(code);
 
-            if (!success || pwsh.HadErrors)
+            if (!success)
             {
                 context.Fail(context.Command);
             }
@@ -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)
         {

src/Microsoft.DotNet.Interactive.Tests/InputsWithinMagicCommandsTests.cs

@@ -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;
@@ -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;
         });
 
@@ -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);
@@ -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]
@@ -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");
     }
@@ -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()
         {

src/Microsoft.DotNet.Interactive.Tests/Microsoft.DotNet.Interactive.Tests.v3.ncrunchproject

@@ -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>

src/Microsoft.DotNet.Interactive.Tests/VariableSharingTests.SetMagicCommand.cs

@@ -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("""
@@ -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]