Extract catalog compilation into a compile_message_catalog() function - #1327
ChrisJr404 wants to merge 1 commit into
Conversation
akx
left a comment
akx
left a comment
There was a problem hiding this comment.
Some initial change requests within.
| The same operation is available as a function, so applications can compile | ||
| catalogs from their own code without shelling out to the command line: | ||
|
|
||
| .. autofunction:: babel.messages.frontend.compile_message_catalog |
There was a problem hiding this comment.
Doesn't sound like a great import path, does it? I'd move this one module up.
| This performs the same work as the ``pybabel compile`` command, exposed so | ||
| that applications can compile catalogs without shelling out to the command | ||
| line. Either point it at a base ``directory`` (optionally narrowed to a | ||
| single ``locale``), or pass an explicit ``input_file`` and ``output_file``. | ||
|
|
There was a problem hiding this comment.
I'm not sure this needs to refer to pybabel compile as such?
| :param use_fuzzy: also compile fuzzy translations | ||
| :param statistics: log translation statistics for each catalog | ||
| :param log: the logger to report progress to, defaulting to Babel's own | ||
| :return: the number of catalogs that had errors |
There was a problem hiding this comment.
For programmatic use, this sounds rather useless. I'd rather like it to return a namedtuple, result dataclass, typeddict, something like that, with more detail than just an integer.
statistics should also go in there instead of get logged implicitly here.
The original code already returned a catalogs_and_errors; this sounds worse already...
| directory: str | os.PathLike[str] | None = None, | ||
| locale: str | Locale | None = None, | ||
| domain: str | Iterable[str] = "messages", | ||
| input_file: str | os.PathLike[str] | None = None, | ||
| output_file: str | os.PathLike[str] | None = None, | ||
| use_fuzzy: bool = False, | ||
| statistics: bool = False, | ||
| log: logging.Logger | None = None, |
There was a problem hiding this comment.
If you're introducing a programmatic API like this, most of these should probably be Explicit Is Better Than Implicit, so no defaults. The flags and logger should also preferably be kwarg-only.
Also, we may call domain domains here and not accept a single string, that way the code below doesn't need to listify_value().
| directory, locale, current_domain, input_file, output_file, | ||
| )) | ||
| if not triples: | ||
| raise OptionError(f'no message catalogs found for domain {current_domain!r}') |
There was a problem hiding this comment.
This sounds like a FileNotFoundError that the command class can translate to an OptionError.
2 participants
Closes #760.
This pulls the compile logic out of the
CompileCatalogcommand into a plaincompile_message_catalog()function, so applications can compile catalogs from their own code without shelling out to the CLI (that was the use case in the betty PR linked from the issue). The command just calls the function now, so the command line behavior is unchanged.I kept the signature close to the compile options (directory/locale/domain, or an explicit input and output file), added tests covering the file and directory modes plus the error counting, and dropped a short note under the compile docs.