#6017: order of django-admin.py options matters, but shouldn't
--------------------------+-------------------------------------------------
Reporter: toddobryan | Owner: nobody
Status: new | Component: Core framework
Version: SVN | Resolution:
Keywords: | Stage: Accepted
Has_patch: 1 | Needs_docs: 0
Needs_tests: 1 | Needs_better_patch: 0
--------------------------+-------------------------------------------------
Changes (by russellm):
* needs_tests: 0 => 1
* stage: Unreviewed => Accepted
Old description:
> In ticket #5369, ``django-admin.py`` and ``manage.py`` are supposed to
> read all options, looking for ``--settings`` and ``--pythonpath``, and
> then use those two if they're present to access app-supplied commands.
> The code (which was mine) has a bug, in that options stop getting read as
> soon as something other than ``--settings`` or ``--pythonpath`` gets hit.
>
> Without the patch, you currently get: {{{
> >>> from django.core.management import LaxOptionParser, get_version
> >>> from django.core.management.base import BaseCommand
> >>> parser = LaxOptionParser(version=get_version(),
> option_list=BaseCommand.option_list)
> >>> options, args = parser.parse_args('django-admin.py command --option
> --settings=settingsmodule --pythonpath=/home/user/django-dir'.split())
> >>> print options
> {'pythonpath': None, 'settings': None}
> >>> args
> ['django-admin.py', 'command', '--settings=settingsmodule', '--
> pythonpath=/home/user/django-dir']
> }}}
> where both ``--pythonpath`` and ``--settings`` get ignored.
>
> With the included patch, you get:{{{
> >>> from django.core.management import LaxOptionParser, get_version
> >>> from django.core.management.base import BaseCommand
> >>> parser = LaxOptionParser(version=get_version(),
> option_list=BaseCommand.option_list)
> >>> options, args = parser.parse_args('django-admin.py command --option
> --settings=settingsmodule --pythonpath=/home/user/django-dir'.split())
> >>> print options
> {'pythonpath': '/home/user/django-dir', 'settings': 'settingsmodule'}
> >>> args
> ['django-admin.py', 'command', '--option']
> }}}
> as desired.
>
> If someone wants me to stick the doctest code somewhere, please tell me
> where. I think I may need to make a whole new folder, but I don't want to
> make a mess.
New description:
In ticket #5369, ``django-admin.py`` and ``manage.py`` are supposed to
read all options, looking for ``--settings`` and ``--pythonpath``, and
then use those two if they're present to access app-supplied commands. The
code (which was mine) has a bug, in that options stop getting read as soon
as something other than ``--settings`` or ``--pythonpath`` gets hit.
Without the patch, you currently get:
{{{
>>> from django.core.management import LaxOptionParser, get_version
>>> from django.core.management.base import BaseCommand
>>> parser = LaxOptionParser(version=get_version(),
option_list=BaseCommand.option_list)
>>> options, args = parser.parse_args('django-admin.py command --option
--settings=settingsmodule --pythonpath=/home/user/django-dir'.split())
>>> print options
{'pythonpath': None, 'settings': None}
>>> args
['django-admin.py', 'command', '--settings=settingsmodule', '--
pythonpath=/home/user/django-dir']
}}}
where both ``--pythonpath`` and ``--settings`` get ignored.
With the included patch, you get:
{{{
>>> from django.core.management import LaxOptionParser, get_version
>>> from django.core.management.base import BaseCommand
>>> parser = LaxOptionParser(version=get_version(),
option_list=BaseCommand.option_list)
>>> options, args = parser.parse_args('django-admin.py command --option
--settings=settingsmodule --pythonpath=/home/user/django-dir'.split())
>>> print options
{'pythonpath': '/home/user/django-dir', 'settings': 'settingsmodule'}
>>> args
['django-admin.py', 'command', '--option']
}}}
as desired.
If someone wants me to stick the doctest code somewhere, please tell me
where. I think I may need to make a whole new folder, but I don't want to
make a mess.
Comment:
The patch itself looks good; my quick poking didn't reveal any problems,
but that's just quick poking. Since you worked out a way to regression
test this bit, it should be a bit more comprehensive than one test. There
are several branch paths in the patch - these should all be tested (at the
very least, -X , --X and --X=Y option types should all be checked, in
various orders and combinations with --settings etc).
Also, a minor point of style - calling the test directory 'management' is
sufficient.
I've attached a slightly revised patch, with a couple more tests and
cleaned up comments; if you can bulk out the patch with some more tests,
I'll commit.
--
Ticket URL: <http://code.djangoproject.com/ticket/6017#comment:2>
Django Code <http://code.djangoproject.com/>
The web framework for perfectionists with deadlines
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/django-updates?hl=en
-~----------~----~----~----~------~----~------~--~---
