shaise / FreeCAD_SheetMetal

A simple sheet metal workbench for FreeCAD
http://theseger.com/projects/2015/06/sheet-metal-addon-for-freecad/
GNU Lesser General Public License v2.1
190 stars 55 forks source link

Fix translation bug on enum #316

Closed hasecilu closed 5 months ago

hasecilu commented 5 months ago

Directly assigning the content of the text of the enum to the enum causes problems when the enum elements are translated.

Fix https://github.com/shaise/FreeCAD_SheetMetal/issues/315

shaise commented 5 months ago

hi @hasecilu Can you please try the following: create a "tub" base shape when freecad is set to english, save it, then change the language, restart freecad and reload the file you saved. Try now to recompute it and see if it works. Thanks!!!

hasecilu commented 5 months ago

I made the test and it worked, even on properties view the combobox for shapeType remains in English and you can make changes.

hasecilu commented 5 months ago

Another comment about translation: I noted that in 197020672bda31ea1177e74c0140013d52d6d1a0 you created a new function with, I think, the main purpose of translating strings def smAddProperty(obj, proptype, name, proptip, defval = None, paramgroup = "Parameters"): but as you can remember from similar change on Fasteners WB that doesn't work.

To test that, I created a new translation files from 0, (German with pylupdate5, French with pylupdate6) and none of them picked the strings from smAddProperty. Only the strings that are still using FreeCAD.Qt.translate("App::Property","Junction Gap") are being picked.

Would you like to revert the changes, so instead of using translate inside that function it should be used outside at the moment of property creation.

image

shaise commented 5 months ago

Hi, Thanks. Yes, if it hurts the string collection function then it should be reverted until I find a better way to do it because it is quite limiting and inflated the code.

hasecilu commented 5 months ago

Since log messages are informative for users I decided to mark them for translation.

I tried to also mark the smWarnDialog() instances but I got all files marked as modified because of line endings. See https://github.com/hasecilu/FreeCAD_SheetMetal/commit/8d690295767462da8525f39a9dcb2e887d8b8699?diff=unified&w=0.

When reviewing please check I didn't move any code from its original place.

        smWarnDialog(
            FreeCAD.Qt.translate(
                "QMessageBox",
                "The selected geometry does not belong to the active Body.\n"
                "Please make the container of this item active by\n"
                "double clicking on it."
            )
        )

Ok, opening with another NeoVim distro works (one that doesn't change line endings) image

hasecilu commented 5 months ago

It's done. The problem with the line endings seems that my editor replaced Windows line endings for Linux. I'm not sure if removing them causes problems on Windows but could be automated using precommit Could be used the one used on FreeCAD that sets max line length to 100 char. https://github.com/FreeCAD/FreeCAD/blob/main/.pre-commit-config.yaml

shaise commented 5 months ago

Don't worry about the line endings. All capable windows editors (such as notepad++ or VSCode) can handle it without issues. I still don't like the code bloating by long translation lines such as: FreeCAD.Qt.translate("App::Property" But I understand the need for the translation scripts. I will continue to look for a solution that will use a simple _tr() code.

hasecilu commented 5 months ago

Yeah that add some noise to the code, an alternative is to enclose code that we don't want the formatter formats with # fmt: off and # fmt: on but it's still more noise lines.

Ooops, it seems I didn't pushed some missing strings, maybe I added them on #314 . Do you have choose your preferred command names there?

image

shaise commented 5 months ago

Hi, Regarding the command names, just make it like the wiki, this is ok. PR #314 is now conflicting, so if more changes are needed, the PR should be fixed, or replaced with a new one.