Closed Bug 1213373 Opened 9 years ago Closed 8 years ago

[research] better way to collapse whitespace in Jinja2 trans blocks

Categories

(Input :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: willkg, Assigned: willkg)

Details

We recently ditched tower. One of the things that tower did that we added code in fjord to do in order to ditch tower was collapse whitespace in strings. When strings appear in code, it's unlikely they'll have consecutive sequences of whitespace. When they do, we should consider that a bug and fix it. However, trans blocks in HTML templates often have consecutive sequences of whitespace because we indent blocks of text to make the files easier to read.

Jinja's gettext functions strip whitespace from the beginning and end of strings, but does not collapse whitespace within the string.

This bug covers looking at what trans blocks do. If they collapse whitespace on their own, then we can ditch all our whitespace collapsing code.
The blocktrans tag in django 1.7+ has a "trimmed" thing that collapses whitespace in the middle and strips whitespace at the beginning and end of the string.

https://docs.djangoproject.com/en/1.8/topics/i18n/translation/#blocktrans-template-tag

We're using "trans" in our templates, so I still need to figure out where that trans tag comes from (django-jinja? jinja? fjord code?) and see if it does something similar or not.
Grabbing this since I'm working on it.
Assignee: nobody → willkg
Status: NEW → ASSIGNED
I think the trans statement comes from Jinja. It says nothing about collapsing whitespace:

http://jinja.pocoo.org/docs/dev/templates/#i18n

Need to investigate further.
I'm pretty sure the answer is "no". Django has a blocktrans that has a "trimmed" option that collapses whitespace, but Jinja's trans block does not collapse whitespace. We're using Jinja's, so we don't get the advantage of this.

The thing we do in Fjord is kind of messy and I'd rather not do it since it's got a lot of pieces.

Do both msgid translation and msgid extraction work off of the parsed ast for jinja templates? If that's true, then maybe we could write our own trans block tag and switch to using that. Then maybe we could ditch all the other whitespace collapsing monkeypatching stuff we're doing.

Relatedly, is a good idea that we override trans to *always* collapse whitespace. I know there were situations when writing email templates in kitsune where whitespace is important and it was difficult because of this. Are there other situations where collapsing whitespace makes things difficult? Maybe we should add a "wrap" feature?

Other than writing a trans block parser, I'm not sure what else we could do here.
I'm going to push off any further work here until a rainy day.
I fiddled with tweaking _parse_block in a different way so that it selectively collapses whitespace if "trimmed" is in the variable list of the tag.

This works with "./manage.py extract" since it parses Jinja2 templates and extracts the msgid from the resulting AST.

This works with django-jinja's "./manage.py makemessages" since django-jinja tweaks the start/end regexes for blocktrans so that it includes Jinja2's trans tags which then makes the template look enough like a Django template that Django's makemessages can extract the strings.

I wrote up a bug in the Jinja2 tracker to add "trimmed" or something like it to the trans tag:

https://github.com/mitsuhiko/jinja2/issues/504

If we had that, we could ditch the code we have for collapsing whitespace in trans blocks. However, we still have the problem of needing extract since makemessages works "by accident". Maybe it makes sense to push django-jinja to write their own makemessages rather than doing what they're doing now.
Summary: [research] does trans blocks collapse whitespace? → [research] better way to collapse whitespace in Jinja2 trans blocks
I wrote a new library to deal with all of this. We'll deal with these issues in the new library and then migrate off after all the things are fixed. That work is in https://github.com/mozilla/puente

Going to mark it WONTFIX here.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.