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

[Tools] Refactor updatets to support two-lang mods #5037

Merged

Conversation

chennes
Copy link
Member

@chennes chennes commented Sep 17, 2021

The script to extract translation strings from source code files was designed to skip the C++ translations of directories with Python. In some cases this was manually worked around, but in others it was not. This PR is an attempt to refactor that code to ensure that both C++ and Python translations are included whenever a Mod has both languages in it.


  • Your pull request is confined strictly to a single module
  • "Small" bug
  • Your branch is rebased on latest master
  • All FreeCAD unit tests are confirmed to pass by running ./bin/FreeCAD --run-test 0
  • All commit messages are well-written e
  • Your pull request is well written and has a good description, and its title starts with the module name
  • No ticket

@chennes
Copy link
Member Author

chennes commented Sep 17, 2021

@yorikvanhavre and @luzpaz can you have a look at this PR and see if this actually works to get the necessary translation strings?

@yorikvanhavre
Copy link
Member

Yeah, that script evolved a lot over time... Good cleaning! Maybe we could simply remove the pyfolders list completely and let the script run both C++ and py gathering in all Mod folders? That way nobody has to worry about that anymore...

@berndhahnebach berndhahnebach added the Translation Translation and localization related label Sep 17, 2021
@chennes
Copy link
Member Author

chennes commented Sep 17, 2021

If that doesn't break anything, then yeah, I think that's the best solution. I figured there was some reason we were so carefully avoiding running both lupdate and pylupdate on the same directory, but obviously we have some where we have to now.

@chennes chennes force-pushed the fixMixedLanguageTranslationExtraction branch from 4c20726 to def6d05 Compare September 17, 2021 15:22
@chennes
Copy link
Member Author

chennes commented Sep 17, 2021

@yorikvanhavre OK, I just force-pushed another pass at refactoring the code. Because of the variability in the directory structures, I still ended up explicitly listing out all of the directories that need to be scanned, but at least now if a new one isn't being done, it will be obvious why! I run both lupdate and pylupdate on all of them, and just combine the results of each. I don't have a great way of testing whether this really worked, though. The TS files are generated, but I don't know what sort of havoc this will wreak on CrowdIn.

@luzpaz
Copy link
Contributor

luzpaz commented Sep 18, 2021

@chennes we need to add src/Mod/Plot as well, see FreeCAD/FreeCAD-translations#60

I've started on Plot here, but it's not complete yet #5042

@chennes
Copy link
Member Author

chennes commented Sep 18, 2021

Plot is omitted here because it doesn't have a Resources/translations directory. That is probably most appropriately added in #5042.

@berndhahnebach
Copy link
Contributor

would you rebase the PR to make it run on CI?

@berndhahnebach
Copy link
Contributor

Following a link to the branch on the CI-repository:

https://gitlab.com/berndhahnebach/FreeCAD/-/commits/PR_5060

The CI-status is available on the latest commit of the branch.
If there is no status available the PR should be rebased on latest master.
Check pipeline branches to see if your PR has been run by the CI.

https://gitlab.com/berndhahnebach/FreeCAD/-/pipelines?scope=branches

@chennes chennes marked this pull request as ready for review September 24, 2021 12:58
@FreeCAD-Tools
Copy link
Contributor

FreeCAD-Tools commented Sep 25, 2021

The Crowdin build needs to be updated when this script is merged. For add ignored (or skipped) strings in FEM and other workbenches.
@yorikvanhavre @luzpaz

@berndhahnebach
Copy link
Contributor

Copy link
Member

@yorikvanhavre yorikvanhavre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I'll merge and do a crowdin update and we compare... It is always possible to revert if anything goes wrong. For reference, here is the situation before the change:
Screenshot from 2021-09-29 14-17-55

@yorikvanhavre yorikvanhavre merged commit 319b490 into FreeCAD:master Sep 29, 2021
@yorikvanhavre
Copy link
Member

Ok this seems consequent. This is after the merge:
Screenshot from 2021-09-29 14-28-52

@yorikvanhavre
Copy link
Member

Updated files in 77925c4 Everything seems good!
Your refactored script does not update the main src/Gui/translations/FreeCAD.ts file or am I mistaken?

@chennes
Copy link
Member Author

chennes commented Sep 29, 2021

It does: that's the very first entry in the directories list, line 56.

@yorikvanhavre
Copy link
Member

Ok! I was fooled because the script didn't change a line of the ts file, but that's good :)

@FreeCAD-Tools
Copy link
Contributor

FreeCAD-Tools commented Sep 29, 2021

@yorikvanhavre new strings is not added to FEM workbench... Why is this happening?

It seemed to me that after this pull-request is merged, translations of FEM workbench commands should have appeared.
Which are not there now.

For example this (Constraint contact) not presented in Fem.ts:

CmdFemConstraintContact::CmdFemConstraintContact()
: Command("FEM_ConstraintContact")
{
sAppModule = "Fem";
sGroup = QT_TR_NOOP("Fem");
sMenuText = QT_TR_NOOP("Constraint contact");
sToolTipText = QT_TR_NOOP("Creates a FEM constraint for contact between faces");

and any other commands.

изображение

изображение

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Translation Translation and localization related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants