In my code, I often use concatenated strings for stuff other than SQL queries. That's because it's the only way that I can see which allows me to indent the code properly without having ugly linebreaks all over the place, and keep the originally intended string just the way it should be outputted.

Thus, my code contains sections like these:

$form['default_duration'] = array(
  ...other properties...,
  '#description' => t('Specify how long an invitation will be valid by '.
                      'default. The host user can customize this '.
                      'timespan for each invited user.'),
);

That's a perfectly valid, non-dynamic string, yet it can't be parsed by potx because it consists of multiple tokens. Would it be possible to add support for parsing strings like this one as well?

CommentFileSizeAuthor
#3 potx-concatenated-strings-try2.patch1.53 KBjpetso
#1 potx-concatenated-strings.patch1.55 KBjpetso
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jpetso’s picture

jpetso CreditAttribution: jpetso commented
Status: Active » Needs review
FileSize
potx-concatenated-strings.patch1.55 KB

Yay, I managed to come up with a patch by myself. Works great, please review.

GoofyX’s picture

GoofyX CreditAttribution: GoofyX commented

Thanks for the patch. It also works fine here for me (tested it with the tinymce module, which suffers from this).

jpetso’s picture

jpetso CreditAttribution: jpetso commented
FileSize
potx-concatenated-strings-try2.patch1.53 KB

Seems I wanted to end the loop by placing a condition at the end of the loop, which is why the previous patch contains a do/while loop.
Of course, with while(TRUE) this is unnecessary and doesn't look just as clean, so here's the same patch with the more appropriate standard while loop.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs review » Closed (works as designed)

Well, I looked into the Drupal coding standards and did not find anything against concatenating stuff, but in practice this type of concatenation is not used in Drupal modules, developers are used to writing long lines and their intelligent IDEs wrapping the lines as required.

I also think that by allowing concatenations, we set a bad precedent. Concatenating dynamic parts into a string is a big no-no, so we found it easiest before to allow no dynamic string manipulation on extracted strings. Also why allow concatenation in t()-like stuff but not with format_plural() strings for example. This gets inconsistent.

As far as I see, we enforce the static string nature by sticking to our rigid standards even if they seem to be unfriendly if looked at a simple editor or IDE.

jpetso’s picture

jpetso CreditAttribution: jpetso commented

Ok, I can see your point... it won't be included then.

Anyways, even good editors usually don't display this in an optimal kind of way: when breaking lines, the continuation of the line would normally start with the same indentation as the original line, which is the right thing to do from a logical point of view. The right thing to do, however, would be to have the line further indented until after "t(", or maybe just two spaces more than the original line's indentation. What looks best and is most readable really has to be determined on a case-by-case basis, the best editor can't do this for you.

Also, there might be cases when concatenating is done not for appropriate indenting, but for dealing with the two string types' different handling of escape characters.

In that light, I find it a bit sad that a nearly unnoticable decrease in performance matters enough to throw away better readibility, not only for Drupal core (which certainly can demand "higher" code standards for consistency) but also for *all* of contributions, even if the given contribs are consistent within the whole module.

By the way: As for format_plural() and friends, certainly this can be refactored and the same lines of code could (and should) be used by all such calls. I did not look closely enough to recognize that there are more functions in need for such string retrieval, but this should pose no problems to the approach in general.

But well. Guess I'll have to live with that decision.

ShaneOnABike’s picture

ShaneOnABike CreditAttribution: ShaneOnABike commented

I'm using D7 version and think that this would be a great feature. Is there any way to make this happen as visually in code it makes (to me at least) more sense to have strings in multiple lines. There must be a way we can parse the entire thing and combine the string like t() does and output that to a file?