WordPress / openverse

Openverse is a search engine for openly-licensed media. This monorepo includes all application code.
https://openverse.org
MIT License
240 stars 194 forks source link

Clean up repeated static-string interpolations #4980

Open sarayourfriend opened 6 days ago

sarayourfriend commented 6 days ago

Current Situation

We use "static" interpolations for certain kinds of string content, typically brand marks. For example, we use {openverse} 73 times in our English messages to interpolate "Openverse" into strings. This is done to prevent translators from accidentally translating brand marks which should not be translated (and is usually most important for things that could be ambiguous to translators like the name of an institution which should not be translated, e.g., an American museum whose name is in English and the institution provides no translation into supported languages, a museum in Aotearoa/New Zealand whose name is in re reo Māori, etc).

In the case of Openverse, because it is so common, we repeat the interpolation arguments of openverse: "Openverse" at least 53 times when calling i18n.t and related functions.

For example, we have the following translation string with the key hero.disclaimer.content: "All {openverse} content is under a {license} or is in the public domain.". As such, the code utilising this string must interpolate "Openverse" for the openverse template variable:

<i18n-t
  scope="global"
  keypath="hero.disclaimer.content"
  tag="p"
  class="mt-4 text-sr"
>
  <template #openverse>Openverse</template>
  <template #license>
    <VLink
      href="https://creativecommons.org/licenses/"
      class="text-default underline hover:text-default"
      >{{ $t("hero.disclaimer.license") }}</VLink
    >
  </template>
</i18n-t>

This is both tedious to maintain, and easy to error.

Suggested Improvement

There are two methods we could use to avoid needing to pass interpolation arguments for static content in code utilising messages:

  1. We could use Vue I18n's "literal interpolation" syntax. To do so, we would replace all usages of things like {openverse} with {'Openverse'}. This allows us to reuse our existing translator help messages of not translating things between ### (though we need a small change to json-to-pot to allow any kinds of content for the the "variable" name, not just letters and hyphens).
  2. We could use Vue I18n's linked messages. We would add a new "static" message, "OPENVERSE": "Openverse" that is not sent to translators, and is instead added to the final translation JSON files. Then instances of things like {openverse} would be changed to @:OPENVERSE. We can use whatever kind(s) of key(s) we want for this to designate non-translatable content not to send to translators. We would still need to add an explanation for translators to ignore the @:<key> content, or we could just wrap it in ### and reuse the same message.

I think the first option may be the best, simply because it utilises our pre-existing tool for indicating to translators not to translate certain content. Linked messages are convenient and DRY (not relevant for the "Openverse" example but is so for longer strings), but require remembering the identification method, which may be harder to remember when writing or reading than the literal interpolation format.

Therefore, I propose we replace {openverse} in our messages with {'Openverse'} and do the same for other statically-interpolated content in our strings. When I searched, I found these, in addition to Openverse:

There may be others, but we can just start with those and already clean up a lot of interpolation arguments passed in code calling i18n.t or using the i18n components.

Benefit

Remove redundant, repeated static strings from the codebase, and the need for them in the first place.

dhruvkb commented 4 days ago

This issue is excellently documented and the changes involved seem very straightforward. @sarayourfriend do you think this is a potential good-first-issue?

sarayourfriend commented 4 days ago

I'll put help wanted. I don't think this is necessarily a good first issue because to test it will require the contributor to run the POT generation and verify the outputs. They will need to make changes to the regex for interpolation detection, as mentioned briefly in the issue description (but not in detail). All of that on top of setting up the development environment makes it too much for a first issue, I think.