-
Notifications
You must be signed in to change notification settings - Fork 64
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
i18n #77
Conversation
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.
Looks good so far, dropped a few suggestions/opinions here and there 👍
except KeyError: | ||
response = self.responses["en"][item] | ||
print(f"Could not find a translation ({self.language}) for the requested i18n item: {item}. Please file an issue on GitHub.") | ||
response = response.replace("{prefix}", self.prefix) |
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.
Feels like this should be handled in the .format
elsewhere, seems odd to only handle this replacement here.
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 it like this because it's the only var that is fixed and present across several strings.
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.
Wouldn't it feel redundant to .format(prefix) everywhere?
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.
True, although I don't like the concept of one variable being handled differently from the others, might get confusing down the line
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.
Yeah, I can see your point too.
I will leave this unresolved for now while I think about it.
Co-authored-by: Martin Stone <1611702+d7415@users.noreply.github.com>
Ready for review, working on a translation and the fairly simple rl!language command. |
Implement i18n module for easy l10n with json files.
Closes #74