Updated: Comment #N

Problem/Motivation

In many cases, transliterating one character can result in multiple characters of output. For instance, you might transliterate ö to oe in German.

If a caller of transliteration methods needs to limit the length of the resulting string from transliterating a string that contained ö somewhere in the middle, they might not want to split between the o and the e in the output. However, they would not have any way to know where it was and wasn't OK to truncate.

Therefore, it would be useful if the transliterate methods themselves had an optional maximum length parameter, which would ensure the return value was less than this length, split on the boundary between transliterations of individual characters.

Proposed resolution

Add an optional max_length parameter, which would return this length or fewer characters, ensuring that the full transliteration of each character (rather than partials) got into the return value. Callers who don't care about splitting on these boundaries could call the function without this parameter, and they could then truncate their output after the fact.

Remaining tasks

Add the parameter. Write a test that verifies it works.

User interface changes

None.

API changes

New optional parameter to the transliteration methods, with backwards-compatible behavior (no truncation).

None.

Original report by alippai

As the transliteration can differ in the output string length from the input we should provide a maxlength parameter.
E.g.: I think we shouldn't split "oe" from ö->oe conversion in German, if we limit the outputs length later.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon’s picture

It seems to me that anyone calling the transliteration methods could just chop the resulting string to the maximum length that they need. I don't see why the transliteration class needs to do that itself?

jhodgdon’s picture

Category: task » feature
amateescu’s picture

Status: Active » Closed (won't fix)

I agree with #1, this is really the responsability of the caller, not of the system that actually performs the transliteration.

alippai’s picture

The caller can't determine the correct truncation of the output. If you want to transliterate the string "ö" (it results in "oe") and truncate it to one character the correct output is "" and not "o" in my opinion.

jhodgdon’s picture

Status: Closed (won't fix) » Active

That is an excellent point in #4, which I missed in the original feature request report. Maybe we should reconsider, in which case we would want to have two additional arguments, I think: one for the max length to return, and a boolean to require that the split happen on an input character boundary if TRUE and just truncate if not... I'm not sure that the case in #4 is universally desirable, but I can see that if it was desirable, it would need to happen in the transliteration class itself.

It shouldn't be that hard to do... anyone want to make a patch?

jhodgdon’s picture

Status: Active » Needs review
FileSize
3.6 KB

I've added an issue summary. And here's a patch, including an addition to the tests. Tests pass on my system...

Hopefully it is not too late in the D8 process to get this in. It is a relatively small change. Someone want to review it quickly?

amateescu’s picture

Status: Needs review » Reviewed & tested by the community

It shouldn't be too late for this kind of small additions, and the patch looks good to me.

alippai’s picture

I'd go with default value PHP_INT_MAX for $max_length and concatenation could go in the else branch. With or without these I'm okay with the patch. +1 from me.

jhodgdon’s picture

I think a default value of NULL to indicate "we don't want to do this" is more consistent with the rest of the Drupal API.

alexpott’s picture

Title: Extend transliterate() with maxlength » Change notice: Extend transliterate() with maxlength
Priority: Normal » Major
Status: Reviewed & tested by the community » Active
Issue tags: +API addition, +Needs change record

Committed c9ab009 and pushed to 8.x. Thanks!

As outlined in #4 this change is very hard to implement outside of the transliteration service.

jhodgdon’s picture

Title: Change notice: Extend transliterate() with maxlength » Extend transliterate() with maxlength
Priority: Major » Normal
Status: Active » Fixed

I added a note/code with the new parameter to the change notice that we had before (from the original issue):
https://drupal.org/node/1842748

xjm’s picture

Issue tags: -Needs change record

Untagging. Please remove the "Needs change notification" tag when the change notice task is complete.

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

Added issue summary