-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
feat: i18n framework based on extracting translations from plugin's /languages/ to /language/ #8435
base: master
Are you sure you want to change the base?
feat: i18n framework based on extracting translations from plugin's /languages/ to /language/ #8435
Conversation
commit 14f7f98 Author: lin onetwo <[email protected]> Date: Sat Jun 8 19:45:17 2024 +0800 Revert "refactor: remove the "tree" tid" This reverts commit fb70f0b. commit fb70f0b Author: lin onetwo <[email protected]> Date: Sat Jun 8 19:36:28 2024 +0800 refactor: remove the "tree" tid commit 5c8b392 Merge: 6603794 33bc77f Author: lin onetwo <[email protected]> Date: Sat Jun 8 19:36:13 2024 +0800 Merge remote-tracking branch 'upstream/master' into feat/i18n commit 6603794 Author: linonetwo <[email protected]> Date: Fri Feb 9 23:18:45 2024 +0800 docs: Headings -> Heading commit 7f2b8b2 Author: linonetwo <[email protected]> Date: Tue Feb 6 22:23:46 2024 +0800 i18n: more for menubal commit 8840a57 Author: linonetwo <[email protected]> Date: Tue Feb 6 22:07:58 2024 +0800 i18n: add menubar translation for example commit 33456a5 Author: linonetwo <[email protected]> Date: Tue Feb 6 21:37:07 2024 +0800 feat: add tree to plugin to reveal l10n structure commit 2579d7f Author: linonetwo <[email protected]> Date: Tue Feb 6 21:36:51 2024 +0800 i18n: add more l10n commit 810b5d5 Author: linonetwo <[email protected]> Date: Tue Feb 6 21:08:50 2024 +0800 feat: add comment, so when use <$text text=<<lingo>> by mistake, it shows commit ab9572e Author: linonetwo <[email protected]> Date: Tue Feb 6 21:00:30 2024 +0800 docs: more tiddlyweb l10n commit f3eefa6 Author: linonetwo <[email protected]> Date: Tue Feb 6 20:06:29 2024 +0800 refactor: update translation of tiddlyweb commit aaf3182 Author: linonetwo <[email protected]> Date: Tue Feb 6 19:43:04 2024 +0800 Delete translateMacro.tid commit 36a9d49 Author: linonetwo <[email protected]> Date: Tue Feb 6 19:36:02 2024 +0800 refactor: update translate macro to reuse lingo macro commit bef871d Author: lin onetwo <[email protected]> Date: Sat Dec 16 01:40:55 2023 +0800 docs: more usage about inline commit 9fbd71e Author: lin onetwo <[email protected]> Date: Sat Dec 16 01:34:45 2023 +0800 Update LingoMacro.tid commit a8c5bc0 Author: lin onetwo <[email protected]> Date: Sat Dec 16 01:33:47 2023 +0800 docs: about mode:"inline" commit 2055f36 Author: lin onetwo <[email protected]> Date: Sat Dec 16 01:27:40 2023 +0800 refactor: use function instead of set variable commit 7d321fe Author: lin onetwo <[email protected]> Date: Sat Dec 16 00:26:55 2023 +0800 refactor: use lingo and procedure commit cacd236 Author: lin onetwo <[email protected]> Date: Wed Dec 13 23:22:12 2023 +0800 feat: support block mode so you can transclude whole tiddler commit a6b6132 Author: linonetwo <[email protected]> Date: Sat Oct 28 20:38:26 2023 +0800 feat: t macro and docs
Confirmed: linonetwo has already signed the Contributor License Agreement (see contributing.md) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did just review the code. I'll give some more feedback. Once I did test it in detail.
editions/tw5.com/tiddlers/macros/examples/lingo Macro (language plugin examples).tid
Outdated
Show resolved
Hide resolved
@linonetwo -- I thought you would have a closer look at the $:/core/modules/pluginswitcher.js, which is responsible to switch languages and themes. I intended to suggest that the new functionality of extracting and removing languages should be part of the pluginswitcher.js It is registered at startup.js but the main code is at /pluginswitcher.js
This mechanism makes sure that only those shadow tiddlers are in the shadow-store that are part of the activated language. IMO there is no reason to have all languages extracted from the plugins at startup. The number of language related shadow tiddlers should be constant and as low as possible. We only have 1 core plugin in this PR, but the number of shadow tiddlers is increased by 174 tiddlers. The menubar-plugin de-DE translations.multids only contains 25 new texts + readme it should be 26 shadows more and not 174. We have 33 possible languages in TW, which makes a total of 858 potential shadow tiddlers for menu-bar plugin alone. The current total number of TW shadow tiddlers is: 2476 for v5.3.5 -- With the implementation in this PR the menubar has the potential to increase the number of shadows by 35%, which will directly impact the core performance for every filter that uses I was not happy, that every possible language is part of the plugin (because of size). -- But OK -- It was decided, that that's not a problem. But I definitely think that only 1 set of language shadow tiddlers should be active at any given time. |
In current implementation, you can use another language plugin to provide l10n. See my last comment.
AFAIK It does this by calling
In |
I think it should not be needed to delete unused shadow tiddlers. Only used shadows should be extracted into the en-GB will need to be always extracted since it is the "source of truth" and the final fallback. If the $:/language setting is de-DE then the German tiddlers are extracted into the same namespace that en-GB already uses. So no extra shadows are ever created. The same should be true for every other language, that is part of a plugin. |
…Titles will be clear cause l10n lost
@pmario I've update the logic in The logic have to be called in I'm using |
I do like your translation concept better than my initial one, which is much more complex. 🥇 I think the plugin menubar/language/ directory should be renamed to The code and functionality looks good to me now.
I did miss that. That's a good thing. So we could decide to change the current behaviour if we want to, without compatibility problems. I did test:
|
@linonetwo -- Please keep my de-AT and de-DE translations within this PR. So it's easier to test. -- I'll close my menubar-translation PR. |
I was using But notice that core translations are like |
@Jermolene @pmario Is current implementation good enough? |
See my last post: #8435 (comment) |
@pmario isn't that updated in last commit? |
Would it be possible to seperate plugin language packs from plugin itself? IMO this can avoid unnecessary language to be installed and avoid the size of the tiddlywiki file be increased too much when the plugin has many languages. |
@Leilei332 Yes. Mario already ask same question above. |
@Jermolene Need decision on this, more and more my plugins are providing i18n, so need to decide a standard way for them before they are getting too much. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @linonetwo this PR is very important and I apologise for not giving it attention until now. I do think the approach is broadly correct. There's a lot of brittleness from the dependency on the plugin naming convention which I think we need to address (see below)
$tw.utils.each(pluginInfo[tiddler.fields.title].tiddlers,function(constituentTiddler,constituentTitle) { | ||
var constituentTiddlers = pluginInfo[tiddler.fields.title].tiddlers; | ||
$tw.utils.each(constituentTiddlers,function(constituentTiddler,constituentTitle) { | ||
// Skip translation tiddlers of plugin that follows `$:/plugins/xxx/yyy/languages/` pattern, will handle it later |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dependency on the format of plugin names is very brittle.
An option we should consider is to extend the plugin format to include languages in additional properties to the existing "tiddlers" property:
{
"tiddlers": {
"$:/config/DefaultColourMappings/menubar-foreground": {
"title": "$:/config/DefaultColourMappings/menubar-foreground",
"text": "#fff"
},
"$:/config/DefaultColourMappings/menubar-background": {
....
"languages": {
"en-GB": {
"$:/plugins/tiddlywiki/menubar/readme": {
"title": "$:/plugins/tiddlywiki/menubar/readme",
"text": "This is English"
}
},
"fr-FR": {
"$:/plugins/tiddlywiki/menubar/readme": {
"title": "$:/plugins/tiddlywiki/menubar/readme",
"text": "This is French"
}
}
}
On the Node.js side there would need to be some reorganisation so that the language tiddlers can be distinguished from the ordinary plugin tiddlers. One approach might be to allow plugins to opt-in to a new folder format where there is are "tiddlers" and "languages" folders within the plugin folder. Perhaps
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was reading science fiction novel A Deepness in the Sky last night, didn't saw this. I will put more time here in next few days.
One approach might be to allow plugins to opt-in to a new folder format where there is are "tiddlers" and "languages" folders within the plugin folder. Perhaps
You mean when dev plugins? But TiddlyWiki way is very flexible, use metadata to describe the tiddler, don't care the physical location of it. Is it worthwile to break this?
Adding a new "languages":
field in JSON plugin will mark them, but how do we know it belongs to there when packaging plugin JSON? By a field or file type? Then we can simply use this information in boot.js:1496.
I suggest add a new tiddler type text/vnd.tiddlywiki-language
or text/vnd.tiddlywiki-lingo
or text/vnd.tiddlywiki-localization
or text/vnd.tiddlywiki-l10n
, or lingo/vnd.tiddlywiki
(This is valid as GPT said...)
title: $:/anything/xxxx/yyy/
type: text/vnd.tiddlywiki-localization
CloseAll/Button: close all
ColourPicker/Recent: Recent:
Don't need to add /language
or /languages
in the title, while this is still recommended so it looks organized in "tree" tab of plugin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, no, they can't have same title, otherwise they will overwrite each other in constituentTiddlers
.
Difference from language plugins, plugins are enabling every language tiddlers at the same time.
Then how about we keep the convension on $:/languages/
, have convension here will make the code and format much simpler.
title: $:/languages/en-GB/plugins/linonetwo/xxx/
CloseAll/Button: close all
ColourPicker/Recent: Recent:
so we can check them by prefix $:/languages/
, instead of constituentTitle.split('/')[4] === 'languages'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or we can remove the "skip" here, it only reduce the count of shadow tiddlers. It doesn't matter if we still load them. Or use type: text/vnd.tiddlywiki-localization
to skip them, but we keep the old logic in core/modules/language.js
, do you like the logic modified in core/modules/language.js
?
(But have to admit that I'm afriad of modifying plugin packing and unpacking so I said this.)
I'm developing a game that learn the tiddler concept of TW, in that game each plugin can declare If it also works here, I can know language plugin
And push these tiddlers right into Just sharing my design in my system here, not intended to introduce it to TW. |
@Jermolene I think we also need to consider i18n for doc site. (Here is an exmple) Current implementation make it impossible to create multiple version of documentation, I just aware of it, and can only switch plugin's language. So the way that adding new structure only on plugin will also weaken the i18n framework. While the old runtime way in #7821 works with doc site. Can we use a new field or new format (like TestCaseTiddlers) or new file extension (like BTW, can we edit |
Why make it complicated, if it can be easy. -- Just create 2 editions and link them. A CI/CD process can handle the automatic creation -- done. |
Sorry for being late.
This is based on logic in #7821 (comment)
Lingo macro keep untouched. Will handle fallback between de-AT and de-DE. fixes #8263
How it works:
$tw.wiki.unpackPluginTiddlers
will be called, and in this method we can modifyshadowTiddlers
, which will be used by$tw.wiki.getTiddler
from wikitext.$tw.utils.activatePluginTranslations
method inunpackPluginTiddlers
xxx/languages/
, and this PR make TW auto move it toxxx/language/
, so lingo macro works withxxx/language/
base path.unpackPluginTiddlers
will be called at least 3 times, wasting lots of repeated work inactivatePluginTranslations
. But I can't skip it, because it resetshadowTiddlers = Object.create(null);
each time it's called.Downside of this implementation is it not able to translate user wiki any more. Which was achieved by modify lingo macro in #7821: