Code

Opened 7 years ago

Closed 8 months ago

#5849 closed New feature (fixed)

Strip indentation and leading/trailing spaces/linebreaks from contents of blocktrans block.

Reported by: Dmitri Fedortchenko <zeraien@…> Owned by: bouke
Component: Internationalization Version: master
Severity: Normal Keywords: blocktrans strip indentation
Cc: chris@…, phartig@…, macek@…, dbrgn, bbr@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The idea is to strip whitespace indentation from the contents of the blocktrans tag, as well as leading and trailing whitespace and linebreaks.

Blocktrans tags allow you to split your translation strings onto multiple lines and this is a good feature, especially if using the "plural" mode.

Picture this:

{% blocktrans number as count %}
There is {{count}} message
waiting for you
{% plural %}
There are {{count}} messages
waiting for you
{% endblocktrans %}

The biggest issue is if you indent the above block. Suddenly those msgids are obsolete and you need to edit all your language files....

Indentation is often changed in HTML since you might nest that block in a new DIV element or whatnot, and the ability to spread the msgids along many lines makes for cleaner looking code.

Furthermore, the current blocktrans tag implementation generates some ugly translation strings when using make-messages if you have indented your msgid block.
Since most HTML is somehow indented, the above can easily look like this:

msgid "\n"
"\t\t\t\t\tThere are %(count)s messages\n"
"\t\t\t\t\twaiting for you\n"
"\t\t\t\t\t"

Note the last line of tabs, since it includes the indentation of the "endblocktrans" tag as well.

See this thread for more:
http://groups.google.com/group/django-developers/browse_thread/thread/60f0a0ac2347c5f5

Attachments (3)

blocktrans_strip.diff (3.4 KB) - added by Dmitri Fedortchenko <zeraien@…> 7 years ago.
Patch that adds stripping of indentation from contents of blocktrans block.
strip_blocktrans_tests.py (3.8 KB) - added by Dmitri Fedortchenko <zeraien@…> 7 years ago.
Some unit tests to show and test outcome of stripping process
blocktrans_strip.2.diff (3.4 KB) - added by Dmitri Fedortchenko <zeraien@…> 7 years ago.
Original patch could not be applied after rev 6689, this is an update.

Download all attachments as: .zip

Change History (17)

Changed 7 years ago by Dmitri Fedortchenko <zeraien@…>

Patch that adds stripping of indentation from contents of blocktrans block.

Changed 7 years ago by Dmitri Fedortchenko <zeraien@…>

Some unit tests to show and test outcome of stripping process

comment:1 Changed 7 years ago by Simon G <dev@…>

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

Accepted, with a possible improvements depending on MT's judgement :)

comment:2 Changed 7 years ago by Dmitri Fedortchenko <zeraien@…>

I would like to start by pointing out that as it is right now, the patch breaks a few blocktrans msgids from the admin app.
However I feel that it might be worth it anyway, and I am sure there can be a workaround. Perhaps a transitional period where two checks are made, one with stripping and then a second AS IS if the first fails...

Changed 7 years ago by Dmitri Fedortchenko <zeraien@…>

Original patch could not be applied after rev 6689, this is an update.

comment:3 Changed 3 years ago by gabrielhurley

  • Severity set to Normal
  • Type set to Cleanup/optimization

comment:4 Changed 3 years ago by acdha

  • Cc chris@… added
  • Easy pickings unset
  • UI/UX unset

comment:5 Changed 3 years ago by charettes

I happen to use blocktrans template tag in a non-html template where indentation is important.

What about adding a spaceless_blocktrans block? It would maintain backward compatibility while fixing this issue.

comment:6 Changed 3 years ago by phartig@…

  • Cc phartig@… added

comment:7 Changed 3 years ago by Tuttle

  • Cc macek@… added
  • Component changed from Template system to Internationalization
  • Type changed from Cleanup/optimization to New feature

I also hate those \t's and \n's in blocktrans! When passing PO's to 3rd party translation agencies, they're not always able to handle escapes correctly.

To avoid \t's and \n's, I'm forced to have blocktranses on a single line. With plural form and variable interpolation it can easily become 300 chars line!

If we want to stay backwards compatible and not adding any new tag, wouldn't it be possible to add a "trimmed" option inside the blocktrans option?

Something like: {% blocktrans trimmed ... %}

It would:

  • strip all whitespace from start and end of the string,
  • do s/[\t\n]+/ /g

Just as the browser does.

Thanks for consideration. (I also dare to change the ticket properties.)

comment:8 Changed 2 years ago by dbrgn

  • Cc dbrgn added

I think both a blocktrans argument or a new tag would be acceptable.

comment:9 in reply to: ↑ description Changed 2 years ago by bbr

  • Cc bbr@… added

Replying to Dmitri Fedortchenko <zeraien@…>:

The biggest issue is if you indent the above block. Suddenly those msgids are obsolete and you need to edit all your language files....

Indentation is often changed in HTML since you might nest that block in a new DIV element or whatnot, and the ability to spread the msgids along many lines makes for cleaner looking code.

Another problem that I haven't seen mentioned, that is implied above: The same blocktrans text could be used at different indentation levels and gets interpreted as different msgids, thus needs to be translated more than necessary.

Version 0, edited 2 years ago by bbr (next)

comment:10 Changed 17 months ago by mpessas

  • Owner changed from nobody to mpessas
  • Patch needs improvement unset
  • Status changed from new to assigned

comment:11 Changed 14 months ago by mpessas

comment:12 Changed 9 months ago by bouke

  • Owner changed from mpessas to bouke

comment:13 Changed 8 months ago by bouke

  • Triage Stage changed from Accepted to Ready for checkin

RFC as review comments by aaugustin have been solved

comment:14 Changed 8 months ago by Anssi Kääriäinen <akaariai@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 7a7c789d5a7ce1c62e3739cb2624454189410ece:

Fixed #5849 -- Strip whitespace from blocktrans

Add the trimmed option to the blocktrans tag to trim any newlines and
whitespace from its content.

This allows the developer to indent the blocktrans tag without adding
new lines and whitespace to the msgid in the PO file.

Thanks to mpessas for the initial patch and Dmitri Fedortchenko for the
report.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.