About how to optimize and improve translation of commands

[wmayer]

See also git commit fe7f4c4be7

If your IDE supports regular expression of the file search you can use

Code: Select all

\ssGroup\s+=\s+QT_TR_NOOP

to find the remaining commands (which is 324 atm).

[Evgeniy]

See also git commit fe7f4c4be7

Not bad. I have been analyzing duplicates in Crowdin and "Standard-view" value is repeats many many times.

Code: Select all

sGroup = QT_TRANSLATE_NOOP("Workbench","Something");

in the command classes because as soon as we want to change the context string it needs to be done again for hundreds of classes.

There is a rational grain in this.

Why is this crutch often used in the code:

Code: Select all

#if 0 // needed for Qt's lupdate utility
    qApp->translate("Workbench", "Something");
#endif

Why not something like this:

in Command.cpp

Code: Select all

... begining

string Sketch_workbench = QT_TRANSLATE_NOOP("Workbench","Sketcher");

... command classes

sGroup = Sketch_workbench
...
sGroup = Sketch_workbench
...
sGroup = Sketch_workbench

in CommandConstraints.cpp

Code: Select all

sGroup = Sketch_workbench
...
sGroup = Sketch_workbench
...
sGroup = Sketch_workbench

Will there be a Sketch_workbench variable is visible in other files like: CommandConstraints.cpp, CommandCreateGeo.cpp, CommandSketcherTools.cpp etc. or needs modification of each header file?

It seems better to have one entry point for all values.
And the programmer will clearly know that the string has a translation.

If your IDE supports regular expression of the file search you can use

Im not use IDE yet because I mostly write in Python. I didn 't really dive into C lang

What IDE you are using for C lang developing of FreeCAD?

[Evgeniy]

Updated ts file look better:

Code: Select all

  <context>
    <name>Workbench</name>
    <message>
      <location filename="../../Command.cpp" line="142"/>
      <location filename="../../Command.cpp" line="265"/>
      <location filename="../../Command.cpp" line="295"/>
      <location filename="../../Command.cpp" line="339"/>
      <location filename="../../Command.cpp" line="380"/>
      <location filename="../../Command.cpp" line="507"/>
      <location filename="../../Command.cpp" line="679"/>
      <location filename="../../Command.cpp" line="718"/>
      <location filename="../../Command.cpp" line="756"/>
      <location filename="../../Command.cpp" line="863"/>
      <location filename="../../Command.cpp" line="947"/>
      <location filename="../../CommandAlterGeometry.cpp" line="73"/>
      <location filename="../../CommandConstraints.cpp" line="1097"/>
      <location filename="../../CommandConstraints.cpp" line="1335"/>
      <location filename="../../CommandConstraints.cpp" line="1569"/>
      <location filename="../../CommandConstraints.cpp" line="1794"/>
      <location filename="../../CommandConstraints.cpp" line="2108"/>
      <location filename="../../CommandConstraints.cpp" line="2314"/>
      <location filename="../../CommandConstraints.cpp" line="2663"/>
      <location filename="../../CommandConstraints.cpp" line="2911"/>
      <location filename="../../CommandConstraints.cpp" line="3165"/>
      <location filename="../../CommandConstraints.cpp" line="3411"/>
      <location filename="../../CommandConstraints.cpp" line="3561"/>
      <location filename="../../CommandConstraints.cpp" line="4199"/>
      <location filename="../../CommandConstraints.cpp" line="4907"/>
      <location filename="../../CommandConstraints.cpp" line="5209"/>
      <location filename="../../CommandConstraints.cpp" line="5485"/>
      <location filename="../../CommandConstraints.cpp" line="5789"/>
      <location filename="../../CommandConstraints.cpp" line="5933"/>
      <location filename="../../CommandConstraints.cpp" line="6460"/>
      <location filename="../../CommandConstraints.cpp" line="6677"/>
      <location filename="../../CommandConstraints.cpp" line="6977"/>
      <location filename="../../CommandConstraints.cpp" line="7141"/>
      <location filename="../../CommandConstraints.cpp" line="7602"/>
      <location filename="../../CommandConstraints.cpp" line="7729"/>
      <location filename="../../CommandCreateGeo.cpp" line="384"/>
      <location filename="../../CommandCreateGeo.cpp" line="687"/>
      <location filename="../../CommandCreateGeo.cpp" line="728"/>
      <location filename="../../CommandCreateGeo.cpp" line="1052"/>
      <location filename="../../CommandCreateGeo.cpp" line="1095"/>
      <location filename="../../CommandCreateGeo.cpp" line="1850"/>
      <location filename="../../CommandCreateGeo.cpp" line="2093"/>
      <location filename="../../CommandCreateGeo.cpp" line="2362"/>
      <location filename="../../CommandCreateGeo.cpp" line="2389"/>
      <location filename="../../CommandCreateGeo.cpp" line="2623"/>
      <location filename="../../CommandCreateGeo.cpp" line="3418"/>
      <location filename="../../CommandCreateGeo.cpp" line="3448"/>
      <location filename="../../CommandCreateGeo.cpp" line="3775"/>
      <location filename="../../CommandCreateGeo.cpp" line="4118"/>
      <location filename="../../CommandCreateGeo.cpp" line="4416"/>
      <location filename="../../CommandCreateGeo.cpp" line="4448"/>
      <location filename="../../CommandCreateGeo.cpp" line="4950"/>
      <location filename="../../CommandCreateGeo.cpp" line="4994"/>
      <location filename="../../CommandCreateGeo.cpp" line="5025"/>
      <location filename="../../CommandCreateGeo.cpp" line="5308"/>
      <location filename="../../CommandCreateGeo.cpp" line="5335"/>
      <location filename="../../CommandCreateGeo.cpp" line="5515"/>
      <location filename="../../CommandCreateGeo.cpp" line="5544"/>
      <location filename="../../CommandCreateGeo.cpp" line="5572"/>
      <location filename="../../CommandCreateGeo.cpp" line="5852"/>
      <location filename="../../CommandCreateGeo.cpp" line="5881"/>
      <location filename="../../CommandCreateGeo.cpp" line="5912"/>
      <location filename="../../CommandCreateGeo.cpp" line="6142"/>
      <location filename="../../CommandCreateGeo.cpp" line="6456"/>
      <location filename="../../CommandCreateGeo.cpp" line="6575"/>
      <location filename="../../CommandCreateGeo.cpp" line="6759"/>
      <location filename="../../CommandCreateGeo.cpp" line="6935"/>
      <location filename="../../CommandCreateGeo.cpp" line="7191"/>
      <location filename="../../CommandCreateGeo.cpp" line="7384"/>
      <location filename="../../CommandCreateGeo.cpp" line="7411"/>
      <location filename="../../CommandCreateGeo.cpp" line="7438"/>
      <location filename="../../CommandCreateGeo.cpp" line="7466"/>
      <location filename="../../CommandCreateGeo.cpp" line="7493"/>
      <location filename="../../CommandCreateGeo.cpp" line="7520"/>
      <location filename="../../CommandCreateGeo.cpp" line="7547"/>
      <location filename="../../CommandCreateGeo.cpp" line="7578"/>
      <location filename="../../CommandSketcherBSpline.cpp" line="100"/>
      <location filename="../../CommandSketcherBSpline.cpp" line="131"/>
      <location filename="../../CommandSketcherBSpline.cpp" line="162"/>
      <location filename="../../CommandSketcherBSpline.cpp" line="193"/>
      <location filename="../../CommandSketcherBSpline.cpp" line="224"/>
      <location filename="../../CommandSketcherBSpline.cpp" line="255"/>
      <location filename="../../CommandSketcherBSpline.cpp" line="384"/>
      <location filename="../../CommandSketcherBSpline.cpp" line="452"/>
      <location filename="../../CommandSketcherBSpline.cpp" line="525"/>
      <location filename="../../CommandSketcherBSpline.cpp" line="602"/>
      <location filename="../../CommandSketcherBSpline.cpp" line="756"/>
      <location filename="../../CommandSketcherBSpline.cpp" line="898"/>
      <location filename="../../CommandSketcherTools.cpp" line="102"/>
      <location filename="../../CommandSketcherTools.cpp" line="214"/>
      <location filename="../../CommandSketcherTools.cpp" line="301"/>
      <location filename="../../CommandSketcherTools.cpp" line="375"/>
      <location filename="../../CommandSketcherTools.cpp" line="421"/>
      <location filename="../../CommandSketcherTools.cpp" line="465"/>
      <location filename="../../CommandSketcherTools.cpp" line="508"/>
      <location filename="../../CommandSketcherTools.cpp" line="562"/>
      <location filename="../../CommandSketcherTools.cpp" line="616"/>
      <location filename="../../CommandSketcherTools.cpp" line="670"/>
      <location filename="../../CommandSketcherTools.cpp" line="722"/>
      <location filename="../../CommandSketcherTools.cpp" line="844"/>
      <location filename="../../CommandSketcherTools.cpp" line="929"/>
      <location filename="../../CommandSketcherTools.cpp" line="1024"/>
      <location filename="../../CommandSketcherTools.cpp" line="1539"/>
      <location filename="../../CommandSketcherTools.cpp" line="1585"/>
      <location filename="../../CommandSketcherTools.cpp" line="1628"/>
      <location filename="../../CommandSketcherTools.cpp" line="1662"/>
      <location filename="../../CommandSketcherTools.cpp" line="1944"/>
      <location filename="../../CommandSketcherTools.cpp" line="2084"/>
      <location filename="../../CommandSketcherTools.cpp" line="2147"/>
      <location filename="../../CommandSketcherTools.cpp" line="2211"/>
      <location filename="../../CommandSketcherVirtualSpace.cpp" line="92"/>
      <location filename="../../Workbench.cpp" line="37"/>
      <source>Sketcher</source>
      <translation>Эскизирование</translation>
    </message>

if we will be use:

Code: Select all

#if 0 // needed for Qt's lupdate utility
    qApp->translate("Workbench", "Something");
#endif

location block compressed to one string:

Code: Select all

  <context>
    <name>Workbench</name>
    <message>
      <location filename="../../Command.cpp" line="xxx"/>
      <source>Sketcher</source>
      <translation>Эскизирование</translation>
    </message>

[wmayer]

Why is this crutch often used in the code:

Code: Select all

#if 0 // needed for Qt's lupdate utility
    qApp->translate("Workbench", "Something");
#endif

The advantage is that it's dead code and the compiler will ignore it while lupdate finds the strings and add them to the .ts files.

Why not something like this:

Code: Select all

... begining

string Sketch_workbench = QT_TRANSLATE_NOOP("Workbench","Sketcher");

... command classes

sGroup = Sketch_workbench
...
sGroup = Sketch_workbench
...
sGroup = Sketch_workbench

It's theoretical possible but it has two disadvantages. You need to declare Sketch_workbench as global variable which is in general considered bad programming practice. And since in the Sketcher module the command classes are split into several source files you either must choose different identifiers or declare them with "extern". This complicates things quite a bit.

[Evgeniy]

Code: Select all

#if 0 // needed for Qt's lupdate utility
    qApp->translate("Workbench", "Something");
#endif

The advantage is that it's dead code and the compiler will ignore it while lupdate finds the strings and add them to the .ts files.

I understand this. And I noticed, these blocks:

Code: Select all

#if 0 // needed for Qt's lupdate utility
    qApp->translate("Workbench", "Value");
#endif

are widely used in FreeCAD sources.

If we use it, we can change context, but we not change value.
And sGroup = "sketcher" won't tell the programmer anything. it is impossible to find out whether a string has a translation or not.
If we want to correct the name of the workbench, then we will have to search (it is unclear how) and rename all the lines.

For example if we use varibles we can change context and value by one string.

Code: Select all

varible = QT_TRANSLATE_NOOP("WorkbenchNewContext","SketcherNewName");

sGroup = varible
sGroup = varible
sGroup = varible

For example what if we place this varible in "PreCompiled.cpp" or "PreCompiled.h"?
That file used in all command files:
Command.cpp , CommandAlterGeometry.cpp , CommandConstraints.cpp , CommandCreateGeo.cpp etc...
Maybe is possible to place this variable in it, for not create varible globally?

------

By the way. Im not find block

Code: Select all

#if 0 // needed for Qt's lupdate utility
    qApp->translate("Workbench", "Sketcher");
#endif

in commit https://github.com/FreeCAD/FreeCAD/comm ... ce7b96560e
Do you add that block to commit?

[Evgeniy]

cut

[wmayer]

If we use it, we can change context, but we not change value.

Why not?

And sGroup = "sketcher" won't tell the programmer anything. it is impossible to find out whether a string has a translation or not.

In the end for a programmer this is irrelevant.

Command.cpp , CommandAlterGeometry.cpp , CommandConstraints.cpp , CommandCreateGeo.cpp etc...
Maybe is possible to place this variable in it, for not create varible globally?

If a global variable should be avoided and the group name Sketcher only in a single place then the easiest solution is to change the functions CreateSketcherCommands, CreateSketcherCommandsCreateGeo, ... that you find in Command*.cpp files. As argument they could use a std::list where all commands are added to (instead of using rcCmdMgr.addCommand).

Then in AppSketcherGui.cpp the commands of the list can be added to the CommandManager and there setGroupName("Sketcher") can be called.

[Evgeniy]

I still came to the conclusion that my first solution with QT_TRANSLATE_NOOP ("Workbench", "sKetch") looks more correct than using variables or string.

Yes, it does not allow to rename all variables by one line. But it clearly shows all locations where the translation was applied and how many times it was applied. And show translation context. And with this commit https://github.com/FreeCAD/FreeCAD/commit/bc57ba6027 it should be work well.

Yes, in the end, this term will have a long context in Crowdin, but still for some reason it seems to me that this is the right solution.