Skip to content
This repository was archived by the owner on Jan 12, 2024. It is now read-only.

Improvements to magic command argument parsing #170

Merged
rmshaffer merged 4 commits into from
Jun 15, 2020
Merged

Improvements to magic command argument parsing #170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ef88877
Use updated parsing method everywhere
rmshaffer Jun 15, 2020
c1c911f
Improve handling of quotes and spaces
rmshaffer Jun 15, 2020
069ede8
Small improvement to switch statement
rmshaffer Jun 15, 2020
d22d506
Add some tests for the new parsing
rmshaffer Jun 15, 2020
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
86 changes: 43 additions & 43 deletions src/Jupyter/Magic/AbstractMagic.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Text.RegularExpressions;
using System.Threading.Tasks;
using Microsoft.Jupyter.Core;
using Microsoft.Quantum.IQSharp.Common;
Expand Down Expand Up @@ -63,24 +64,6 @@ public Func<string, IChannel, Task<ExecutionResult>> SafeExecute(Func<string, IC
}
};

/// <summary>
/// Parses the input to a magic command, interpreting the input as
/// a name followed by a JSON-serialized dictionary.
/// </summary>
public static (string, Dictionary<string, string>) ParseInput(string input)
{
if (input == null) return (string.Empty, new Dictionary<string, string> { });
var BLANK_SPACE = new char[1] { ' ' };

var inputParts = input.Split(BLANK_SPACE, 2, StringSplitOptions.RemoveEmptyEntries);
var name = inputParts.Length > 0 ? inputParts[0] : string.Empty;
var args = inputParts.Length > 1
? JsonConverters.JsonToDict(inputParts[1])
: new Dictionary<string, string> { };

return (name, args);
}

