MagicMirror icon indicating copy to clipboard operation
MagicMirror copied to clipboard

Translate error?

Open BKeyport opened this issue 3 years ago • 13 comments

Hello and thank you for opening an issue.

Platform: Raspberry Pi 4B, Raspbian GNU/Linux 10 (buster)

Node Version: v16.14.2

MagicMirror² Version: 2.19.0

Description: Translation error using "this.translate" - the word "LOADING" results in "Loading …"

Steps to Reproduce/** Actual results**: Using this code snippet to generate output: this.stats.cpuTemp = this.translate('LOADING').toLowerCase(); always results in "loading …" or this.stats.cpuTemp = this.translate('LOADING'); always results in "Loading …"

Expected Results: Describe what you expected to see. this.stats.cpuTemp = this.translate('LOADING').toLowerCase(); resulting in "loading" or this.stats.cpuTemp = this.translate('LOADING'); always results in "LOADING"

Note: This only occurs on the ALL CAPS version of the word. Otherwise, it works as expected.

Configuration: What does the used config.js file look like? Don't forget to remove any sensitive information! N/A - not a configuration error.

BKeyport avatar May 16 '22 01:05 BKeyport

AFAIK this is expected behaviour. The "LOADING" string is used in the default modules for displaying "Loading ..." as long as they are loading. You find the english translation in translations/en.json in this repository: "LOADING": "Loading …", and this is translated into other languages with the three dots too.

khassel avatar May 16 '22 21:05 khassel

Sorry, the bug system didn't handle my report the way I wanted.

to clarify, MM is showing the & code, not the "..."

(updated original post to show correctly)

BKeyport avatar May 16 '22 21:05 BKeyport

2022-05-16

BKeyport avatar May 16 '22 21:05 BKeyport

o.k., can reproduce this. The translate function seems to work only in the getDom function or in the njk templates if the string contains e.g. &hellip.

khassel avatar May 16 '22 21:05 khassel

@BKeyport @khassel you have to use the safe filter from nunjuck to display it.

E.g. https://github.com/fewieden/MMM-Fuel/blob/232333659c4d7ed7233805505fd127edfe20713b/templates/MMM-Fuel.njk#L8

fewieden avatar May 17 '22 07:05 fewieden

This really has turned into an improper translate issue then. "LOADING" shouldn't include the ellipsis, nor should it be changing case unless specifically specified.

BKeyport avatar May 17 '22 18:05 BKeyport

I think the decision to translate "LOADING" to "Loading ..." was made long ago in this project for a specific use case (display this message when loading a module).

We could change this when we remove all the &hellip from the translation files and append this in the default modules. But it would break other modules using this which are expecting the "...".

So you have to convince @MichMich to change the current behaviour ...

khassel avatar May 17 '22 20:05 khassel

I've never seen this used in any module except with the one that I have with the error. (I've changed that, too, so it's eliminated)

The difference would be a minor one at best, and honestly, wouldn't be noticed by most.

So, how about it, @MichMich? Should we change this? I'm a solid vote for yes.

BKeyport avatar May 18 '22 04:05 BKeyport

I'm not against small changes like these, but in this case the issue is more that punctuation marks are language specific. When we put the ellipsis in the template, this creates a wrong output for some languages. Isn't @fewieden's suggestion the solution to your problem?

MichMich avatar May 18 '22 07:05 MichMich

My suggestion would be to drop the … from the translation.

BKeyport avatar May 18 '22 07:05 BKeyport

That would make more sense, but does change the actual output and how it looks. I think it's good to put some extra thought in this. @khassel @fewieden Any thoughts?

MichMich avatar May 18 '22 07:05 MichMich

@MichMich as already mentioned before moving the ellipsis outside of the translation would be very bad.

Context: At work, I also have to do translations of the system, and when I notice someone splitting translations into two labels or appending a prefix/suffix in the code I have to reject the PR because it simply doesn't work across multiple languages/locales/dir the same way.

Having the ellipsis or not, in the end, is a UI/UX choice. If I recall correctly it pretty much has been there forever but that doesn't mean it couldn't be changed.

Looking at the code for the translate filter it actually seems that we try to automatically apply the safe filter of some sorts. That I was using successfully in the template.

this._nunjucksEnvironment.addFilter("translate", (str, variables) => {
    return nunjucks.runtime.markSafe(this.translate(str, variables));
});

I tried adding some logs there

if (str === 'LOADING') {
    console.log('raw', this.translate(str, variables));
    console.log('markSafe', nunjucks.runtime.markSafe(this.translate(str, variables)).toString());
    console.log('saveFilter', this._nunjucksEnvironment.filters.safe(this.translate(str, variables)).toString());
}

which produces the output

raw Loading … module.js:177
markSafe Loading … module.js:178
saveFilter Loading … module.js:179

So our current attempt to mark it as safe in the translation filter directly doesn't work. It is the same as doing

this._nunjucksEnvironment.addFilter("translate", (str, variables) => {
    return this.translate(str, variables);
});

fewieden avatar May 19 '22 06:05 fewieden

AFAIS the change to use return nunjucks.runtime.markSafe was introduced with this PR for not needing safe in the templates:

  • with return nunjucks.runtime.markSafe(this.translate(str, variables)); and {{ "LOADING" | translate }} result in mm website is Loading ...
  • with return this.translate(str, variables); and {{ "LOADING" | translate }} result in mm website is Loading …

khassel avatar May 19 '22 21:05 khassel

Is this still a problem with latest release v2.22.0?

All &hellip were replaced with ... with this PR

khassel avatar Jan 13 '23 21:01 khassel

It appears that would eliminate this. If that's the case, this could be closed as complete. I stopped using the module that had the problem, so, I'm not testing anymore.

BKeyport avatar Jan 13 '23 22:01 BKeyport

Thanks for the feedback. So I will close this, if anyone sees the need to open it again please comment below.

khassel avatar Jan 13 '23 22:01 khassel