Skip to content

remove basecommand.merge_options() #904

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

Closed
wants to merge 1 commit into from
Closed

Conversation

gvalkov
Copy link
Contributor

@gvalkov gvalkov commented Apr 19, 2013

Fixes issue #70.

This patch removes the error prone step of merging the baseparsers'
options (the 'Generic Options') with those of the subparser. In fact,
commands are already capable of parsing all options present in the
'Generic Options' option group. The option parsing logic is as follows:

Given a command line such as:
  '--timeout 5 install --user pkg'
pip.parseopt() returns command 'install' with arguments:
  '--timeout 5 --user pkg'
pip.main() executes the above as:
  commands['install'](parser).main('--timeout 5 --user pkg', options)
@hltbra
Copy link
Contributor

hltbra commented Apr 19, 2013

Fixes #350 also.

👍

@ghost ghost assigned hltbra Apr 19, 2013
options, cmd_args = parser.parse_args(args)
# args: ['--timeout=5', 'install', '--user', 'INITools']
# cmd_args: ['install', '--user', 'INITools']
# note: parser calls 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.

Are these comments needed?

Copy link
Contributor

Choose a reason for hiding this comment

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

It was good for me, because in the bottom part there is:

args.remove(cmd_args[0])
cmd_args = args  # --timeout=5 --user INITools

That is a complement to the first comments.

@hltbra
Copy link
Contributor

hltbra commented May 1, 2013

I suggest merging #918 and #919 before this pull request, because those contain tests that are useful to this pull request. It may need rebasing, btw.

@qwcode
Copy link
Contributor

qwcode commented Sep 17, 2013

included in PR #1202

@qwcode qwcode closed this Sep 17, 2013
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 5, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation type: refactor Refactoring code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants