Meta bug: non-plural marked strings

There are some strings in the translations files that are not marked as plural. This is a meta bug for them.
I’ll list the string id’s (named “Key” in Transifex), and update the topic whenever I find another strings.
I hope that when the fix is applied, you change “N” to “Y” in “Fixed” column! :slight_smile:

EDIT: 8 by @meglio and some changes

List:
client.en file:

|Fixed| String
|  Y  | js.flagging.delete_confirm
|  Y  | js.flagging.custom_message.at_least
|  Y  | js.flagging.custom_message.more
|  Y  | js.flagging.custom_message.left
|  N  | js.posts_long
|  N  | js.likes_long
|  N  | js.views_long
|  Y  | js.topic.auto_close_immediate
|  N  | js.topic.feature_topic.confirm_pin_globally
|  Y  | js.topic.filter_to
|  N  | js.too_few_topics_and_posts_notice
|  N  | js.too_few_topics_notice
|  N  | js.too_few_posts_notice
|  Y  | js.composer.group_mentioned
|  Y  | admin_js.admin.user.tl3_requirements.table_title
|  Y  | admin_js.admin.user.delete_all_posts_confirm
|  N  | js.composer.error.title_too_short
|  N  | js.composer.error.title_too_long
|  N  | js.composer.error.post_length
|  N  | js.summary.description
|  N  | js.summary.description_time
|  N  | js.user.password.instructions
|  N  |  js.too_few_topics_and_posts_notice
|  N  |  js.too_few_topics_notice
|  N  |  js.too_few_posts_notice

poll/server.en file:

|Fixed| String
|  N  | poll.edit_window_expired.cannot_change_polls
|  N  | poll.edit_window_expired.op_cannot_edit_options
|  N  | poll.edit_window_expired.staff_cannot_add_or_remove_options
|  N  | education.reviving_old_topic
|  N  | education.get_a_room
|  N  | education.too_many_replies
|  N  | site_settings.errors.invalid_string_min_max
|  N  | site_settings.errors.invalid_string_min 
|  N  | site_settings.errors.invalid_string_max
3 Likes

@team: Ping? :smile:

Why don’t you submit a pull request fixing all those strings? :wink:

1 Like

After I mentioned the group, the PR idea came to my mind :smiley:
I’ll do my best, just need to check how plurals are handled…

1 Like

Not all of them are easy fixes. The following strings have multiple numbers and would be a candidate for Message Format:

  • js.flagging.delete_confirm
  • js.too_few_topics_and_posts_notice
  • js.too_few_topics_notice
  • js.too_few_posts_notice
  • admin_js.admin.user.delete_all_posts_confirm

I’m not sure if the following are still used. Looks like they were removed with the topic list refactor a few versions back…

  • js.posts_long
  • js.likes_long
  • js.views_long
3 Likes

I think I can handle them as well. I understood how do they work in the code and will do a PR! :slight_smile:
Thanks!

But for the deleted strings I think you can remove them without a PR…

Made a PR! https://github.com/discourse/discourse/pull/4382 :slight_smile:

For:

  • js.too_few_topics_and_posts_notice
  • js.too_few_topics_notice
  • js.too_few_posts_notice
    There are 4 variables, and I think that the string will be really complicated to deal with.
    What do you think?

And I think that these two are also deleted:

  • js.topic.feature_topic.confirm_pin_globally
  • js.post.more_links
5 Likes

I tried to fix js.too_few_topics_and_posts_notice, which is:

Let’s get this discussion started! There are currently %{currentTopics} / %{requiredTopics} topics and %{currentPosts} / %{requiredPosts} posts. New visitors need some conversations to read and respond to.

The important part to fix is:

There are currently %{currentTopics} / %{requiredTopics} topics and %{currentPosts} / %{requiredPosts} posts.

Which turned into this:
There { CURRENTTOPICS, plural, one {is currently <strong>1} other {are currently <strong>#}} / { REQUIREDTOPICS, plural, one {1</strong> topic} other {#</strong> topics}} and { CURRENTPOSTS, plural, one {<strong>1} other {<strong>#}} / { REQUIREDPOSTS, plural, one {1</strong> post} other {#</strong> posts}}.

And it can be simplified by just pluralizing the required parts (requiredTopics, requiredPosts), but I’m not sure how other languages handle this.

Was the change of placeholders name necessary? For example {{post_count}} to {{count}}.

All locales will need to be revised if I’m not mistaken.

Not sure really, that’s what I found in the file.
It seems that “count” is used always for plural strings, so that the translation engine know which variable is the plural one.

I think the name does not matter. The code has just to be consistent with itself. But as far as I can tell, changing these names breaks all locales until the changes are manually applied on Transifex.

Take a look at:
siteSettingRate: I18n.t('logs_error_rate_notice.rate', { count: siteSettingLimit, duration: duration }),
And the translation:
rate: one: "1 error/%{duration}" other: "%{count} errors/%{duration}"

How can the translation system choose the plural form? It should check the variable, but which one?
Here comes that “count” is always used as the “plural variable” that the system depends on when choosing the forms.

2 Likes

You should make the first post a wiki-post.
Please add this key: js.topic.feature_topic.confirm_pin_globally

1 Like