/// <summary>
/// Parses the input to a magic command, interpreting the input as
/// a name followed by a JSON-serialized dictionary.
Expand All @@ -97,46 +80,63 @@ public static Dictionary<string, string> ParseInputParameters(string input, stri
{
Dictionary<string, string> inputParameters = new Dictionary<string, string>();

var args = input.Split(null as char[], StringSplitOptions.RemoveEmptyEntries);
// This regex looks for four types of matches:
// 1. (\{.*\})
Copy link
Contributor

@ricardo-espinoza ricardo-espinoza Jun 15, 2020

Choose a reason for hiding this comment

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

Thanks for the detailed explanation of the RE in the comments.

Copy link
Contributor Author

@rmshaffer rmshaffer Jun 15, 2020

Choose a reason for hiding this comment

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

That was as much for myself as for anyone else! Otherwise there's no way that I would be able to reverse-engineer this regex a month from now. 😄

// Matches anything enclosed in matching curly braces.
// 2. [^\s"]+(?:\s*=\s*)(?:"[^"]*"|[^\s"]*)*
// Matches things that look like key=value, allowing whitespace around the equals sign,
// and allowing value to be a quoted string, e.g., key="value".
// 3. [^\s"]+(?:"[^"]*"[^\s"]*)*
// Matches things that are single words, not inside quotes.
// 4. (?:"[^"]*"[^\s"]*)+
// Matches quoted strings.
var regex = new Regex(@"(\{.*\})|[^\s""]+(?:\s*=\s*)(?:""[^""]*""|[^\s""]*)*|[^\s""]+(?:""[^""]*""[^\s""]*)*|(?:""[^""]*""[^\s""]*)+");
var args = regex.Matches(input).Select(match => match.Value);

// If we are expecting a first inferred-name parameter, see if it exists.
// If so, serialize it to the dictionary as JSON and remove it from the list of args.
if (args.Length > 0 &&
!args[0].StartsWith("{") &&
!args[0].Contains("=") &&
if (args.Any() &&
!args.First().StartsWith("{") &&
!args.First().Contains("=") &&
!string.IsNullOrEmpty(firstParameterInferredName))
{
using (var writer = new StringWriter())
{
Json.Serializer.Serialize(writer, args[0]);
inputParameters[firstParameterInferredName] = writer.ToString();
}
args = args.Where((_, index) => index != 0).ToArray();
using var writer = new StringWriter();
Json.Serializer.Serialize(writer, args.First());
inputParameters[firstParameterInferredName] = writer.ToString();
args = args.Skip(1);
}

// See if the remaining arguments look like JSON. If so, try to parse as JSON.
// Otherwise, try to parse as key=value pairs and serialize into the dictionary as JSON.
if (args.Length > 0 && args[0].StartsWith("{"))
// See if the remaining arguments look like JSON. If so, parse as JSON.
if (args.Any() && args.First().StartsWith("{"))
{
var jsonArgs = JsonToDict(string.Join(" ", args));
var jsonArgs = JsonToDict(args.First());
foreach (var (key, jsonValue) in jsonArgs)
{
inputParameters[key] = jsonValue;
}

return inputParameters;
}
else

// Otherwise, try to parse as key=value pairs and serialize into the dictionary as JSON.
foreach (string arg in args)
{
foreach (string arg in args)
var tokens = arg.Split("=", 2);
var key = tokens[0].Trim();
var value = tokens.Length switch
{
var tokens = arg.Split("=", 2);
var key = tokens[0].Trim();
var value = (tokens.Length == 1) ? true as object : tokens[1].Trim() as object;
using (var writer = new StringWriter())
{
Json.Serializer.Serialize(writer, value);
inputParameters[key] = writer.ToString();
}
}
// If there was no value provided explicitly, treat it as an implicit "true" value
1 => true as object,

// Trim whitespace and also enclosing single-quotes or double-quotes before returning
2 => Regex.Replace(tokens[1].Trim(), @"^['""]|['""]$", string.Empty) as object,

// We called arg.Split("=", 2), so there should never be more than 2
_ => throw new InvalidOperationException()
};
using var writer = new StringWriter();
Json.Serializer.Serialize(writer, value);
inputParameters[key] = writer.ToString();
}

return inputParameters;
Expand Down
5 changes: 4 additions & 1 deletion src/Kernel/Magic/PackageMagic.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ namespace Microsoft.Quantum.IQSharp.Kernel
/// </summary>
public class PackageMagic : AbstractMagic
{
private const string ParameterNamePackageName = "__packageName__";

/// <summary>
/// Constructs a new magic command that adds package references to
/// a given references collection.
Expand All @@ -39,7 +41,8 @@ public PackageMagic(IReferences references) : base(
/// <inheritdoc />
public override ExecutionResult Run(string input, IChannel channel)
{
var (name, _) = ParseInput(input);
var inputParameters = ParseInputParameters(input, firstParameterInferredName: ParameterNamePackageName);
var name = inputParameters.DecodeParameter<string>(ParameterNamePackageName);
var status = new Jupyter.TaskStatus($"Adding package {name}");
var statusUpdater = channel.DisplayUpdatable(status);
void Update() => statusUpdater.Update(status);
Expand Down
5 changes: 4 additions & 1 deletion src/Kernel/Magic/WorkspaceMagic.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ namespace Microsoft.Quantum.IQSharp.Kernel
/// </summary>
public class WorkspaceMagic : AbstractMagic
{
private const string ParameterNameCommand = "__command__";

/// <summary>
/// Given a workspace, constructs a new magic symbol to control
/// that workspace.
Expand Down Expand Up @@ -51,7 +53,8 @@ public void CheckIfReady()
/// <inheritdoc />
public override ExecutionResult Run(string input, IChannel channel)
{
var (command, _) = ParseInput(input);
var inputParameters = ParseInputParameters(input, firstParameterInferredName: ParameterNameCommand);
var command = inputParameters.DecodeParameter<string>(ParameterNameCommand);

if (string.IsNullOrWhiteSpace(command))
{
Expand Down
68 changes: 53 additions & 15 deletions src/Tests/AzureClientMagicTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,17 +50,32 @@ public void TestConnectMagic()
resourceGroupName={resourceGroupName}
workspaceName={workspaceName}
storageAccountConnectionString={storageAccountConnectionString}");
Assert.AreEqual(azureClient.LastAction, AzureClientAction.Connect);
Assert.AreEqual(AzureClientAction.Connect, azureClient.LastAction);
Assert.IsFalse(azureClient.RefreshCredentials);
Assert.AreEqual(azureClient.ConnectionString, storageAccountConnectionString);
Assert.AreEqual(subscriptionId, azureClient.SubscriptionId);
Assert.AreEqual(resourceGroupName, azureClient.ResourceGroupName);
Assert.AreEqual(workspaceName, azureClient.WorkspaceName);
Assert.AreEqual(storageAccountConnectionString, azureClient.ConnectionString);

// valid input with extra whitespace and quotes
connectMagic.Test(
@$"subscriptionId = {subscriptionId}
resourceGroupName= ""{resourceGroupName}""
workspaceName ={workspaceName}
storageAccountConnectionString = '{storageAccountConnectionString}'");
Assert.AreEqual(AzureClientAction.Connect, azureClient.LastAction);
Assert.IsFalse(azureClient.RefreshCredentials);
Assert.AreEqual(subscriptionId, azureClient.SubscriptionId);
Assert.AreEqual(resourceGroupName, azureClient.ResourceGroupName);
Assert.AreEqual(workspaceName, azureClient.WorkspaceName);
Assert.AreEqual(storageAccountConnectionString, azureClient.ConnectionString);

// valid input with forced login
connectMagic.Test(
@$"refresh subscriptionId={subscriptionId}
resourceGroupName={resourceGroupName}
workspaceName={workspaceName}
storageAccountConnectionString={storageAccountConnectionString}");

Assert.IsTrue(azureClient.RefreshCredentials);
}

Expand All @@ -71,13 +86,19 @@ public void TestStatusMagic()
var azureClient = new MockAzureClient();
var statusMagic = new StatusMagic(azureClient);
statusMagic.Test(string.Empty);
Assert.AreEqual(azureClient.LastAction, AzureClientAction.GetJobStatus);
Assert.AreEqual(AzureClientAction.GetJobStatus, azureClient.LastAction);

// single argument - should print job status
azureClient = new MockAzureClient();
statusMagic = new StatusMagic(azureClient);
statusMagic.Test($"{jobId}");
Assert.AreEqual(azureClient.LastAction, AzureClientAction.GetJobStatus);
Assert.AreEqual(AzureClientAction.GetJobStatus, azureClient.LastAction);

// single argument with quotes - should print job status
azureClient = new MockAzureClient();
statusMagic = new StatusMagic(azureClient);
statusMagic.Test($"\"{jobId}\"");
Assert.AreEqual(AzureClientAction.GetJobStatus, azureClient.LastAction);
}

[TestMethod]
Expand All @@ -87,11 +108,11 @@ public void TestSubmitMagic()
var azureClient = new MockAzureClient();
var submitMagic = new SubmitMagic(azureClient);
submitMagic.Test(string.Empty);
Assert.AreEqual(azureClient.LastAction, AzureClientAction.SubmitJob);
Assert.AreEqual(AzureClientAction.SubmitJob, azureClient.LastAction);

// single argument
submitMagic.Test($"{operationName}");
Assert.AreEqual(azureClient.LastAction, AzureClientAction.SubmitJob);
Assert.AreEqual(AzureClientAction.SubmitJob, azureClient.LastAction);
Assert.IsTrue(azureClient.SubmittedJobs.Contains(operationName));
}

Expand All @@ -102,11 +123,11 @@ public void TestExecuteMagic()
var azureClient = new MockAzureClient();
var executeMagic = new ExecuteMagic(azureClient);
executeMagic.Test(string.Empty);
Assert.AreEqual(azureClient.LastAction, AzureClientAction.ExecuteJob);
Assert.AreEqual(AzureClientAction.ExecuteJob, azureClient.LastAction);

// single argument
executeMagic.Test($"{operationName}");
Assert.AreEqual(azureClient.LastAction, AzureClientAction.ExecuteJob);
Assert.AreEqual(AzureClientAction.ExecuteJob, azureClient.LastAction);
Assert.IsTrue(azureClient.ExecutedJobs.Contains(operationName));
}

Expand All @@ -117,13 +138,19 @@ public void TestOutputMagic()
var azureClient = new MockAzureClient();
var outputMagic = new OutputMagic(azureClient);
outputMagic.Test(string.Empty);
Assert.AreEqual(azureClient.LastAction, AzureClientAction.GetJobResult);
Assert.AreEqual(AzureClientAction.GetJobResult, azureClient.LastAction);

// single argument - should print job status
// single argument - should print job result
azureClient = new MockAzureClient();
outputMagic = new OutputMagic(azureClient);
outputMagic.Test($"{jobId}");
Assert.AreEqual(azureClient.LastAction, AzureClientAction.GetJobResult);
Assert.AreEqual(AzureClientAction.GetJobResult, azureClient.LastAction);

// single argument with quotes - should print job result
azureClient = new MockAzureClient();
outputMagic = new OutputMagic(azureClient);
outputMagic.Test($"'{jobId}'");
Assert.AreEqual(AzureClientAction.GetJobResult, azureClient.LastAction);
}

[TestMethod]
Expand All @@ -133,7 +160,7 @@ public void TestJobsMagic()
var azureClient = new MockAzureClient();
var jobsMagic = new JobsMagic(azureClient);
jobsMagic.Test(string.Empty);
Assert.AreEqual(azureClient.LastAction, AzureClientAction.GetJobList);
Assert.AreEqual(AzureClientAction.GetJobList, azureClient.LastAction);
}

[TestMethod]
Expand All @@ -143,13 +170,18 @@ public void TestTargetMagic()
var azureClient = new MockAzureClient();
var targetMagic = new TargetMagic(azureClient);
targetMagic.Test(targetId);
Assert.AreEqual(azureClient.LastAction, AzureClientAction.SetActiveTarget);
Assert.AreEqual(AzureClientAction.SetActiveTarget, azureClient.LastAction);

// single argument with quotes - should set active target
targetMagic = new TargetMagic(azureClient);
targetMagic.Test($"\"{targetId}\"");
Assert.AreEqual(AzureClientAction.SetActiveTarget, azureClient.LastAction);

// no arguments - should print active target
azureClient = new MockAzureClient();
targetMagic = new TargetMagic(azureClient);
targetMagic.Test(string.Empty);
Assert.AreEqual(azureClient.LastAction, AzureClientAction.GetActiveTarget);
Assert.AreEqual(AzureClientAction.GetActiveTarget, azureClient.LastAction);
}
}

Expand All @@ -170,6 +202,9 @@ internal enum AzureClientAction
public class MockAzureClient : IAzureClient
{
internal AzureClientAction LastAction = AzureClientAction.None;
internal string SubscriptionId = string.Empty;
internal string ResourceGroupName = string.Empty;
internal string WorkspaceName = string.Empty;
internal string ConnectionString = string.Empty;
internal bool RefreshCredentials = false;
internal string ActiveTargetId = string.Empty;
Expand Down Expand Up @@ -205,6 +240,9 @@ public async Task<ExecutionResult> ExecuteJobAsync(IChannel channel, AzureSubmis
public async Task<ExecutionResult> ConnectAsync(IChannel channel, string subscriptionId, string resourceGroupName, string workspaceName, string storageAccountConnectionString, bool refreshCredentials)
{
LastAction = AzureClientAction.Connect;
SubscriptionId = subscriptionId;
ResourceGroupName = resourceGroupName;
WorkspaceName = workspaceName;
ConnectionString = storageAccountConnectionString;
RefreshCredentials = refreshCredentials;
return ExecuteStatus.Ok.ToExecutionResult();
Expand Down