Merge lp:~james-w/django-configglue/fix-argv into lp:django-configglue

[James Westby]

Stop 'value' being added to argv when '--unknown-option=value' is passed.

Hi,

As discussed this fixes a problem with Django's LaxOptionParser.

I added the test as you suggested and it fails before as expected, and succeeds
after.

I ended up writing my own init script that avoided the issues I was having, so
this isn't blocking our deployment, but it would still be good to have a solution.

Thanks,

James

[Ricardo Kirkner (ricardokirkner)]

A few remarks:

1. l.23-24: please leave only one blank line here
2. l.71: there is now explanation (nor test) of why this monkeypatch is needed
3. l.72-74: please remove the trailing blank lines

When done, please also add a commit message for tarmac to be able to merge this branch.

review: Needs Fixing

[James Westby (james-w)]

Hi,

I made those changes as requested (dropping the monkeypatch.)

Thanks,

James

[lp:~james-w/django-configglue/fix-argv]

70. By James Westby on 2012-01-13

Tweaks from review, thanks Ricardo.

[Ricardo Kirkner (ricardokirkner)]

LGTM. Thanks!

review: Approve

=== modified file 'django_configglue/management/__init__.py'
--- django_configglue/management/__init__.py	2011-07-27 00:14:34 +0000
+++ django_configglue/management/__init__.py	2012-01-13 15:14:23 +0000
@@ -5,17 +5,67 @@
 # except where third-party/django/LICENSE applies.
 
 
+from optparse import BadOptionError
 import sys
 
 import django
 import django.core.management
 from configglue.glue import schemaconfigglue
-from django.core.management import ManagementUtility, LaxOptionParser
+from django.core.management import ManagementUtility, LaxOptionParser as _LaxOptionParser
 from django.core.management.base import BaseCommand, handle_default_options
 from django.conf import settings
 from django_configglue import utils
 
 
+class LaxOptionParser(_LaxOptionParser):
+    # Subclasses django's to avoid a bug
+
+    def _process_long_opt(self, rargs, values):
+        arg = rargs.pop(0)
+
+        # Value explicitly attached to arg?  Pretend it's the next
+        # argument.
+        if "=" in arg:
+            (opt, next_arg) = arg.split("=", 1)
+            rargs.insert(0, next_arg)
+            had_explicit_value = True
+        else:
+            opt = arg
+            had_explicit_value = False
+
+        try:
+            opt = self._match_long_opt(opt)
+        except BadOptionError:
+            # Here's the addition in our subclass, we take care to take
+            # the value back out of rargs so that it isn't duplicated if
+            # this code path is hit.
+            if had_explicit_value:
+                rargs.pop(0)
+            raise
+        option = self._long_opt[opt]
+        if option.takes_value():
+            nargs = option.nargs
+            if len(rargs) < nargs:
+                if nargs == 1:
+                    self.error(_("%s option requires an argument") % opt)
+                else:
+                    self.error(_("%s option requires %d arguments")
+                               % (opt, nargs))
+            elif nargs == 1:
+                value = rargs.pop(0)
+            else:
+                value = tuple(rargs[0:nargs])
+                del rargs[0:nargs]
+
+        elif had_explicit_value:
+            self.error(_("%s option does not take a value") % opt)
+
+        else:
+            value = None
+
+        option.process(opt, value, values, self)
+
+
 class GlueManagementUtility(ManagementUtility):
     # This function was mostly taken from the django project.
     # Please see the license file in the third-party/django directory.

=== modified file 'django_configglue/tests/test_configglue.py'
--- django_configglue/tests/test_configglue.py	2011-09-16 23:11:02 +0000
+++ django_configglue/tests/test_configglue.py	2012-01-13 15:14:23 +0000
@@ -5,6 +5,7 @@
 import inspect
 import textwrap
 from cStringIO import StringIO
+from optparse import BadOptionError
 from unittest import TestCase
 
 import django
@@ -27,7 +28,7 @@
 from django.conf.project_template import settings as project_settings
 from mock import patch
 
-from django_configglue.management import GlueManagementUtility
+from django_configglue.management import GlueManagementUtility, LaxOptionParser
 from django_configglue.utils import (
     SETTINGS_ENCODING,
     configglue,
@@ -408,6 +409,15 @@
         self.assertTrue('Show settings attributes' in self.capture['stdout'])
 
 
+class LaxOptionParserTestCase(TestCase):
+
+    def test_explicit_value_for_unknown_option(self):
+        parser = LaxOptionParser()
+        rargs = ["--foo=bar"]
+        self.assertRaises(BadOptionError, parser._process_long_opt, rargs, [])
+        self.assertEqual([], rargs)
+
+
 class UpperCaseDictOptionTestCase(TestCase):
     def test_parse(self):
         class MySchema(Schema):