deltachat / deltachat-desktop

Email-based instant messaging for Desktop.
GNU General Public License v3.0
941 stars 166 forks source link

i18n: wrong order of substitutions for some strings #3897

Open WofWca opened 5 months ago

WofWca commented 5 months ago

How to find (some) strings that are wrong: search for %2.*%1. There are 94 occurrences for 16 languages currently.

The responsible code:

https://github.com/deltachat/deltachat-desktop/blob/b342a1d47b505e68caaec71f79c381c3f304405a/src/shared/localize.ts#L73-L83

It assumes that the substitutions are always in the same order for all strings for all languages. And if you feel like "it's just a matter of looking at the digits after %, keep in mind that some strings only have one substitution, and some strings have several substitutions, but they are not numbered (such as ask_send_following_n_files_to

It feels like it's about time to use a library for this. Or write one, and cover it with tests. Or convert the original xml files to a better-supported format with a tried-and-tested converter. We already have code that converts XML to JSON.

Also as a part of this I guess this

https://github.com/deltachat/deltachat-desktop/blob/dd8361e65e35529056f5bb2a343a593a1a2f5586/src/renderer/components/dialogs/ConfirmSendingFiles.tsx#L48

needs to be fixed to support non-English plural rules.

r10s commented 5 months ago

good catch! thanks a lot!

it's all a bit tricky, indeed.

isn't there a printf()-or-so with indexed parameters on javascript? then one would just need to convert the argument syntax.

otherwise, if it helps, indexed-parameters and not-indexed-parameters are not mixed in the same string. and, there is currently max. 3 indexed parameters, so, instead of a loop, we could do sth. like the following pseudocode:

substitude_translations(str, param, plurals_amount = 0) {
  str.replace('%1$s', param[0])
  str.replace('%2$s', param[1])
  str.replace('%3$s', param[2])

  str.replace_once('%s', param[0])
  str.replace_once('%s', param[1])
  str.replace_once('%s', param[2])

  str.replace('%d', plurals_amount)
}
WofWca commented 5 months ago

isn't there a printf()-or-so with indexed parameters on javascript?

There is not built-in thing that solves this problem.

maxphilippov commented 3 months ago
(function() {
  var substitutions = ['ADD', 'BAD', 'CAD', 'DAD'];

  ///

  const replace = (n) => {
      if (
        substitutions === undefined ||
        typeof substitutions[n] === 'undefined'
      ) {
        console.error(`Missing ${n}`)
        return ''
      }
      return substitutions[n].toString()
  }

  var c = 0;

  return '%2$s가 %$s %1$s를 멤버로 추가함 %s xxx'.replace(/(%\d\$[\w\d])/g, (match) => {
      let num = match.match(/\d/) - 1
      c = Math.max(num, c)
      return replace(num)
    }).replace(/(%[\w\d])/g, () => {
      return replace(++c)
    })
})()

What do you think of something like this? We have 2 passes: first replaces all the 'numbered' replacements, second treats others after the biggest number used before? So in this example %2$s가 %s %1$s를 멤버로 추가함 %s xxx resulting indices are [2, 3, 1, 4]. Didn't thoroughly test that, just a 'back-of-the-envelope' solution