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

Quick fix for #101 #102

Merged
merged 4 commits into from
Sep 15, 2015
Merged

Quick fix for #101 #102

merged 4 commits into from
Sep 15, 2015

Conversation

aaaaalbert
Copy link
Collaborator

Make repy.py stop interpreting command-line arguments once it sees
the first non-option (which is usually the name of the program to run
in the sandbox).

Make `repy.py` stop interpreting command-line arguments once it sees
the first non-option (which is usually the name of the program to run
in the sandbox).
@aaaaalbert
Copy link
Collaborator Author

Note that writing unit tests for this is currently held up by SeattleTestbed/utf#60 (I think).

Check that the Repy sandbox parses command-line arguments correctly.
@aaaaalbert
Copy link
Collaborator Author

The unit test I created takes a different approach than originally planned, and now launches the test sandbox from within the ut file using Popen. (I thought I would use #pragma repy directives, thus my referring to the UTF issue previously).

@@ -331,6 +331,7 @@ def main():
### anywhere if this is repurposed...
usage = "USAGE: repy.py [options] resource_file program_to_run.r2py [program args]"
parser = optparse.OptionParser(usage=usage)
parser.disable_interspersed_args()
Copy link
Contributor

Choose a reason for hiding this comment

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

You may even add a comment explaining the importance of restoring traditional Unix syntax and what can go wrong if command-line options are consumed by repy.py. Not adding a comment here seems like an easy way for a future developer to miss the importance of this behaviour.

Example comment:
disable_interspersed_args() restores traditional Unix syntax, where option parsing stops with the first non-option argument. disable_interspersed_args() should be used if you have a command processor which runs another command which has options of its own and you want to make sure these options don’t get confused. For example, each command might have a different set of options.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Great point, thanks. Addressed in 6c20115.

@vladimir-v-diaz
Copy link
Contributor

I reviewed the follow-up commits and verified that ut_repv2api_commandlineargs.py passes.
There are two review comments of mine that were not covered (example: #102 (comment)).. if they will not be addressed, I can go ahead and merge this PR.

$ python utf.py -f ut_repyv2api_commandlineargs.py
Testing module: repyv2api
Running: ut_repyv2api_commandlineargs.py [ PASS ]

@vladimir-v-diaz
Copy link
Contributor

LGTM. Merging.

vladimir-v-diaz added a commit that referenced this pull request Sep 15, 2015
@vladimir-v-diaz vladimir-v-diaz merged commit 5686420 into master Sep 15, 2015
@aaaaalbert aaaaalbert deleted the fix-greedy-optparse-#101 branch September 15, 2015 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants