Repository: python-babel/babel
Pull request #394 (2016-04-22 06:27:17)

2.3.4

sils1297: which are cherry picked and already reviewed?
akx: @sils1297 All changes here are already in master.
akx: @sils1297 : Namely,

* changes in c4056d2 are from fbc1648 and 414aec5..ee8abd6 (CR in#386 and #389 respectively)
* f8ada14 is cherry-pick of 670a84e (CR in #393)
akx: Bah, it seems I fudged up the cherry-pick somehow. Just a moment...
codecov-io: ## [Current coverage][1] is `90.07%`
> Merging **#394** into **release/2.3** will increase coverage by **+0.06%** as of [`46585f5`][3]


```diff
@@            release/2.3    #394   diff @@
===========================================
  Files                24      24       
  Stmts              3935    3950    +15
  Branches              0       0       
  Methods               0       0       
===========================================
+ Hit                3542    3558    +16
  Partial               0       0       
+ Missed              393     392     -1
```

> Review entire [Coverage Diff][4] as of [`46585f5`][3]


[1]: https://codecov.io/github/python-babel/babel?ref=46585f5a0479fb44c75e909f83523de90f76743f
[2]: https://codecov.io/github/python-babel/babel/features/suggestions?ref=46585f5a0479fb44c75e909f83523de90f76743f
[3]: https://codecov.io/github/python-babel/babel/commit/46585f5a0479fb44c75e909f83523de90f76743f
[4]: https://codecov.io/github/python-babel/babel/compare/3086b0c016d02c128832d07bc445f088bee4a4c6...46585f5a0479fb44c75e909f83523de90f76743f

> Powered by [Codecov](https://codecov.io). Updated on successful CI builds.
sils1297: why does the commit message of https://github.com/python-babel/babel/pull/394/commits/f61910287e7f841675d0473596003d834dc1b165 have no reason? Can you please never merge a revert without a reason?
akx: @sils1297 Sure thing, will mandate refs in revert commit messages in the future. (GitMate rule idea? :) )

For that particular bug, see #391 anyway.

Patch:
diff --git a/CHANGES b/CHANGES
index 396710b..d4c0132 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,17 @@
 Babel Changelog
 ===============
 
+Version 2.3.4
+-------------
+
+(Bugfix release, released on April 22th)
+
+Bugfixes
+~~~~~~~~
+
+* CLDR: The lxml library is no longer used for CLDR importing, so it should not cause strange failures either. Thanks to @aronbierbaum for the bug report and @jtwang for the fix. (https://github.com/python-babel/babel/pull/393)
+* CLI: Every last single CLI usage regression should now be gone, and both distutils and stand-alone CLIs should work as they have in the past. Thanks to @paxswill and @ajaeger for bug reports. (https://github.com/python-babel/babel/pull/389)
+
 Version 2.3.3
 -------------
 
diff --git a/babel/__init__.py b/babel/__init__.py
index 7f810c7..ecc4059 100644
--- a/babel/__init__.py
+++ b/babel/__init__.py
@@ -21,4 +21,4 @@ from babel.core import UnknownLocaleError, Locale, default_locale, \
     negotiate_locale, parse_locale, get_locale_identifier
 
 
-__version__ = '2.3.3'
+__version__ = '2.3.4'
diff --git a/babel/messages/frontend.py b/babel/messages/frontend.py
index 62e7c03..d190a2c 100644
--- a/babel/messages/frontend.py
+++ b/babel/messages/frontend.py
@@ -22,7 +22,7 @@ from locale import getpreferredencoding
 
 from babel import __version__ as VERSION
 from babel import Locale, localedata
-from babel._compat import StringIO, string_types
+from babel._compat import StringIO, string_types, text_type
 from babel.core import UnknownLocaleError
 from babel.messages.catalog import Catalog
 from babel.messages.extract import DEFAULT_KEYWORDS, DEFAULT_MAPPING, check_and_call_extract_file, extract_from_dir
@@ -39,6 +39,48 @@ except ImportError:
     from configparser import RawConfigParser
 
 
+def listify_value(arg, split=None):
+    """
+    Make a list out of an argument.
+
+    Values from `distutils` argument parsing are always single strings;
+    values from `optparse` parsing may be lists of strings that may need
+    to be further split.
+
+    No matter the input, this function returns a flat list of whitespace-trimmed
+    strings, with `None` values filtered out.
+
+    >>> listify_value("foo bar")
+    ['foo', 'bar']
+    >>> listify_value(["foo bar"])
+    ['foo', 'bar']
+    >>> listify_value([["foo"], "bar"])
+    ['foo', 'bar']
+    >>> listify_value([["foo"], ["bar", None, "foo"]])
+    ['foo', 'bar', 'foo']
+    >>> listify_value("foo, bar, quux", ",")
+    ['foo', 'bar', 'quux']
+
+    :param arg: A string or a list of strings
+    :param split: The argument to pass to `str.split()`.
+    :return:
+    """
+    out = []
+
+    if not isinstance(arg, (list, tuple)):
+        arg = [arg]
+
+    for val in arg:
+        if val is None:
+            continue
+        if isinstance(val, (list, tuple)):
+            out.extend(listify_value(val, split=split))
+            continue
+        out.extend(s.strip() for s in text_type(val).split(split))
+    assert all(isinstance(val, string_types) for val in out)
+    return out
+
+
 class Command(_Command):
     # This class is a small shim between Distutils commands and
     # optparse option parsing in the frontend command line.
@@ -47,8 +89,21 @@ class Command(_Command):
     as_args = None
 
     #: Options which allow multiple values.
+    #: This is used by the `optparse` transmogrification code.
     multiple_value_options = ()
 
+    #: Options which are booleans.
+    #: This is used by the `optparse` transmogrification code.
+    # (This is actually used by distutils code too, but is never
+    # declared in the base class.)
+    boolean_options = ()
+
+    #: Option aliases, to retain standalone command compatibility.
+    #: Distutils does not support option aliases, but optparse does.
+    #: This maps the distutils argument name to an iterable of aliases
+    #: that are usable with optparse.
+    option_aliases = {}
+
     #: Log object. To allow replacement in the script command line runner.
     log = distutils_log
 
@@ -110,6 +165,7 @@ class compile_catalog(Command):
         self.statistics = False
 
     def finalize_options(self):
+        self.domain = listify_value(self.domain)
         if not self.input_file and not self.directory:
             raise DistutilsOptionError('you must specify either the input file '
                                        'or the base directory')
@@ -118,9 +174,7 @@ class compile_catalog(Command):
                                        'or the base directory')
 
     def run(self):
-        domains = self.domain.split()
-
-        for domain in domains:
+        for domain in self.domain:
             self._run_domain(domain)
 
     def _run_domain(self, domain):
@@ -174,12 +228,12 @@ class compile_catalog(Command):
                 if len(catalog):
                     percentage = translated * 100 // len(catalog)
                 self.log.info(
-                    '%d of %d messages (%d%%) translated in %r',
+                    '%d of %d messages (%d%%) translated in %s',
                     translated, len(catalog), percentage, po_file
                 )
 
             if catalog.fuzzy and not self.use_fuzzy:
-                self.log.info('catalog %r is marked as fuzzy, skipping', po_file)
+                self.log.info('catalog %s is marked as fuzzy, skipping', po_file)
                 continue
 
             for message, errors in catalog.check():
@@ -188,7 +242,7 @@ class compile_catalog(Command):
                         'error: %s:%d: %s', po_file, message.lineno, error
                     )
 
-            self.log.info('compiling catalog %r to %r', po_file, mo_file)
+            self.log.info('compiling catalog %s to %s', po_file, mo_file)
 
             outfile = open(mo_file, 'wb')
             try:
@@ -249,7 +303,7 @@ class extract_messages(Command):
         ('add-comments=', 'c',
          'place comment block with TAG (or those preceding keyword lines) in '
          'output file. Separate multiple TAGs with commas(,)'),  # TODO: Support repetition of this argument
-        ('strip-comments', None,
+        ('strip-comments', 's',
          'strip the comment TAGs from the comments.'),
         ('input-paths=', None,
          'files or directories that should be scanned for messages. Separate multiple '
@@ -263,6 +317,12 @@ class extract_messages(Command):
     ]
     as_args = 'input-paths'
     multiple_value_options = ('add-comments', 'keywords')
+    option_aliases = {
+        'keywords': ('--keyword',),
+        'mapping-file': ('--mapping',),
+        'output-file': ('--output',),
+        'strip-comments': ('--strip-comment-tags',),
+    }
 
     def initialize_options(self):
         self.charset = 'utf-8'
@@ -299,8 +359,7 @@ class extract_messages(Command):
         else:
             keywords = DEFAULT_KEYWORDS.copy()
 
-        for kwarg in (self.keywords or ()):
-            keywords.update(parse_keywords(kwarg.split()))
+        keywords.update(parse_keywords(listify_value(self.keywords)))
 
         self.keywords = keywords
 
@@ -325,11 +384,13 @@ class extract_messages(Command):
         if self.input_paths:
             if isinstance(self.input_paths, string_types):
                 self.input_paths = re.split(',\s*', self.input_paths)
-        else:
+        elif self.distribution is not None:
             self.input_paths = dict.fromkeys([
                 k.split('.', 1)[0]
                 for k in (self.distribution.packages or ())
             ]).keys()
+        else:
+            self.input_paths = []
 
         if not self.input_paths:
             raise DistutilsOptionError("no input files or directories specified")
@@ -338,11 +399,7 @@ class extract_messages(Command):
             if not os.path.exists(path):
                 raise DistutilsOptionError("Input path: %s does not exist" % path)
 
-        if self.add_comments:
-            if isinstance(self.add_comments, string_types):
-                self.add_comments = self.add_comments.split(',')
-        else:
-            self.add_comments = []
+        self.add_comments = listify_value(self.add_comments or (), ",")
 
         if self.distribution:
             if not self.project:
@@ -531,7 +588,7 @@ class init_catalog(Command):
 
     def run(self):
         self.log.info(
-            'creating catalog %r based on %r', self.output_file, self.input_file
+            'creating catalog %s based on %s', self.output_file, self.input_file
         )
 
         infile = open(self.input_file, 'rb')
@@ -662,7 +719,7 @@ class update_catalog(Command):
             raise DistutilsOptionError('no message catalogs found')
 
         for locale, filename in po_files:
-            self.log.info('updating catalog %r based on %r', filename, self.input_file)
+            self.log.info('updating catalog %s based on %s', filename, self.input_file)
             infile = open(filename, 'rb')
             try:
                 catalog = read_po(infile, locale=locale, domain=domain)
@@ -825,6 +882,7 @@ class CommandLineInterface(object):
             strs = ["--%s" % name]
             if short:
                 strs.append("-%s" % short)
+            strs.extend(cmdclass.option_aliases.get(name, ()))
             if name == as_args:
                 parser.usage += "<%s>" % name
             elif name in cmdclass.boolean_options:
diff --git a/docs/conf.py b/docs/conf.py
index fc0530f..b27a04c 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -53,7 +53,7 @@ copyright = u'2016, The Babel Team'
 # The short X.Y version.
 version = '2.3'
 # The full version, including alpha/beta/rc tags.
-release = '2.3.3'
+release = '2.3.4'
 
 # The language for content autogenerated by Sphinx. Refer to documentation
 # for a list of supported languages.
diff --git a/scripts/import_cldr.py b/scripts/import_cldr.py
index f684de3..8de7043 100755
--- a/scripts/import_cldr.py
+++ b/scripts/import_cldr.py
@@ -18,12 +18,9 @@ import re
 import sys
 
 try:
-    import lxml.etree as ElementTree
+    from xml.etree import cElementTree as ElementTree
 except ImportError:
-    try:
-        from xml.etree import cElementTree as ElementTree
-    except ImportError:
-        from xml.etree import ElementTree
+    from xml.etree import ElementTree
 
 # Make sure we're using Babel source, and not some previously installed version
 CHECKOUT_ROOT = os.path.abspath(os.path.join(


Test patch:
diff --git a/tests/messages/test_frontend.py b/tests/messages/test_frontend.py
index 4c3c4a5..7488bcc 100644
--- a/tests/messages/test_frontend.py
+++ b/tests/messages/test_frontend.py
@@ -27,7 +27,7 @@ import pytest
 from babel import __version__ as VERSION
 from babel.dates import format_datetime
 from babel.messages import frontend, Catalog
-from babel.messages.frontend import CommandLineInterface, extract_messages
+from babel.messages.frontend import CommandLineInterface, extract_messages, update_catalog
 from babel.util import LOCALTZ
 from babel.messages.pofile import read_po, write_po
 from babel._compat import StringIO
@@ -1092,7 +1092,7 @@ msgstr[2] ""
                                  '-d', self._i18n_dir()])
         assert not os.path.isfile(mo_file), 'Expected no file at %r' % mo_file
         self.assertEqual("""\
-catalog %r is marked as fuzzy, skipping
+catalog %s is marked as fuzzy, skipping
 """ % (po_file), sys.stderr.getvalue())
 
     def test_compile_fuzzy_catalog(self):
@@ -1104,7 +1104,7 @@ catalog %r is marked as fuzzy, skipping
                                      '-d', self._i18n_dir()])
             assert os.path.isfile(mo_file)
             self.assertEqual("""\
-compiling catalog %r to %r
+compiling catalog %s to %s
 """ % (po_file, mo_file), sys.stderr.getvalue())
         finally:
             if os.path.isfile(mo_file):
@@ -1123,7 +1123,7 @@ compiling catalog %r to %r
                                      '-d', self._i18n_dir()])
             assert os.path.isfile(mo_file)
             self.assertEqual("""\
-compiling catalog %r to %r
+compiling catalog %s to %s
 """ % (po_file, mo_file), sys.stderr.getvalue())
         finally:
             if os.path.isfile(mo_file):
@@ -1143,8 +1143,8 @@ compiling catalog %r to %r
             for mo_file in [mo_foo, mo_bar]:
                 assert os.path.isfile(mo_file)
             self.assertEqual("""\
-compiling catalog %r to %r
-compiling catalog %r to %r
+compiling catalog %s to %s
+compiling catalog %s to %s
 """ % (po_foo, mo_foo, po_bar, mo_bar), sys.stderr.getvalue())
 
         finally:
@@ -1232,9 +1232,47 @@ def test_parse_keywords():
     }
 
 
+def configure_cli_command(cmdline):
+    """
+    Helper to configure a command class, but not run it just yet.
+
+    :param cmdline: The command line (sans the executable name)
+    :return: Command instance
+    """
+    args = shlex.split(cmdline)
+    cli = CommandLineInterface()
+    cmdinst = cli._configure_command(cmdname=args[0], argv=args[1:])
+    return cmdinst
+
+
+def configure_distutils_command(cmdline):
+    """
+    Helper to configure a command class, but not run it just yet.
+
+    This will have strange side effects if you pass in things
+    `distutils` deals with internally.
+
+    :param cmdline: The command line (sans the executable name)
+    :return: Command instance
+    """
+    d = Distribution(attrs={
+        "cmdclass": vars(frontend),
+        "script_args": shlex.split(cmdline),
+    })
+    d.parse_command_line()
+    assert len(d.commands) == 1
+    cmdinst = d.get_command_obj(d.commands[0])
+    cmdinst.ensure_finalized()
+    return cmdinst
+
+
 @pytest.mark.parametrize("split", (False, True))
-def test_extract_keyword_args_384(split):
+@pytest.mark.parametrize("arg_name", ("-k", "--keyword", "--keywords"))
+def test_extract_keyword_args_384(split, arg_name):
     # This is a regression test for https://github.com/python-babel/babel/issues/384
+    # and it also tests that the rest of the forgotten aliases/shorthands implied by
+    # https://github.com/python-babel/babel/issues/390 are re-remembered (or rather
+    # that the mechanism for remembering them again works).
 
     kwarg_specs = [
         "gettext_noop",
@@ -1248,17 +1286,15 @@ def test_extract_keyword_args_384(split):
     ]
 
     if split:  # Generate a command line with multiple -ks
-        kwarg_text = " ".join("-k %s" % kwarg_spec for kwarg_spec in kwarg_specs)
+        kwarg_text = " ".join("%s %s" % (arg_name, kwarg_spec) for kwarg_spec in kwarg_specs)
     else:  # Generate a single space-separated -k
-        kwarg_text = "-k \"%s\"" % " ".join(kwarg_specs)
+        kwarg_text = "%s \"%s\"" % (arg_name, " ".join(kwarg_specs))
 
     # (Both of those invocation styles should be equivalent, so there is no parametrization from here on out)
 
-    cmdline = "extract -F babel-django.cfg --add-comments Translators: -o django232.pot %s ." % kwarg_text
-
-    args = shlex.split(cmdline)
-    cli = CommandLineInterface()
-    cmdinst = cli._configure_command(cmdname=args[0], argv=args[1:])
+    cmdinst = configure_cli_command(
+        "extract -F babel-django.cfg --add-comments Translators: -o django232.pot %s ." % kwarg_text
+    )
     assert isinstance(cmdinst, extract_messages)
     assert set(cmdinst.keywords.keys()) == set((
         '_',
@@ -1280,3 +1316,41 @@ def test_extract_keyword_args_384(split):
         'ungettext',
         'ungettext_lazy',
     ))
+
+
+@pytest.mark.parametrize("kwarg,expected", [
+    ("LW_", ("LW_",)),
+    ("LW_ QQ Q", ("LW_", "QQ", "Q")),
+    ("yiy         aia", ("yiy", "aia")),
+])
+def test_extract_distutils_keyword_arg_388(kwarg, expected):
+    # This is a regression test for https://github.com/python-babel/babel/issues/388
+
+    # Note that distutils-based commands only support a single repetition of the same argument;
+    # hence `--keyword ignored` will actually never end up in the output.
+
+    cmdinst = configure_distutils_command(
+        "extract_messages --no-default-keywords --keyword ignored --keyword '%s' "
+        "--input-dirs . --output-file django233.pot --add-comments Bar,Foo" % kwarg
+    )
+    assert isinstance(cmdinst, extract_messages)
+    assert set(cmdinst.keywords.keys()) == set(expected)
+
+    # Test the comma-separated comment argument while we're at it:
+    assert set(cmdinst.add_comments) == set(("Bar", "Foo"))
+
+
+def test_update_catalog_boolean_args():
+    cmdinst = configure_cli_command("update --no-wrap -N --ignore-obsolete --previous -i foo -o foo -l en")
+    assert isinstance(cmdinst, update_catalog)
+    assert cmdinst.no_wrap is True
+    assert cmdinst.no_fuzzy_matching is True
+    assert cmdinst.ignore_obsolete is True
+    assert cmdinst.previous is False  # Mutually exclusive with no_fuzzy_matching
+
+
+def test_extract_cli_knows_dash_s():
+    # This is a regression test for https://github.com/python-babel/babel/issues/390
+    cmdinst = configure_cli_command("extract -s -o foo babel")
+    assert isinstance(cmdinst, extract_messages)
+    assert cmdinst.strip_comments