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

Suggestion: avoid __MSG_*__ format #1

Open
ivilata opened this issue Feb 12, 2021 · 2 comments
Open

Suggestion: avoid __MSG_*__ format #1

ivilata opened this issue Feb 12, 2021 · 2 comments
Labels
enhancement New feature or request

Comments

@ivilata
Copy link

ivilata commented Feb 12, 2021

Hi! First of all, hats off to this module, it makes dynamic translation of extension HTML messages very efficient and flexible. 😄 We are using it in this extension and it works like a charm.

I was wondering whether the __MSG_*__ format in message ids really adds anything. I know it's necessary for text replacement in the manifest.json and CSS files, but with data-i18n* attributes explicitly and only referring to message ids, the format can be entirely avoided, thus avoiding a lot of noise in the HTML file. As you can see, we altered Localizer to remove that requirement and it works perfectly, even with predefined messages like @@ui_locale.

For backwards compatibility, getMessageTag may remove the __MSG_*__ wrapper if present as it does now, otherwise just use the tag as is. If you like the idea I can provide a PR, but my JS abilities are very limited.

Thanks!

@rugk rugk added the enhancement New feature or request label Feb 12, 2021
@rugk
Copy link
Member

rugk commented Feb 12, 2021

Hi,
first of all great to hear you use and like the library! 😃

I was wondering whether the MSG*_ format in message ids really adds anything.

Looking back at what I've made that long ago I do tend to agree to your reasoning.
Maybe the only advantage could be if you copy/paste the strings from somewhere, but unless you copy from your manifest.json (which happens rarely), it is actually more
Maybe it was just inspired by all these libs, which just go over the whole website source and "string replace" exactly this with a RegEx. You could there only locate the strings as such.

For backwards compatibility, getMessageTag may remove the MSG*_ wrapper if present as it does now, otherwise just use the tag as is. If you like the idea I can provide a PR, but my JS abilities are very limited.

Yes, I'm fine with a PR that does this and is backwards-compatible!
I'm still reluctant to declare this as the supposed way in the Readme/doc though, maybe just offer both features side-by-side…

--

Also BTW note that for including this lib in your project I'd suggest to use git submodules. You then don't need to do this (Edit: oh this is just the LICENSE credit, of course you can and need to do that, just don't need to mention the commit) as git does it for you (keep track of the included version, locally at least) and you can easily update it. 😃

@ivilata
Copy link
Author

ivilata commented Feb 15, 2021

Thanks @rugk, I just created PR #2 (more info in its description).

Thanks also for the hint on submodules, we use them pretty extensively but this time I wanted to avoid adding all Localizer files to the extension… though I found later I can specify them here. I'll probably convert that to a submodule, esp. if the PR is eventually merged. 😉

Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants