Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Make use of new ungettext_lazy function at appropriate places. #702

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
2 participants
Contributor

uruz commented Feb 6, 2013

@claudep claudep commented on an outdated diff Feb 6, 2013

django/contrib/comments/admin.py
@@ -78,7 +75,7 @@ def _bulk_flag(self, request, queryset, action, done_message):
msg = ungettext('1 comment was successfully %(action)s.',
'%(count)s comments were successfully %(action)s.',
n_comments)
- self.message_user(request, msg % {'count': n_comments, 'action': done_message(n_comments)})
+ self.message_user(request, msg % {'count': n_comments, 'action': done_message % n_comments})
# Only register the default admin if the model is the built-in comment model
# (this won't be true if there's a custom comment app).
@claudep

claudep Feb 6, 2013

Member

If we fix this, we should do it right, because currently, it is awfully wrong, i18n-wise. The proper way is to duplicate the strings in each flag_* methods ('%(count)s comments were successfully flagged.', '%(count)s comments were successfully approved.', ...).

@claudep claudep commented on an outdated diff Feb 6, 2013

django/utils/timesince.py
@@ -19,12 +19,12 @@ def timesince(d, now=None, reversed=False):
Adapted from http://blog.natbat.co.uk/archive/2003/Jun/14/time_since
"""
chunks = (
- (60 * 60 * 24 * 365, lambda n: ungettext('year', 'years', n)),
- (60 * 60 * 24 * 30, lambda n: ungettext('month', 'months', n)),
- (60 * 60 * 24 * 7, lambda n : ungettext('week', 'weeks', n)),
- (60 * 60 * 24, lambda n : ungettext('day', 'days', n)),
- (60 * 60, lambda n: ungettext('hour', 'hours', n)),
- (60, lambda n: ungettext('minute', 'minutes', n))
+ (60 * 60 * 24 * 365, ungettext_lazy('year', 'years')),
+ (60 * 60 * 24 * 30, ungettext_lazy('month', 'months')),
+ (60 * 60 * 24 * 7, ungettext_lazy('week', 'weeks')),
+ (60 * 60 * 24, ungettext_lazy('day', 'days')),
+ (60 * 60, ungettext_lazy('hour', 'hours')),
+ (60, ungettext_lazy('minute', 'minutes'))
@claudep

claudep Feb 6, 2013

Member

I would suggest to use the ungettext_lazy('%(number)d minute', ''%(number)d minutes', 'number') syntax, if possible.

@claudep claudep commented on the diff Feb 6, 2013

django/utils/timesince.py
@@ -45,13 +45,13 @@ def timesince(d, now=None, reversed=False):
count = since // seconds
@claudep

claudep Feb 6, 2013

Member

Could you also fix this awful return '0 ' + ugettext('minutes'), just above?

@claudep claudep commented on an outdated diff Feb 6, 2013

django/utils/timesince.py
if i + 1 < len(chunks):
# Now get the second item
seconds2, name2 = chunks[i + 1]
count2 = (since - (seconds * count)) // seconds2
if count2 != 0:
- s += ugettext(', %(number)d %(type)s') % {'number': count2, 'type': name2(count2)}
+ s += ugettext(', %(number)d %(type)s') % {'number': count2, 'type': name2 % count2}
return s
def timeuntil(d, now=None):
@claudep

claudep Feb 6, 2013

Member

I would have put the strings in a list, then finally ugettext(", ").join(the_list)

Member

claudep commented Feb 6, 2013

Note that my remarks all address preexisting flaws in the code, so you are not to blame :-)

Member

claudep commented Feb 6, 2013

Thanks. committed in d18f796

@claudep claudep closed this Feb 6, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment