Repository navigation
Improved javascript template string expression extracting - #792
gitaarik wants to merge 1 commit into
Conversation
Sorry, something went wrong.
|
Hi @akx . I've been working on this feature for a while. In my setup I need these changes. However I have some problems when using my own fork in my project. So I have some questions regarding using a fork of babel in my project, and also what needs to be done to get this merged, so I don't have to use my fork anymore. In my project, which uses Pipenv. Inside the Pipfile, I use a VCS descriptor to specify my python-babel fork: babel = {editable = true, ref = "javascript-improvements-1", git = "https://github.com/gitaarik/babel"}
This works partly. When I run python-babel's babel.messages.extract.extract_from_dir, it finds gettext() functions that it didn't found before. However, when I run my Django devserver, on some requests (not all of them, I'm not sure yet why it only happens on some requests), the request fails with a exception coming from python-babel: RuntimeError: The babel data files are not available. This usually happens because you are using a source checkout from Babel and you did not build the data files. Just make sure to run "python setup.py import_cldr" before installing the library. I noticed that when I go into the virtualenv directory (which is created by Pipenv and can be retrieved with pipenv --venv) and then from there go to src/babel/, I can run the command python setup.py import_cld to generate the locale files and then it seems to work. I will have to look into the possibilities to integrate this in my project in a way that the locale files would be generated when I build and deploy my project. In the meantime it would be good for us if the changes would eventually be merged into the official package. So I have two questions:
|
Sorry, something went wrong.
|
Hi @gitaarik, sorry for the latency. I had somehow inadvertently turned all notifications about Babel off (or maybe had just dropped the ball on this one). Could you rebase this on master and maybe add a test that shows how this works? |
Sorry, something went wrong.
akx
left a comment
There was a problem hiding this comment.
Beyond the other comments, I'd appreciate some tests :)
Sorry, something went wrong.
|
|
||
| fake_file_obj = io.BytesIO(expression_contents.encode()) | ||
|
|
||
| for item in extract_javascript(fake_file_obj, keywords, comment_tags, options): |
There was a problem hiding this comment.
How does this interact with e.g. line numbering?
Sorry, something went wrong.
There was a problem hiding this comment.
line numbering has been fixed in #939. There is now also a test for that.
Sorry, something went wrong.
| def parse_template_string(template_string, fileobj, keywords, comment_tags, options): | ||
|
|
||
| prev_character = None | ||
| level = 0 | ||
| inside_str = False | ||
| expression_contents = '' | ||
|
|
||
| for character in template_string[1:-1]: | ||
|
|
||
| if not inside_str and character in ('"', "'", '`'): | ||
| inside_str = character | ||
| elif inside_str == character and prev_character != r'\\': | ||
| inside_str = False | ||
|
|
||
| if level: | ||
| expression_contents += character | ||
|
|
||
| if not inside_str: | ||
|
|
||
| if character == '{' and prev_character == '$': | ||
| level += 1 | ||
|
|
||
| elif level and character == '}': | ||
|
|
||
| level -= 1 | ||
|
|
||
| if level == 0 and expression_contents: | ||
|
|
||
| expression_contents = expression_contents[0:-1] | ||
|
|
||
| fake_file_obj = io.BytesIO(expression_contents.encode()) | ||
|
|
||
| for item in extract_javascript(fake_file_obj, keywords, comment_tags, options): | ||
| yield item | ||
|
|
||
| expression_contents = '' | ||
|
|
||
| prev_character = character |
There was a problem hiding this comment.
Could you remove the empty lines within the blocks, so the spacing of this code (which is quite airy 😄 ) matches the rest of the codebase?
Sorry, something went wrong.
There was a problem hiding this comment.
Done in #939.
Sorry, something went wrong.
Tests have been added in #939. |
Sorry, something went wrong.
Add option parse_template_string that improves parsing of javascript templates expressions, like:
`${gettext('my text')}`And it also works with nested expressions like:
${`some text ${gettext('my text')} more text`}You have to enable template string extraction with an option in the option_map:
extract_from_dir( # ... options_map={ '**': {'parse_template_string': True} } )Then it should work.