[PATH] Fix bug with translated UI #5242

sliptonic · 2021-12-09T16:23:48Z

https://forum.freecadweb.org/viewtopic.php?f=15&t=64336
Many files were touched in this PR.
All the Black reformatting was consolidated to a single commit.
Many of the .UI files have had certain comboboxes marked not translatable to avoid wasted effort. All these changes are in a single commit.
The remaining commits each fix a single module with the currentText/currentData bug.

Thank you for creating a pull request to contribute to FreeCAD! To ease integration, we ask you to conform to the following items. Pull requests which don't satisfy all the items below might be rejected. If you are in doubt with any of the items below, don't hesitate to ask for help in the FreeCAD forum!

Your pull request is confined strictly to a single module. That is, all the files changed by your pull request are either in App, Base, Gui or one of the Mod subfolders. If you need to make changes in several locations, make several pull requests and wait for the first one to be merged before submitting the next ones
In case your pull request does more than just fixing small bugs, make sure you discussed your ideas with other developers on the FreeCAD forum
Your branch is rebased on latest master git pull --rebase upstream master
All FreeCAD unit tests are confirmed to pass by running ./bin/FreeCAD --run-test 0
All commit messages are well-written ex: Fixes typo in Draft Move command text
Your pull request is well written and has a good description, and its title starts with the module name, ex: Draft: Fixed typos
Commit messages include issue #<id> or fixes #<id> where <id> is the FreeCAD bug tracker issue number in case a particular commit solves or is related to an existing issue on the tracker. Ex: Draft: fix typos - fixes #0004805

And please remember to update the Wiki with the features added or changed once this PR is merged.
Note: If you don't have wiki access, then please mention your contribution on the 0.20 Changelog Forum Thread.

chennes

I've added some comments below: some of them are clearly "future work", but this seemed like a good opportunity to add them to the radar.

chennes · 2021-12-09T16:34:23Z

src/Mod/Path/PathScripts/PathAdaptiveGui.py

@@ -1,4 +1,4 @@
-# -*- coding: utf-8 -*-
+# -*- cmding: utf-8 -*-


chennes · 2021-12-09T16:34:44Z

src/Mod/Path/PathScripts/PathAdaptiveGui.py

@@ -74,7 +80,9 @@ def getForm(self):
        form.Tolerance.setTickInterval(1)
        form.Tolerance.setValue(10)
        form.Tolerance.setTickPosition(QtGui.QSlider.TicksBelow)
-        form.Tolerance.setToolTip("Influences calculation performance vs stability and accuracy.")
+        form.Tolerance.setToolTip(
+            "Influences calculation performance vs stability and accuracy."


Tooltips should get translated too.

chennes · 2021-12-09T16:35:22Z

src/Mod/Path/PathScripts/PathAdaptiveGui.py

        formLayout.addRow(QtGui.QLabel("Stock to Leave"), form.StockToLeave)

        # Force inside out
        form.ForceInsideOut = QtGui.QCheckBox()
        form.ForceInsideOut.setChecked(True)
-        formLayout.addRow(QtGui.QLabel("Force Clearing Inside-Out"), form.ForceInsideOut)
+        formLayout.addRow(
+            QtGui.QLabel("Force Clearing Inside-Out"), form.ForceInsideOut


chennes · 2021-12-09T17:07:08Z

src/Mod/Path/PathScripts/PathDressupPathBoundaryGui.py

@@ -147,7 +175,10 @@ def setupFromExisting():
            elif PathJobGui.StockFromExistingEdit.IsStock(self.obj):
                setupFromExisting()
            else:
-                PathLog.error(translate('PathJob', "Unsupported stock object %s") % self.obj.Stock.Label)
+                PathLog.error(
+                    translate("PathJob", "Unsupported stock object %s")


Not necessarily for this PR, but long-term -- from a translation point of view, it's best if you can separate the string that must be translated from the part that translators shouldn't edit. So in this case I suggest something like
translate("PathJob","Unsupported stock object") + " %s" % self.obj.Stock.Label
or
translate("PathJob","Unsupported stock object") + f" {self.obj.Stock.Label}"
This prevents a translator from accidentally removing the "%s", extra spaces, punctuation, etc. from the string.

chennes · 2021-12-09T17:13:52Z

src/Mod/Path/PathScripts/PathAdaptiveGui.py

-        form.Side.addItem("Inside")
-        form.Side.addItem("Outside")
+        form.side.addItem(
+            QtCore.QT_TRANSLATE_NOOP("Path", "Inside"), "Inside"


Not for this PR (obviously!) but someday it might be nice to make these Enums instead of strings so that a linter/typechecker can catch mistakes.

chennes · 2021-12-09T17:20:21Z

src/Mod/Path/PathScripts/PathPocketBaseGui.py

        if not FeatureFacing & self.pocketFeatures():
            form.facingWidget.hide()
            form.clearEdges.hide()

        if FeaturePocket & self.pocketFeatures():
            form.extraOffset_label.setText(translate("PathPocket", "Pass Extension"))
-            form.extraOffset.setToolTip(translate("PathPocket", "The distance the facing operation will extend beyond the boundary shape."))
+            form.extraOffset.setToolTip(
+                translate(


If in this codebase this is a synonym for QtCore.QT_TRANSLATE_NOOP I suggest standardizing on one or the other. You'd probably save yourself some vertical space if you used the shorter one, if you're using Black.

chennes · 2021-12-09T17:24:55Z

src/Mod/Path/PathScripts/PathPreferencesPathJob.py

-                path,
-                "job_*.json")[0]
+        foo = QtGui.QFileDialog.getOpenFileName(
+            QtGui.QApplication.activeWindow(), "Path - Job Template", path, "job_*.json"


I think these window titles need a translate() (here and below)

chennes · 2021-12-09T17:29:45Z

src/Mod/Path/PathScripts/PathSlotGui.py

@@ -190,37 +254,37 @@ def updateVisibility(self, sentObj=None):
        if hideFeatures:
            # reset values
            self.CATS = [None, None]
-            self.selectInComboBox('Start to End', self.form.pathOrientation)
+            self.selectInComboBox("Start to End", self.form.pathOrientation)


I have not looked at how self.selectInComboBox() works, but you might need to audit your usage of it to ensure that it's going to work with the new user data.

Russ4262 · 2021-12-09T23:18:11Z

src/Mod/Path/PathScripts/PathSurfaceGui.py

+        form.boundBoxSelect.clear()
+        form.boundBoxSelect.addItem(QtCore.QT_TRANSLATE_NOOP("Path", "Stock"), "Stock")
+        form.boundBoxSelect.addItem(
+            QtCore.QT_TRANSLATE_NOOP("Path", "BaseBoundBox"), "BaseBoundBox"
+        )


If I may, I suggest a little different approach to this type of solution. Ideally, translations would be available in the CL version of FreeCAD also. Making the proposed changes does not allow for this.

I suggest modifying the enumerations lists, or the parent method containing them, to contain these translate commands. I am referring to https://github.com/sliptonic/FreeCAD/blob/ed945419d416ac6f2d98317aef76ec385d6371f6/src/Mod/Path/PathScripts/PathSurface.py#L202-L215 for this particular operation. The opPropertyEnumerations() referenced with the link returns a list of enumerations so CL users can query the class and identify the options available. Having these translations there makes sense, as the tooltips for the parent properties are available in the tuples stored in the opPropertyDefinitions() method, and those tooltips are translated there. Ideally, I think some of these methods should be @classmethod type methods.

Putting these proposed translation commands of the enumeration elements within the base module, rather than the GUI module, and then automating the addItem() population task for each combobox item makes the translations readily available across the operation, GUI or CL. As the proposed method stands, the enumeration element translations are only available via the GUI, whereas moving these to the appropriate methods in the base operation module will make for a more robust and manageable set of code, and shorter code since we could replace all the addItem() calls with for e in enumerationProps: iteration handler for a list of tuples that identify combobox items and their respective enumeration properties.

Not all ops make use of the database-style methods, opPropertyEnumerations() and similar opPropertyDefinitions(), that I put in place when editing Slot, Profile, Surface, and Waterline. These methods make certain other actions possible for users.

Hope this makes sense. Just my thoughts.

I've moved the PR to draft. It looks like we have a constellation of translation related issues. We need to figure out exactly what to use before deploying the full-scale copy-paste that I tried in this PR.

Russ, I think I understand what you're getting at. Can you take one module and do a concrete implementation so we can see it?

I was able to do that tonight, though I need to do some lesson plans for my Algebra 1 class now! Haha.

Refer to sliptonic#20.

freecadci · 2021-12-12T21:35:57Z

for feature branch PR_5242. Pipeline 427881631 was triggered at ed94541. All CI branches and pipelines.

sliptonic added the WB CAM Related to the CAM/Path Workbench label Dec 9, 2021

chennes reviewed Dec 9, 2021

View reviewed changes

Russ4262 reviewed Dec 9, 2021

View reviewed changes

sliptonic added the WB Draft Related to the Draft Workbench label Dec 9, 2021

sliptonic closed this Dec 14, 2021

sliptonic force-pushed the bug/translation branch from ed94541 to 2bfbbb9 Compare December 14, 2021 23:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PATH] Fix bug with translated UI #5242

[PATH] Fix bug with translated UI #5242

sliptonic commented Dec 9, 2021 •

edited

chennes left a comment

chennes Dec 9, 2021

chennes Dec 9, 2021

chennes Dec 9, 2021

chennes Dec 9, 2021

chennes Dec 9, 2021

chennes Dec 9, 2021

chennes Dec 9, 2021

chennes Dec 9, 2021

Russ4262 Dec 9, 2021 •

edited

sliptonic Dec 10, 2021

Russ4262 Dec 10, 2021

freecadci commented Dec 12, 2021

		@@ -1,4 +1,4 @@
		# -- coding: utf-8 --
		# -- cmding: utf-8 --

[PATH] Fix bug with translated UI #5242

[PATH] Fix bug with translated UI #5242

Conversation

sliptonic commented Dec 9, 2021 • edited

chennes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Russ4262 Dec 9, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

freecadci commented Dec 12, 2021

sliptonic commented Dec 9, 2021 •

edited

Russ4262 Dec 9, 2021 •

edited