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

Conversation

@rmshaffer
Copy link
Contributor

This PR improves the magic command argument parsing code in the AbstractMagic base class by using regular expression matching rather than manual string parsing.

Previously, arguments were supported only as key=value. This had two problems:

  1. It was inconsistent with %config magic, which supports key = value (with whitespace around the equals sign).
  2. It did not allow a way to specify value as a string containing whitespace. This is desirable, for example, when specifying a job name for an Azure Quantum submission.

With these changes, arguments such as key = value and key = "value with whitespace" are supported, where whitespace is optionally allowed on both sides of the equals sign. We also support key and "value only" arguments to magic commands that expect a single-token argument.

Parsing behavior for arguments provided as JSON (i.e., when called from Python) is unaffected by these changes, and there are a number of existing tests which validate that code path.


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

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

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. 😄

@rmshaffer rmshaffer merged commit 8771325 into feature/azure-client Jun 15, 2020
@rmshaffer rmshaffer deleted the rmshaffer/magic-parsing branch June 15, 2020 22:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants