Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Translations keys are normalized but msgid only trimmed #60

Open
mussinbenarbia opened this issue Aug 20, 2024 · 6 comments
Open

Translations keys are normalized but msgid only trimmed #60

mussinbenarbia opened this issue Aug 20, 2024 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@mussinbenarbia
Copy link

Hello, thank you for your hard work on this project.

We recently ran into an issue caused by the string to be translated containing characters like \n. When translations are loaded in the createGettext function, there seems to be a step that normalizes each key, here

This will get rid of \n and s+ and will trim the keys. The problem is when we try to actually get the translated string, msgid is only trimmed, here meaning that the msgid and the corresponding key in the translations object will not match due to one being normalized and the other not.

We are planning to apply a patch but would also love to contribute to the project if helpful. Would you recommend applying the normalization to the msgid as well or remove the normalization of the keys?

@lzurbriggen
Copy link
Collaborator

@mussinbenarbia We recently created a new 4.0 pre-release version that made some changes to that, but it hasn't been thoroughly tested:

Breaking Changes

  • Message ids are not trimmed anymore. this was likely initially implemented to deal with ids in HTML (using <translate>/v-translate)
  • Line breaks are now normalized to LF across the board (\r\n in messages will always become \n)

v4.0.0-alpha.4

I believe there are some issues with the bin files that I will try to address in the next week, but if you are patient you don't need to creat a patch.

@mussinbenarbia
Copy link
Author

Thank you for the quick feedback!

@mussinbenarbia
Copy link
Author

@lzurbriggen

Line breaks are now normalized to LF across the board (\r\n in messages will always become \n)

Does this mean that the same will be applied to msgid? For example:

gettext(`This is a string.
When extracted it will contain newline characters.`)

When normalized with the new step, if I understand correctly, this will be This is a string.\r\nWhen extracted it will contain newline characters.

But the msgid, at runtime will still be This is a string.\nWhen extracted it will contain newline characters..

Will this not cause the lookup for the translated string to fail?

@lzurbriggen
Copy link
Collaborator

@mussinbenarbia

When normalized with the new step, if I understand correctly, this will be This is a string.\r\nWhen extracted it will contain newline characters.

The idea is that all \r\n become \n in all messageids: when extracting, when reading in the translations.json and when looking up msgids, so the lookup should succeed. It's possible I missed something, did you run into an issue already?

@mussinbenarbia
Copy link
Author

No issue as I haven't tried v4 (Our project still uses the component and v-translate directives so we'll need to remove those before updating)

I looked at the code in the v4 branch and I understand the changes now. The code will now call normalizeMsgId() both in getTranslation() and also in normalizeTranslations() so the lookup should match 👍

Kind of an unrelated question but I noticed that normalizeMsgId is implemented as:

export function normalizeMsgId(key: string) {
  return key.replace(/\r?\n|\r/, "").trim();
}

Why is there a need to even remove these characters? I'm only asking because the Vue 2 version did not have this normalization step and I'm curious why it was introduced. Also is it intended that it only replaces the first occurrence?
Since it's not replaceAll, and it's also not using the /g flag, this will only match the first character.

@lzurbriggen lzurbriggen self-assigned this Aug 23, 2024
@lzurbriggen lzurbriggen added the bug Something isn't working label Aug 23, 2024
@lzurbriggen
Copy link
Collaborator

I definitely don't want to trim msgids anymore. I'll have another look at this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants