-
Notifications
You must be signed in to change notification settings - Fork 94
Dictionary mode #519
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
base: master
Are you sure you want to change the base?
Dictionary mode #519
Conversation
|
This is a huge set of changes, can you describe what it does? It changes both localisations and logic and presentation and build, are these changes all related to one feature? |
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 config can't be merged as-is. Don't change the default URL, mode, name, or color.
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.
@vykliuk, yeah, revert your changes to the config file in this branch/PR, and just keep them locally.
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.
This was apparently entirely inadvertent. I've reverted it at @vykliuk's request.
It's for Paradigm Dictionary mode: |
| fs.writeFileSync(INDEX_FILE, content); | ||
| } | ||
|
|
||
| generateRegistry(); |
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.
This seems to auto-create an index.ts which refers to all the files in langs, but if I read it correctly, the generated file will just contain something like
import Dictionary from './Dictionary';
import { spaPlugin } from './langs/spa';
import { uumPlugin } from './langs/uum';
import { LanguagePlugin } from './types';
export const languageRegistry: Record<string, LanguagePlugin> = {
"spa": spaPlugin,
"uum": uumPlugin,
};
export default Dictionary;
I understand that this build step is here to avoid having to remember to do
+import { tatPlugin } from './langs/tat';
…
+ "tat": tatPlugin,on adding Tatar etc., but OTOH it adds some complexity to the build. Do we expect lots of langs?
(Does the regular localisation code also do this kind of thing?)
|
I haven't looked very deeply into the react code, @sushain97 knows that better, but I think from just reading it, it looks good to me, the new stuff seems pretty self-contained. I would prefer squashing into one commit for the Dictionary stuff and one for the fairly unrelated src/strings updates – or just not including the src/strings updates if they're just from running |
No description provided.