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

Fixed wrongly used plural form #1166

Merged
merged 1 commit into from
Apr 24, 2024
Merged

Fixed wrongly used plural form #1166

merged 1 commit into from
Apr 24, 2024

Conversation

lslezak
Copy link
Contributor

@lslezak lslezak commented Apr 24, 2024

Problem

  • Merging the translation from Weblate failed with
    web/po/ca.po:836: number of format specifications in 'msgid_plural' and 'msgstr[0]' does not match
    web/po/ca.po:848: number of format specifications in 'msgid_plural' and 'msgstr[0]' does not match
    web/po/ca.po:8[5](https://github.com/openSUSE/agama/actions/runs/8797558854/job/24142706299#step:9:6)9: number of format specifications in 'msgid_plural' and 'msgstr[0]' does not match
    web/po/ca.po:872: number of format specifications in 'msgid_plural' and 'msgstr[0]' does not match
    msgfmt: found 4 fatal errors
    
  • See https://github.com/openSUSE/agama/actions/runs/8797558854/job/24142706299#step:9:10

Details

There are actually two problems.

Building Translated Texts

Building translated strings should be avoided. Example:

const vg = _("Logical Volume Manager (LVM) volume group");
if (...) {return sprintf(n_( "Install in a new %1$s..."), vg)};
if (...) {return sprintf(n_( "Install in a new %1$s..."), vg)};

The problem is that the translator does not see the whole text and the translated parts then might not fit well together.
Another problem is that the text around might change the context and the injected message might need a different translation. But in this case that is not possible.

It is better to always use the full texts. It might seem annoying to have several similar strings but more important is that the translator has full control for the final text and each of them can be translated slightly differently if needed.

Wrongly Used Plural Form

This is a more serious problem. The code displays a different text for LVM with a single PV and a different text for LVM with multiple PV. That makes sense, if there is one PV we can display it in the text, but there can be a dozen of PVs and then the text would be too long.

But the problem is that it used a gettext plural form n_() function for that logic.

n_("Install in a new %1$s on %2$s shrinking...", "Install in a new %1$s shrinking...", num)

Then gettext complains about the different number of format specifiers. But the real problem is elsewhere.

⚠️ There are languages which do not differ between singular and plural forms, they have one form for both. For example Japanese or Vietnamese. In that case you will always get the singular form even for num >= 2!! ⚠️

When using n_() function both texts should be the same, the only difference should be singular/plural form for some words!

Solution

Refactor the code to use the simple _() translation and put the logic into the code.

Testing

  • Tested manually

Screenshots

storage_summary

@coveralls
Copy link

Coverage Status

coverage: 74.804% (-0.02%) from 74.825%
when pulling afd4ebe on plural_form_fix
into d141f86 on master.

@lslezak lslezak merged commit 4036658 into master Apr 24, 2024
2 checks passed
@lslezak lslezak deleted the plural_form_fix branch April 24, 2024 08:57
@imobachgs imobachgs mentioned this pull request May 17, 2024
imobachgs added a commit that referenced this pull request May 17, 2024
Prepare for releasing Agama 8. It includes the following pull requests:

* #884
* #886
* #914
* #918
* #956
* #957
* #958
* #959
* #960
* #961
* #962
* #963
* #964
* #965
* #966
* #969
* #970
* #976
* #977
* #978
* #979
* #980
* #981
* #983
* #984
* #985
* #986
* #988
* #991
* #992
* #995
* #996
* #997
* #999
* #1003
* #1004
* #1006
* #1007
* #1008
* #1009
* #1010
* #1011
* #1012
* #1014
* #1015
* #1016
* #1017
* #1020
* #1022
* #1023
* #1024
* #1025
* #1027
* #1028
* #1029
* #1030
* #1031
* #1032
* #1033
* #1034
* #1035
* #1036
* #1038
* #1039
* #1041
* #1042
* #1043
* #1045
* #1046
* #1047
* #1048
* #1052
* #1054
* #1056
* #1057
* #1060
* #1061
* #1062
* #1063
* #1064
* #1066
* #1067
* #1068
* #1069
* #1071
* #1072
* #1073
* #1074
* #1075
* #1079
* #1080
* #1081
* #1082
* #1085
* #1086
* #1087
* #1088
* #1089
* #1090
* #1091
* #1092
* #1093
* #1094
* #1095
* #1096
* #1097
* #1098
* #1099
* #1100
* #1102
* #1103
* #1104
* #1105
* #1106
* #1109
* #1110
* #1111
* #1112
* #1114
* #1116
* #1117
* #1118
* #1119
* #1120
* #1121
* #1122
* #1123
* #1125
* #1126
* #1127
* #1128
* #1129
* #1130
* #1131
* #1132
* #1133
* #1134
* #1135
* #1136
* #1138
* #1139
* #1140
* #1141
* #1142
* #1143
* #1144
* #1145
* #1146
* #1147
* #1148
* #1149
* #1151
* #1152
* #1153
* #1154
* #1155
* #1156
* #1157
* #1158
* #1160
* #1161
* #1162
* #1163
* #1164
* #1165
* #1166
* #1167
* #1168
* #1169
* #1170
* #1171
* #1172
* #1173
* #1174
* #1175
* #1177
* #1178
* #1180
* #1181
* #1182
* #1183
* #1184
* #1185
* #1187
* #1188
* #1189
* #1190
* #1191
* #1192
* #1193
* #1194
* #1195
* #1196
* #1198
* #1199
* #1200
* #1201
* #1203
* #1204
* #1205
* #1206
* #1207
* #1208
* #1209
* #1210
* #1211
* #1212
* #1213
* #1214
* #1215
* #1216
* #1217
* #1219
* #1220
* #1221
* #1222
* #1223
* #1224
* #1225
* #1226
* #1227
* #1229
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants