Posted by SamLerner on September 29, 2008 at 7:34pm
4 followers
| Project: | Flag |
| Version: | 6.x-1.0-beta4 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | SamLerner |
| Status: | closed (fixed) |
Issue Summary
CivicActions is reviewing and upgrading multiple modules for use on client sites. Part of this work is a coding standards review. Attached you will find a patch based on a review with the coder module and a careful examination of the code. Thanks!
| Attachment | Size |
|---|---|
| flag_coder_i18n.patch | 2.36 KB |
Comments
#1
Thanks! I committed to the Drupal 5 version also, which didn't have any conflicts other than hook_menu(), which didn't need to be updated anyway.
#2
Here's another patch with a couple small i18n fixes for includes/flag.views_bookmark.inc that we didn't discover until just now.
#3
quicksketch: FYI, there's one other internationalization issue not covered by these patches:
flag/flag.module:+901: Invalid localization code: format_plural(intval($count),$singular,$plural) In format_plural(), the singular and plural strings are literal strings and should not be enclosed within t().
The t() variable should not be used on variables, and the potx module will not be able to extract these strings for translation. It's also not what the t() function was intended for. If you need to translate dynamic variables, then the i18nstrings
tt()is a good alternative. The above line of code is within the_flag_format_plural()function, which I couldn't find used anywhere, so this may be a non-issue.Cheers,
Stella
#4
Thanks stella, I'm not sure what that function is used for, let's discuss in a separate issue. Sam, I committed your #2 to both Drupal 5 and 6 branches.
#5
That's not a bug. This is user (administrator) input, so we can't hardcode it.
(It's used in the D5 version to generate messages such as "no votes", "1 vote", "3 votes" in views. It was left in D6 as a reminder that we don't have this functionality yet. I see that people aren't clamoring for this functionality, and, besides, it's a job better suited for Views itself or for "Views Bonus", so maybe it's time to remove it.)
#6
Created a new issue for the use of t() on $variables at #316149: Use of t() on $variables
#7
Automatically closed -- issue fixed for two weeks with no activity.