Translations

Development
1

I've installed gettext 0.21 and it seems to work:

./packages/lib/services/KeymapService.js:215: warning: unterminated string
Updated pot file. Total strings: 731 => 742
Building en_GB...
Building ar...

................................................................................. done.
Building bg_BG...

............................................................................... done.
Building bs_BA...

.................................................................................. done.
Building ca...

................................................................................. done.
Building cs_CZ...

.................................................................................. done.
Building da_DK...

.................................................................................. done.
Building de_DE...

................................................................................... done.
Building el_GR...

.................................................................................. done.
Building en_US...

............................................................................ done.
Building eo...

................................................................................. done.
Building es_ES...

.................................................................................. done.
Building et_EE...

.................................................................................. done.
Building eu...

............................................................................... done.
Building fa...

................................................................................. done.
Building fr_FR...

................................................................................... done.
Building gl_ES...

............................................................................... done.
Building hr_HR...

................................................................................ done.
Building id_ID...

....................................................................................... done.
Building it_IT...

.................................................................................. done.
Building ja_JP...

................................................................................... done.
Building ko...

.................................................................................. done.
Building nb_NO...

................................................................................. done.
Building nl_BE...

............................................................................... done.
Building nl_NL...

.................................................................................. done.
Building pl_PL...

.................................................................................. done.
Building pt_BR...

.................................................................................. done.
Building pt_PT...

................................................................................... done.
Building ro...

................................................................................. done.
Building ru_RU...

................................................................................. done.
Building sl_SI...

............................................................................... done.
Building sr_RS...

................................................................................. done.
Building sv...

................................................................................ done.
Building th_TH...

................................................................................... done.
Building tr_TR...

.................................................................................. done.
Building vi...

.................................................................................. done.
Building zh_CN...

.................................................................................. done.
Building zh_TW...

................................................................................. done.

The warning comes from the following in ./packages/lib/services/KeymapService.js:

'`',

But the code works. I've tested it. So not sure why this warning pops up. I think JS or gettext requires the backtick to be escaped.
However, there are other backticks in that file, so this error must have shown up since the keyservice was added, but I have never seen that error before. Maybe there was an exception for it?
Or maybe the old gettext didn't care?

I really don't know what's going on. Any ideas?

2

Can you provide the lines before and after this? It might just be a forgotten quote somewhere.

3 
	'/',                     // [191]
	'`',                     // [192]
	'',                      // [193]

the thing is if I escape the backtick, it then complains for line:

		{ accelerator: 'Cmd+`', command: 'textCode' },

and then

		{ accelerator: 'Ctrl+`', command: 'textCode' },

In JS one can use the backtick to create strings, so that's probably the reason for the warning. My point is that accelerator code has been there for a while, but I've never seen any warnings in that regard. So something is off. Also, we can't just escape backticks, if there's no need for that in the code.

4

Any update on this? Is there an exception we have to create for this file? How do we get rid of the warning?

5

I guess we should avoid having backticks within strings if it confuses gettext. Does it work if you replace it with 'Ctrl+' + String.fromCharCode(96) (char code 96 being a backtick)

6

That's not going to work, because the json files uses the backtick and afaik Electron won't accept that as an accelerator.

7

It’s unrelated to json though, since gettext only parses js files.

8

I just mentioned the json (keymap-desktop.json) file, because a backtick is written to it as follows:

  {
    "command": "textCode",
    "accelerator": "Shift+Cmd+`"
  },

And it is read correctly.

The same is true for the current code. It is interpreted correctly and it runs as it should.
It just seems that gettext or whatever spits out the warning has an issue with it.

The thing is, there is no issue, otherwise the linter would have complained already.... (And the code would never have worked in the first place.)

I suspect that gettext 0.21 throws that warning where 0.19 did not. Either way, I think it is the wrong approach to change the code for gettext.

9

Yes for sure the code is fine, and the problem is with the buggy gettext parser. I don't think there are many instances of backticks within quotes in the code, so I think it's a reasonable fix to get gettext to work, and we can always add a comment to explain why it's like that. I'd rather have this than translations being silently dropped in any case.

10
  1. I don't see any translations dropped in this file (despite the warning)
  2. This file has a no user facing messages, but inconsistencies in the errors that are thrown (some are translatable strings, some are not)

Error messages that are thrown via throw new Error should not be translated anyway.
This file, since it has no user facing messages, should be excluded from being parsed.
All problems solved.

Update: I would like to remove all _('') constructs from ./packages/lib/services/KeymapService.ts since the thrown error messages should not be translated.

11

I thought we were talking about the backtick, of which there's only one in the project. Excluding the file from being parsed is all fine, till the day we add a translation to it and we don't realise the file is actually excluded. So no, we shouldn't exclude anything, we shouldn't have to wonder if a file is being translated or not, that should work automatically.

Update: I would like to remove all _('') constructs from ./packages/lib/services/KeymapService.ts since the thrown error messages should not be translated.

No, I've spent quite a bit of time fixing the mess that was the translation strings in this file, and what's left are errors we should display.

12

Yes, we were talking about the backtick. But there are 3 occurrences that lead to the warning, not just one.

Ok, my fault then. I thought that throw new Error statements show up only in stacktraces. And I'd find it useless, if I had to read an error message in a stacktrace in a language I do not understand.

13

Right, some error messages indeed end up in the log while others are displayed to the users. Errors that are unlikely to happen sometimes aren't translated (even if they are displayed) but in this particular case they are a bit more likely so we translate them.

14

I always had the rule in my head "errors thrown -> log and stack", errors printed with UI methods or stdout -> user facing.
Thus I totally mixed this up.

15

I've looked at the backtick issue and the problem was not with the backticks themselves but with the regex at the top of the file, which was making the rest of the parsing fail. I've moved that regex outside the file and it works. Fix is there: https://github.com/laurent22/joplin/commit/046433a947d5507f55537cd5c7e3437f2fe400b4

Could you try on your side and double-check that there's no warning?

16

Yep, no more warning!

17

I'm going to push the new translations soon. I'm testing a few things first, but I think we are good to go.