keepassxreboot / keepassxc

KeePassXC is a cross-platform community-driven port of the Windows application “Keepass Password Safe”.
https://keepassxc.org/
Other
20.74k stars 1.44k forks source link

Improve translator documentation in CONTRIBUTING #10300

Open C0rn3j opened 7 months ago

C0rn3j commented 7 months ago

Entrys is obviously not a word, it would be nice if this was done properly instead and showed Entry/Entries based on the actual count, and the same for all the other words.

In English this is super simple:

In other languages this is more complex and has to be kept in mind if translations are meant to be perfect too, for example Czech:

And then there's declension.

While we are at it, there's currently inconsistent capitalization, to point out one case:
Delete Entry vs
Move entry

Outside of share/translations, all the occurrences in the current master for entry(s) are as follows:

# grep -Ri '(s)' # and then some additional manual filtering for comments and functions
src/cli/Analyze.cpp:            out << QObject::tr("Password for '%1' has been leaked %2 time(s)!", "", count).arg(path).arg(count) << endl;
src/cli/Clip.cpp:        lastLine = QObject::tr("Clearing the clipboard in %1 second(s)...", "", timeout).arg(timeout);
src/cli/Command.cpp:        err << QObject::tr("Missing positional argument(s).") << "\n\n";
src/core/PasswordHealth.cpp:        health->addScoreReason(QObject::tr("Password is used %1 time(s)", "", count).arg(QString::number(count)));
src/core/PasswordHealth.cpp:                health->addScoreReason(QObject::tr("Password expires in %1 day(s)", "", days).arg(days));
src/core/Tools.cpp:            return QObject::tr("over %1 year(s)", nullptr, years).arg(years);
src/core/Tools.cpp:            return QObject::tr("about %1 month(s)", nullptr, months).arg(months);
src/core/Tools.cpp:            return QObject::tr("%1 week(s)", nullptr, weeks).arg(weeks);
src/core/Tools.cpp:            return QObject::tr("%1 day(s)", nullptr, days).arg(days);
src/core/Tools.cpp:            return QObject::tr("%1 hour(s)", nullptr, hours).arg(hours);
src/core/Tools.cpp:        return QObject::tr("%1 minute(s)", nullptr, minutes).arg(minutes);
src/fdosecrets/objects/Service.cpp:            tr(R"(%n Entry(s) was used by %1)", "%1 is the name of an application", secrets.size())
src/format/KdbxXmlReader.cpp:        qWarning("KdbxXmlReader::readDatabase: found %d invalid group reference(s)", m_tmpParent->children().size());
src/format/KdbxXmlReader.cpp:        qWarning("KdbxXmlReader::readDatabase: found %d invalid entry reference(s)", m_tmpParent->children().size());
src/gui/Clipboard.cpp:        QObject::tr("Clearing the clipboard in %1 second(s)…", "", m_secondsToClear).arg(m_secondsToClear));
src/gui/DatabaseWidget.cpp:                    tr("Entries expiring within %1 day(s)", "", expirationOffset).arg(expirationOffset);
src/gui/EditWidgetIcons.cpp:        fileDialog()->getOpenFileNames(this, tr("Select Image(s)"), FileDialog::getLastDir("icons"), filter);
src/gui/EditWidgetIcons.cpp:            msg = tr("Successfully loaded %1 of %n icon(s)", "", filenames.size()).arg(numloaded);
src/gui/EditWidgetIcons.cpp:            msg += "\n" + tr("%n icon(s) already exist in the database", "", numexisting);
src/gui/EditWidgetIcons.cpp:            emit messageEditEntry(msg + "\n" + tr("The following icon(s) failed:", "", errornames.size()) + "\n"
src/gui/GuiTools.cpp:                prompt = QObject::tr("Do you really want to delete %n entry(s) for good?", "", entries.size());
src/gui/GuiTools.cpp:                                               QObject::tr("Delete entry(s)?", "", entries.size()),
src/gui/GuiTools.cpp:                prompt = QObject::tr("Do you really want to move %n entry(s) to the recycle bin?", "", entries.size());
src/gui/GuiTools.cpp:                                               QObject::tr("Move entry(s) to recycle bin?", "", entries.size()),
src/gui/GuiTools.cpp:                            "Entry \"%1\" has %2 reference(s). "
src/gui/MainWindow.cpp:            m_ui->actionEntryRestore->setText(tr("Restore Entry(s)", "", dbWidget->numberOfSelectedEntries()));
src/gui/MainWindow.cpp:            m_ui->actionEntryRestore->setToolTip(tr("Restore Entry(s)", "", dbWidget->numberOfSelectedEntries()));
src/gui/MainWindow.cpp:        m_statusBarLabel->setText(tr("%1 Entry(s)", "", numEntries).arg(numEntries));
src/gui/MainWindow.ui:    <string notr="true" extracomment="Translatable string with plural form set in CPP file">Restore Entry(s)</string>
src/gui/TotpDialog.cpp:    m_ui->timerLabel->setText(tr("Expires in <b>%n</b> second(s)", "", m_step - (epoch % m_step)));
src/gui/csvImport/CsvImportWidget.cpp:        m_ui->messageWidget->showMessage(tr("Error(s) detected in CSV file!").append("\n").append(formatStatusText()),
src/gui/csvImport/CsvImportWidget.cpp:        return text.section('\n', 0, 1).append("\n").append(tr("[%n more message(s) skipped]", "", items - 2));
src/gui/csvImport/CsvParserModel.cpp:                  .arg(tr("%n byte(s)", nullptr, getFileSize()),
src/gui/csvImport/CsvParserModel.cpp:                       tr("%n row(s)", nullptr, getCsvRows()),
src/gui/csvImport/CsvParserModel.cpp:                       tr("%n column(s)", nullptr, qMax(0, getCsvCols() - 1))));
src/gui/dbsettings/DatabaseSettingsWidgetBrowser.cpp:                            tr("Successfully removed %n encryption key(s) from KeePassXC settings.", "", count),
src/gui/dbsettings/DatabaseSettingsWidgetBrowser.cpp:                                tr("Successfully removed permissions from %n entry(s).", "", counter),
src/gui/dbsettings/DatabaseSettingsWidgetEncryption.cpp:    m_ui->parallelismSpinBox->setSuffix(tr(" thread(s)", "Threads for parallel execution (KDF settings)", value));
src/gui/dbsettings/DatabaseSettingsWidgetMaintenance.cpp:        this, tr("Purged Unused Icons"), tr("Purged %n icon(s) from the database.", "", purgeCounter), MessageBox::Ok);
src/gui/dbsettings/DatabaseSettingsWidgetMaintenance.ui:           <string>Delete selected icon(s)</string>
src/gui/entry/EditEntryWidget.cpp:    expirePresetsMenu->addAction(tr("%n hour(s)", nullptr, 12))->setData(QVariant::fromValue(TimeDelta::fromHours(12)));
src/gui/entry/EditEntryWidget.cpp:    expirePresetsMenu->addAction(tr("%n hour(s)", nullptr, 24))->setData(QVariant::fromValue(TimeDelta::fromHours(24)));
src/gui/entry/EditEntryWidget.cpp:    expirePresetsMenu->addAction(tr("%n week(s)", nullptr, 1))->setData(QVariant::fromValue(TimeDelta::fromDays(7)));
src/gui/entry/EditEntryWidget.cpp:    expirePresetsMenu->addAction(tr("%n week(s)", nullptr, 2))->setData(QVariant::fromValue(TimeDelta::fromDays(14)));
src/gui/entry/EditEntryWidget.cpp:    expirePresetsMenu->addAction(tr("%n week(s)", nullptr, 3))->setData(QVariant::fromValue(TimeDelta::fromDays(21)));
src/gui/entry/EditEntryWidget.cpp:    expirePresetsMenu->addAction(tr("%n month(s)", nullptr, 1))->setData(QVariant::fromValue(TimeDelta::fromMonths(1)));
src/gui/entry/EditEntryWidget.cpp:    expirePresetsMenu->addAction(tr("%n month(s)", nullptr, 2))->setData(QVariant::fromValue(TimeDelta::fromMonths(2)));
src/gui/entry/EditEntryWidget.cpp:    expirePresetsMenu->addAction(tr("%n month(s)", nullptr, 3))->setData(QVariant::fromValue(TimeDelta::fromMonths(3)));
src/gui/entry/EditEntryWidget.cpp:    expirePresetsMenu->addAction(tr("%n month(s)", nullptr, 6))->setData(QVariant::fromValue(TimeDelta::fromMonths(6)));
src/gui/entry/EditEntryWidget.cpp:    expirePresetsMenu->addAction(tr("%n year(s)", nullptr, 1))->setData(QVariant::fromValue(TimeDelta::fromYears(1)));
src/gui/entry/EditEntryWidget.cpp:    expirePresetsMenu->addAction(tr("%n year(s)", nullptr, 2))->setData(QVariant::fromValue(TimeDelta::fromYears(2)));
src/gui/entry/EditEntryWidget.cpp:    expirePresetsMenu->addAction(tr("%n year(s)", nullptr, 3))->setData(QVariant::fromValue(TimeDelta::fromYears(3)));
src/gui/entry/EntryAttachmentsWidget.cpp:                                       tr("Are you sure you want to remove %n attachment(s)?", "", indexes.count()),
src/gui/entry/EntryAttachmentsWidget.cpp:        errorMessage = tr("Unable to open file(s):\n%1", "", errors.size()).arg(errors.join('\n'));
src/gui/entry/EntryView.cpp:            listWidget.addItem(tr("+ %1 entry(s)...", nullptr, remaining).arg(remaining));
src/gui/reports/ReportsWidgetBrowserStatistics.cpp:    const auto delEntry = new QAction(icons()->icon("entry-delete"), tr("Delete Entry(s)…", "", selected.size()), this);
src/gui/reports/ReportsWidgetHealthcheck.cpp:    const auto delEntry = new QAction(icons()->icon("entry-delete"), tr("Delete Entry(s)…", "", selected.size()), this);
src/gui/reports/ReportsWidgetHibp.cpp:    const auto delEntry = new QAction(icons()->icon("entry-delete"), tr("Delete Entry(s)…", "", selected.size()), this);
src/gui/reports/ReportsWidgetPasskeys.cpp:    const auto delEntry = new QAction(icons()->icon("entry-delete"), tr("Delete Entry(s)…", "", selected.size()), this);
src/main.cpp:        "filename(s)", QObject::tr("filenames of the password databases to open (*.kdbx)"), "[filename(s)]");
droidmonkey commented 7 months ago

This is how plurals are translated with qt

C0rn3j commented 7 months ago

Indeed, Qt has a specific rule set for each plural form case, which makes this all infinitely easier, and keepassxc apparently attempts to use it.

For example, English rule 1 would be entry, rule 2 would be entries.

The code for this case is:

src/gui/MainWindow.cpp:        
m_statusBarLabel->setText(tr("%1 Entry(s)", "", numEntries).arg(numEntries));

Which seems to conflict with the documentation, where it seems like it should be:

src/gui/MainWindow.cpp:        
m_statusBarLabel->setText(tr("%n Entry(s)", "", numEntries).arg(numEntries));

~~In which case this is still a valid bug report, even though I haven't made my issue clear in the initial post.
It shows up literally on my system, for example in search in the main window, please reopen :)~~

image

KeePassXC - Version 2.7.6
Revision: dd21def

Qt 5.15.12
Debugging mode is disabled.

Operating system: Arch Linux
CPU architecture: x86_64
Kernel: linux 6.7.1-arch1-1

Enabled extensions:
- Auto-Type
- Browser Integration
- SSH Agent
- KeeShare
- YubiKey
- Secret Service Integration

Cryptographic libraries:
- Botan 3.2.0

Might want to add system locale to the debug tab, seems relevant

[0] % localectl                              
System Locale: LANG=en_GB.UTF-8
droidmonkey commented 7 months ago

Sorry but you need to do more work on your end. Go to https://transifex.com/keepassxc/keepassxc to translate your language. There you can also download the latest translation file for your language.

As an aside, Qt uses "%1, %2, %3" etc for fill in placeholders. It will also recognize ansi C placeholders like %n but those are far less flexible.

C0rn3j commented 7 months ago

Sorry but you need to do more work on your end. Go to https://transifex.com/keepassxc/keepassxc to translate your language.

Gave that a shot, my example was already translated months ago with no review, which wouldn't matter anyway as keepassxc's last release is from 2023-08.

Wouldn't it make sense to default missing translations for en_ variants that contain %n or %1 in the string to the en base, instead of showing the source code variants which look like real words with broken grammar?

droidmonkey commented 7 months ago

The en translation is default, this one wasn't translated though.

phoerious commented 7 months ago

If you see (s) in the interface, then the en-US or en-GB translation for that string is missing. You need to fix that on Transifex, not in the code.

C0rn3j commented 7 months ago

Right, the default source en translation is indeed source, with no translated variants, so there's no variants to take from it.

One way would be to fill in en_US from en_GB and vice versa for missing messages with %, but I am getting the vibe that wouldn't be welcome and you'd rather get translators.

https://github.com/keepassxreboot/keepassxc/blob/develop/.github/CONTRIBUTING.md#translations

This is pretty bare, how does one request the permissions to Review things?
There are %n replaces that are hanging in the limbo for 2 years+ for en_UK.

Why would I choose to translate a specific version over develop and vice versa, is there any duplication between them?

It would be very nice if KeePassXC informed the user that the translation for their language is incomplete and some things may look odd as a result, and link them to the translation contributing section.
I had no clue that the English-developed app I use does not have string translations for English, for years.

Some basic explanation about %1/%n would be welcome, took me a while to even figure the 1 and other are clickable and that they are the countable rules.

image

image

droidmonkey commented 7 months ago

We don't control transifex so you would need to read the docs of the website before working on it. (Referencing your comment on the 1 vs other button)

Regarding %1/%n, that has absolutely nothing to do with plurals. That is a replacement schema used in Qt/C/C++ for injecting fill-ins. The plural aspect actually comes from the (s) suffix within the string. The plural choice (1 or other) comes from the third parameter in the tr(...) function.

With all that said, you don't need to know any of this. Just go to Transifex and start translating. We prefer you start on the non-develop set since that is closest to release. Most translations automatically carry over to the other set (that is a Transifex feature).

If you want to be a reviewer for one or more languages, request that through Transifex. Again, read the docs.

C0rn3j commented 7 months ago

Regarding %1/%n, that has absolutely nothing to do with plurals. That is a replacement schema used in Qt/C/C++ for injecting fill-ins. The plural aspect actually comes from the (s) suffix within the string. The plural choice (1 or other) comes from the third parameter in the tr(...) function.

With all that said, you don't need to know any of this.

In which case, people will be confused and end up wasting time making pointless bug reports that would be completely prevented by informing the user better.

I am not alone, just clicking through the % forms shows that other people are confused too:

image

We don't control transifex so you would need to read the docs of the website before working on it

I suggested improving existing KeePassXC's documentation, not Transifex's.
I am still suggesting that.

With all that said, you don't need to know any of this. Just go to Transifex and start translating.

If you want to be a reviewer for one or more languages, request that through Transifex. Again, read the docs.

Alright... Strings I care about are translated already, just need to be reviewed, let's see how to do that.
https://help.transifex.com/en/collections/3441044-starting-with-the-basics

Alright, let's read the doc for reviewing translations...
https://help.transifex.com/en/articles/6318604-reviewing-translations

"Only Admins, Project Maintainers, Language Coordinators, and Reviewers can review and edit reviewed strings."

Great, how do I apply to become a Reviewer again?

Who knows.

Judging by https://help.transifex.com/en/?q=reviewer, it's YOUR job to add Reviewers and there is no way to request it on my side other than asking.

Another funny thing, when attempting to use the Join Team button, one gets this notice:

"NOTE: Please do not mark translations as reviewed. This prevents other users from editing them later on!"

And then you see random languages are half reviewed, some 100% and some 0%.

What gives? Who knows.
Does KeePassXC even care about the review status? Who knows.

image

We prefer you start on the non-develop set since that is closest to release. Most translations automatically carry over to the other set (that is a Transifex feature).

I do not feel like enough information about translating is available on KeePassXC's side, making the translations incomplete even in the main language of the app.

Do you genuinely think it would be a bad idea to fill in the translation contribution short document a bit, to get people started easier?
Linking Transifex documentation where relevant is fine, but having no explanation other than "read the docs" is making this extremely frustrating to work with.

droidmonkey commented 7 months ago

The review status has no bearing on whether translations make it into a release. That's just a nice to have transifex feature that enables translators to control the translation a little more.

I'll add some meat to the contributing docs