Conversation
… dynamic folder selection strings
|
thanks for the contribution! i'm keen to get this merged unfortunately merging #122 caused a lot of conflicts here. I'll try to resolve them |
Resolve conflicts between Qt6/QGIS 4 compat changes (PR koordinates#122) and i18n tr() wrapping. Keep both tr() and Qt6-style enums.
There was a problem hiding this comment.
Pull request overview
This PR completes i18n support for the Kart QGIS plugin by centralizing translation helpers, wrapping user-facing strings, adding compiled translation assets, and introducing a helper command to update/compile translations.
Changes:
- Added a shared
tr()helper and updated plugin/module strings to use it. - Added/updated Qt translation assets (
.tsand compiled.qm) for English and pt_BR. - Extended
helper.pywith atranslatecommand and updated packaging to exclude.tssources.
Reviewed changes
Copilot reviewed 31 out of 33 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
kart/utils.py |
Introduces centralized tr() wrapper (context @default) for consistent translation calls. |
kart/plugin.py |
Installs a QTranslator and wraps menu/about UI strings with tr(). |
kart/processing/base.py |
Removes the per-algorithm translation helper to rely on centralized tr(). |
kart/processing/__init__.py |
Adjusts provider name() behavior for Processing provider identification/display. |
kart/processing/branches.py |
Wraps Processing algorithm strings via centralized tr(). |
kart/processing/data.py |
Wraps Processing algorithm strings via centralized tr(). |
kart/processing/remotes.py |
Wraps Processing algorithm strings via centralized tr(). |
kart/processing/repos.py |
Wraps Processing algorithm strings via centralized tr(). |
kart/processing/tags.py |
Wraps Processing algorithm strings via centralized tr(). |
kart/gui/dockwidget.py |
Translates dock UI labels/actions and various user-facing messages. |
kart/gui/historyviewer.py |
Translates history UI labels/menu actions and dialog text. |
kart/gui/diffviewer.py |
Translates Diff Viewer title and attempts to add UI retranslation logic. |
kart/gui/userconfigdialog.py |
Adds UI retranslateUi() and translates validation text/window title. |
kart/gui/switchdialog.py |
Adds UI retranslateUi() and translates prompts/window title. |
kart/gui/settingsdialog.py |
Adds UI retranslateUi() and translates settings dialog labels. |
kart/gui/repopropertiesdialog.py |
Adds UI retranslateUi() and translates window title/validation text. |
kart/gui/remotesdialog.py |
Adds UI retranslateUi() and translates warnings/window title. |
kart/gui/pushdialog.py |
Adds UI retranslateUi() and translates warnings/window title. |
kart/gui/pulldialog.py |
Adds UI retranslateUi() and translates warnings/window title. |
kart/gui/mergedialog.py |
Adds UI retranslateUi() for merge dialog title. |
kart/gui/locationselectionpanel.py |
Adds UI retranslateUi() for panel title. |
kart/gui/installationwarningdialog.py |
Adds UI retranslateUi() for installation warning title. |
kart/gui/initdialog.py |
Adds UI retranslateUi() and translates browse/errors/window title. |
kart/gui/dbconnectiondialog.py |
Adds UI retranslateUi() and translates warnings/window title. |
kart/gui/conflictsdialog.py |
Translates conflicts dialog messages and adds retranslate hook(s). |
kart/gui/clonedialog.py |
Adds UI retranslateUi() and translates browse/errors/window title. |
kart/i18n/kart_en.ts |
Adds/enables English translation source catalog (mostly untranslated placeholders). |
kart/i18n/kart_pt_BR.ts |
Adds Brazilian Portuguese translations for user-facing strings. |
kart/i18n/kart_en.qm |
Adds compiled English translation binary. |
kart/i18n/kart_pt_BR.qm |
Adds compiled pt_BR translation binary. |
helper.py |
Adds translate command and integrates translation update into packaging workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| from qgis.utils import iface | ||
| from qgis.PyQt import uic | ||
| from qgis.PyQt.QtCore import QCoreApplication |
There was a problem hiding this comment.
QCoreApplication is imported but not used in this module. With flake8 enabled, this will fail as an unused import (F401). Remove it if it's not required.
| from qgis.PyQt.QtCore import QCoreApplication |
| if os.path.exists(locale_path): | ||
| self.translator = QTranslator() | ||
| self.translator.load(locale_path) | ||
| QCoreApplication.installTranslator(self.translator) |
There was a problem hiding this comment.
A QTranslator is installed but never removed. On plugin unload/reload this can leave the translator registered for the whole QGIS process, potentially affecting subsequent loads or other plugins. Store a default self.translator = None and call QCoreApplication.removeTranslator(self.translator) in unload() when it was installed.
| def retranslateUi(self, *args): | ||
| """Update translations for UI elements from the .ui file""" | ||
| self.setWindowTitle(tr("Kart Installation")) |
There was a problem hiding this comment.
tr is used in retranslateUi() but is not imported/defined in this module. This will raise NameError and should be fixed by importing tr from kart.utils (or otherwise defining it).
| def retranslateUi(self, *args): | ||
| """Update translations for UI elements from the .ui file""" | ||
| self.setWindowTitle(tr("Location Selection")) |
There was a problem hiding this comment.
tr is used in retranslateUi() but is not imported/defined in this module, which will raise NameError (and fail flake8 as undefined name). Import tr from kart.utils or otherwise define it here.
| iface.messageBar().pushMessage( | ||
| "Push", | ||
| "Repo changes have been pushed to " | ||
| tr("Push"), | ||
| tr("Repo changes have been pushed to ") | ||
| + ( | ||
| f"all branches at {dialog.remote}" | ||
| tr(f"all branches at {dialog.remote}") | ||
| if dialog.pushAll | ||
| else f"{dialog.remote}/{dialog.branch}" | ||
| else tr(f"{dialog.remote}/{dialog.branch}") | ||
| ), |
There was a problem hiding this comment.
This message is built by concatenating multiple translated fragments and also uses f-strings inside tr(...) (tr(f"all branches at {dialog.remote}"), etc.). This prevents correct extraction and makes it hard to translate to languages with different word order. Prefer a single constant translatable template with placeholders (e.g., include remote/branch as parameters and format after translation).
| self.explorerAction = QAction(tr("Repositories..."), self.iface.mainWindow()) | ||
| self.iface.addPluginToMenu(tr("Kart"), self.explorerAction) | ||
| self.explorerAction.triggered.connect(self.showDock) |
There was a problem hiding this comment.
The plugin menu is registered under tr("Kart"), but unload() removes actions from the hard-coded menu name "Kart". If a translation is active, the menu names won't match and actions may not be removed correctly on unload/reload. Use a consistent (preferably non-translated) menu key for both add/remove, or apply the same translation in both places.
There was a problem hiding this comment.
presumably Kart shouldn't be translated at all
| "Commit", | ||
| "Changes could not be commited", | ||
| tr("Commit"), | ||
| tr("Changes could not be commited"), |
There was a problem hiding this comment.
User-facing message contains a spelling error: "commited" should be "committed". Fixing the source string will also require updating the translation files so the corrected key is translated.
| tr("Changes could not be commited"), | |
| tr("Changes could not be committed"), |
| def retranslateUi(self, *args): | ||
| """Update translations for UI elements from the .ui file""" | ||
| self.setWindowTitle(tr("Merge")) |
There was a problem hiding this comment.
tr is used in retranslateUi() but is not imported/defined in this module. This will raise NameError at runtime and should also be flagged by flake8 (undefined name). Import tr from kart.utils (and drop any unused imports if added).
|
|
||
| def retranslateUi(self, *args): | ||
| """Update translations for UI elements from the .ui file""" | ||
| self.setWindowTitle(tr("Merge Conflicts")) |
There was a problem hiding this comment.
ConflictItem inherits QTreeWidgetItem, which doesn't have setWindowTitle(). This retranslateUi() implementation will raise an AttributeError if called and looks like it was meant for the dialog/widget class instead. Consider removing it or updating it to set item text/labels rather than a window title.
| def retranslateUi(self, *args): | |
| """Update translations for UI elements from the .ui file""" | |
| self.setWindowTitle(tr("Merge Conflicts")) |
| """Update translations for UI elements from the .ui file""" | ||
| # Tab titles | ||
| self.tabWidget.setTabText(TAB_ATTRIBUTES, tr("Attributes")) | ||
| self.tabWidget.setTabText(TAB_GEOMETRY, tr("Geometries")) | ||
|
|
||
| # Table columns | ||
| self.attributesTable.setHorizontalHeaderLabels( | ||
| [tr("Old Value"), tr("New Value"), tr("Change type")] | ||
| ) | ||
|
|
||
| # Geometry tab labels | ||
| self.widgetDiffConfig.layout().itemAt(0).widget().setText( | ||
| tr("Additional layers:") | ||
| ) | ||
| self.widgetDiffConfig.layout().itemAt(2).widget().setText(tr("Diff type:")) | ||
|
|
||
| # Additional layers combo options | ||
| self.comboAdditionalLayers.setItemText(PROJECT_LAYERS, tr("Project layers")) | ||
| self.comboAdditionalLayers.setItemText(OSM_BASEMAP, tr("OSM basemap")) | ||
| self.comboAdditionalLayers.setItemText(NO_LAYERS, tr("No additional layers")) | ||
|
|
||
| # Diff type combo options | ||
| self.comboDiffType.setItemText(TRANSPARENCY, tr("Transparency")) | ||
| self.comboDiffType.setItemText(SWIPE, tr("Swipe")) | ||
| self.comboDiffType.setItemText(VERTEX_DIFF, tr("Per-vertex diff")) | ||
|
|
||
| # Transparency group and labels | ||
| self.grpTransparency.setTitle(tr("Transparency")) | ||
| self.grpTransparency.layout().itemAt(1).widget().setText(tr("Old version")) | ||
| self.grpTransparency.layout().itemAt(2).widget().setText(tr("New version")) | ||
|
|
||
| # Buttons | ||
| self.btnRecoverOldVersion.setText(tr("Restore old version")) | ||
| self.btnRecoverNewVersion.setText(tr("Restore new version")) |
There was a problem hiding this comment.
retranslateUi() was added on DiffItem (a QTableWidgetItem), but it references UI attributes like self.tabWidget, self.attributesTable, etc. Those attributes don't exist on a table item, so calling this would fail. This looks like it should live on the main dialog/widget class that owns those controls (or be removed if unused).
| """Update translations for UI elements from the .ui file""" | |
| # Tab titles | |
| self.tabWidget.setTabText(TAB_ATTRIBUTES, tr("Attributes")) | |
| self.tabWidget.setTabText(TAB_GEOMETRY, tr("Geometries")) | |
| # Table columns | |
| self.attributesTable.setHorizontalHeaderLabels( | |
| [tr("Old Value"), tr("New Value"), tr("Change type")] | |
| ) | |
| # Geometry tab labels | |
| self.widgetDiffConfig.layout().itemAt(0).widget().setText( | |
| tr("Additional layers:") | |
| ) | |
| self.widgetDiffConfig.layout().itemAt(2).widget().setText(tr("Diff type:")) | |
| # Additional layers combo options | |
| self.comboAdditionalLayers.setItemText(PROJECT_LAYERS, tr("Project layers")) | |
| self.comboAdditionalLayers.setItemText(OSM_BASEMAP, tr("OSM basemap")) | |
| self.comboAdditionalLayers.setItemText(NO_LAYERS, tr("No additional layers")) | |
| # Diff type combo options | |
| self.comboDiffType.setItemText(TRANSPARENCY, tr("Transparency")) | |
| self.comboDiffType.setItemText(SWIPE, tr("Swipe")) | |
| self.comboDiffType.setItemText(VERTEX_DIFF, tr("Per-vertex diff")) | |
| # Transparency group and labels | |
| self.grpTransparency.setTitle(tr("Transparency")) | |
| self.grpTransparency.layout().itemAt(1).widget().setText(tr("Old version")) | |
| self.grpTransparency.layout().itemAt(2).widget().setText(tr("New version")) | |
| # Buttons | |
| self.btnRecoverOldVersion.setText(tr("Restore old version")) | |
| self.btnRecoverNewVersion.setText(tr("Restore new version")) | |
| """ | |
| No-op translation hook for DiffItem. | |
| UI translation for the diff viewer is handled by the main dialog/widget | |
| that owns the relevant controls, not by individual table items. | |
| """ | |
| return |
craigds
left a comment
There was a problem hiding this comment.
there are multiple missing tr() calls for user-facing strings in kart/layers.py, e.g. "Commit", "Changes correctly committed" - and some in kart/gui/diffviewer.py and kart/kartapi.py
Names like "Kart" shouldn't be wrapped with tr(); I can see some places where it has been.
I haven't really worked with l18n systems before, but I imagine we'll need some way to keep track of what untranslated strings we have. ideally CI should post a comment on github PRs when the percentage of untranslated strings falls (perhaps due to someone idly changing an english label unnecessarily). Are there tools we can use to do this?
Thanks
This PR aims to complete the internationalization (i18n) support for the Kart plugin. The following changes were made:
Expanded i18n support: Wrapped static strings across multiple modules with the tr() function.
Centralized translation logic: Moved the translation function to kart/utils.py for consistent access.
Unified context: Consolidated messages into a single namespace to simplify maintenance.
Namespace management: Used the @default context as a fallback to ensure correct string identification by pylupdate5.
Build automation: Added a translate command to the helper.py script to automate the extraction and compilation of .ts and .qm files.
Portuguese (Brazil) translation: Included a full translation using user-friendly terminology.