From 414aec5864053a28f873d098412017be24ce0626 Mon Sep 17 00:00:00 2001 From: Aarni Koskela Date: Thu, 14 Apr 2016 11:23:01 +0300 Subject: [PATCH] Harmonize extraction keyword parsing between distutils and standalone CLI Fixes #388 Refs #384 Refs #311 --- babel/messages/frontend.py | 65 +++++++++++++++++++++++++++------ tests/messages/test_frontend.py | 43 ++++++++++++++++++++++ 2 files changed, 97 insertions(+), 11 deletions(-) diff --git a/babel/messages/frontend.py b/babel/messages/frontend.py index 62e7c03e..b975ff38 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,15 @@ 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 = () + #: Log object. To allow replacement in the script command line runner. log = distutils_log @@ -110,6 +159,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 +168,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): @@ -299,8 +347,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 @@ -338,11 +385,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: diff --git a/tests/messages/test_frontend.py b/tests/messages/test_frontend.py index 049fc299..118d3f29 100644 --- a/tests/messages/test_frontend.py +++ b/tests/messages/test_frontend.py @@ -1245,6 +1245,27 @@ def configure_cli_command(cmdline): 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): # This is a regression test for https://github.com/python-babel/babel/issues/384 @@ -1293,6 +1314,28 @@ def test_extract_keyword_args_384(split): )) +@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) -- 2.47.2