Internationalization of xbps #647
Conversation
xbps-install - pt_BR * We still need to finish man xbps-install.1 for pt_BR.
|
Screenshot in pt_BR for xbps-install 
|
|
Example for Brazil: VoidBR - Void Linux adapted for Brazil https://plus.diolinux.com.br/t/voidbr-void-linux-adaptado-ao-brasil/76554 |
|
Although the Void Linux developers reject pull requests related to implementing pt_BR support, the functionality is operational, as demonstrated in the screenshot. It is recommended that the code be reviewed by a larger number of project maintainers, since the same mechanism can be extended to enable additional localizations. |
meator
left a comment
meator
left a comment
There was a problem hiding this comment.
To my knowledge, Void has never really tried to do i18n. If it were to begin now, there would be a lot of resources that would have to be adapted to i18n. I have reviewed your changes, but if I were you, I wouldn't try to fix all of the problems just yet, I would wait for a maintainer to show interest in i18n. Because right now I'm afraid that your changes won't be merged.
This PR doesn't handle the build system side of things. #350, #608 or a PR of equivalent functionality would have to be merged before _()ing all translatable strings and before writing translations.
Your changes are poorly formatted (why so many newlines?) and include non-English comments in code. Both of these things are not desirable.
Your PR only handles xbps-install. I assume it is a proof-of-concept of implementing translations in XBPS. Translations for everything would be preferable if this PR should be merged and released (but see my comment in the first paragraph before embarking on such journey).
My review was quick, I didn't check whether you _()-ed all appropriate strings in xbps-install. A proper transition to gettext would mean sieving through all user strings, adding _(), adding TRANSLATORS comments into appropriate places, sometimes changing the code more to adapt to gettext better or to have nicer input strings (that would later get formatted)...
To do things 100% correctly, code would have to be added to flip the xbps-install summary table in RTL languages. Other more intrusive changes might be also needed in some or all languages.
|
|
||
|
|
||
| setlocale(LC_ALL, ""); | ||
| bindtextdomain("xbps-install", "/usr/share/locale"); |
There was a problem hiding this comment.
You shouldn't hardcode paths like that. This would require tighter build system integration, see my other comments on that.
There was a problem hiding this comment.
I am also not 100% sure on this, but setlocale(LC_ALL, ""); can have unanticipated consequences in a codebase which was locale-unaware before. This could lead to non-English users seeing snippets of translated/localized strings in xbps-install while the rest of the strings remain in English. This can be distracting and it could be considered intrusive to non-English users who are used to English XBPS.
| rv = preset; | ||
| else if (response == 'y' || response == 'Y') | ||
|
|
||
| // Apenas acrescente S/s como equivalentes | ||
| else if (response == 'y' || response == 'Y' || response == 's' || response == 'S') | ||
| rv = true; | ||
| else if (response == 'n' || response == 'N') | ||
| rv = false; |
There was a problem hiding this comment.
This is not acceptable. If you really want to do this, the following might be better:
response = fgetc(stdin);
// TRANSLATORS: The following string isn't displayed to the user, it is used to
// determine what the user's response to a yes/no question means.
// The characters in this string should be the same characters
// as in the TODO string (without the separator, / or otherwise).
// Only the first character of the user's response is considered,
// for example, the default English version consideres "y", "ye",
// "yes" and "yn" the same. You therefore cannot include whole
// words here.
// TODO: One letter may occupy multiple bytes. I suspect that the
// current setup doesn't handle that.
// This string defines a positive response. See the string below
// for a negative one.
const char * positive_response = _("yY");
// TRANSLATORS: This string defines a negative response. See the translators
// comment for positive_response above for more info.
const char * negative_response = _("nN");
if (response == "\n")
rv = preset;
else if (strchr(positive_response, response) != NULL)
rt = true;
else if (strchr(negative_response, response) != NULL)
rt = false;
// ...| #include <libintl.h> | ||
| #include <locale.h> | ||
| #define _(STRING) gettext(STRING) |
There was a problem hiding this comment.
This should be put into a common header (without the locale.h header, which should be only needed in main()).
There was a problem hiding this comment.
Large chunks of this manpage remain in English.
There was a problem hiding this comment.
I have restored the fork so that it matches the original Void Linux repository.
Where should we start so that everyone can follow the process?
Should we begin with translating the manual pages for xbps-install and xbps-remove, and then submit separate pull requests for each command?
There was a problem hiding this comment.
Where should we start?
We should start by taking some time off. By asking the XBPS maintainers to merge this PR, you're indirectly asking them to develop and maintain i18n infrastructure for XBPS. I am not an XBPS maintainer, but I feel that such task would require some work and at least the approval of several XBPS maintainers.
XBPS doesn't receive that much development nowadays (which is great for stability). I consider it a more or less finished project (with a few bugfixes and added flags here and there). Adding such a large feature would require the consensus of several maintainers (in my opinion, which isn't worth much here, since I'm not an XBPS maintainer).
If you haven't already, visit the #xbps channel on Libera.Chat and ask around, you'll get more qualified opinions there.
2 participants
We still need to finish man xbps-install.1 for pt_BR.