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

Also fail on broken -x 123, should be -x123 for now #4061

Merged
merged 1 commit into from
Jul 10, 2015

Conversation

benjaoming
Copy link
Contributor

Fix for #4058

@MCGallaspy
Copy link
Contributor

I want to settle this option parsing stuff once and for all, so I made a big ol' regex that should match any valid long or short form argument. Here it is:

(-[0-9a-zA-Z]\s?(?P<sh_value>\w+|'[^']*')|--[\w-]+[\s=]{1}(?P<lg_value>\w+|'[^']*'))

And at pythex.org, with examples. Disregard the initial ^ there, that's so it works in multiline mode.

What this matches in plain english:

First, it either tries to match short-form or long-form arguments.

A short-form argument is matched if there is exactly 1 - followed by 1 alphanumeric character (\w). Then there is exactly one optional whitespace (in particular, = is not a valid character). Then it can match a value, as described below.

A long-form argument is matched if there are exactly two dashes (--) followed by 1 or more alphanumeric characters, a dash (-), or an underscore (_). This is followed by exactly 1 of a whitespace character or =, and then a value as described below.

Values are matched the same way for long- or short-form arguments. They can either be 1 or more alphanumeric or _ (\w+) or anything inside of two ' (aside from ' itself), including nothing. The ' are also matched, so if you don't want 'em then you gotta strip 'em. This allows us to pass whitespace characters within arguments. This also shouldn't break any existing uses, as it continues to match them.

Do with this as you will!

@MCGallaspy
Copy link
Contributor

Okay, after further discussion it's not a simple matter of dropping in a such a regex... Seems like the best course of action is to merge this in for now. However, we also decided it was better to live with the "lesser evil" of having options out of order than having to maintain a separate version of docopt. It's too much to revert it for 0.14, but let's put a moratorium on further changes and revert it for 0.15.

MCGallaspy added a commit that referenced this pull request Jul 10, 2015
Also fail on broken -x 123, should be -x123 for now
@MCGallaspy MCGallaspy merged commit 5409cc5 into learningequality:0.14.x Jul 10, 2015
@MCGallaspy MCGallaspy removed the has PR label Jul 10, 2015
@benjaoming
Copy link
Contributor Author

@MCGallaspy #4039

@benjaoming
Copy link
Contributor Author

I've opened an upstream PR for docopt to fix some of this.

@benjaoming benjaoming deleted the option-failure-fix branch July 14, 2015 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants